publish bison version number in generated output about bison

[bazsi]

You are right. I just always used hex numbers for this purpose, but the ifdef in your example makes sense. So I am okay with using this as is.

[akimd]

Hi @bazsi
Sorry for the delays, I didn't get any notification :(

I have a few questions:

• why do you actually use YYEMPTY? In most cases, it's quite an implementation detail. Using github's search engine, I could find where you actually depended on this symbol.

• starting with 3.6.1, YYEMPTY got renamed to _EMPTY when api.prefix is used: I guess you didn't mean _EMPTY, right?

• why don't you %require "3.6" and avoid all this portability hassle? Your releases can just ship the generated files, and you're done.

the migration from the old style to the new one took us almost 2 years

Wow... Don't hesitate getting in touch with us sooner!

Cheers!

[akimd]

@bazsi Ping!

[bazsi]

Sorry, this scrolled away in my inbox and forgot to answer.

I have a few questions:

• why do you actually use YYEMPTY? In most cases, it's quite an implementation detail. Using github's search engine, I could find where you actually depended on this symbol.

I am not sure you remember but we are using multiple grammars in syslog-ng with the same lexer instance. The reason is that the parsing of the syslog-ng configuration file sections are delegated to plugins. This means that the main grammar would invoke the grammar of the plugin to perform the parsing of the respective plugin.

What can happen is that when this plugin grammar ends (for instance via YYACCEPT or simply when the grammar processes its first rule) it might have looked up a token that is stored in yychar. This is just basic look ahead, where the new symbol is used to decide if the grammar is finished or not.

If we return to the parent grammar at that point this token is lost.

This we inject this token back with a code fragment like this:

if (yychar != AFSOCKET_EMPTY)
  cfg_lexer_unput_token(lexer, &yylval);
YYACCEPT;

• starting with 3.6.1, YYEMPTY got renamed to _EMPTY when api.prefix is used: I guess you didn't mean _EMPTY, right?

No I had PREFIX there but probably github chopped it off.

• why don't you %require "3.6" and avoid all this portability hassle? Your releases can just ship the generated files, and you're done.

We do ship generated grammar files.

However it is common practice that distros potentially patch our grammar files and they need bison to regenerate that. We already decided to increase our requirement but only up to 3.5.1 because ubuntu focal is still at that version.

the migration from the old style to the new one took us almost 2 years

Wow... Don't hesitate getting in touch with us sooner!

It wasn't much you could do, we were mostly waiting for new versions to enter into various distros. And then working around incompatibilities within the broad set of biosn versions we wanted to support.

[akimd]

Hi @bazsi

starting with 3.6.1, YYEMPTY got renamed to PREFIX_EMPTY when api.prefix is used

Actually, that's 3.6. Changes in the third number are only bug fixes and are not expected to have such a large effect.

I am not sure you remember but we are using multiple grammars in syslog-ng with the same lexer instance.

Yep, I remember.

What can happen is that when this plugin grammar ends (for instance via YYACCEPT or simply when the grammar processes its first rule) it might have looked up a token that is stored in yychar. This is just basic look ahead, where the new symbol is used to decide if the grammar is finished or not.

If we return to the parent grammar at that point this token is lost.

Gosh… That's a tricky one. I should really look at your set up at some point, and see what solution we could provide.

This we inject this token back with a code fragment like this:

if (yychar != AFSOCKET_EMPTY)
  cfg_lexer_unput_token(lexer, &yylval);
YYACCEPT;

That makes perfect sense!

We do ship generated grammar files.

However it is common practice that distros potentially patch our grammar files and they need bison to regenerate that. We already decided to increase our requirement but only up to 3.5.1 because ubuntu focal is still at that version.

I understand.

the migration from the old style to the new one took us almost 2 years

Wow... Don't hesitate getting in touch with us sooner!

It wasn't much you could do, we were mostly waiting for new versions to enter into various distros. And then working around incompatibilities within the broad set of bison versions we wanted to support.

I understand your concern.

So let's address your real question.

I've considered adding such version macros long long ago, but never really encountered a case where this was truly needed. We maintain backward compatibility. But accidents do happen, unfortunately. And in the present case, with 64aec0a and the following ones, I had not anticipated it could be a problem for anybody (I didn't have much choice actually, since anyway that token type is now exposed in the header, so it must be subject to api.prefix).

