[Sema] Split special builtin type lookups into a separate function

[tambre]

Summary

In case further such cases appear in the future we've got a generic function to add them to.
Additionally changed the ObjC special case to check the language and the identifier builtin ID instead of the name.

Addresses the cleanup suggestion from D87917.

[rjmccall]

Thanks. Minor improvement and then LGTM.

clang/lib/Sema/SemaLookup.cpp

924

I'm pretty sure this builtin only exists in ObjC mode, so there's no need to separately check the language mode. (Even if you did want to check it, it would be better to check the builtin mode first, as that's a very trivial check.)

[tambre]

clang/lib/Sema/SemaLookup.cpp

924

Fixed, thanks for the review!

clang/include/clang/Sema/Sema.h

public:
   /// It is preferable to use the elaborated form and explicitly handle
   /// ambiguity and overloaded.
   NamedDecl *LookupSingleName(Scope *S, DeclarationName Name,
                               SourceLocation Loc,
                               LookupNameKind NameKind,
                               RedeclarationKind Redecl
                                 = NotForRedeclaration);
   bool LookupBuiltin(LookupResult &R);
+  void LookupNecessaryTypesForBuiltin(Scope *S, unsigned ID);
   bool LookupName(LookupResult &R, Scope *S,
                   bool AllowBuiltinCreation = false);
   bool LookupQualifiedName(LookupResult &R, DeclContext *LookupCtx,
                            bool InUnqualifiedLookup = false);
   bool LookupQualifiedName(LookupResult &R, DeclContext *LookupCtx,
                            CXXScopeSpec &SS);
   bool LookupParsedName(LookupResult &R, Scope *S, CXXScopeSpec *SS,
                         bool AllowBuiltinCreation = false,

clang/lib/Sema/SemaDecl.cpp

 Scope *Sema::getNonFieldDeclScope(Scope *S) {
   while (((S->getFlags() & Scope::DeclScope) == 0) ||
          (S->getEntity() && S->getEntity()->isTransparentContext()) ||
          (S->isClassScope() && !getLangOpts().CPlusPlus))
     S = S->getParent();
   return S;
 }
 
-/// Looks up the declaration of "struct objc_super" and
-/// saves it for later use in building builtin declaration of
-/// objc_msgSendSuper and objc_msgSendSuper_stret. If no such
-/// pre-existing declaration exists no action takes place.
-static void LookupPredefedObjCSuperType(Sema &ThisSema, Scope *S,
-                                        IdentifierInfo *II) {
-  if (!II->isStr("objc_msgSendSuper"))
-    return;
-  ASTContext &Context = ThisSema.Context;
-
-  LookupResult Result(ThisSema, &Context.Idents.get("objc_super"),
-                      SourceLocation(), Sema::LookupTagName);
-  ThisSema.LookupName(Result, S);
-  if (Result.getResultKind() == LookupResult::Found)
-    if (const TagDecl *TD = Result.getAsSingle<TagDecl>())
-      Context.setObjCSuperType(Context.getTagDeclType(TD));
-}
-
 static StringRef getHeaderName(Builtin::Context &BuiltinInfo, unsigned ID,
                                ASTContext::GetBuiltinTypeError Error) {
   switch (Error) {
   case ASTContext::GE_None:
     return "";
   case ASTContext::GE_Missing_type:
     return BuiltinInfo.getHeaderName(ID);
   case ASTContext::GE_Missing_stdio:

 
 /// LazilyCreateBuiltin - The specified Builtin-ID was first used at
 /// file scope.  lazily create a decl for it. ForRedeclaration is true
 /// if we're creating this built-in in anticipation of redeclaring the
 /// built-in.
 NamedDecl *Sema::LazilyCreateBuiltin(IdentifierInfo *II, unsigned ID,
                                      Scope *S, bool ForRedeclaration,
                                      SourceLocation Loc) {
-  LookupPredefedObjCSuperType(*this, S, II);
+  LookupNecessaryTypesForBuiltin(S, ID);
 
   ASTContext::GetBuiltinTypeError Error;
   QualType R = Context.GetBuiltinType(ID, Error);
   if (Error) {
     if (!ForRedeclaration)
       return nullptr;
 
     // If we have a builtin without an associated type we should not emit a
if (IdentifierInfo *II = Previous.getLookupName().getAsIdentifierInfo()) {
         if (NewFD->getLanguageLinkage() == CLanguageLinkage) {
           // Declarations for builtins with custom typechecking by definition
           // don't make sense. Don't attempt typechecking and simply add the
           // attribute.
           if (Context.BuiltinInfo.hasCustomTypechecking(BuiltinID)) {
             NewFD->addAttr(BuiltinAttr::CreateImplicit(Context, BuiltinID));
           } else {
             ASTContext::GetBuiltinTypeError Error;
-            LookupPredefedObjCSuperType(*this, S, NewFD->getIdentifier());
+            LookupNecessaryTypesForBuiltin(S, BuiltinID);
             QualType BuiltinType = Context.GetBuiltinType(BuiltinID, Error);
 
             if (!Error && !BuiltinType.isNull() &&
                 Context.hasSameFunctionTypeIgnoringExceptionSpec(
                     NewFD->getType(), BuiltinType))
               NewFD->addAttr(BuiltinAttr::CreateImplicit(Context, BuiltinID));
           }
         } else if (BuiltinID == Builtin::BI__GetExceptionInfo &&
if (getLangOpts().CPlusPlus) {
     // during delayed parsing anyway.
     if (!CurContext->isRecord())
       CheckCXXDefaultArguments(NewFD);
 
     // If this function declares a builtin function, check the type of this
     // declaration against the expected type for the builtin.
     if (unsigned BuiltinID = NewFD->getBuiltinID()) {
       ASTContext::GetBuiltinTypeError Error;
-      LookupPredefedObjCSuperType(*this, S, NewFD->getIdentifier());
+      LookupNecessaryTypesForBuiltin(S, BuiltinID);
       QualType T = Context.GetBuiltinType(BuiltinID, Error);
       // If the type of the builtin differs only in its exception
       // specification, that's OK.
       // FIXME: If the types do differ in this way, it would be better to
       // retain the 'noexcept' form of the type.
       if (!T.isNull() &&
           !Context.hasSameFunctionTypeIgnoringExceptionSpec(T,
                                                             NewFD->getType()))

clang/lib/Sema/SemaLookup.cpp

if (II) {
         }
       }
     }
   }
 
   return false;
 }
 
+/// Looks up the declaration of "struct objc_super" and
+/// saves it for later use in building builtin declaration of
+/// objc_msgSendSuper and objc_msgSendSuper_stret.
+static void LookupPredefedObjCSuperType(Sema &Sema, Scope *S) {
+  ASTContext &Context = Sema.Context;
+  LookupResult Result(Sema, &Context.Idents.get("objc_super"), SourceLocation(),
+                      Sema::LookupTagName);
+  Sema.LookupName(Result, S);
+  if (Result.getResultKind() == LookupResult::Found)
+    if (const TagDecl *TD = Result.getAsSingle<TagDecl>())
+      Context.setObjCSuperType(Context.getTagDeclType(TD));
+}
+
+void Sema::LookupNecessaryTypesForBuiltin(Scope *S, unsigned ID) {
+  if (ID == Builtin::BIobjc_msgSendSuper)
+    LookupPredefedObjCSuperType(*this, S);
+}
+
 /// Determine whether we can declare a special member function within
 /// the class at this point.
 static bool CanDeclareSpecialMemberFunction(const CXXRecordDecl *Class) {
   // We need to have a definition for the class.
   if (!Class->getDefinition() || Class->isDependentContext())
     return false;
 
   // We can't be in the middle of defining the class.