Re: syntax_error constructor is declared inline

[Akim Demaille]

Hi Frank!

> Le 11 mai 2018 à 05:28, Frank Heckenbach  a écrit :
> 
>> I don't think the FSF cares about that as long as the main repo
>> remains hosted on a genuine free software platform (but IANArms).
>> I can see there are already several forks on Github.
> 
> As I said, if you'd like to use one of those, I can put my patches
> there (as merge requests where supported), or if you prefer them in
> some other form, just let me know.

Branches on GH are fine by me.  However, since, at least
historically, patches have been discussed publicly on bison-patches,
and since the archives are there, please always email them here.
That’s were questions, answers, comments of any form should be.

GH branches are convenient to play with (and to merge actually),
but not to submit/discuss it.

Re: syntax_error constructor is declared inline

[Frank Heckenbach]

Hi Akim!

> > Actually, where do you keep the current development sources?
> 
> It is on Savannah, but I'm really using only as a simple git server.
> Since it's been quite a while since I last worked on the GNU project,
> I was wandering if Savannah had newer management features, like
> Merge Requests, etc.

I didn't find any; just posting the patches like I did.

> So yes, we (maintainers) do directly push onto the main repo :)
> 
> > I also
> > have accounts on Github and sf.net (and can get others if
> > necessary), so I'm fine to post my patches there if you prefer (if
> > FSF doesn't mind).
> 
> I don't think the FSF cares about that as long as the main repo
> remains hosted on a genuine free software platform (but IANArms).
> I can see there are already several forks on Github.

As I said, if you'd like to use one of those, I can put my patches
there (as merge requests where supported), or if you prefer them in
some other form, just let me know.

Regards,
Frank

Re: syntax_error constructor is declared inline

[Frank Heckenbach]

Akim Demaille wrote:

> >> Your idea of letting Bison to keep track of the type does not work
> >> in mid-actions as it is needed to decide which copy constructor to
> >> use, unlike C then.
> > 
> > And likewise for %destructor and %printer.
> 
> This is something I have not studied yet, I'll have to look at it.
> I'm really disappointed that we have to double-code the symbol types.

I think it's basically unavoidable. There really is both the static
type (which Bison uses to decide which reductions to apply) and the
dynamic type (which can be overridden with $). So maybe
$ is not the best feature to begin with, but I guess that's a
moot point now. I don't currently use it at all (then again, my
parsers are not so time-critical), but apparently others do and it's
probably much too late to drop that feature (even if only in C++).

> I did not run benchmarks, but I expect more space and more time with
> std::variants.  Not to mention the C++-98/03 compatibility :)

That compatibility will be hard to keep if we want move semantics ...

Regards,
Frank

Re: syntax_error constructor is declared inline

[Akim Demaille]

Hi Frank!

> Le 9 mai 2018 à 05:00, Frank Heckenbach  a écrit :
> 
> I'm afraid I'm not overly involved with Savannah, either. I had an
> account there as I made some patches to GNU make some years ago, and
> I saw on https://www.gnu.org/software/bison/ that Savannah is (meant
> to be?) the official development platform, so I put my patches
> there, but otherwise I'm not bound to it.

Ok.

> Actually, where do you keep the current development sources?

It is on Savannah, but I’m really using only as a simple git server.
Since it’s been quite a while since I last worked on the GNU project,
I was wandering if Savannah had newer management features, like
Merge Requests, etc.

So yes, we (maintainers) do directly push onto the main repo :)

> I also
> have accounts on Github and sf.net (and can get others if
> necessary), so I'm fine to post my patches there if you prefer (if
> FSF doesn't mind).

I don’t think the FSF cares about that as long as the main repo
remains hosted on a genuine free software platform (but IANArms).
I can see there are already several forks on Github.

Re: syntax_error constructor is declared inline

[Akim Demaille]

> Le 9 mai 2018 à 05:00, Frank Heckenbach  a écrit :
> 
> Hans Åberg wrote:
> 
>>> On 8 May 2018, at 11:13, Akim Demaille  wrote:
>>> 
>>> I'm all rusted on Bison.
>> 
>> Welcome back, though!
> 
> Yes, nice to see you again!

