Index: clang/docs/ReleaseNotes.rst =================================================================== --- clang/docs/ReleaseNotes.rst +++ clang/docs/ReleaseNotes.rst @@ -149,6 +149,12 @@ because there is no way to fully qualify the enumerator name, so this "extension" was unintentional and useless. This fixes `Issue 42372 `_. +- When forming a member expression, now consider any qualifiers written on an + anonymous structure or union as also applying to the field being referenced. + This fixes an issue where qualifiers were being ignored, allowing you to + assign to a ``const`` field. Note that qualifiers are ignored in C++ and for + Microsoft's extension of anonymous objects. This fixes + `Issues 48099 `_. Improvements to Clang's diagnostics ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Index: clang/include/clang/Basic/DiagnosticSemaKinds.td =================================================================== --- clang/include/clang/Basic/DiagnosticSemaKinds.td +++ clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -7010,6 +7010,7 @@ "cannot assign to non-static data member within const member function %1|" "cannot assign to %select{variable %2|non-static data member %2|lvalue}1 " "with %select{|nested }3const-qualified data member %4|" + "|" // Unused for anonymous objects "read-only variable is not assignable}0">; def note_typecheck_assign_const : Note< @@ -7018,7 +7019,8 @@ "variable %1 declared const here|" "%select{non-|}1static data member %2 declared const here|" "member function %q1 is declared const here|" - "%select{|nested }1data member %2 declared const here}0">; + "%select{|nested }1data member %2 declared const here|" + "anonymous %select{union|struct}1 declared const here}0">; def warn_unsigned_always_true_comparison : Warning< "result of comparison of %select{%3|unsigned expression}0 %2 " Index: clang/lib/Sema/SemaDecl.cpp =================================================================== --- clang/lib/Sema/SemaDecl.cpp +++ clang/lib/Sema/SemaDecl.cpp @@ -5498,7 +5498,10 @@ if (RecordDecl *OwningClass = dyn_cast(Owner)) { Anon = FieldDecl::Create( Context, OwningClass, DS.getBeginLoc(), Record->getLocation(), - /*IdentifierInfo=*/nullptr, Context.getTypeDeclType(Record), TInfo, + /*IdentifierInfo=*/nullptr, + Context.getTypeDeclType(Record).withFastQualifiers( + DS.getTypeQualifiers()), + TInfo, /*BitWidth=*/nullptr, /*Mutable=*/false, /*InitStyle=*/ICIS_NoInit); Anon->setAccess(AS); @@ -5520,7 +5523,9 @@ assert(DS.getAttributes().empty() && "No attribute expected"); Anon = VarDecl::Create(Context, Owner, DS.getBeginLoc(), Record->getLocation(), /*IdentifierInfo=*/nullptr, - Context.getTypeDeclType(Record), TInfo, SC); + Context.getTypeDeclType(Record).withFastQualifiers( + DS.getTypeQualifiers()), + TInfo, SC); // Default-initialize the implicit variable. This initialization will be // trivial in almost all cases, except if a union member has an in-class Index: clang/lib/Sema/SemaExpr.cpp =================================================================== --- clang/lib/Sema/SemaExpr.cpp +++ clang/lib/Sema/SemaExpr.cpp @@ -13351,6 +13351,7 @@ ConstMember, ConstMethod, NestedConstMember, + ConstAnonymousObject, // Only used as a note ConstUnknown, // Keep as last element }; @@ -13370,6 +13371,10 @@ // next checked expression is the result of a dereference. bool IsDereference = false; bool NextIsDereference = false; + // Used when diagnosing indirect fields that are const because the anonymous + // object is const rather than the field. This allows us to diagnose the + // field rather than the anonymous object. + const FieldDecl *LastNamedField = nullptr; // Loop to process MemberExpr chains. while (true) { @@ -13388,16 +13393,44 @@ if (!IsTypeModifiable(Field->getType(), IsDereference)) { if (!DiagnosticEmitted) { - S.Diag(Loc, diag::err_typecheck_assign_const) - << ExprRange << ConstMember << false /*static*/ << Field - << Field->getType(); + // If the field is an anonymous union or structure, we want to + // diagnose the last named field rather than the unnamed object. + // This requires us to combine the qualifiers from the anonymous + // object with those of the field in order for the diagnostic to + // make sense. + if (Field->isAnonymousStructOrUnion()) { + assert(LastNamedField && + "we should have seen a named field by this point"); + S.Diag(Loc, diag::err_typecheck_assign_const) + << ExprRange << ConstMember + << false /*static*/ << LastNamedField + << LastNamedField->getType().withFastQualifiers( + Field->getType().getLocalFastQualifiers()); + } else { + S.Diag(Loc, diag::err_typecheck_assign_const) + << ExprRange << ConstMember << false /*static*/ << Field + << Field->getType(); + } DiagnosticEmitted = true; } - S.Diag(VD->getLocation(), diag::note_typecheck_assign_const) - << ConstMember << false /*static*/ << Field << Field->getType() - << Field->getSourceRange(); + // If the field is an anonymous union or structure, we want the note + // to be about the anonymous object being const rather than the + // indirect field. + if (Field->isAnonymousStructOrUnion()) { + S.Diag(VD->getLocation(), diag::note_typecheck_assign_const) + << ConstAnonymousObject + << Field->getType() + ->castAs() + ->getDecl() + ->isStruct(); + } else { + S.Diag(VD->getLocation(), diag::note_typecheck_assign_const) + << ConstMember << false /*static*/ << Field << Field->getType() + << Field->getSourceRange(); + } } E = ME->getBase(); + LastNamedField = Field; continue; } else if (const VarDecl *VDecl = dyn_cast(VD)) { if (VDecl->getType().isConstQualified()) { Index: clang/test/Sema/anonymous-struct-union.c =================================================================== --- clang/test/Sema/anonymous-struct-union.c +++ clang/test/Sema/anonymous-struct-union.c @@ -119,3 +119,26 @@ struct s3 s; s.A = 1; // expected-warning {{'A' is deprecated}} } + +struct GH48099 { + const struct { // expected-note {{anonymous struct declared const here}} + int i; + }; + volatile union { + int j; + }; +}; + +// Ensure that qualifiers from the anonymous object are reflected on the +// fields being hoisted into the outer context. +void GH48099_test(void) { + struct GH48099 x; + + // It's the access path that picks up the qualifiers, not the direct + // declaration of the field itself. So 'i' and 'j' are both 'int'. + _Static_assert(_Generic(x.i, int : 1, default : 0), "i is not int?"); + _Static_assert(_Generic(x.j, int : 1, default : 0), "j is not int?"); + + // Test that the member access picks up the qualifiers. + x.i = 12; // expected-error {{cannot assign to non-static data member 'i' with const-qualified type 'const int'}} +} Index: clang/test/Sema/anonymous-struct-union.cpp =================================================================== --- /dev/null +++ clang/test/Sema/anonymous-struct-union.cpp @@ -0,0 +1,34 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s +// RUN: %clang_cc1 -fsyntax-only -verify -fms-extensions -Wno-microsoft-anon-tag -x c %s +// expected-no-diagnostics + +struct trivial { + int a; +}; + +struct GH48099 { + const struct { + int i; + }; +#ifndef __cplusplus + // This extension is only allowed in C. + const struct trivial; +#endif +}; + +// In C++, qualifiers are ignored on the anonymous objects. +void GH48099_test(void) { + struct GH48099 x; + +#ifdef __cplusplus + // Test that the member access ignores the qualifiers. This would fail in C. + x.i = 12; // ok +#else + // Test that the member access ignores the qualifiers for a Microsoft C + // anonymous structure declaration. This would fail in C++ because the + // declaration in GH48099 does not declare anything (so 'a' isn't lifted into + // the outer structure). + x.a = 12; // ok +#endif +} +