Skip to content

Fix GLSL global const diagnostic regression#7808

Merged
csyonghe merged 2 commits intomasterfrom
copilot/fix-7807
Jul 18, 2025
Merged

Fix GLSL global const diagnostic regression#7808
csyonghe merged 2 commits intomasterfrom
copilot/fix-7807

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Jul 17, 2025

This PR fixes a regression introduced by PR #7711 where the diagnostic for global const variables with initializers was incorrectly triggering in GLSL mode.

Problem

In GLSL mode, global non-static const variables represent real constants, not uniform parameters. However, the diagnostic added in PR #7711 was treating them the same as HLSL, causing false positive errors when compiling valid GLSL code:

// This should be valid in GLSL mode but was incorrectly flagging error 31224
const float PI = 3.14159;
const vec3 GRAVITY = vec3(0.0, -9.8, 0.0);

Solution

Added a condition to exclude GLSL modules from the diagnostic check in slang-check-decl.cpp:

// Before: Always diagnosed global const with initializers
if (isGlobalDecl(decl) && (hasConst || hasUniform) && !hasStatic &&
    !hasSpecializationConstant && decl->initExpr)

// After: Skip diagnostic for GLSL modules  
auto moduleDecl = getModuleDecl(decl);
if (!moduleDecl || !moduleDecl->hasModifier<GLSLModuleModifier>())

Testing

Added comprehensive test coverage:

  • tests/diagnostics/glsl-global-const-with-init.slang - Verifies GLSL mode allows global const with initializers
  • tests/diagnostics/hlsl-global-const-with-init-still-errors.slang - Ensures HLSL mode still produces the diagnostic

All existing diagnostic tests continue to pass (223/223), confirming no regressions.

Fixes #7807.


💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

@CLAassistant
Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@csyonghe csyonghe added pr: non-breaking PRs without breaking changes CoPilot labels Jul 17, 2025
…GLSL module

Co-authored-by: csyonghe <2652293+csyonghe@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix regression of global const diagnostic regression in GLSL mode. Fix GLSL global const diagnostic regression Jul 17, 2025
Copilot AI requested a review from csyonghe July 17, 2025 16:25
Copy link
Copy Markdown
Collaborator

@jkwak-work jkwak-work left a comment

Choose a reason for hiding this comment

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

Wow. This looks good to me.

@csyonghe csyonghe marked this pull request as ready for review July 17, 2025 23:32
@csyonghe csyonghe requested a review from a team as a code owner July 17, 2025 23:32
@csyonghe csyonghe enabled auto-merge July 17, 2025 23:32
@csyonghe csyonghe added this pull request to the merge queue Jul 17, 2025
Merged via the queue into master with commit 447d73f Jul 18, 2025
23 of 34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CoPilot pr: non-breaking PRs without breaking changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix regression of global const diagnostic regression in GLSL mode.

4 participants