[clang] 3925fbc - nullptr returned from ActOnTag() is not a valid result

Aaron Ballman via cfe-commits cfe-commits at lists.llvm.org
Tue Jan 17 13:04:46 PST 2023


Author: Aaron Ballman
Date: 2023-01-17T16:04:35-05:00
New Revision: 3925fbc80019f72bf3f5174736f348acfb5768b0

URL: https://github.com/llvm/llvm-project/commit/3925fbc80019f72bf3f5174736f348acfb5768b0
DIFF: https://github.com/llvm/llvm-project/commit/3925fbc80019f72bf3f5174736f348acfb5768b0.diff

LOG: nullptr returned from ActOnTag() is not a valid result

DeclResult tracks two states: valid/invalid and usable/unusable.
Passing a null pointer to the constructor creates a valid but unusable
result and we wanted an invalid result instead. This changes some
functions to return a DeclResult rather than a Decl * to make it harder
to get this incorrect in callers.

Discovered when working on https://reviews.llvm.org/D141280.

Co-authored-by: Haojian Wu <hokein at google.com>
Co-authored-by: Aaron Ballman <aaron at aaronballman.com>

Differential Revision: https://reviews.llvm.org/D141580

Added: 
    

Modified: 
    clang/include/clang/Sema/Sema.h
    clang/lib/Parse/ParseDecl.cpp
    clang/lib/Sema/SemaDecl.cpp
    clang/lib/Sema/SemaDeclCXX.cpp
    clang/lib/Sema/SemaTemplate.cpp
    clang/test/Parser/cxx-undeclared-identifier.cpp
    clang/test/Parser/recovery.cpp
    clang/test/SemaCXX/invalid-template-params.cpp
    clang/test/SemaCXX/rdar42746401.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 35e319879a98d..7a95989ee6d44 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -3314,22 +3314,24 @@ class Sema final {
     OOK_Macro,
   };
 
