Trouble with grammar version 2 declaration with Inform 6.43

[DavidGriffith]

Now that I've released inform6unix for Inform 6.42, I started a new development branch pulling in the latest Inform 6.43 code. It looks like PunyInform is doing some stuff that's no longer legal.

This is a brief taste. I'll work on fixing this and do a pull request.

/home/dave/proj/int-fiction/inform/inform6unix/inform-6.43 -v3 +punyinform/lib punyinform/tests/test-parser.inf punyinform/tests/test-parser.z3
Inform 6.43 for Linux (in development)
"punyinform/lib/globals.h", line 87: Error:  Once an action has been defined it is too late to change the grammar version.
> Constant Grammar__Version = 2;
line 156: Error:  The 'topic' token is only available if you are using grammar version 2 or later
>  * topic
line 156: Error:  '/' can only be used with grammar version 2 or later
>  * topic 'into'/
line 156: Error:  '/' can only be applied to prepositions
>  * topic 'into'/
line 156: Error:  '/' can only be applied to prepositions
>  * topic 'into'/'on'
line 163: Error:  '/' can only be used with grammar version 2 or later
>  * 'to'/
line 163: Error:  '/' can only be applied to prepositions
>  * 'to'/
line 163: Error:  '/' can only be applied to prepositions
>  * 'to'/'with'
"punyinform/lib/grammar.h", line 7: Error:  The 'topic' token is only available if you are using grammar version 2 or later
>  * topic
"punyinform/lib/grammar.h", line 10: Error:  The 'topic' token is only available if you are using grammar version 2 or later
>  * creature 'about' topic
"punyinform/lib/grammar.h", line 10: Error:  The 'topic' token is only available if you are using grammar version 2 or later
>  * creature 'to' topic
"punyinform/lib/grammar.h", line 10: Error:  The 'topic' token is only available if you are using grammar version 2 or later
>  * 'that' creature topic
"punyinform/lib/grammar.h", line 22: Error:  '/' can only be used with grammar version 2 or later
>  * 'up'/
"punyinform/lib/grammar.h", line 22: Error:  '/' can only be applied to prepositions
>  * 'up'/
"punyinform/lib/grammar.h", line 22: Error:  '/' can only be applied to prepositions
>  * 'up'/'over'
"punyinform/lib/grammar.h", line 22: Error:  '/' can only be used with grammar version 2 or later
>  * 'into'/
"punyinform/lib/grammar.h", line 22: Error:  '/' can only be applied to prepositions
>  * 'into'/
"punyinform/lib/grammar.h", line 22: Error:  '/' can only be applied to prepositions
>  * 'into'/'onto'
"punyinform/lib/grammar.h", line 22: Error:  '/' can only be used with grammar version 2 or later
>  * 'out' 'of'/
"punyinform/lib/grammar.h", line 22: Error:  '/' can only be applied to prepositions
>  * 'out' 'of'/
"punyinform/lib/grammar.h", line 22: Error:  '/' can only be applied to prepositions
>  * 'out' 'of'/'from'
"punyinform/lib/grammar.h", line 43: Error:  '/' can only be used with grammar version 2 or later
>  * multiexcept 'in'/
"punyinform/lib/grammar.h", line 43: Error:  '/' can only be applied to prepositions
>  * multiexcept 'in'/
"punyinform/lib/grammar.h", line 43: Error:  '/' can only be applied to prepositions
>  * multiexcept 'in'/'into'
"punyinform/lib/grammar.h", line 43: Error:  '/' can only be used with grammar version 2 or later
>  * multiexcept 'in'/'into'/
"punyinform/lib/grammar.h", line 43: Error:  '/' can only be applied to prepositions
>  * multiexcept 'in'/'into'/
"punyinform/lib/grammar.h", line 43: Error:  '/' can only be applied to prepositions
>  * multiexcept 'in'/'into'/'down'
"punyinform/lib/grammar.h", line 43: Error:  '/' can only be used with grammar version 2 or later

