Mesa (master): 29 new commits

Carl Worth cworth at kemper.freedesktop.org
Tue Jul 29 22:11:59 UTC 2014


URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=a62354a9877e327c44ce4f0898359b4a3db41727
Author: Carl Worth <cworth at cworth.org>
Date:   Tue Jul 1 15:10:02 2014 -0700

    glsl: Add flex options to eliminate the default rule
    
    We've had bugs in the past where we have been inadvertently matching the
    default rule.
    
    Just as we did in the pre-processor in the previous commit, we can use:
    
    	%option warn nodefault
    
    in the compiler to instruct flex to not generate the default rule, and
    further to warn if our set of rules could let any characters go unmatched.
    
    With this warning active, flex actually warns that the catch-all rule we
    recently added to the compiler could never be matched. Since that is all
    safely determined at compile time now, we can safely drop this run-time
    compiler error message, (as we do in this commit).
    
    Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
    Reviewed-by: Kristian Høgsberg <krh at bitplanet.net>

URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=bc8721f16f2f4a08e6ad935723cc52de1748b63f
Author: Carl Worth <cworth at cworth.org>
Date:   Tue Jul 1 15:04:38 2014 -0700

    glsl/glcpp: Add flex options to eliminate the default rule.
    
    We've had multiple bugs in the past where we have been inadvertently matching
    the default rule, (which we never want to do). We recently added a catch-all
    rule to avoid this, (and made this rule robust for future start conditions).
    
    Kristian pointed out that flex allows us to go one step better. This syntax:
    
    	%option warn nodefault
    
    instructs flex to not generate the default rule at all. Further, flex will
    generate a warning at compile time if the set of rules we provide are
    inadequate, (such that it would be possible for the default rule to be
    matched).
    
    With this warning in place, I found that the catch-all rule was in fact
    missing something. The catch-all rule uses a pattern of "." which doesn't
    match newlines. So here we extend the newline-matching rule to all start
    conditions. That is enough to convince flex that it really doesn't need
    any default rule.
    
    Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
    Reviewed-by: Kristian Høgsberg <krh at bitplanet.net>

URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=4ebff9bca638d96b562640c093b2dcc5d02fb443
Author: Carl Worth <cworth at cworth.org>
Date:   Tue Jul 1 15:02:14 2014 -0700

    glsl/glcpp: Combine the two rules matching any character
    
    Using a single rule here means that we can use the <*> syntax to match
    all start conditions. This makes the catch-all rule more robust against
    the addition of future start conditions, (no need to maintain an ever-
    growing list of start conditions for this rul).
    
    Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
    Reviewed-by: Kristian Høgsberg <krh at bitplanet.net>

URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=80e9301d9be3f1fd85803a1fe5d9c6ac899e4e19
Author: Carl Worth <cworth at cworth.org>
Date:   Tue Jul 1 14:55:56 2014 -0700

    glsl/glcpp: Alphabetize lists of start conditions
    
    There is no behavioral change here. It's just easier to verify that lists
    of start conditions include all expected conditions when they appear in a
    consistent order.
    
    The <INITIAL> state is special, so it appears first in all lists. All others
    appear in alphabetical order.
    
    Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
    Reviewed-by: Kristian Høgsberg <krh at bitplanet.net>

URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=f9c99aefea85fe23bbe4b7802542eaf8f70d6798
Author: Carl Worth <cworth at cworth.org>
Date:   Wed Jun 18 17:03:19 2014 -0700

    glsl/glcpp: Add a catch-all rule for unexpected characters.
    
    In some of the recent glcpp bug-fixing, we found that glcpp was emitting
    unrecognized characters from the input source file to stdout, and dropping
    them from the source passed onto the compiler proper.
    
    This was obviously confusing, and totally undesired.
    
    The bogus behavior comes from an implicit default rule in flex, which is
    that any unmatched character is implicitly matched and printed to stdout.
    
    To avoid this implicit matching and printing, here we add an explicit
    catch-all rule. If this rule ever matches it prints an internal compiler
    error. The correct response for any such error is fixing glcpp to handle
    the unexpected character in the correct way.
    
    Reviewed-by: Jordan Justen <jordan.l.justen at intel.com>

URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=4757c74c840460f387a466894baf1c45624467c7
Author: Carl Worth <cworth at cworth.org>
Date:   Thu Jun 26 14:04:01 2014 -0700

    glsl/glcpp: Treat carriage return as equivalent to line feed.
    
    Previously, the '\r' character was not explicitly matched by any lexer
    rule. This means that glcpp would have been using the default flex rule to
    match '\r' characters, (where they would have been printed to stdout rather
    than actually correctly handled).
    
    With this commit, we treat '\r' as equivalent to '\n'. This is clearly an
    improvement the bogus printing to stdout. The resulting behavior is compliant
    with the GLSL specification for any source file that uses exclusively '\r' or
    '\n' to separate lines.
    
    For shaders that use a multiple-character line separator, (such as "\r\n"),
    glcpp won't be precisely compliant with the specification, (treating these as
    two newline characters rather than one), but this should not introduce any
    semantic changes to the shader programs.
    
    Reviewed-by: Jordan Justen <jordan.l.justen at intel.com>

URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=12d583b21a7e538d7fa44c3129e95b407d03c01a
Author: Carl Worth <cworth at cworth.org>
Date:   Thu Jun 26 10:56:41 2014 -0700

    glsl/glcpp: Add test for a multi-line comment within an #if 0 block
    
    This test is written to exercise a bug which I recently wrote, (but
    fortunately caught and fixed before ever committing it).
    
    For the curious:
    
      The bug happened when the NEWLINE_CATCHUP code didn't actually return the
      NEWLINE token (due to the skipping). This resulted in the lexer continuing
      on through all the subsequent rules while still in the NEWLINE_CATCHUP start
      condition, (which then triggered the internal-compiler-error catch-all
      rule).
    
      What is intended is for the return of the NEWLINE token to start a new
      iteration of the lexer loop, at which time the NEWLINE_CATCHUP-handling code
      will reset from the <NEWLINE_CATCHUP> to the <INITIAL> start condition.
    
    Reviewed-by: Jordan Justen <jordan.l.justen at intel.com>

URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=346d712e87f09b434706ee21de02860f104c1c3c
Author: Carl Worth <cworth at cworth.org>
Date:   Thu Jun 26 10:08:34 2014 -0700

    glsl/glcpp: Test that macro parameters substitute immediately after periods
    
    At one point while rewriting the lexing rule for pre-processing numbers, I
    made it a bit too aggressive and within a replacement list sucked up a
    parameter name that appeared immediately after a period. This caused the
    parameter name to be unreplaced when the macro was expanded.
    
    It was in some piglit tests that I originally found this issue. Here, I'm
    adding a test to "make check" to ensure that this behavior remains correct.
    
    Reviewed-by: Jordan Justen <jordan.l.justen at intel.com>

URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=285c9392ad9c4ca862bc6f0cd4138bfc38cc3cee
Author: Carl Worth <cworth at cworth.org>
Date:   Wed Jun 25 14:17:37 2014 -0700

    glsl/glcpp: Add (non)-support for ++ and -- operators
    
    These operators aren't defined for preprocessor expressions, so we never
    implemented them. This led them to be misinterpreted as strings of unary
    '+' or '-' operators.
    
    In fact, what is actually desired is to generate an error if these operators
    appear in any preprocessor condition.
    
    So this commit looks like it is strictly adding support for these
    operators. And it is supporting them as far as passing them through to the
    subsequent compiler, (which was already happening anyway).
    
    What's less apparent in the commit is that with these tokens now being lexed,
    but with no change to the grammar for preprocessor expressions, these
    operators will now trigger errors there.
    
    A new "make check" test is added to verify the desired behavior.
    
    This commit fixes the following Khronos GLES3 CTS test:
    
    	invalid_op_1_vertex
    	invalid_op_1_fragment
    	invalid_op_2_vertex
    	invalid_op_2_fragment
    
    Reviewed-by: Jordan Justen <jordan.l.justen at intel.com>

URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=34cd293c8ac6df6284f94fd456eee7ddc847c5df
Author: Carl Worth <cworth at cworth.org>
Date:   Wed Jun 25 13:41:47 2014 -0700

    glsl/glcpp: Emit error for duplicate parameter name in function-like macro
    
    This will emit an error for something like:
    
    	#define FOO(x,x) ...
    
    Obviously, it's not a legal thing to do, and it's easy to check.
    
    Add a "make check" test for this as well.
    
    This fixes the following Khronos GLES3 CTS tests:
    
    	invalid_function_definitions.unique_param_name_vertex
    	invalid_function_definitions.unique_param_name_fragment
    
    Reviewed-by: Jordan Justen <jordan.l.justen at intel.com>

URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=fe1e0ac8524677c40c84f238650e0e198f0e8d36
Author: Carl Worth <cworth at cworth.org>
Date:   Wed Jun 25 13:28:38 2014 -0700

    glsl/glcpp: Add an explanatory comment for "loc != NULL" check
    
    Just reading the code, it looked like a bug that _define_object_macro had this
    check, but _define_function_macro did not. Upon further reading, that's
    because the check is to allow for our builtins to be defined, (and there are
    no builtin function-like macros).
    
    Add my new understanding as a comment to help the next reader.
    
    Reviewed-by: Jordan Justen <jordan.l.justen at intel.com>

URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=18c589d20ea684f6415b28d563fd2edaae0d7b2c
Author: Carl Worth <cworth at cworth.org>
Date:   Wed Jun 25 13:55:18 2014 -0700

    glsl/glcpp: Drop the HASH_ prefix from token names like HASH_IF
    
    Previously, we had a single token for "#if" but now that we have two separate
    tokens, it looks much better to see:
    
    	HASH_TOKEN IF
    
    than:
    
    	HASH_TOKEN HASH_IF
    
    (Note, that for the same reason we use HASH_TOKEN instead of HASH, we also use
    DEFINE_TOKEN instead of DEFINE to avoid a conflict with the <DEFINE> start
    condition in the lexer.)
    
    There should be no behavioral change from this commit.
    
    Reviewed-by: Jordan Justen <jordan.l.justen at intel.com>

URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=de0b4b6607c755257a63a72d0079ef9f2aaccbc2
Author: Kenneth Graunke <kenneth at whitecape.org>
Date:   Wed Jun 11 22:04:09 2014 -0700

    glsl: Properly lex extra tokens when handling # directives.
    
    Without this, in the <PP> state, we would hit Flex's default rule, which
    prints tokens to stdout, rather than returning them as tokens. (Or, after the
    previous commit, we would hit the new catch-all rule and generate an internal
    compiler error.)
    
    With this commit in place, we generate the desired syntax error.
    
    This manifested as a weird bug where shaders with semicolons after
    extension directives, such as:
    
       #extension GL_foo_bar : enable;
    
    would print semicolons to the screen, but otherwise compile just fine
    (even though this is illegal).
    
    Fixes Piglit's extension-semicolon.frag test.
    
    This also fixes the following Khronos GLES3 conformance tests, (and for real
    this time):
    
    	invalid_char_in_name_vertex
    	invalid_char_in_name_fragment
    
    Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
    Reviewed-by: Carl Worth <cworth at cworth.org>
    Reviewed-by: Jordan Justen <jordan.l.justen at intel.com>

URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=f196eb2d39d334b3ac8fbc5c0513dcb25e725155
Author: Carl Worth <cworth at cworth.org>
Date:   Wed Jun 25 13:07:44 2014 -0700

    glsl: Add an internal-error catch-all rule
    
    This is to avoid the default, silent flex rule which simply prints the
    character to stdout.
    
    For the following Khronos GLES3 conformance tests:
    
    	invalid_char_in_name_vertex
    	invalid_char_in_name_fragment
    
    With this commit, these tests now report Pass where they previously reported
    Fail, but Mesa isn't behaving correctly yet. It's now reporting the internal
    error where what is really desired is a syntax error.
    
    Reviewed-by: Jordan Justen <jordan.l.justen at intel.com>

URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=f062f0506a5b827667b7eb52136d8420b7e8113b
Author: Carl Worth <cworth at cworth.org>
Date:   Wed Jun 25 12:20:22 2014 -0700

    glsl/glcpp: Correctly parse directives with intervening comments
    
    It's legal (though highly bizarre) for a pre-processor directive to look like
    this:
    
    	#  /* why? */ define FOO bar
    
    This behavior comes about since the specification defines separate logical
    phases in a precise order, and comment-removal occurs in a phase before the
    identification of directives.
    
    Our implementation does not use an actual separate phase for comment removal,
    so some extra care is necessary to correctly parse this. What we want is for
    '#' to introduce a directive iff it is the first token on a line, (ignoring
    whitespace and comments). Previously, we had a lexical rule that worked only
    for whitespace (not comments) with the following regular expression to find a
    directive-introducing '#' at the beginning of a line:
    
    	HASH		^{HSPACE}*#{HSPACE}*
    
    In this commit, we switch to instead use a simple literal match of '#' to
    return a HASH_TOKEN token and add a new <HASH> start condition for whenever
    the HASH_TOKEN is the first non-space token of a line. This requires the
    addition of the new bit of state: first_non_space_token_this_line.
    
    This approach has a couple of implications on the glcpp parser:
    
    	1. The parser now sees two separate tokens, (such as HASH_TOKEN and
    	   HASH_DEFINE) where it previously saw one token (HASH_DEFINE) for
    	   the sequence "#define". This is a straightforward change throughout
    	   the grammar.
    
    	2. The parser may now see a SPACE token before the HASH_TOKEN token of
    	   a directive. Previously the lexical regular expression for {HASH}
    	   would eat up the space and there would be no SPACE token.
    
    This second implication is a bit of a nuisance for the parser. It causes a
    SPACE token to appear in a production of the grammar with the following two
    definitions of a control_line:
    
    	control_line
    	SPACE control_line
    
    This is really ugly, since normally a space would simply be a token
    separator, so it wouldn't appear in the tokens of a production. This leads to
    a further problem with interleaved spaces and comments:
    
    	/* ... */    /* ... */ #define /* ..*/
    
    For this, we must not return several consecutive SPACE tokens, or else we would need an arbitrary number of new productions:
    
    	SPACE SPACE control_line
    	SPACE SPACE SPACE control_line
    	ad nauseam
    
    To avoid this problem, in this commit we also change the lexer to emit only a
    single SPACE token for any series of consecutive spaces, (whether from actual
    whitespace or comments). For this compression, we add a new bit of parser
    state: last_token_was_space. And we also update the expected results of all
    necessary test cases for the new compression of space tokens.
    
    Fortunately, the compression of spaces should not lead to any semantic changes
    in terms of what the eventual GLSL compiler sees.
    
    So there's a lot happening in this commit, (particularly for such a tiny
    feature). But fortunately, the lexer itself is looking cleaner than ever. The
    only ugly bit is all the state updating, but it is at least isolated to a
    single shared function.
    
    Of course, a new "make check" test is added for the new feature, (directives
    with comments and whitespace interleaved in many combinations).
    
    And this commit fixes the following Khronos GLES3 CTS tests:
    
    	function_definition_with_comments_vertex
    	function_definition_with_comments_fragment
    
    Reviewed-by: Jordan Justen <jordan.l.justen at intel.com>

URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=dfdf9dc082cbab332457ea2dbe012eeb0d164ce4
Author: Carl Worth <cworth at cworth.org>
Date:   Wed Jun 25 11:59:55 2014 -0700

    glsl/glcpp: Rename HASH token to HASH_TOKEN
    
    This is in preparation for the planned addition of a new <HASH> start
    condition to the lexer. Both start conditions and token types are, of course,
    in the same default C namespace, so a start condition and a token type with
    the same name will collide. (And unfortunately, they are both apparently
    implemented as equivalent numeric types so the collision is undetected at
    compile time and simply leads to unpredictable behavior at run time.)
    
    Reviewed-by: Jordan Justen <jordan.l.justen at intel.com>

URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=0d5f5d127b2ccac29b12bc7377ccd8cd5b29c70b
Author: Carl Worth <cworth at cworth.org>
Date:   Wed Jun 25 11:52:02 2014 -0700

    glsl/glcpp: Don't use start-condition stack when switching to/from <DEFINE>
    
    This commit does not cause any behavioral change for any valid program. Prior
    to entering the <DEFINE> start condition, the only valid start condition is
    <INITIAL>, so whether pushing/popping <DEFINE> onto the stack or explicit
    returning to <INITIAL> is equivalent.
    
    The reason for this change is that we are planning to soon add a start
    condition for <HASH> with the following semantics:
    
    	<HASH>: We just saw a directive-introducing '#'
    
    	<DEFINE>: We just saw "#define" starting a directive
    
    With these two start conditions in place, the only correct behavior is to
    leave <DEFINE> by returning to <INITIAL>. But the old push/pop code would have
    returned to the <HASH> start condition which would then cause an error when
    the next directive-introducing '#' would be encountered.
    
    Reviewed-by: Jordan Justen <jordan.l.justen at intel.com>

URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=2fdc1f50c46b7f0a6abd8b2ef4739ac96958cb0a
Author: Carl Worth <cworth at cworth.org>
Date:   Wed Jun 25 11:46:49 2014 -0700

    glsl/glcpp: Add a -d/--debug option to the standalone glcpp program
    
    The verbose debug output from the parser is quite useful when debugging, and
    having this available as a command-line option is much more convenient than
    manually forcing this into the code when needed, (which is what I had been
    doing for too long previously).
    
    Reviewed-by: Jordan Justen <jordan.l.justen at intel.com>

URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=8e8f8ff1b25a4cfd332d9e7c86d3d56cdff98d6a
Author: Carl Worth <cworth at cworth.org>
Date:   Fri Jun 20 19:17:28 2014 -0700

    glsl/glcpp: Fix off-by-one error in column in first-line error messages
    
    For the first line we were initializing the column to 1, but for all
    subsequent lines we were initializing the column to 0. The column number is
    advanced for each token read before any error message is printed. So the 0
    value is the correct initialization, (so that the first column is reported as
    column 1).
    
    With this extremely minor change, many of the .expected files are updated such
    that error messages for the first line now have the correct column number in
    them.
    
    Reviewed-by: Jordan Justen <jordan.l.justen at intel.com>

URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=0742e0acd302d90dd7f326cf641da61ac60a8508
Author: Carl Worth <cworth at cworth.org>
Date:   Fri Jun 20 19:11:27 2014 -0700

    glsl/glcpp: Minor tweak to wording of error message
    
    It makes more sense to print the directive name with the preceding '#'.
    
    Reviewed-by: Jordan Justen <jordan.l.justen at intel.com>

URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=f583f214d5b8292aca91ec2217296b7ed17d9df4
Author: Carl Worth <cworth at cworth.org>
Date:   Fri Jun 20 16:18:23 2014 -0700

    glsl/glcpp: Stop using a lexer start condition (<SKIP>) for token skipping.
    
    Here, "skipping" refers to the lexer not emitting any tokens for portions of
    the file within an #if condition (or similar) that evaluates to false.
    
    Previously, the lexer had a special <SKIP> start condition used to control
    this skipping. This start condition was not handled like a normal start
    condition. Instead, there was a particularly ugly block of code set to be
    included at the top of the generated lexing loop that would change from
    <INITIAL> to <SKIP> or from <SKIP> to <INITIAL> depending on various pieces of
    parser state, (such as parser->skip_state and parser->lexing_directive).
    
    Not only was that an ugly approach, but the <SKIP> start condition was
    complicating several glcpp bug fixes I attempted recently that want to use
    start conditions for other purposes, (such as a new <HASH> start condition).
    
    The recently added RETURN_TOKEN macro gives us a convenient way to implement
    skipping without using a lexer start condition. Now, at the top of the
    generated lexer, we examine all the necessary parser state and set a new
    parser->skipping bit. Then, in RETURN_TOKEN, we examine parser->skipping to
    determine whether to actually emit the token or not.
    
    Besides this, there are only a couple of other places where we need to examine
    the skipping bit (other than when returning a token):
    
    	* To avoid emitting an error for #error if skipped.
    
    	* To avoid entering the <DEFINE> start condition for a #define that is
              skipped.
    
    With all of this in place in the present commit, there are hopefully no
    behavioral changes with this patch, ("make check" still passes all of the
    glcpp tests at least).
    
    Reviewed-by: Jordan Justen <jordan.l.justen at intel.com>

URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=09b4e12900ae496cbbc2a8864b721eefffb9d74a
Author: Carl Worth <cworth at cworth.org>
Date:   Fri Jun 20 15:30:21 2014 -0700

    glsl/glcpp: Abstract a bit of common code for returning string tokens
    
    Now that we have a common macro for returning tokens, it makes sense to
    perform some of the common work there, (such as copying string values).
    
    Reviewed-by: Jordan Justen <jordan.l.justen at intel.com>

URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=828686d4ebe055d1c6583192bc57742e101ff931
Author: Carl Worth <cworth at cworth.org>
Date:   Fri Jun 20 14:58:48 2014 -0700

    glsl/glcpp: Drop extra, final newline from most output
    
    The glcpp parser is line-based, so it needs to see a NEWLINE token at the end
    of each line. This causes a trick for files that end without a final newline.
    
    Previously, the lexer for glcpp punted in this case by unconditionally
    returning a NEWLINE token at end-of-file, (causing most files to have an extra
    blank line at the end). Here, we refine this by lexing end-of-file as a
    NEWLINE token only if the immediately preceding token was not a NEWLINE token.
    
    The patch is a minor change that only looks huge for two reasons:
    
    	1. Almost all glcpp test result ".expected" files are updated to drop
    	   the extra newline.
    
    	2. All return statements from the lexer are adjusted to use a new
    	   RETURN_TOKEN macro that tracks the last-token-was-a-newline state.
    
    Reviewed-by: Jordan Justen <jordan.l.justen at intel.com>

URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=5dbdc341e8e8502ab5d98784a24b54c6f3907472
Author: Carl Worth <cworth at cworth.org>
Date:   Fri Jun 20 14:28:20 2014 -0700

    glsl/glcpp: Add testing for EOF sans newline (and fix for <DEFINE>, <COMMENT>)
    
    The glcpp implementation has long had code to support a file that ends without
    a final newline. But we didn't have a "make check" test for this.
    
    Additionally, the <EOF> action was restricted only to the <INITIAL> state so
    it would fail to get invoked if the EOF was encountered in the <COMMENT> or
    the <DEFINE> case. Neither of these was a bug, per se, since EOF in either
    of these cases is an error anyway, (either "unterminated comment" or
    "missing macro name for #define").
    
    But with the new explicit support for these cases, we not generate clean error
    messages in these cases, (rather than "unexpected $end" from before).
    
    Reviewed-by: Jordan Justen <jordan.l.justen at intel.com>

URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=21dda50549c5f220eff7ec04a72fb02e5eb09e76
Author: Carl Worth <cworth at cworth.org>
Date:   Fri Jun 20 13:44:51 2014 -0700

    glsl/glcpp: Remove some un-needed calls to NEWLINE_CATCHUP
    
    The NEWLINE_CATCHUP code is only intended to be invoked after we lex an actual
    newline character ('\n'). The two extra calls here were apparently added
    accidentally because the pattern happened to contain a (negated) '\n',
    (see commit 6005e9cb283214cd57038c7c5e7758ba72ec6ac2).
    
    I don't think either case could have caused any actual bug. (In the first
    case, the pattern matched right up to the next newline, so the NEWLINE_CATCHUP
    code was just about to be called. In the second case, I don't think it's
    possible to actually enter the <SKIP> start condition after commented newlines
    without any intervening newline.)
    
    But, if nothing else, the code is cleaner without these extra calls.
    
    Reviewed-by: Jordan Justen <jordan.l.justen at intel.com>

URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=cc335c0e57a92fd78141894350607eca3622465a
Author: Carl Worth <cworth at cworth.org>
Date:   Thu Jun 19 11:57:06 2014 -0700

    glsl/glcpp: Add support for comments between #define and macro identifier
    
    The recent adddition of an error for "#define followed by a non-identifier"
    was a bit to aggressive since it used a regular expression in the lexer to
    flag any character that's not legal as the first character of an identifier.
    
    But we need to allow comments to appear here, (since we aren't removing
    comments in a preliminary pass). So we refine the error here to only flag
    characters that could not be an identifier, nor a comment, nor whitespace.
    
    We also augment the existing comment support to be active in the <DEFINE>
    state as well.
    
    Reviewed-by: Jordan Justen <jordan.l.justen at intel.com>

URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=ea2e9300ece89dffdc4a228dc86bea28a7cd58e2
Author: Carl Worth <cworth at cworth.org>
Date:   Tue Jun 17 11:55:07 2014 -0700

    glsl/glcpp: Emit proper error for #define with a non-identifier
    
    Previously, if the preprocessor encountered a #define with a non-identifier,
    such as:
    
    	#define 123 456
    
    The lexer had no explicit rules to match non-identifiers in the <DEFINE> start
    state. Because of this, flex's default rule was being invoked, (printing
    characters to stdout), and all text was being discarded by the compiler until
    the next identifier. As one can imagine, this led to all sorts of interesting
    and surprising results.
    
    Fix this by adding an explicit rule complementing the existing
    identifier-based rules that should catch all non-identifiers after #define and
    reliably give a well-formatted error message.
    
    A new test is added to "make check" to ensure this bug stays fixed.
    
    This commit also fixes the following Khronos GLES3 CTS test:
    
    	define_non_identifier_vertex
    
    (The "fragment" variant was passing earlier only because the preprocessor was
    behaving so randomly and causing the compilation to fail. It's lucky, in fact,
    that the "vertex" version succesfully compiled so we could find and fix this
    bug.)
    
    Reviewed-by: Jordan Justen <jordan.l.justen at intel.com>

URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=9e45fb6f5127cecd02a1420aaea59a7012e74404
Author: Carl Worth <cworth at cworth.org>
Date:   Mon Jul 28 09:38:30 2014 -0700

    glsl/glcpp: Add testing for directives preceded by a space
    
    This test simply has one of each directive, all of which are preceded by a
    single space character.

URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=da7f226a2711450bf92dcc9e2e1e1052111683ff
Author: Carl Worth <cworth at cworth.org>
Date:   Mon Jul 28 08:59:25 2014 -0700

    glsl/glcpp: Fix to emit spaces following directives
    
    The glcpp lexer and parser use the space_tokens state bit to avoid emitting
    tokens for spaces while parsing a directive. Previously, this bit was only
    being set again by the first non-space token following a directive.
    
    This led to a bug where a space, (or a comment that should emit a space),
    immediately following a directive, (optionally searated by newlines), would be
    omitted from the output.
    
    Here we fix the bug by also setting the space_tokens bit whenever we lex a
    newline in the standard start conditions.




More information about the mesa-commit mailing list