Thanks :)


>> Your idea of letting Bison to keep track of the type does not work
>> in mid-actions as it is needed to decide which copy constructor to
>> use, unlike C then.
> 
> And likewise for %destructor and %printer.

This is something I have not studied yet, I’ll have to look at it.
I’m really disappointed that we have to double-code the symbol types.
I did not run benchmarks, but I expect more space and more time with
std::variants.  Not to mention the C++-98/03 compatibility :)

Re: syntax_error constructor is declared inline

[Frank Heckenbach]

Akim Demaille wrote:

> > I will apply this patch in my C++17 version, but since I'm not a
> > Bison maintainer, I can't promise it will go in the C++98 parser, or
> > in the official Bison sources at all. For now, you can apply the
> > patch manually (your installation directory may vary).
> 
> I will try to make soon a maintenance release of Bison, with
> bug fixing and gnulib updates.  Then it will be great to address
> modern C++.  However, I'd prefer sticking to a single skeleton
> rather than having to maintain several (not to mention the CI
> headaches...)
>
> I see that you also have submitted this on Savannah as
> https://savannah.gnu.org/patch/?9616.  But are you aware
> of any means to merge you submission in a way similar to
> Gitlab, or Git***?  What service does Savanah bring on
> this regard?

I'm afraid I'm not overly involved with Savannah, either. I had an
account there as I made some patches to GNU make some years ago, and
I saw on https://www.gnu.org/software/bison/ that Savannah is (meant
to be?) the official development platform, so I put my patches
there, but otherwise I'm not bound to it.

Actually, where do you keep the current development sources? I also
have accounts on Github and sf.net (and can get others if
necessary), so I'm fine to post my patches there if you prefer (if
FSF doesn't mind).

Regards,
Frank

Re: syntax_error constructor is declared inline

[Frank Heckenbach]

Hans Åberg wrote:

> > I don't particularly like those extra headers (see my mail in the
> > other thread "Generated headers"), but for now I don't want to
> > change more than necessary.
> 
> They conflict if using multiple parsers with different namespaces. 

Indeed (see my other mail).

> > But I generally agree that fewer options are necessary due to more
> > features of C++. In particular, in my C++17 version, I had to
> > disable "%printer" for variants and "%destructor" (with and without
> > variants) in favour of standard C++ overloading. This was also a
> > consequence of the problems with type overrides (e.g. in mid-rule
> > actions).
> 
> Those were written for C. If one can always use C++ features, that is better.

Sure. And with std::variant, they're not just better in an abstract
way, but very concretely as type overrides (required in mid-rules
actions with values) simply didn't work with the old variant.

Regards,
Frank

Re: syntax_error constructor is declared inline

[Hans Åberg]

> On 15 Mar 2018, at 07:41, Frank Heckenbach  wrote:
> 
> Hans Åberg wrote:
> 
 It could be deliberate to avoid them being exported. But C++ now
 has namespaces, which cann be used to avoid name conflicts.
>>> 
>>> It's all in a namespace anyway (yy by default) and we're only
>>> talking about a constructor, so no name conflicts. I think it was a
>>> simple mistake.
>> 
>> It would seem reasonable that the C++ parser has fewer options,
>> with say always headers for example. The stack, location and
>> position headers might be in the parser header, so there is no
>> conflict when using multiple parsers.
> 
> I don't particularly like those extra headers (see my mail in the
> other thread "Generated headers"), but for now I don't want to
> change more than necessary.

They conflict if using multiple parsers with different namespaces. 

> But I generally agree that fewer options are necessary due to more
> features of C++. In particular, in my C++17 version, I had to
> disable "%printer" for variants and "%destructor" (with and without
> variants) in favour of standard C++ overloading. This was also a
> consequence of the problems with type overrides (e.g. in mid-rule
> actions).

Those were written for C. If one can always use C++ features, that is better.

