From f062f0506a5b827667b7eb52136d8420b7e8113b Mon Sep 17 00:00:00 2001 From: Carl Worth Date: Wed, 25 Jun 2014 12:20:22 -0700 Subject: 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 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 --- src/glsl/glcpp/tests/128-space-before-hash.c.expected | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src/glsl/glcpp/tests/128-space-before-hash.c.expected') diff --git a/src/glsl/glcpp/tests/128-space-before-hash.c.expected b/src/glsl/glcpp/tests/128-space-before-hash.c.expected index 5d44f4161eb..9babb6fb078 100644 --- a/src/glsl/glcpp/tests/128-space-before-hash.c.expected +++ b/src/glsl/glcpp/tests/128-space-before-hash.c.expected @@ -1,6 +1,6 @@ - + #version 300 - #pragma Testing spaces before hash +#pragma Testing spaces before hash #line 3 -- cgit v1.2.3