You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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.
Resolvesmicrosoft#8965
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?
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
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.
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.
Const methods lose writes to mutable receiver fields
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
As part of the addition of
constmember functions, this PR introduces themutablekeyword.Conservatively I've made it an error to place an object containing a
mutablemember in a constant resource.Resolves #8965