Change frozen_string_literal to be a tri-state

[casperisfine]

An explicit false is not equivalent to the comment being missing, because the default can be switched with a runtime flag:

$ ruby --enable-frozen-string-literal -e 'p "foo".frozen?'
true

NB: I chose to make it a second boolean flag as I'm not sure what the API compatibility guarantees are, but maybe a three state enum might be better?

@kddnewton

[kddnewton]

Yeah I'd prefer a uint8_t since it has multiple states as opposed to two bools if you don't mind

[casperisfine]

Alright, doing that then.

[casperisfine]

Alright I think I fixed all issues. I changed unspecified to be 0 so it's easier to initialize strucs.

Also I use int8_t not uint8_t as I think it makes a bit more sense to use -1 / 0 / 1 than 0 / 1 / 2.

[kddnewton]

Okay I'm trying to get my head around how to compile this and I'm now not so sure about these changes. I'm going to lay out a bunch of information, and maybe you can find where I'm missing something? Here's a table of the various combinations of options in Ruby:

# frozen_string_literal	--{enable,disable}-frozen-string-literal	"".frozen?
unspecified	unspecified	false
false	unspecified	false
true	unspecified	true
unspecified	disable	false
false	disable	false
true	disable	true
unspecified	enable	true
false	enable	false
true	enable	true

We need to map these options from the combination of pm_parser_t#frozen_string_literal, pm_options_t#frozen_string_literal, and pm_string_node_t#flags.

From the table above, it seems like not specifying a CLI option and passing --disable-frozen-string-literal both result in the same triplet. So I think pm_options_t#frozen_string_literal can be a boolean. Then looking at only the bottom 6 (because we consider the top 6 two triplets of the same), the only difference is an unspecified comment changes the value based on the CLI. In that case, if we set pm_parser_t#frozen_string_literal to pm_options_t#frozen_string_literal and then change pm_parser_t#frozen_string_literal based on the comment, it seems like we'll get the right values.

So I guess what I'm saying is I'm not understanding what we're changing here.

[casperisfine]

Alright, I'll try to explain. So first for the context, I'm working on changing the default value of frozen_string_literal: https://bugs.ruby-lang.org/issues/20205

Which I'm implementing in Shopify/ruby#549 for now.

So MRI now needs to know if a literal string is frozen/mutable/unspecified, and each will result in a different instruction (putobject/putstring/putchilledstring respectively).

Does that make sense now?

[kddnewton]

Ahh I think I understand now, thanks for the explanation. In this case I think I understand that the table is now:

So if I understand your proposal correctly, it would change the table to be:

# frozen_string_literal	--{enable,disable}-frozen-string-literal	""
unspecified	unspecified	chilled
false	unspecified	mutable
true	unspecified	frozen
unspecified	disable	mutable
false	disable	mutable
true	disable	frozen
unspecified	enable	frozen
false	enable	mutable
true	enable	frozen

And for compiling we should check:

• if FROZEN -> frozen
• if MUTABLE -> mutable
• else -> chilled

Is that right?

I've added a couple of minor comments on the code itself.