[DavidGriffith]

The core of the problem seems to be that Inform 6.43 doesn't like Constant Grammar__Version = 2;, telling me that somehow that constant is being set twice if it's in globals.h. I tried putting it in the first library file, parser.h. All that did was stop the being set twice complaint. The Grammar__Version still isn't being set. At this point I'm starting to suspect a bug in the compiler. Tomorrow I'll do some bisecting to see when and where this problem started.

[johanberntsson]

Thanks for looking into this. If you're on Linux you can use runtest.bash in the tests folder to see if something is broken before creating the pull request

[DavidGriffith]

I tracked down the appearance of the problem to DavidKinder/Inform6@2f1bd4a. Then I moved around where Constant Grammar__Version = 2; should go in the PunyInform library. Putting it at the very beginning of parser.h did remove the complaint about setting Grammar__Version twice, but the subsequent complaints remained. Poking further, I noticed that the compiler seems to have a problem with SceneryReply() in test-parser.inf in relation to when it encounters Constant Grammar__Version = 2; .

PunyInform/tests/test-parser.inf

Lines 83 to 89 in ef880bd

	[SceneryReply w1 w2;
	Push:
	if(w1=='bird' && w2=='birds') "The birds fly away as you try.";
	"Now how would you do that?";
	default:
	rfalse;
	];

If I put Constant Grammar__Version = 2; before that block, the compiler won't complain and the resulting game seems to perform as expected. However, the tests/runtests.rb won't allow me to check individual test games on the fly.

I'm not clear why that change in the compiler was deemed necessary and I'll explore that question later. I'm left with four questions for you:

1. Why did you put that SceneryReply() function declaration where you did?
2. Why is Constant Grammar__Version set where it is in globals.h?
3. Why does globals.h need to be explicitly included in a game?
4. Why can't Constant Grammar__Version appear at the beginning of parser.h?

[fredrikr]

Thanks for the heads-up. Looks like the compiler has a problem though, not the library?

Maybe you need to make a check at that point in the code in verbs.c and only set grammar version if it hasn't been set already?

1. Probably just happened to write it there, but it seems harmless, as it has nothing to do with actions or grammar?

2 & 4. PunyInform sets grammar version at one point only, and it's in the very first file to be included, long before creating any actions. Surely, it can't get any more correct or less risky than that?

3. globals.h holds constants, properties etc, so after including it, you can write routines which use these entities. We require routines to be written before including puny.h (which in turn includes all other library files), so that we can just not compile code sections that won't be needed. E.g. if the user hasn't included an InScope routine, we can exclude the code that would call it and take care of whatever it did. This is all done to make the compiled library as small and fast as possible.

[fredrikr]

Ah, I see now. SceneryReply does refer to actions, of course!

In some cases when using the Inform 6 compiler, it's safe to refer to things that haven't been defined yet, and they will be backpatched without problems. It's hard to know as a user when it's safe and when it's not.

I think it's fine if that SceneryReply routine has to move to after the inclusion of globals.h, which would put it after the code that sets the grammar version. It can even be moved to after the inclusiong of puny.h, as long as it's before the inclusion of ext_cheap_scenery.h

Sometimes in PunyInform projects, you need to create a constant referring to a routine before including puny,h, and then define the routine itself after including puny,h, because the routine can't be compiled until puny.h has been compiled, but it has to be present before puny.h is included. This isn't one of those cases though.

[DavidGriffith]

I made the suggested changes. If this works for you, I think you should mention this new restriction in the PunyInform documentation.

[DavidGriffith]

To restate my withdrawal of #120:

I have withdrawn my pull request because it is premature and probably the wrong thing to do. It appears that a more viable fix is to change the Inform6 compiler. We will wait until the discussion at https://intfiction.org/t/placement-of-constant-grammar-version-2/68141 is complete before addressing what might need to be done with PunyInform.