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@intel.com>
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@intel.com>
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@intel.com>
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@intel.com>
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@intel.com>
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@intel.com>
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@intel.com>
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@intel.com>
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@whitecape.org>
Reviewed-by: Carl Worth <cworth@cworth.org>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
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@intel.com>
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@intel.com>
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@intel.com>
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@intel.com>
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@intel.com>
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@intel.com>
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@intel.com>
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@intel.com>
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@intel.com>
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@intel.com>
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 6005e9cb28).
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@intel.com>
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@intel.com>
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@intel.com>
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.
With MESA_EXTENSION_OVERRIDE=GL_ARB_compute_shader, this fixes piglit:
built-in-constants tests/spec/arb_compute_shader/minimum-maximums.txt
Signed-off-by: Jordan Justen <jordan.l.justen@intel.com>
Reviewed-by: Chris Forbes <chrisf@ijw.co.nz>
Instead of falling back to just the block name (which we won't find),
look for the first element of the block array. We'll deal with the rest
in the backend by arranging for the blocks to be laid out contiguously.
V2: Squashed together patches 3, 5 of V1, plus a naming tweak.
Signed-off-by: Chris Forbes <chrisf@ijw.co.nz>
Reviewed-by: Ilia Mirkin <imirkin@alum.mit.edu>
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Previously this was a block index with special semantics for -1.
With ARB_gpu_shader5, this need not be a compile-time constant, so
allow any rvalue here and convert the -1 to a NULL pointer.
Signed-off-by: Chris Forbes <chrisf@ijw.co.nz>
Reviewed-by: Ilia Mirkin <imirkin@alum.mit.edu>
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Without doing a lot more work, we have no idea which indices may
be used at runtime, so just mark them all.
Signed-off-by: Chris Forbes <chrisf@ijw.co.nz>
Reviewed-by: Ilia Mirkin <imirkin@alum.mit.edu>
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
This is needed to make Unigine Heaven 4.0 and Unigine Valley 1.0 work
with sample shading.
Also, if this is disabled, the error message at least makes sense now.
Reviewed-by: Brian Paul <brianp@vmware.com>
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
This fixes following tests in es3conform:
shaders.switch.default_not_last_dynamic_vertex
shaders.switch.default_not_last_dynamic_fragment
and makes following tests in Piglit pass:
glsl-1.30/execution/switch/fs-default-notlast-fallthrough
glsl-1.30/execution/switch/fs-default_notlast
No Piglit regressions.
v2: take away unnecessary ir_if, just use conditional assignment
v3: use foreach_in_list instead of foreach_list
Signed-off-by: Tapani Pälli <tapani.palli@intel.com>
Reviewed-by: Roland Scheidegger <sroland@vmware.com> (v2)
Reviewed-by: Ian Romanick <ian.d.romanick@intel.com> (v3)
components() includes matrix columns, so if this code encountered a
matrix, it would ask for something like a vec9 or vec16. This is
clearly not what you want.
Earlier code now prevents this from seeing matrices, but we should still
use vector_elements, for clarity.
Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
It doesn't handle things like (vector * matrix) correctly, and
apparently Matt's intention was to bail.
Fixes shader compilation in Natural Selection 2.
Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
Instead of hand-rolling it.
v2 [mattst88]: Rename get_size to length. Expand comment in ir_reader.
Reviewed-by: Ian Romanick <ian.d.romanick@intel.com> [v1]
Reviewed-by: Matt Turner <mattst88@gmail.com>
Signed-off-by: Connor Abbott <connor.abbott@intel.com>
v2 [mattst88]: Remove trailing whitespace. Rename get_size to length.
Mark as const.
Reviewed-by: Ian Romanick <ian.d.romanick@intel.com> [v1]
Reviewed-by: Matt Turner <mattst88@gmail.com>
Signed-off-by: Connor Abbott <connor.abbott@intel.com>
This complements the existing append function. It's implemented in a
rather simple way right now; it could be changed if performance is a
concern.
Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>
Reviewed-by: Matt Turner <mattst88@gmail.com>
Signed-off-by: Connor Abbott <connor.abbott@intel.com>
If we saw a tree that looked like
vec3
/ \
vec3 float
/ \
vec3 float
/ \
vec3 float
We would see that all of the expression types were vec3, and then
rebalance to
vec3
/ \
vec3 vec3 <-- should be float
/ \ / \
vec3 float float float
This patch adds code to visit the rebalanced tree and update the
expression types from the bottom up.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=80880
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Vectors are falling in to the ir_dereference_array() path.
Without this change, the following glsl aborts the debug driver,
or gets the wrong answer in release:
mat2x2 a = mat2( vec2( 1.0, vertex.x ), vec2( 0.0, 1.0 ) );
Also submitting piglit tests, will reference in bug.
v2: Rebase on Mesa master.
v3: Remove unneeded check for arrays, which are covered by
process_array_constructor(), recommended by Timothy Arceri.
Signed-off-by: Cody Northrop <cody@lunarg.com>
Reviewed-by: Courtney Goeltzenleuchter <courtney@lunarg.com>
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=79373
V2: - Don't assume everyone wants interpolateAtSample() lowered to
interpolateAtOffset. It turns out this isn't what we want most
of the time for i965. Lowering can be added later in an ir pass
which drivers opt into, rather than bolting it straight into the
builtin definition.
- Only expose the interpolateAt* builtins in the fragment language.
Signed-off-by: Chris Forbes <chrisf@ijw.co.nz>
Reviewed-by: Ilia Mirkin <imirkin@alum.mit.edu>
Will be used to implement interpolateAt*() from ARB_gpu_shader5
Signed-off-by: Chris Forbes <chrisf@ijw.co.nz>
Reviewed-by: Ilia Mirkin <imirkin@alum.mit.edu>
The new interpolateAt* builtins have strange restrictions on the
<interpolant> parameter.
- It must be a shader input, or an element of a shader input array.
- It must not include a swizzle.
V2: Don't abuse ir_var_mode_shader_in for this; make a new flag.
Signed-off-by: Chris Forbes <chrisf@ijw.co.nz>
Reviewed-by: Ilia Mirkin <imirkin@alum.mit.edu>
The lexer was insisting that there be at least one character after "#pragma"
and before the end of the line. This caused an error for a line consisting
only of "#pragma" which volates at least the following sentence from the GLSL
ES Specification 3.00.4:
The scope as well as the effect of the optimize and debug pragmas is
implementation-dependent except that their use must not generate an
error. [Page 12 (Page 28 of PDF)]
and likely the following sentence from that specification and also in
GLSLangSpec 4.30.6:
If an implementation does not recognize the tokens following #pragma,
then it will ignore that pragma.
Add a "make check" test to ensure no future regressions.
This change fixes at least part of the following Khronos GLES3 CTS test:
preprocessor.pragmas.pragma_vertex
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
