diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -306,6 +306,8 @@ ``-Wunused-label`` warning. - Implemented `WG14 N2508 `_, so labels can placed everywhere inside a compound statement. +- Reject type definitions in the ``type`` argument of ``__builtin_offsetof`` + according to `WG14 N2350 `_. C++ Language Changes in Clang ----------------------------- diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -1646,6 +1646,8 @@ "%0 cannot be defined in a condition">; def err_type_defined_in_enum : Error< "%0 cannot be defined in an enumeration">; +def err_type_defined_in_offsetof : Error< + "%0 cannot be defined in '%select{__builtin_offsetof|offsetof}1'">; def note_pure_virtual_function : Note< "unimplemented pure virtual method %0 in %1">; diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h --- a/clang/include/clang/Parse/Parser.h +++ b/clang/include/clang/Parse/Parser.h @@ -61,6 +61,7 @@ friend class ColonProtectionRAIIObject; friend class ParsingOpenMPDirectiveRAII; friend class InMessageExpressionRAIIObject; + friend class OffsetOfStateRAIIObject; friend class PoisonSEHIdentifiersRAIIObject; friend class ObjCDeclContextSwitch; friend class ParenBraceBracketBalancer; @@ -247,6 +248,8 @@ /// function call. bool CalledSignatureHelp = false; + Sema::OffsetOfKind OffsetOfState = Sema::OffsetOfKind::OOK_Outside; + /// The "depth" of the template parameters currently being parsed. unsigned TemplateParameterDepth; diff --git a/clang/include/clang/Parse/RAIIObjectsForParser.h b/clang/include/clang/Parse/RAIIObjectsForParser.h --- a/clang/include/clang/Parse/RAIIObjectsForParser.h +++ b/clang/include/clang/Parse/RAIIObjectsForParser.h @@ -341,6 +341,19 @@ } }; + class OffsetOfStateRAIIObject { + Sema::OffsetOfKind &OffsetOfState; + Sema::OffsetOfKind OldValue; + + public: + OffsetOfStateRAIIObject(Parser &P, Sema::OffsetOfKind Value) + : OffsetOfState(P.OffsetOfState), OldValue(P.OffsetOfState) { + OffsetOfState = Value; + } + + ~OffsetOfStateRAIIObject() { OffsetOfState = OldValue; } + }; + /// RAII object that makes sure paren/bracket/brace count is correct /// after declaration/statement parsing, even when there's a parsing error. class ParenBraceBracketBalancer { diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -3272,6 +3272,16 @@ TUK_Friend // Friend declaration: 'friend struct foo;' }; + enum OffsetOfKind { + // Not parsing a type within __builtin_offsetof. + OOK_Outside, + // Parsing a type within __builtin_offsetof. + OOK_Builtin, + // Parsing a type within macro "offsetof", defined in __buitin_offsetof + // To improve our diagnostic message. + OOK_Macro, + }; + Decl *ActOnTag(Scope *S, unsigned TagSpec, TagUseKind TUK, SourceLocation KWLoc, CXXScopeSpec &SS, IdentifierInfo *Name, SourceLocation NameLoc, const ParsedAttributesView &Attr, @@ -3280,7 +3290,7 @@ bool &IsDependent, SourceLocation ScopedEnumKWLoc, bool ScopedEnumUsesClassTag, TypeResult UnderlyingType, bool IsTypeSpecifier, bool IsTemplateParamOrArg, - SkipBodyInfo *SkipBody = nullptr); + OffsetOfKind OOK, SkipBodyInfo *SkipBody = nullptr); Decl *ActOnTemplatedFriendTag(Scope *S, SourceLocation FriendLoc, unsigned TagSpec, SourceLocation TagLoc, diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp --- a/clang/lib/Parse/ParseDecl.cpp +++ b/clang/lib/Parse/ParseDecl.cpp @@ -4891,7 +4891,7 @@ DSC == DeclSpecContext::DSC_type_specifier, DSC == DeclSpecContext::DSC_template_param || DSC == DeclSpecContext::DSC_template_type_arg, - &SkipBody); + OffsetOfState, &SkipBody); if (SkipBody.ShouldSkip) { assert(TUK == Sema::TUK_Definition && "can only skip a definition"); diff --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp --- a/clang/lib/Parse/ParseDeclCXX.cpp +++ b/clang/lib/Parse/ParseDeclCXX.cpp @@ -2030,7 +2030,7 @@ DSC == DeclSpecContext::DSC_type_specifier, DSC == DeclSpecContext::DSC_template_param || DSC == DeclSpecContext::DSC_template_type_arg, - &SkipBody); + OffsetOfState, &SkipBody); // If ActOnTag said the type was dependent, try again with the // less common call. diff --git a/clang/lib/Parse/ParseExpr.cpp b/clang/lib/Parse/ParseExpr.cpp --- a/clang/lib/Parse/ParseExpr.cpp +++ b/clang/lib/Parse/ParseExpr.cpp @@ -2580,6 +2580,14 @@ } case tok::kw___builtin_offsetof: { SourceLocation TypeLoc = Tok.getLocation(); + auto K = Sema::OffsetOfKind::OOK_Builtin; + if (Tok.getLocation().isMacroID()) { + StringRef MacroName = Lexer::getImmediateMacroNameForDiagnostics( + Tok.getLocation(), PP.getSourceManager(), getLangOpts()); + if (MacroName == "offsetof") + K = Sema::OffsetOfKind::OOK_Macro; + } + OffsetOfStateRAIIObject InOffsetof(*this, K); TypeResult Ty = ParseTypeName(); if (Ty.isInvalid()) { SkipUntil(tok::r_paren, StopAtSemi); diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -16266,7 +16266,7 @@ SourceLocation ScopedEnumKWLoc, bool ScopedEnumUsesClassTag, TypeResult UnderlyingType, bool IsTypeSpecifier, bool IsTemplateParamOrArg, - SkipBodyInfo *SkipBody) { + OffsetOfKind OOK, SkipBodyInfo *SkipBody) { // If this is not a definition, it must have a name. IdentifierInfo *OrigName = Name; assert((Name != nullptr || TUK == TUK_Definition) && @@ -17039,10 +17039,16 @@ cast_or_null(PrevDecl)); } + if (OOK != OOK_Outside && TUK == TUK_Definition) { + Diag(New->getLocation(), diag::err_type_defined_in_offsetof) + << New << static_cast(OOK == OOK_Macro); + Invalid = true; + } + // C++11 [dcl.type]p3: // A type-specifier-seq shall not define a class or enumeration [...]. - if (getLangOpts().CPlusPlus && (IsTypeSpecifier || IsTemplateParamOrArg) && - TUK == TUK_Definition) { + if (!Invalid && getLangOpts().CPlusPlus && + (IsTypeSpecifier || IsTemplateParamOrArg) && TUK == TUK_Definition) { Diag(New->getLocation(), diag::err_type_defined_in_type_specifier) << Context.getTagDeclType(New); Invalid = true; diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -16922,15 +16922,15 @@ if (SS.isEmpty()) { bool Owned = false; bool IsDependent = false; - return ActOnTag(S, TagSpec, TUK_Friend, TagLoc, SS, Name, NameLoc, - Attr, AS_public, + return ActOnTag(S, TagSpec, TUK_Friend, TagLoc, SS, Name, NameLoc, Attr, + AS_public, /*ModulePrivateLoc=*/SourceLocation(), MultiTemplateParamsArg(), Owned, IsDependent, /*ScopedEnumKWLoc=*/SourceLocation(), /*ScopedEnumUsesClassTag=*/false, /*UnderlyingType=*/TypeResult(), /*IsTypeSpecifier=*/false, - /*IsTemplateParamOrArg=*/false); + /*IsTemplateParamOrArg=*/false, /*OOK=*/OOK_Outside); } NestedNameSpecifierLoc QualifierLoc = SS.getWithLocInContext(Context); diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp --- a/clang/lib/Sema/SemaTemplate.cpp +++ b/clang/lib/Sema/SemaTemplate.cpp @@ -10053,13 +10053,12 @@ bool Owned = false; bool IsDependent = false; - Decl *TagD = ActOnTag(S, TagSpec, Sema::TUK_Reference, - KWLoc, SS, Name, NameLoc, Attr, AS_none, - /*ModulePrivateLoc=*/SourceLocation(), - MultiTemplateParamsArg(), Owned, IsDependent, - SourceLocation(), false, TypeResult(), - /*IsTypeSpecifier*/false, - /*IsTemplateParamOrArg*/false); + Decl *TagD = ActOnTag( + S, TagSpec, Sema::TUK_Reference, KWLoc, SS, Name, NameLoc, Attr, AS_none, + /*ModulePrivateLoc=*/SourceLocation(), MultiTemplateParamsArg(), Owned, + IsDependent, SourceLocation(), false, TypeResult(), + /*IsTypeSpecifier*/ false, + /*IsOffsetOfInMacro=*/false, /*OOK=*/OOK_Outside); assert(!IsDependent && "explicit instantiation of dependent name not yet handled"); if (!TagD) diff --git a/clang/test/C/C2x/n2350.c b/clang/test/C/C2x/n2350.c new file mode 100644 --- /dev/null +++ b/clang/test/C/C2x/n2350.c @@ -0,0 +1,50 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s +// RUN: %clang_cc1 -fsyntax-only -std=c89 -verify %s +// RUN: %clang_cc1 -fsyntax-only -std=c99 -verify %s +// RUN: %clang_cc1 -fsyntax-only -std=c11 -verify %s +// RUN: %clang_cc1 -fsyntax-only -std=c17 -verify %s +// RUN: %clang_cc1 -fsyntax-only -std=c2x -verify %s + +// Reject definitions in __builtin_offsetof +// https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2350.htm +int simple(void) { + return __builtin_offsetof(struct A // expected-error{{'A' cannot be defined in '__builtin_offsetof'}} + { + int a; + struct B // expected-error{{'B' cannot be defined in '__builtin_offsetof'}} + { + int c; + int d; + } x; + }, a); +} + +#define offsetof(TYPE, MEMBER) __builtin_offsetof(TYPE, MEMBER) + + +int macro(void) { + return offsetof(struct A // expected-error{{'A' cannot be defined in 'offsetof'}} + // expected-error@-1{{'B' cannot be defined in 'offsetof'}} + { + int a; + struct B // verifier seems to think the error is emitted by the macro + // In fact the location of the error is "B" on the line above + { + int c; + int d; + } x; + }, a); +} + +#undef offsetof + +#define offsetof(TYPE, MEMBER) (&((TYPE *)0)->MEMBER) + +// no warning for traditional offsetof as a function-like macro +int * macro_func(void) { + return offsetof(struct A // no-warning + { + int a; + int b; + }, a); +} diff --git a/clang/test/Parser/declarators.c b/clang/test/Parser/declarators.c --- a/clang/test/Parser/declarators.c +++ b/clang/test/Parser/declarators.c @@ -80,10 +80,6 @@ struct test10 { int a; } static test10x; struct test11 { int a; } const test11x; -// PR6216 -void test12(void) { - (void)__builtin_offsetof(struct { char c; int i; }, i); -} // rdar://7608537 struct test13 { int a; } (test13x); diff --git a/clang/test/SemaCXX/offsetof.cpp b/clang/test/SemaCXX/offsetof.cpp --- a/clang/test/SemaCXX/offsetof.cpp +++ b/clang/test/SemaCXX/offsetof.cpp @@ -83,3 +83,20 @@ expected-error {{invalid application of 'offsetof' to a field of a virtual base}} }; } + +// Reject definitions in __builtin_offsetof +// https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2350.htm +int test_definition(void) { + return __builtin_offsetof(struct A // expected-error{{'A' cannot be defined in '__builtin_offsetof'}} + { + int a; + struct B // FIXME: error diagnostic message for nested definitions + // https://reviews.llvm.org/D133574 + // fixme-error{{'A' cannot be defined in '__builtin_offsetof'}} + { + int c; + int d; + }; + B x; + }, a); +}