-  Decl *ActOnTag(Scope *S, unsigned TagSpec, TagUseKind TUK,
-                 SourceLocation KWLoc, CXXScopeSpec &SS, IdentifierInfo *Name,
-                 SourceLocation NameLoc, const ParsedAttributesView &Attr,
-                 AccessSpecifier AS, SourceLocation ModulePrivateLoc,
-                 MultiTemplateParamsArg TemplateParameterLists, bool &OwnedDecl,
-                 bool &IsDependent, SourceLocation ScopedEnumKWLoc,
-                 bool ScopedEnumUsesClassTag, TypeResult UnderlyingType,
-                 bool IsTypeSpecifier, bool IsTemplateParamOrArg,
-                 OffsetOfKind OOK, SkipBodyInfo *SkipBody = nullptr);
-
-  Decl *ActOnTemplatedFriendTag(Scope *S, SourceLocation FriendLoc,
-                                unsigned TagSpec, SourceLocation TagLoc,
-                                CXXScopeSpec &SS, IdentifierInfo *Name,
-                                SourceLocation NameLoc,
-                                const ParsedAttributesView &Attr,
-                                MultiTemplateParamsArg TempParamLists);
+  DeclResult ActOnTag(Scope *S, unsigned TagSpec, TagUseKind TUK,
+                      SourceLocation KWLoc, CXXScopeSpec &SS,
+                      IdentifierInfo *Name, SourceLocation NameLoc,
+                      const ParsedAttributesView &Attr, AccessSpecifier AS,
+                      SourceLocation ModulePrivateLoc,
+                      MultiTemplateParamsArg TemplateParameterLists,
+                      bool &OwnedDecl, bool &IsDependent,
+                      SourceLocation ScopedEnumKWLoc,
+                      bool ScopedEnumUsesClassTag, TypeResult UnderlyingType,
+                      bool IsTypeSpecifier, bool IsTemplateParamOrArg,
+                      OffsetOfKind OOK, SkipBodyInfo *SkipBody = nullptr);
+
+  DeclResult ActOnTemplatedFriendTag(Scope *S, SourceLocation FriendLoc,
+                                     unsigned TagSpec, SourceLocation TagLoc,
+                                     CXXScopeSpec &SS, IdentifierInfo *Name,
+                                     SourceLocation NameLoc,
+                                     const ParsedAttributesView &Attr,
+                                     MultiTemplateParamsArg TempParamLists);
 
   TypeResult ActOnDependentTag(Scope *S,
                                unsigned TagSpec,

diff  --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
index 75937c0d6a952..4a26f0443634c 100644
--- a/clang/lib/Parse/ParseDecl.cpp
+++ b/clang/lib/Parse/ParseDecl.cpp
@@ -4972,7 +4972,7 @@ void Parser::ParseEnumSpecifier(SourceLocation StartLoc, DeclSpec &DS,
       DSC == DeclSpecContext::DSC_type_specifier,
       DSC == DeclSpecContext::DSC_template_param ||
           DSC == DeclSpecContext::DSC_template_type_arg,
-      OffsetOfState, &SkipBody);
+      OffsetOfState, &SkipBody).get();
 
   if (SkipBody.ShouldSkip) {
     assert(TUK == Sema::TUK_Definition && "can only skip a definition");

diff  --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 0fd68c55cc172..1de4caee01bed 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -16583,17 +16583,16 @@ static bool isAcceptableTagRedeclContext(Sema &S, DeclContext *OldDC,
 ///
 /// \param SkipBody If non-null, will be set to indicate if the caller should
 /// skip the definition of this tag and treat it as if it were a declaration.
-Decl *Sema::ActOnTag(Scope *S, unsigned TagSpec, TagUseKind TUK,
-                     SourceLocation KWLoc, CXXScopeSpec &SS,
-                     IdentifierInfo *Name, SourceLocation NameLoc,
-                     const ParsedAttributesView &Attrs, AccessSpecifier AS,
-                     SourceLocation ModulePrivateLoc,
-                     MultiTemplateParamsArg TemplateParameterLists,
-                     bool &OwnedDecl, bool &IsDependent,
-                     SourceLocation ScopedEnumKWLoc,
-                     bool ScopedEnumUsesClassTag, TypeResult UnderlyingType,
-                     bool IsTypeSpecifier, bool IsTemplateParamOrArg,
-                     OffsetOfKind OOK, SkipBodyInfo *SkipBody) {
+DeclResult
+Sema::ActOnTag(Scope *S, unsigned TagSpec, TagUseKind TUK, SourceLocation KWLoc,
+               CXXScopeSpec &SS, IdentifierInfo *Name, SourceLocation NameLoc,
+               const ParsedAttributesView &Attrs, AccessSpecifier AS,
+               SourceLocation ModulePrivateLoc,
+               MultiTemplateParamsArg TemplateParameterLists, bool &OwnedDecl,
+               bool &IsDependent, SourceLocation ScopedEnumKWLoc,
+               bool ScopedEnumUsesClassTag, TypeResult UnderlyingType,
+               bool IsTypeSpecifier, bool IsTemplateParamOrArg,
+               OffsetOfKind OOK, SkipBodyInfo *SkipBody) {
   // If this is not a definition, it must have a name.
   IdentifierInfo *OrigName = Name;
   assert((Name != nullptr || TUK == TUK_Definition) &&
@@ -16619,7 +16618,7 @@ Decl *Sema::ActOnTag(Scope *S, unsigned TagSpec, TagUseKind TUK,
                 TUK == TUK_Friend, isMemberSpecialization, Invalid)) {
       if (Kind == TTK_Enum) {
         Diag(KWLoc, diag::err_enum_template);
-        return nullptr;
+        return true;
       }
 
       if (TemplateParams->size() > 0) {
@@ -16627,7 +16626,7 @@ Decl *Sema::ActOnTag(Scope *S, unsigned TagSpec, TagUseKind TUK,
         // be a member of another template).
 
         if (Invalid)
-          return nullptr;
+          return true;
 
         OwnedDecl = false;
         DeclResult Result = CheckClassTemplate(
@@ -16646,7 +16645,7 @@ Decl *Sema::ActOnTag(Scope *S, unsigned TagSpec, TagUseKind TUK,
 
     if (!TemplateParameterLists.empty() && isMemberSpecialization &&
         CheckTemplateDeclScope(S, TemplateParameterLists.back()))
-      return nullptr;
+      return true;
   }
 
   // Figure out the underlying type if this a enum declaration. We need to do
@@ -16762,26 +16761,26 @@ Decl *Sema::ActOnTag(Scope *S, unsigned TagSpec, TagUseKind TUK,
       DC = computeDeclContext(SS, false);
       if (!DC) {
         IsDependent = true;
-        return nullptr;
+        return true;
       }
     } else {
       DC = computeDeclContext(SS, true);
       if (!DC) {
         Diag(SS.getRange().getBegin(), diag::err_dependent_nested_name_spec)
           << SS.getRange();
-        return nullptr;
+        return true;
       }
     }
 
     if (RequireCompleteDeclContext(SS, DC))
-      return nullptr;
+      return true;
 
     SearchDC = DC;
     // Look-up name inside 'foo::'.
     LookupQualifiedName(Previous, DC);
 
     if (Previous.isAmbiguous())
-      return nullptr;
+      return true;
 
     if (Previous.empty()) {
       // Name lookup did not find anything. However, if the
@@ -16793,7 +16792,7 @@ Decl *Sema::ActOnTag(Scope *S, unsigned TagSpec, TagUseKind TUK,
       if (Previous.wasNotFoundInCurrentInstantiation() &&
           (TUK == TUK_Reference || TUK == TUK_Friend)) {
         IsDependent = true;
-        return nullptr;
+        return true;
       }
 
       // A tag 'foo::bar' must already exist.
@@ -16810,7 +16809,7 @@ Decl *Sema::ActOnTag(Scope *S, unsigned TagSpec, TagUseKind TUK,
     //    -- every member of class T that is itself a type
     if (TUK != TUK_Reference && TUK != TUK_Friend &&
         DiagnoseClassNameShadow(SearchDC, DeclarationNameInfo(Name, NameLoc)))
-      return nullptr;
+      return true;
 
     // If this is a named struct, check to see if there was a previous forward
     // declaration or definition.
@@ -16874,7 +16873,7 @@ Decl *Sema::ActOnTag(Scope *S, unsigned TagSpec, TagUseKind TUK,
 
     // Note:  there used to be some attempt at recovery here.
     if (Previous.isAmbiguous())
-      return nullptr;
+      return true;
 
     if (!getLangOpts().CPlusPlus && TUK != TUK_Reference) {
       // FIXME: This makes sure that we ignore the contexts associated
@@ -17531,7 +17530,7 @@ Decl *Sema::ActOnTag(Scope *S, unsigned TagSpec, TagUseKind TUK,
     if (New->isBeingDefined())
       if (auto RD = dyn_cast<RecordDecl>(New))
         RD->completeDefinition();
-    return nullptr;
+    return true;
   } else if (SkipBody && SkipBody->ShouldSkip) {
     return SkipBody->Previous;
   } else {

diff  --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index cf1242beffe99..c8d384c643b3d 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -16922,12 +16922,10 @@ FriendDecl *Sema::CheckFriendTypeDecl(SourceLocation LocStart,
 
 /// Handle a friend tag declaration where the scope specifier was
 /// templated.
-Decl *Sema::ActOnTemplatedFriendTag(Scope *S, SourceLocation FriendLoc,
-                                    unsigned TagSpec, SourceLocation TagLoc,
-                                    CXXScopeSpec &SS, IdentifierInfo *Name,
-                                    SourceLocation NameLoc,
-                                    const ParsedAttributesView &Attr,
-                                    MultiTemplateParamsArg TempParamLists) {
+DeclResult Sema::ActOnTemplatedFriendTag(
+    Scope *S, SourceLocation FriendLoc, unsigned TagSpec, SourceLocation TagLoc,
+    CXXScopeSpec &SS, IdentifierInfo *Name, SourceLocation NameLoc,
+    const ParsedAttributesView &Attr, MultiTemplateParamsArg TempParamLists) {
   TagTypeKind Kind = TypeWithKeyword::getTagTypeKindForTypeSpec(TagSpec);
 
   bool IsMemberSpecialization = false;
@@ -16940,7 +16938,7 @@ Decl *Sema::ActOnTemplatedFriendTag(Scope *S, SourceLocation FriendLoc,
     if (TemplateParams->size() > 0) {
       // This is a declaration of a class template.
       if (Invalid)
-        return nullptr;
+        return true;
 
       return CheckClassTemplate(S, TagSpec, TUK_Friend, TagLoc, SS, Name,
                                 NameLoc, Attr, TemplateParams, AS_public,
@@ -16955,7 +16953,7 @@ Decl *Sema::ActOnTemplatedFriendTag(Scope *S, SourceLocation FriendLoc,
     }
   }
 
-  if (Invalid) return nullptr;
+  if (Invalid) return true;
 
   bool isAllExplicitSpecializations = true;
   for (unsigned I = TempParamLists.size(); I-- > 0; ) {
@@ -16991,7 +16989,7 @@ Decl *Sema::ActOnTemplatedFriendTag(Scope *S, SourceLocation FriendLoc,
     QualType T = CheckTypenameType(Keyword, TagLoc, QualifierLoc,
                                    *Name, NameLoc);
     if (T.isNull())
-      return nullptr;
+      return true;
 
     TypeSourceInfo *TSI = Context.CreateTypeSourceInfo(T);
     if (isa<DependentNameType>(T)) {

diff  --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 1b30116192616..da74267cc65d2 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -10186,7 +10186,7 @@ Sema::ActOnExplicitInstantiation(Scope *S, SourceLocation ExternLoc,
       /*ModulePrivateLoc=*/SourceLocation(), MultiTemplateParamsArg(), Owned,
       IsDependent, SourceLocation(), false, TypeResult(),
       /*IsTypeSpecifier*/ false,
-      /*IsTemplateParamOrArg=*/false, /*OOK=*/OOK_Outside);
+      /*IsTemplateParamOrArg=*/false, /*OOK=*/OOK_Outside).get();
   assert(!IsDependent && "explicit instantiation of dependent name not yet handled");
 
   if (!TagD)

diff  --git a/clang/test/Parser/cxx-undeclared-identifier.cpp b/clang/test/Parser/cxx-undeclared-identifier.cpp
index c5bf8ae9944ee..4751dadd1c498 100644
--- a/clang/test/Parser/cxx-undeclared-identifier.cpp
+++ b/clang/test/Parser/cxx-undeclared-identifier.cpp
@@ -15,5 +15,5 @@ namespace ImplicitInt {
 // PR7180
 int f(a::b::c); // expected-error {{use of undeclared identifier 'a'}}
 
-class Foo::Bar { // expected-error {{use of undeclared identifier 'Foo'}} \
-                 // expected-error {{expected ';' after class}}
+class Foo::Bar { // expected-error {{use of undeclared identifier 'Foo'}}
+                 // expected-error {{expected unqualified-id}}

diff  --git a/clang/test/Parser/recovery.cpp b/clang/test/Parser/recovery.cpp
index 5a20f98d32912..4e2811c4cac92 100644
--- a/clang/test/Parser/recovery.cpp
+++ b/clang/test/Parser/recovery.cpp
@@ -212,6 +212,6 @@ struct ::, struct ::; // expected-error 2 {{expected identifier}} expected-error
 enum ::, enum ::; // expected-error 2 {{expected identifier}}
 struct ::__super, struct ::__super; // expected-error 2 {{expected identifier}} expected-error 2 {{expected '::' after '__super'}}
 struct ::template foo, struct ::template bar; // expected-error 2 {{expected identifier}} expected-error 2 {{declaration of anonymous struct must be a definition}} expected-warning {{declaration does not declare anything}}
-struct ::foo struct::; // expected-error {{no struct named 'foo' in the global namespace}} expected-error {{expected identifier}} expected-error {{declaration of anonymous struct must be a definition}}
+struct ::foo struct::; // expected-error {{no struct named 'foo' in the global namespace}} expected-error {{expected identifier}}
 class :: : {} a;  // expected-error {{expected identifier}} expected-error {{expected class name}}
 }

diff  --git a/clang/test/SemaCXX/invalid-template-params.cpp b/clang/test/SemaCXX/invalid-template-params.cpp
index 6f19aa9d5ddbe..4eb8edc0141fe 100644
--- a/clang/test/SemaCXX/invalid-template-params.cpp
+++ b/clang/test/SemaCXX/invalid-template-params.cpp
@@ -15,9 +15,8 @@ class C0 {
 public:
   template<typename T0, typename T1 = T0 // missing closing angle bracket
   struct S0 {}; // expected-error {{'S0' cannot be defined in a type specifier}}
-                // expected-error at -1 {{cannot combine with previous 'type-name' declaration specifier}}
-                // expected-error at -2 {{expected ',' or '>' in template-parameter-list}}
-                // expected-error at -3 {{declaration does not declare anything}}
+                // expected-error at -1 {{expected ',' or '>' in template-parameter-list}}
+                // expected-error at -2 {{declaration does not declare anything}}
   C0() : m(new S0<int>) {} // expected-error {{expected '(' for function-style cast or type construction}}
                            // expected-error at -1 {{expected expression}}
   S0<int> *m; // expected-error {{expected member name or ';' after declaration specifiers}}

diff  --git a/clang/test/SemaCXX/rdar42746401.cpp b/clang/test/SemaCXX/rdar42746401.cpp
index ff9dff456ce47..a2a941f2ae836 100644
--- a/clang/test/SemaCXX/rdar42746401.cpp
+++ b/clang/test/SemaCXX/rdar42746401.cpp
@@ -4,4 +4,4 @@ template <int>
 class b;
 class c; // expected-note{{forward declaration}}
 
-::b<0> struct c::d // expected-error{{incomplete type}} expected-error{{cannot combine}} expected-error{{expected unqualified-id}}
+::b<0> struct c::d // expected-error{{incomplete type}} expected-error{{expected unqualified-id}}


        


More information about the cfe-commits mailing list