8000 Add EXT_abort by r-potter · Pull Request #317 · KhronosGroup/GLSL · GitHub
[go: up one dir, main page]
Skip to content

Add EXT_abort - #317

Open
r-potter wants to merge 2 commits into
KhronosGroup:mainfrom
r-potter:EXT_abort
Open

Add EXT_abort#317
r-potter wants to merge 2 commits into
KhronosGroup:mainfrom
r-potter:EXT_abort

Conversation

@r-potter
@r-potter r-potter commented Mar 23, 2026
Copy link
Copy Markdown

cc @gnl21

@arcady-lunarg
arcady-lunarg commented Jun 24, 2026
Copy link
Copy Markdown

Can this be merged? It's already implemented in glslang.

@rdb
rdb commented Jun 28, 2026
Copy link
Copy Markdown

One grammar question: like GL_EXT_debug_printf, this extension doesn't actually define a syntactic path to allow a literal string in a function-call argument. The wording ("the first argument must be a literal string") suggests the string is a special argument rather than an expression; yet glslang parses strings as a primary-expression, so abortEXT(("foo")) is accepted by glslang even though nothing in this extension suggests that it should be.

So: what is actually intended here? I would like to know so that so that the grammar I have proposed in GL_EXT_spirv_intrinsics_string (#333) can be kept consistent with what GL_EXT_abort and GL_EXT_debug_printf do. Adding an explicit production to the extension text may be useful to remove any ambiguity.

Also, minor typo: "Last Modified Date" mentions "September 31, 2024", even though September has only 30 days.

@jeffbolznv
jeffbolznv commented Aug 26, 2026
Copy link
Copy Markdown
Contributor

I think you're right that there is a missing edit to the grammar, that ("foo") should not be treated as a literal string, and that glslang is missing validation for this for debugPrintf (and presumably also for abort, but I haven't looked).

r-potter
r-potter commented Sep 1, 2026
View reviewed changes
Comment thread extensions/ext/GLSL_EXT_abort.txt Resolved
@HansKristian-Work
HansKristian-Work commented Sep 17, 2026
Copy link
Copy Markdown

Wasn't this okay to merge now?

@r-potter
r-potter commented Sep 17, 2026
Copy link
Copy Markdown
Author

It should be, yes. It was blocked on the literal string question, but I implemented that for both debug printf and abort

@rdb
rdb commented Sep 17, 2026
Copy link
Copy Markdown

There are no grammar productions listed yet, is that intentional? As-is there's still no grammar rule to enable passing a literal string as an argument to a function.

@rdb
rdb commented Sep 17, 2026
Copy link
Copy Markdown

I would suggest something like this. If that is considered unnecessary if the existence of such a rule can be inferred from the prose, then never mind.

Additions to Chapter 9 of the OpenGL Shading Language Specification
(Shading Language Grammar)

    Add the following rule:

        function_call_argument:
            assignment_expression
            STRING_LITERAL

    Replace the "function_call_header_with_parameters" rule as follows:

        function_call_header_with_parameters:
            function_call_header function_call_argument
            function_call_header_with_parameters COMMA function_call_argument

@r-potter
r-potter commented Sep 17, 2026
Copy link
Copy Markdown
Author

I would have no objections to doing this, although if we do then we should probably also do so for printf.

@gnl21 @jeffbolznv Do you have opinions on this one?

@gnl21
gnl21 commented Sep 17, 2026
Copy link
Copy Markdown
Contributor

I agree that it would good to add something like @rdb suggests. I was expecting to see something like that in response to Jeff's comment.

@gnl21
gnl21 commented Sep 17, 2026
Copy link
Copy Markdown
Contributor

Since we now have 3 extensions that need the same thing, maybe we should update the core grammar with this so that all the extensions can refer to it. I think (but I'd have to check) that this wouldn't be a problem for the unextended spec if STRING_LITERAL were unconditionally banned.

r-potter
r-potter commented Sep 17, 2026
View reviewed changes
Comment thread extensions/ext/GLSL_EXT_abort.txt
implementation.
The set of format specifiers is implementation-dependent, but must
include at least "%d" and "%i" (int), "%u" (uint), and "%f" (float).

@r-potter r-potter Sep 17, 2026
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leaving this here for now, although I like @gnl21's suggestion of a core grammar rule as an alternative:

Suggested change
8000
Add the following grammar rule to chapter 9:
function_call_argument:
assignment_expression
STRING_LITERAL
Replace the "function_call_header_with_parameters" rule as follows:
function_call_header_with_parameters:
function_call_header function_call_argument
function_call_header_with_parameters COMMA function_call_argument

@r-potter
r-potter commented Sep 17, 2026
Copy link
Copy Markdown
Author

Since we now have 3 extensions that need the same thing, maybe we should update the core grammar with this so that all the extensions can refer to it. I think (but I'd have to check) that this wouldn't be a problem for the unextended spec if STRING_LITERAL were unconditionally banned.

Would the proposal here be:

  • Add the change to the core grammar
  • Explicitly ban it in 6.1 Function Definitions so that user defined functions can't do the same thing

Happy to make the change, just need to know exactly what we want

@spencer-lunarg
spencer-lunarg commented Nov 4, 2026
Copy link
Copy Markdown
Contributor

any update on this? I forgot this wasn't merged yet

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@r-potter @arcady-lunarg @rdb @jeffbolznv @HansKristian-Work @gnl21 @spencer-lunarg
0