Re: syntax_error constructor is declared inline

[Hans Åberg]

> On 14 Mar 2018, at 23:04, Frank Heckenbach  wrote:
> 
>> It could be deliberate to avoid them being exported. But C++ now
>> has namespaces, which cann be used to avoid name conflicts.
> 
> It's all in a namespace anyway (yy by default) and we're only
> talking about a constructor, so no name conflicts. I think it was a
> simple mistake.

It would seem reasonable that the C++ parser has fewer options, with say always 
headers for example. The stack, location and position headers might be in the 
parser header, so there is no conflict when using multiple parsers.

Re: syntax_error constructor is declared inline

[Frank Heckenbach]

Hans Åberg wrote:

> It could be deliberate to avoid them being exported. But C++ now
> has namespaces, which cann be used to avoid name conflicts.

It's all in a namespace anyway (yy by default) and we're only
talking about a constructor, so no name conflicts. I think it was a
simple mistake.

Re: syntax_error constructor is declared inline

[Hans Åberg]

> On 14 Mar 2018, at 19:22, Kaz Kylheku  wrote:
> 
> Here is a rule from a production Makefile to remove // comments in Lex code 
> that prevent it from being C90

Flex is a separate project:
https://github.com/westes/flex
https://en.wikipedia.org/wiki/Flex_(lexical_analyser_generator)

Re: syntax_error constructor is declared inline

[Vishal V]

Thank you for the workaround suggestions. I ended up using a sed rule to
specifically remove the offending keyword:

sed -i '/inline/{ N;
s/inline\s*\n\s*parser::syntax_error::syntax_error/parser::syntax_error::syntax_error/
}' parser.cpp

By keeping it specific, I hope the rule will simply have no effect once the
issue is fixed in a future release.

Thanks,
Vishal


On Wed, Mar 14, 2018 at 11:22 AM, Kaz Kylheku  wrote:

> On 2018-03-13 13:31, Hans Åberg wrote:
>
>> On 12 Mar 2018, at 20:08, Vishal V  wrote:
>>>
>>> Bison 3.0.4 marks the constructor for the syntax_error class as 'inline'
>>> when generating a C++ scanner, which results in undefined references when
>>> the exception is thrown from a separate scanner file. Since this is the
>>> stated purpose of the syntax_error class (see
>>> http://lists.gnu.org/archive/html/help-bison/2013-07/msg00010.html),
>>> this
>>> appears to be a bug.
>>>
>>
>> As a workaround, you might try removing the inline specifier by adding
>> to the .yy grammar file:
>> %code {
>>   ...
>>   #define inline
>>   ...
>> }
>>
>
> The developers of this stuff are pretty fucking careless about what goes
> into these skeleton files.
>
> They think they can just throw in random new stuff and nobody's build will
> break.
>
> Here is a rule from a production Makefile to remove // comments in Lex
> code that prevent it from being C90:
>
> lex.yy.c: $(top_srcdir)parser.l
> $(call ABBREV,LEX)
> $(call SH,rm -f $@)
> $(call SH,  \
>   if $(TXR_LEX) $(LEX_DBG_FLAGS) $< ; then  \
> sed -e s@//.*@@ < $@ > $@.tmp ; \
> mv $@.tmp $@ ;  \
>   else  \
> exit 1 ;\
>   fi)
> $(call SH,chmod a-w $@)
>
> Commit: http://www.kylheku.com/cgit/txr/commit/?id=038bed1fa17743cbb
> fe4d219b6798d88254eef2c
>
> Here is build recipe which removes an unwanted/incorrect yyparse
> declaration from y.tab.h:
>
> y.tab.c: $(top_srcdir)parser.y
> $(call ABBREV,YACC)
> $(call SH,  \
>   if [ -e y.tab.h ]; then mv y.tab.h y.tab.h.old ; fi)
> $(call SH,rm -f y.tab.c)
> $(call SH,  \
>   if $(TXR_YACC) -v -d $< ; then\
> chmod a-w y.tab.c ; \
> sed -e '/yyparse/d' < y.tab.h > y.tab.h.tmp &&  \
>   mv y.tab.h.tmp y.tab.h ;  \
> if cmp -s y.tab.h y.tab.h.old ; then\
>   mv y.tab.h.old y.tab.h ;  \
> fi ;\
>   else  \
> rm y.tab.c ;\
> false ; \
>   fi)
>
>
> Commit: http://www.kylheku.com/cgit/txr/commit/?id=fdb54c85577859e26
> 525463c2e21801cd9b2377c
>

Re: syntax_error constructor is declared inline

[Kaz Kylheku]

On 2018-03-13 13:31, Hans Åberg wrote:

On 12 Mar 2018, at 20:08, Vishal V  wrote:

Bison 3.0.4 marks the constructor for the syntax_error class as 
'inline'
when generating a C++ scanner, which results in undefined references 
when
the exception is thrown from a separate scanner file. Since this is 
the

stated purpose of the syntax_error class (see
http://lists.gnu.org/archive/html/help-bison/2013-07/msg00010.html), 
this

appears to be a bug.


As a workaround, you might try removing the inline specifier by adding
to the .yy grammar file:
%code {
  ...
  #define inline
  ...
}


The developers of this stuff are pretty fucking careless about what goes 
into these skeleton files.


They think they can just throw in random new stuff and nobody's build 
will break.


Here is a rule from a production Makefile to remove // comments in Lex 
code that prevent it from being C90:


lex.yy.c: $(top_srcdir)parser.l
$(call ABBREV,LEX)
$(call SH,rm -f $@)
$(call SH,  \
  if $(TXR_LEX) $(LEX_DBG_FLAGS) $< ; then  \
sed -e s@//.*@@ < $@ > $@.tmp ; \
mv $@.tmp $@ ;  \
  else  \
exit 1 ;\
  fi)
$(call SH,chmod a-w $@)

Commit: 
http://www.kylheku.com/cgit/txr/commit/?id=038bed1fa17743cbbfe4d219b6798d88254eef2c


Here is build recipe which removes an unwanted/incorrect yyparse 
declaration from y.tab.h:


y.tab.c: $(top_srcdir)parser.y
$(call ABBREV,YACC)
$(call SH,  \
  if [ -e y.tab.h ]; then mv y.tab.h y.tab.h.old ; fi)
$(call SH,rm -f y.tab.c)
$(call SH,  \
  if $(TXR_YACC) -v -d $< ; then\
chmod a-w y.tab.c ; \
sed -e '/yyparse/d' < y.tab.h > y.tab.h.tmp &&  \
  mv y.tab.h.tmp y.tab.h ;  \
if cmp -s y.tab.h y.tab.h.old ; then\
  mv y.tab.h.old y.tab.h ;  \
fi ;\
  else  \
rm y.tab.c ;\
false ; \
  fi)


Commit: 
http://www.kylheku.com/cgit/txr/commit/?id=fdb54c85577859e26525463c2e21801cd9b2377c

Re: syntax_error constructor is declared inline

[Hans Åberg]

> On 14 Mar 2018, at 15:46, Frank Heckenbach  wrote:
> 
> Hans Åberg wrote:
> 
>> It is sort of strange in C++ to not have a header, and having
>> inlines not in those.
> 
> Sure, I think it was just a mistake. Bison puts inlines for some
> classes it uses internally (by_state, stack_symbol_type) in the C++
> file, that's OK (though they don't really need the "inline" keyword
> then, since it doesn't mean "can be inlined", but "can be defined in
> several compilation units" in modern C++, but it doesn't hurt), and
> probably by accident they caught the syntax_error inline
> constructor, too.

It could be deliberate to avoid them being exported. But C++ now has 
namespaces, which cann be used to avoid name conflicts.

Re: syntax_error constructor is declared inline

[Frank Heckenbach]

Hans Åberg wrote:

> It is sort of strange in C++ to not have a header, and having
> inlines not in those.

Sure, I think it was just a mistake. Bison puts inlines for some
classes it uses internally (by_state, stack_symbol_type) in the C++
file, that's OK (though they don't really need the "inline" keyword
then, since it doesn't mean "can be inlined", but "can be defined in
several compilation units" in modern C++, but it doesn't hurt), and
probably by accident they caught the syntax_error inline
constructor, too.

Regards,
Frank

Re: syntax_error constructor is declared inline

[Hans Åberg]

> On 14 Mar 2018, at 04:03, Frank Heckenbach  wrote:
> 
> Hans Åberg wrote:
> 
>>> On 13 Mar 2018, at 23:23, Frank Heckenbach  wrote:
>>> 
 Bison 3.0.4 marks the constructor for the syntax_error class as 'inline'
 when generating a C++ scanner, which results in undefined references when
 the exception is thrown from a separate scanner file. Since this is the
 stated purpose of the syntax_error class (see
 http://lists.gnu.org/archive/html/help-bison/2013-07/msg00010.html), this
 appears to be a bug.
>>> 
>>> Anyway, do you use the "--defines" option or "%defines" setting to
>>> generate a header that you include in your other source files? If
>>> so, AFAICS, the inline constructor is generated in the same header,
>>> so it should be available in your other source file, too.
>> 
>> It ends up in the .cc file even with %defines, it seems. But it
>> should properly be in the header file, and probably the other
>> inlines too.
> 
> I checked it and found that it's only put in the header if
> 
>  %define api.token.constructor
> 
> is set. But that can only be set if
> 
>  %define api.value.type
> 
> is also set. So this indeed seems to be wrong for non-variant users.
> 
> Instead of removing "inline" as suggested in the other bug report
> (which would then break with api.token.constructor), the following
> patch will always put the definition in the header.

It is sort of strange in C++ to not have a header, and having inlines not in 
those. In C, there is a style of include the lexer .c file into the parser .c 
file; perhaps the C++ parser intends to preserve that possibility.

> I will apply this patch in my C++17 version, but since I'm not a
> Bison maintainer, I can't promise it will go in the C++98 parser, or
> in the official Bison sources at all. For now, you can apply the
> patch manually (your installation directory may vary).

Perhaps if there is active development of Bison in the future, these things 
might be integrated.

Re: syntax_error constructor is declared inline

[Frank Heckenbach]

Hans Åberg wrote:

> > On 13 Mar 2018, at 23:23, Frank Heckenbach  wrote:
> > 
> >> Bison 3.0.4 marks the constructor for the syntax_error class as 'inline'
> >> when generating a C++ scanner, which results in undefined references when
> >> the exception is thrown from a separate scanner file. Since this is the
> >> stated purpose of the syntax_error class (see
> >> http://lists.gnu.org/archive/html/help-bison/2013-07/msg00010.html), this
> >> appears to be a bug.
> > 
> > Anyway, do you use the "--defines" option or "%defines" setting to
> > generate a header that you include in your other source files? If
> > so, AFAICS, the inline constructor is generated in the same header,
> > so it should be available in your other source file, too.
> 
> It ends up in the .cc file even with %defines, it seems. But it
> should properly be in the header file, and probably the other
> inlines too.

I checked it and found that it's only put in the header if

  %define api.token.constructor

is set. But that can only be set if

  %define api.value.type

is also set. So this indeed seems to be wrong for non-variant users.

Instead of removing "inline" as suggested in the other bug report
(which would then break with api.token.constructor), the following
patch will always put the definition in the header.

I will apply this patch in my C++17 version, but since I'm not a
Bison maintainer, I can't promise it will go in the C++98 parser, or
in the official Bison sources at all. For now, you can apply the
patch manually (your installation directory may vary).

Regards,
Frank

--- /usr/share/bison/c++.m4
+++ /usr/share/bison/c++.m4
@@ -161,7 +161,10 @@
 /// Syntax errors thrown from user actions.
 struct syntax_error : std::runtime_error
 {
-  syntax_error (]b4_locations_if([const location_type& l, ])[const 
std::string& m);]b4_locations_if([
+  syntax_error (]b4_locations_if([const location_type& l, ])[const 
std::string& m)
+   : std::runtime_error (m)]b4_locations_if([
+   , location (l)])[
+  {}]b4_locations_if([
   location_type location;])[
 };
 
@@ -279,13 +282,7 @@
 # --
 # Provide the implementation needed by the public types.
 m4_define([b4_public_types_define],
-[[  inline
-  ]b4_parser_class_name[::syntax_error::syntax_error (]b4_locations_if([const 
location_type& l, ])[const std::string& m)
-: std::runtime_error (m)]b4_locations_if([
-, location (l)])[
-  {}
-
-  // basic_symbol.
+[[// basic_symbol.
   template 
   inline
   ]b4_parser_class_name[::basic_symbol::basic_symbol ()

Re: syntax_error constructor is declared inline

[Hans Åberg]

> On 13 Mar 2018, at 23:23, Frank Heckenbach  wrote:
> 
>> Bison 3.0.4 marks the constructor for the syntax_error class as 'inline'
>> when generating a C++ scanner, which results in undefined references when
>> the exception is thrown from a separate scanner file. Since this is the
>> stated purpose of the syntax_error class (see
>> http://lists.gnu.org/archive/html/help-bison/2013-07/msg00010.html), this
>> appears to be a bug.
> 
> Anyway, do you use the "--defines" option or "%defines" setting to
> generate a header that you include in your other source files? If
> so, AFAICS, the inline constructor is generated in the same header,
> so it should be available in your other source file, too.

It ends up in the .cc file even with %defines, it seems. But it should properly 
be in the header file, and probably the other inlines too.

Re: syntax_error constructor is declared inline

[Frank Heckenbach]

Vishal V wrote:

> Bison 3.0.4 marks the constructor for the syntax_error class as 'inline'
> when generating a C++ scanner, which results in undefined references when
> the exception is thrown from a separate scanner file. Since this is the
> stated purpose of the syntax_error class (see
> http://lists.gnu.org/archive/html/help-bison/2013-07/msg00010.html), this
> appears to be a bug.

Note, I didn't write the code. But I'm currently writing a C++17
skeleton, and if there's a bug, I could fix it in that version.

BTW, the link doesn't explicitly mention separate files, it could
refer to sub-functions in the same file.

Anyway, do you use the "--defines" option or "%defines" setting to
generate a header that you include in your other source files? If
so, AFAICS, the inline constructor is generated in the same header,
so it should be available in your other source file, too.

If you don't generate a header, you don't even get the declaration
of syntax_error (or the parser class).

Am I missing something?

Regards,
Frank

Re: syntax_error constructor is declared inline

[Hans Åberg]

> On 12 Mar 2018, at 20:08, Vishal V  wrote:
> 
> Bison 3.0.4 marks the constructor for the syntax_error class as 'inline'
> when generating a C++ scanner, which results in undefined references when
> the exception is thrown from a separate scanner file. Since this is the
> stated purpose of the syntax_error class (see
> http://lists.gnu.org/archive/html/help-bison/2013-07/msg00010.html), this
> appears to be a bug.

As a workaround, you might try removing the inline specifier by adding to the 
.yy grammar file:
%code {
  ...
  #define inline
  ...
}

> This was mentioned in another unrelated bug report as well:
> http://lists.gnu.org/archive/html/bug-bison/2016-03/msg2.html

It worked in GCC7.

syntax_error constructor is declared inline

[Vishal V]

Hello,

Bison 3.0.4 marks the constructor for the syntax_error class as 'inline'
when generating a C++ scanner, which results in undefined references when
the exception is thrown from a separate scanner file. Since this is the
stated purpose of the syntax_error class (see
http://lists.gnu.org/archive/html/help-bison/2013-07/msg00010.html), this
appears to be a bug.

This was mentioned in another unrelated bug report as well:
http://lists.gnu.org/archive/html/bug-bison/2016-03/msg2.html

Thank you for your attention,
Vishal