Anyway. This should not happen, so it should not be needed. Besides, some languages such as Java do not have equivalent to macros. And each time a backward issue was discovered, we found ways to address it in a version-independent way. In your case, I believe you could have used

#ifndef YYEMPTY
# define YYEMPTY PREFIX_EMPTY
#endif

and use YYEMPTY only (until you %require "3.6").

So I never wanted to cross this line, and I still want to avoid it.

That being said, if you insist this is needed, I'll do it for yacc.c.

Cheers!

[bazsi]

YYEMPTY is an enum at least in newer versions and not a macro. I've tried to find a reliable way of coming up with a solution that works across a set of bison versions, but the only one that seemed to work was to detect it at build time and export as defines. YYEMPTY might be a macro prior to 3.6, so ifndef might work to detect if we are on a new version. I'll check. On the other hand, these version macros could be useful for the next such case. And we already have a string value containing this value, but that's not useful for conditional compilation.

…

On Sat, Nov 7, 2020, 18:20 Akim Demaille ***@***.***> wrote: Hi @bazsi <https://github.com/bazsi> starting with 3.6.1, YYEMPTY got renamed to PREFIX_EMPTY when api.prefix is used Actually, that's 3.6. Changes in the third number are only bug fixes and are not expected to have such a large effect. I am not sure you remember but we are using multiple grammars in syslog-ng with the same lexer instance. Yep, I remember. What can happen is that when this plugin grammar ends (for instance via YYACCEPT or simply when the grammar processes its first rule) it might have looked up a token that is stored in yychar. This is just basic look ahead, where the new symbol is used to decide if the grammar is finished or not. If we return to the parent grammar at that point this token is lost. Gosh… That's a tricky one. I should really look at your set up at some point, and see what solution we could provide. This we inject this token back with a code fragment like this: if (yychar != AFSOCKET_EMPTY) cfg_lexer_unput_token(lexer, &yylval); YYACCEPT; That makes perfect sense! We do ship generated grammar files. However it is common practice that distros potentially patch our grammar files and they need bison to regenerate that. We already decided to increase our requirement but only up to 3.5.1 because ubuntu focal is still at that version. I understand. the migration from the old style to the new one took us almost 2 years Wow... Don't hesitate getting in touch with us sooner! It wasn't much you could do, we were mostly waiting for new versions to enter into various distros. And then working around incompatibilities within the broad set of bison versions we wanted to support. I understand your concern. So let's address your real question. I've considered adding such version macros long long ago, but never really encountered a case where this was truly needed. We maintain backward compatibility. But accidents do happen, unfortunately. And in the present case, with 64aec0a <64aec0a> and the following ones, I had not anticipated it could be a problem for anybody (I didn't have much choice actually, since anyway that token type is now exposed in the header, so it must be subject to api.prefix). Anyway. This *should* not happen, so it *should* not be needed. Besides, some languages such as Java do not have equivalent to macros. And each time a backward issue was discovered, we found ways to address it in a version-independent way. In your case, I believe you could have used #ifndef YYEMPTY # define YYEMPTY PREFIX_EMPTY #endif and use YYEMPTY only (until you %require "3.6"). So I never wanted to cross this line, and I still want to avoid it. That being said, if you insist this is needed, I'll do it for yacc.c. Cheers! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#55 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAFOK5V34KXC5KAREPBGGJDSOV6XDANCNFSM4S3OETXA> .

[akimd]

YYEMPTY is an enum at least in newer versions and not a macro. I've tried to find a reliable way of coming up with a solution that works across a set of bison versions, but the only one that seemed to work was to detect it at build time and export as defines. YYEMPTY might be a macro prior to 3.6, so ifndef might work to detect if we are on a new version.

Yes, it was a macro.

Cheers!

[akimd]

On the other hand, these version macros could be useful for the next such case. And we already have a string value containing this value, but that's not useful for conditional compilation.

I fully agree, these macros never made sense to me. I see them as utterly useless, and would be very happy to get rid of them.

So, are you asking for the numeric version?

[bazsi]

Yes please :) I would try to avoid to change the workaround we have just pushed through by the next such issue the numeric macros would save a ton of efforts. Thanks.

…

