Pewarta Proxy

📋 Server Request & Proxy Logs

Loading active proxy settings...
Developed by Khairul Adha (adhexa.id)
Mengambil log server...

⚙️ Pengaturan Proxy Target

Skip to content

[202x][0015] Implement mutable keyword - #9007

Open
Chris B (llvm-beanz) wants to merge 5 commits into
microsoft:mainfrom
llvm-beanz:mutable
Open

Chris B (llvm-beanz) wants to merge 5 commits into
microsoft:mainfrom
llvm-beanz:mutable

Conversation

@llvm-beanz

Copy link
Copy Markdown
Collaborator

As part of the addition of const member functions, this PR introduces the mutable keyword.

Conservatively I've made it an error to place an object containing a mutable member in a constant resource.

Resolves #8965

As part of the addition of `const` member functions, this PR introduces
the `mutable` keyword.

Conservatively I've made it an error to place an object containing a
`mutable` member in a constant resource.

Resolves microsoft#8965
Copilot AI balanced review requested due to automatic review settings October 6, 2026 16:14

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

The new diagnostic incorrectly identifies texture-buffer failures as constant-buffer failures.

Review effort: Balanced
Findings: 1 Low severity

Open (1)
What changed in this PR

Adds HLSL 202x support for mutable struct members, including const-method mutation and constant-buffer restrictions.

Changes:

  • Enables and parses mutable under HLSL 202x.
  • Rejects mutable-containing types in constant and texture buffers.
  • Adds semantic, AST, code-generation, and compatibility tests.
File Description
tools/​clang/​include/​clang/​Basic/​TokenKinds.def Enables the HLSL 202x keyword.
tools/​clang/​lib/​Parse/​ParseDecl.cpp Gates parsing by HLSL version.
tools/​clang/​lib/​Sema/​SemaHLSL.cpp Rejects mutable fields in constant resources.
tools/​clang/​include/​clang/​Basic/​DiagnosticSemaKinds.td Adds the buffer-placement diagnostic.
tools/​clang/​test/​SemaHLSL/​hlsl/​classes/​mutable_202x_errors.hlsl Tests invalid declarations and resource placement.
tools/​clang/​test/​SemaHLSL/​hlsl/​classes/​const_method_pre202x.hlsl Tests pre-202x rejection.
tools/​clang/​test/​SemaHLSL/​hlsl/​classes/​const_method_202x.hlsl Verifies mutable AST representation.
tools/​clang/​test/​SemaHLSL/​hlsl/​classes/​const_method_202x_this_const.hlsl Tests mutation from const methods.
tools/​clang/​test/​HLSLFileCheck/​hlsl/​classes/​const_method_202x_codegen.hlsl Verifies generated DXIL behavior.
docs/​ReleaseNotes.md Documents the language feature.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread tools/clang/include/clang/Basic/DiagnosticSemaKinds.td Outdated
Copilot AI balanced review requested due to automatic review settings October 6, 2026 17:20

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟢 Approval recommended

The implementation is version-gated, uses established C++ mutable semantics, and includes comprehensive diagnostics and regression coverage.

Review effort: Balanced
Findings: 1 Low severity

Open (1)

@hekota Helena Kotas (hekota) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM!

TextureBuffer<MutableData> textureBuffer; // expected-error {{'mutable' field 'value' is not allowed in constant buffer data}}

groupshared MutableData sharedData;
RWStructuredBuffer<MutableData> structuredBuffer; No newline at end of file

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nit - missing new line

Copilot AI balanced review requested due to automatic review settings October 6, 2026 19:37

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Read-only StructuredBuffer payloads can still contain mutable fields and bypass SRV constness.

Review effort: Balanced
Findings: 1 High severity · 1 Low severity

Open (2)

Comment thread tools/clang/lib/Sema/SemaHLSL.cpp Outdated

@bob80905 Joshua Batista (bob80905) left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Comments from Alex Sepkowski (@alsepkow) and myself

TextureBuffer<MutableData> textureBuffer; // expected-error {{'mutable' field 'value' is not allowed in constant buffer data}}

groupshared MutableData sharedData;
RWStructuredBuffer<MutableData> structuredBuffer;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Like Copilot recommends, it would be good to see / define what happens when StructuredBuffer is used instead.

NestedMutableData data; // expected-error {{'mutable' field 'value' is not allowed in constant buffer data}}
};

MutableData implicitCBufferData; // expected-error {{'mutable' field 'value' is not allowed in constant buffer data}}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This and the below diagnostic are misleading since there are no constant buffers associated with these statements. Should we broaden the diagnostic text?

Copilot AI balanced review requested due to automatic review settings October 9, 2026 15:24

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

The change crosses parser, semantic analysis, resource validation, and code generation paths and warrants final maintainer review.

0 open findings

2 resolved since last review

🧠 Review effort: Balanced

tools/clang/include/clang/Basic/DiagnosticSemaKinds.td
Copilot AI balanced review requested due to automatic review settings October 15, 2026 15:00

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

Mutable writes can be lost, const locals can incorrectly share storage, and template element types can bypass read-only buffer validation.

0 open findings

Previously missed (3)

In code that hasn't changed since last review

Medium severity Mutable fields cause unsafe static promotion of const locals

tools/​clang/​lib/​Parse/​ParseDecl.cpp:3957

Enabling mutable makes the local-const promotion in CGDecl.cpp:145–152 unsafe. For struct M { mutable int value; };, the local in int f() { const M m = {0}; return ++m.value; } takes the EmitStaticVarDecl path. Repeated calls then share storage instead of reinitializing m on each call. Exclude types containing mutable fields from this promotion, using the existing CGM.isTypeConstant check where appropriate, and add a regression that calls the function twice and expects 1 both times.

Medium severity Read-only resource validation misses mutable fields in new specializations

tools/​clang/​lib/​Sema/​SemaHLSL.cpp:5926

The read-only restriction misses previously uninstantiated element specializations, such as template<class T> struct Data { mutable T value; }; StructuredBuffer<Data<int>> buf;. Unlike the ConstantBuffer branch, this path does not require a complete element type before traversal, so DiagnoseElementTypes sees no fields and accepts it. Later global validation stops at the resource object rather than revisiting its element. Complete non-dependent element types before walking their fields, or defer validation until completion, and add a regression where the buffer is the specialization's first use.

Medium severity Const methods lose writes to mutable receiver fields

tools/​clang/​test/​HLSLFileCheck/​hlsl/​classes/​const_method_202x_codegen.hlsl:13

Mutable writes do not survive this const-method call. CGHLSLMS.cpp:1918–1921 marks every const method's this as In, so HLLegalizeParameter::patchWriteOnInParam redirects stores to a temporary without copying them back before inlining. With cache initialized to 0, ls.incrementCache(); return ls.cache; therefore returns 0 instead of 1. Preserve write-back for receivers containing mutable fields, including nested and base-class fields, and extend this test to observe the receiver after the call. Checking only the method's return value hides the lost update.

🧠 Review effort: Balanced

This branch has not been deployed

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

Labels

None yet

Projects

Status: New

Development

Successfully merging this pull request may close these issues.

[202x][0015] Add mutable keyword

4 participants