[Mesa-dev] glsl/glcpp: A bunch of pre-processor cleanups

Wed Jul 23 16:40:43 PDT 2014

On 2014-07-17 16:45:34, Jordan Justen wrote:
> Made it ~25% through. :) I'll be busy for a bit, but I'll continue
> looking at the rest later.
> 
> 01/23 glsl/glcpp: Emit proper error for #define with a non-identifier
>   Reviewed-by: Jordan Justen <jordan.l.justen at intel.com>
> 
> 02/23 glsl/glcpp: Add support for comments between #define and macro identifier
>   Reviewed-by: Jordan Justen <jordan.l.justen at intel.com>
> 
> 03/23 glsl/glcpp: Remove some un-needed calls to NEWLINE_CATCHUP
>   * Reference 6005e9cb in comment?
>   Reviewed-by: Jordan Justen <jordan.l.justen at intel.com>
> 
> 04/23 glsl/glcpp: Add testing for EOF sans newline (and fix for
> <DEFINE>, <COMMENT>)
>   Reviewed-by: Jordan Justen <jordan.l.justen at intel.com>
> 
> 05/23 glsl/glcpp: Drop extra, final newline from most output
>   * In the "<SKIP,INITIAL>\n {" section, you set
>     "parser->last_token_was_newline = 1;"
>     Doesn't "RETURN_TOKEN (NEWLINE);" do this as well?
>   Reviewed-by: Jordan Justen <jordan.l.justen at intel.com>
> 
> 06/23 glsl/glcpp: Abstract a bit of common code for returning string tokens
>   Reviewed-by: Jordan Justen <jordan.l.justen at intel.com>

07/23 glsl/glcpp: Stop using a lexer start condition (<SKIP>) for token skipping.
  Replied with question

08/23 glsl/glcpp: Minor tweak to wording of error message
09/23 glsl/glcpp: Fix off-by-one error in column in first-line error messages
10/23 glsl/glcpp: Add a -d/--debug option to the standalone glcpp program
11/23 glsl/glcpp: Don't use start-condition stack when switching to/from <DEFINE>
12/23 glsl/glcpp: Rename HASH token to HASH_TOKEN
  Reviewed-by: Jordan Justen <jordan.l.justen at intel.com>

13/23 glsl/glcpp: Correctly parse directives with intervening comments
  Commit message: simpoly => simply
  Not tab indented: "parser->first_non_space_token_this_line = 1;"
  Reviewed-by: Jordan Justen <jordan.l.justen at intel.com>

14/23 glsl: Add an internal-error catch-all rule
  In your updated commit message (on git branch), you mention that
  this fixes two Khronos negative tests. But, it looks like we will
  now print an "Internal compiler error" error message. So, we'll fail
  to compile the negative tests, yet report that we have a compiler
  error?

  Does the 15 patch fix the error reported? If so, then maybe you can
  update the commit messages of the two patches? Also, if patch 15
  fixes the error message, then add my Reviewed-by for this patch.

  Do we have a make check test for the errors that this fixes?

15/23 glsl: Properly lex extra tokens when handling # directives.
  Reviewed-by: Jordan Justen <jordan.l.justen at intel.com>

16/23 glsl/glcpp: Drop the HASH_ prefix from token names like HASH_IF
  Commit message: "Note, that HASH_TOKEN instead of HASH": should this
  be something like "Note that for the same reason we use HASH_TOKEN
  instead of HASH"?
  Reviewed-by: Jordan Justen <jordan.l.justen at intel.com>

17/23 glsl/glcpp: Add an explantory comment for "loc != NULL" check
18/23 glsl/glcpp: Emit error for duplicate parameter name in function-like macro
19/23 glsl/glcpp: Add (non)-support for ++ and -- operators
20/23 glsl/glcpp: Test that macro parameters substitute immediately after periods
21/23 glsl/glcpp: Add test for a multi-line comment within an #if 0 block
  Reviewed-by: Jordan Justen <jordan.l.justen at intel.com>

22/23 glsl/glcpp: Add a catch-all rule for unexpected characters.
  Commit message: anyt => any
  Reviewed-by: Jordan Justen <jordan.l.justen at intel.com>

23/23 glsl/glcpp: Treat carriage return as equivalent to line feed.
  Should we add this before the patch that will cause the error to be
  generated?
  Reviewed-by: Jordan Justen <jordan.l.justen at intel.com>

> On Thu, Jun 26, 2014 at 3:19 PM, Carl Worth <cworth at cworth.org> wrote:
> > Here's my latest series of patches to improve conformance of glcpp, (the glsl
> > preprocessor in mesa).
> >
> > Most of these changes are fixes that only a test-suite author could love. Most
> > fix nit-picky tests that do things that no sance application would actually
> > do. They're all reasonable things to do, but few are likely to impact many
> > real applications.
> >
> > The entire series (as well as some earlier patches already reviewed) can be
> > found on the glcpp-fixup branch of my mesa tree:
> >
> >         git://people.freedesktop.org/~cworth/mesa
> >
> > Here's a run-down of what the changes are in this series:
> >
> > Patch 01: Give an error for "#define 123" or similar non-identifier
> >
> >         Not a useful thing to do, of course, but an error we need.
> >
> > Patch 02: Support comment here: "#define /* Ha! */ FOO"
> >
> > Patches 03-12: Many cleanups/rewriting while working on the next patch
> >
> > Patch 13: Support comment here: "# /* Tricky! */ define FOO"
> >
> >         Comments appearing in these places are not likely, but are clearly
> >         valid according to the language specification. There was a bunch of
> >         work necessary to make this fix easy, (and even with all the
> >         preliminary work, the final patch was longer than I wanted).
> >
> >         I am happy that the lexer state at the end of this cleanup is much
> >         simpler and easier to read than it was before.
> >
> > Patch 14: Emit internal error for unrecognized character
> >
> >         This is to make un-subtle all classes of subtle bugs where the default
> >         flex rule was simply printing unrecongized characters to stdout and
> >         dropping them from the GLSL source.
> >
> >         This is not actually in glcpp but in the lexer for the main glsl
> >         compiler.
> >
> > Patch 15: Emit error for bogus extra characters after #extension
> >
> >         This is an example of a fix for one of those subtle bugs from the flex
> >         default-rule. This is a patch from Ken that was sent some time ago.
> >
> > Patches 16-17: Trivial fixups (renaming of token identifiers and new comment)
> >
> > Patch 18: Emit an error for duplicate macro parameter, eg "#define FOO(a, a)"
> >
> > Patch 19: Emit error if "++" or "--" appear in preprocessor condition
> >
> > Patches 20-21: Two new tests for bugs that I wrote (and fixed) while working
> >                on some of the above.
> >
> > Patch 22: Emit internal error for unrecognized character
> >
> >         This is just like patch 14, but for the lexer in glcpp itself.
> >
> > Patch 23: Treat '\r' as equivalent to '\n'
> >
> >         The '\r' character was previously hitting the default lex, "print and
> >         throw away" rule so was being entirely ignored. With patch 22, '\r'
> >         would instead generate an internal error. Fix this by making '\r'
> >         equivalent to '\n'.
> >
> >         I'd like to be even more spec-compliant for '\r', but I think this is
> >         OK for now. I'd also like to add some more-exhaustive testing for
> >         '\r', (such as running all of glcpp-test on the test cases with '\n'
> >         changed to "\r\n").
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: signature
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140723/75f59073/attachment.sig>