On Sun, Nov 8, 2020, 07:36 Akim Demaille ***@***.***> wrote: On the other hand, these version macros could be useful for the next such case. And we already have a string value containing this value, but that's not useful for conditional compilation. I fully agree, these macros never made sense to me. I see them as utterly useless, and would be very happy to get rid of them. So, are you asking for the numeric version? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#55 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAFOK5UENFFSXJWPCZNP72LSOY35BANCNFSM4S3OETXA> .

[akimd]

Balazs, I intend to release 3.7.4 this weekend, but I would like to make sure it addresses your concerns. Could you please give a shot at:

https://www.lrde.epita.fr/~akim/private/bison/bison-3.7.3.15-f4e85.tar.gz
https://www.lrde.epita.fr/~akim/private/bison/bison-3.7.3.15-f4e85.tar.lz
https://www.lrde.epita.fr/~akim/private/bison/bison-3.7.3.15-f4e85.tar.xz

The YYBISON macro is now a version number.

Thanks a lot!

[bazsi]

I would use a hexadecimal number so the major/minor/patch values can be extracted. This is what I see now: ``` /* Identify Bison output, and Bison version. */ #define YYBISON 30703 ``` And I think this should be 0x030703 instead. I was grepping through a number of header files how they usually encode this information and what I have found is that some of those do use decimal values, but in those cases I usually found the broken down version numbers in additional macros. So if we have a single VERSION value I would encode it as a hexadecimal value, so that these expressions would work: #define YYBISON_VERSION_MAJOR ((YYBISON >> 16) & 0xFF) #define YYBISON_VERSION_MINOR ((YYBISON >> 8) & 0xFF) #define YYBISON_VERSION_PATCH ((YYBISON) & 0xFF) Also, I think I would use a bit more specific name, like YYBISON_VERSION_VALUE This is how this looks like in syslog-ng with the current name/decimal encoding: ``` #if YYBISON / 10000 == 3 && (YYBISON % 10000) / 100 <= 5 /* old bison versions (e.g. 3.5.1 and below) when using api.prefix, didn't * generate the PRAGMA_EMPTY enum, but had YYEMPTY, so let's define * PRAGMA_EMPTY to YYEMPTY, so the grammar itself is able to use the new * style */ #define PRAGMA_EMPTY YYEMPTY #endif ``` Thanks.

…

On Wed, Nov 11, 2020 at 4:52 PM Akim Demaille ***@***.***> wrote: Balazs, I intend to release 3.7.4 this weekend, but I would like to make sure it addresses your concerns. Could you please give a shot at: https://www.lrde.epita.fr/~akim/private/bison/bison-3.7.3.15-f4e85.tar.gz https://www.lrde.epita.fr/~akim/private/bison/bison-3.7.3.15-f4e85.tar.lz https://www.lrde.epita.fr/~akim/private/bison/bison-3.7.3.15-f4e85.tar.xz The YYBISON macro is now a version number. Thanks a lot! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#55 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAFOK5SY4F5BMIAG33IIBF3SPKXKFANCNFSM4S3OETXA> .

-- Bazsi

[akimd]

I would use a hexadecimal number so the major/minor/patch values can be extracted.

I fail to see how decimal and hexadecimal would differ here.

And I think this should be 0x030703 instead.

Care to argument?

#define YYBISON_VERSION_MAJOR ((YYBISON >> 16) & 0xFF)
#define YYBISON_VERSION_MINOR ((YYBISON >> 8) & 0xFF)
#define YYBISON_VERSION_PATCH ((YYBISON) & 0xFF)

I don't see any value in these macros. What kind of condition do you think you need to write that would make sense with them? In my experience, simple numbers suffice and are way more easy to manipulate and understand.

Also, I think I would use a bit more specific name, like YYBISON_VERSION_VALUE

Well, __GNUC__ is the version of GCC, I don't see a problem with that. I would have happily used YYBISON_VERSION, but it's already taken for something else :-(

This is how this looks like in syslog-ng with the current name/decimal encoding:

#if YYBISON / 10000 == 3 && (YYBISON % 10000) / 100 <= 5

Why do you need to specify == 3? Most of the time conditions such as

#if 30704 <= YYBISON && YYBISON < 30800

suffice.

Cheers!

[akimd]

Bison 3.7.4 was released with that patch. Cheers!

[bazsi]

Thanks a lot.