[vlc-devel] [PATCH 1/2] CSSGrammar: refactor lex/parse-param

[Alexandre Janniaux]

Hi,

No, it's not that the CSS parser context should only be
exposed to the parser. It's that the lexer doesn't use the
css_parser so we don't need to add an argument to pass it.
Which is basically what I wrote.

If you declare YY_DECL in the lexer code (which is not the
case currently), the generated lexer code will use the
declaration you provided for yylex signature. Otherwise it
uses a default one which is the one in the +diff section
here (but on the lexer side) as defined by api.pure full.

The static link issue relative to this exist even before this
patch, even with simply `vlc_module_name`. I try solving this
through using partial linking and localization of hidden
symbols but it raised issue with C++ code in the end and was
quite painful (and only GNU Make) to integrate into automake.
Another solution would be to use objcopy --redefine-sym to
rename such symbols but it's much alike the same quantity of
work to integrate in automake.

This is the only module using flex/bison anyway, so I don't
know whether we should care or not as this would only be a
local workaround for this module anyway. If it matters to
you I can provide an additional patch for this issue.

Regards,
--
Alexandre Janniaux
Videolabs

On Tue, Mar 10, 2020 at 09:47:35AM +0200, Rémi Denis-Courmont wrote:
> Le tiistaina 10. maaliskuuta 2020, 1.58.29 EET Alexandre Janniaux a écrit :
> > Scanner is used by both lexer and parser, so %param allows binding to
> > both, and css_parser is only used by the parser so there is no need to
> > declare it in the yylex function. As the function was not correctly
> > exposed to the lexer code, it was leading to different prototype for
> > both generated compile unit.
> > ---
> >  modules/codec/webvtt/CSSGrammar.y | 7 +++----
> >  1 file changed, 3 insertions(+), 4 deletions(-)
> >
> > diff --git a/modules/codec/webvtt/CSSGrammar.y
> > b/modules/codec/webvtt/CSSGrammar.y index f9d7fd06b0..e51b132525 100644
> > --- a/modules/codec/webvtt/CSSGrammar.y
> > +++ b/modules/codec/webvtt/CSSGrammar.y
> > @@ -28,10 +28,9 @@
> >   */
> >  %define api.pure full
> >
> > -%parse-param { yyscan_t scanner }
> > +%param { yyscan_t scanner }
> >  %parse-param { vlc_css_parser_t *css_parser }
> > -%lex-param   { yyscan_t scanner }
> > -%lex-param   { vlc_css_parser_t *css_parser }
> > +
> >
> >  %{
> >  #ifdef HAVE_CONFIG_H
> > @@ -64,7 +63,7 @@ typedef void* yyscan_t;
> >
> >  %{
> >  /* See bison pure calling */
> > -#define YY_DECL int yylex(union YYSTYPE *, yyscan_t, vlc_css_parser_t *)
> > +#define YY_DECL int yylex(union YYSTYPE *, yyscan_t)
>
> The function cannot be called yylex since this might be the global namespace
> (when linking statically). Admittedly, that's a preexisting problem.
>
> Is your argument that the CSS parser context should only be exposed to the
> parser, and not the lexer? This needs explaining in the patch description.
>
> >  YY_DECL;
> >
> >  static int yyerror(yyscan_t scanner, vlc_css_parser_t *p, const char *msg)
>
>
> --
> レミ・デニ-クールモン
> http://www.remlab.net/
>
>
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel