Bug 72273 - GLSL preprocessor does not support expressions in #line
Summary: GLSL preprocessor does not support expressions in #line
Status: RESOLVED WONTFIX
Alias: None
Product: Mesa
Classification: Unclassified
Component: glsl-compiler (show other bugs)
Version: git
Hardware: Other All
: medium normal
Assignee: Carl Worth
QA Contact: Intel 3D Bugs Mailing List
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-12-03 17:50 UTC by Ian Romanick
Modified: 2014-04-27 18:32 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments

Description Ian Romanick 2013-12-03 17:50:58 UTC
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*."

However, Mesa's preprocessor rejects shaders like:

#line 20
#line (20)
#line (10+10)
#line +20
Comment 1 Carl Worth 2014-01-28 19:19:40 UTC
(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
Comment 2 Ian Romanick 2014-01-28 20:55:55 UTC
The GLSL spec says:

    #version number profileopt

So, I think integer_constant is fine for #version.
Comment 3 Carl Worth 2014-01-29 01:40:57 UTC
(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
Comment 4 Carl Worth 2014-01-29 01:57:20 UTC
Patches now sent to list:

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

-Carl
Comment 5 Carl Worth 2014-01-29 22:32:58 UTC
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
Comment 6 Carl Worth 2014-01-31 18:07:05 UTC
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
Comment 7 Ian Romanick 2014-02-01 03:48:49 UTC
We have to match the behavior of glslang, which allows expressions.  Other GLSL compilers also allow expressions.
Comment 8 Ian Romanick 2014-02-01 03:53:20 UTC
See Khronos bug #11278.
Comment 9 Carl Worth 2014-02-01 18:18:40 UTC
(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
Comment 10 Carl Worth 2014-02-01 18:57:08 UTC
(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
Comment 11 Ian Romanick 2014-04-27 18:32:25 UTC
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.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.