Skip to content

Conversation

@casperisfine
Copy link
Contributor

@casperisfine casperisfine commented Mar 15, 2024

In preparation for https://bugs.ruby-lang.org/issues/20205.

The frozen_string_literal compilation option will no longer be a boolean but a tri-state: on/off/default.

Blocked on ruby/prism#2603 (cc @kddnewton)

Also fix a few discrepancies with how prism compiler handle these options.

Copy link
Contributor

@kddnewton kddnewton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks right. Will rb_iseq_opt_frozen_string_literal take into account --enable-frozen-string-literal and --disable-frozen-string-literal?

@casperisfine
Copy link
Contributor Author

Will rb_iseq_opt_frozen_string_literal take into account --enable-frozen-string-literal and --disable-frozen-string-literal

Yes, right now it's a boolean so it's easy. Once I change it for a tri-state, if it's at "unset" I just won't call pm_options_frozen_string_literal_set at all.

@casperisfine casperisfine force-pushed the refactor-compile-frozen-string-literal branch from 6b60f6f to 818afcf Compare March 15, 2024 13:39
In preparation for https://bugs.ruby-lang.org/issues/20205.

The `frozen_string_literal` compilation option will no longer
be a boolean but a tri-state: `on/off/default`.
@casperisfine
Copy link
Contributor Author

Alright, now the only failure left is with prism, but it's already an issue on master:

    1) Failure:
  TestRubyLiteral#test_hash_duplicated_key [/home/runner/work/ruby/ruby/src/test/ruby/test_literal.rb:495]:
  duplicated literal key.
  
  1. [11/12] Assertion for "__FILE__"
     | expected: /key "\(eval\ at\ \/home\/runner\/work\/ruby\/ruby\/src\/test\/ruby\/test_literal\.rb:510\)" is duplicated/
     | actual: "".

From my understanding, now that __FILE__ is no longer a pm_static_literal_value the duplicated key warning no longer works.

Not too sure how to fix that.

@byroot byroot merged commit 91bf7eb into ruby:master Mar 15, 2024
@casperisfine
Copy link
Contributor Author

The way the current compiler does it is that strings used as literal hash keys are always considered literals:

      case NODE_STR:
      case NODE_FILE:
        return hash_key || frozen_string_literal_p(iseq);

So I guess prism would need something similar.

@kddnewton kddnewton deleted the refactor-compile-frozen-string-literal branch March 15, 2024 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants