Feature #16602: Add support for `frozen_string_literals` to eval.

• Status: Rejected
• Assignee: -
• Target version: -

[ioquatix (Samuel Williams)]

Would it make sense for eval(..., frozen_string_literal: true) to exist?

[byroot (Jean Boussier)]

eval("# frozen_string_literal: true\n ...") works fine.

e.g. https://github.com/rails/rails/pull/38355/files#diff-e321ca5b1217071d1504c1d2cf37cc32R378

[ioquatix (Samuel Williams)]

Yes, it does, but it:

• Requires modification of the string which is sometimes messy.
• Requires modification of line numbers if you want to do it in a way which is not visible to the user.

I think any option that can be provided on the comment line at the start should also be an option to eval, and *_eval. e.g. coding too.

[sawa (Tsuyoshi Sawada)]

ioquatix (Samuel Williams) wrote in #note-2:

Yes, it does, but it:

• Requires modification of the string which is sometimes messy.

It requires modification because it is indeed modification: You are adding specification to a piece of code that did not have such specification.

I think any option that can be provided on the comment line at the start should also be an option to eval, and *_eval. e.g. coding too.

Once you start asking for such feature, it would be a matter of time that someone would ask for features like:

require "foo", frozen_string_literal: true
load "bar.rb", frozen_string_literal: true

$ ruby --frozen-string-literal=true ./baz.rb

[Eregon (Benoit Daloze)]

ioquatix (Samuel Williams) wrote in #note-2:

Requires modification of the string which is sometimes messy.

How is it messy? Isn't it as simple as @byroot (Jean Boussier) showed?

coding too.

Just using the encoding of the String seems much better for eval.

Requires modification of line numbers if you want to do it in a way which is not visible to the user.

You can pass a line number to eval to make it look like there was no extra line.

I think what @byroot (Jean Boussier) says is sufficient.
The global command-line flag is a very different thing that probably doesn't work for many gems.

The way it's implemented currently is entirely consistent, the parser reads magic comments like # frozen_string_literal: true no matter where it's parsed from.

Also, I think adding # frozen_string_literal to code not adding it itself is a potentially breaking change, so I think it doesn't need to be super convenient.
Basically, it's back to the discussion of enabling frozen_string_literal by default.

[shevegen (Robert A. Heiler)]

Basically, it's back to the discussion of enabling frozen_string_literal by default.

As far as I know it is not so much a discussion (matz decided to enable it after
all) - the primary question was more when it would become the default. I do not know
when it will be the case, but matz said that ruby 3.0 will not have it by default,
primarily due to backwards compatibility (which is simply a decision to be made
either way).

I have no real opinion on this feature suggested by ioquatix (personally I do not
need it; all my .rb files that I use on a daily basis have frozen strings set to
true anyway) but I think the discussion is a bit coming or going from different
point of views. One point may originate from convenience (I would think so
ioquatix); the other about that it is already possible as-is, via the example
shown by byroot, but also "downward" request that could follow, as shown by
saba - e. g. to add the same to more methods, even if the suggestion here did
not include it.

Again, I have no real preference either way, but the convenience aspect should
possibly be considered, whether there may be a net benefit or a net drawback
of the feature at hand. This may also require perhaps other folks to comment
if they were to need the suggested functionality; I can't speak for them as
I would not need the functionality (I almost never use eval() anymore; in
general I try to avoid plain eval(); only using instance_eval and
class_eval if any eval at all these days).

[ioquatix (Samuel Williams)]

@sawa (Tsuyoshi Sawada) your argument convince me that we don't need this feature then. But it's not always the case that "You are adding specification to a piece of code that did not have such specification."

Following that we reject this feature, I'm at a loss how to fix https://github.com/rack/rack/pull/1544 elegantly.

User code may contain:

# frozen_string_literal: true

But we end up calling eval like this:

eval "Rack::Builder.new {\n" + builder_script + "\n}.to_app",
        TOPLEVEL_BINDING, file, 0

So we end up with a kludge of:

Rack::Builder.new {
# frozen_string_literal: true
...
}

Which obviously isn't going to work correctly. The only other solution I can think of is to try and match comments in the user code, extract them outside the block, etc. Messy.

[ioquatix (Samuel Williams)]

• Status changed from Open to Rejected

[Eregon (Benoit Daloze)]

I think I found a good solution for that case in rack:
https://github.com/rack/rack/pull/1544#issuecomment-581123830