Bug 72273

(In reply to comment #0)
>     "#line must have, after macro substitution, one of the following forms
...
>     where line and source-string-number are constant integer *expressions*."

OK. This should be really easy to fix, (basically just replace "integer_constant"
with "expression" in the grammar and then do a bunch of testing).

I notice that the only other place that "integer_constant" appears in the
current grammar is for #version. Should we be allowing expressions there
as well or does the specification demand a numeric literal for that one?

-Carl

The GLSL spec says:

    #version number profileopt

So, I think integer_constant is fine for #version.

(In reply to comment #0)
> The GLSL spec says (emphasis mine):
> 
>     "#line must have, after macro substitution, one of the following forms:
> 
>         #line line
>         #line line source-string-number
> 
>     where line and source-string-number are constant integer *expressions*."

*Grumble*

That's a deviation from the specification for the preprocessor for C,
where N1256 has two basic forms:

  # line digit-sequence newline
  # line digit-sequence "s-char-sequence-opt" newline

And a third one specified as follows:

  A preprocessing directive of the form

    # line pp-tokens new-line

  (that does not match one of the two previous forms) is permitted. The
  preprocessing tokens after line on the directive are processed just as
  in normal text (each identifier currently defined as a macro name is
  replaced by its replacement list of preprocessing tokens). The directive
  resulting after all replacements shall match one of the two previous forms
  and is then processed as appropriate.

In other words, in C, you can have macro-substitution here, but in the end
you have to have digits and nothing but digits for the line number, (which
is basically what we currently have implemented in Mesa).

Anyway, like I said, it's not hard to change. But I wonder if this deviation
is a GLSL specification bug or is actually intended.

Ian, what prompted the bug report? Are there known shaders in the wild
putting parentheses in #line directives or something?

-Carl

Patches now sent to list:

    glcpp: Allow integer expression for #line directive.
    glcpp: Add testing for #line with integer expression.

-Carl

As discussed on the mailing list, my patches as sent allow an expression
for <line> but not for <source-string-number>.

But allowing expressions for both results in grammar ambiguities, (unless
we did something ugly like disallowing whitespace within these expressions).

I'm not sure what the right answer is, but my vote is to declare the GLSL
specification buggy here and stick with an implementation allowing only
integer literals.

I'll be happy to hear other ideas.

-Carl

Closing as NOTABUG.

The specification needs to be changed here one way or the other.

We can wait for that change before deciding what to change in Mesa, if anything.

-Cal

We have to match the behavior of glslang, which allows expressions.  Other GLSL compilers also allow expressions.

See Khronos bug #11278.

(In reply to comment #7)
> We have to match the behavior of glslang, which allows expressions.  Other
> GLSL compilers also allow expressions.

Can you tell me what that behavior is?

The text of the GLSL specification is ill-specified. Trying to implement it as written leads to a grammar ambiguity.

-Carl

(In reply to comment #9)
> (In reply to comment #7)
> > We have to match the behavior of glslang, which allows expressions.  Other
> > GLSL compilers also allow expressions.
> 
> Can you tell me what that behavior is?

More concretely, how should the following lines be parsed:

#line 1+3

#line 1 +3

#line 1 + 3

-Carl

No other desktop GL vendor supports this, and it doesn't appear that they intend to change.  Given the other problems associated with changing our handling of #line, I'm closing this as WONTFIX.  We can revisit this later if necessary.