[PATCH] D73495: [CodeGen] Attach no-builtin attributes to function definitions with no Decl

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.


================
Comment at: clang/lib/CodeGen/CGCall.cpp:1917
         const auto *NBA = Fn->getAttr<NoBuiltinAttr>();
-        bool HasWildcard = NBA && llvm::is_contained(NBA->builtinNames(), "*");
-        if (getLangOpts().NoBuiltin || HasWildcard)
-          FuncAttrs.addAttribute("no-builtins");
-        else {
-          auto AddNoBuiltinAttr = [&FuncAttrs](StringRef BuiltinName) {
-            SmallString<32> AttributeName;
-            AttributeName += "no-builtin-";
-            AttributeName += BuiltinName;
-            FuncAttrs.addAttribute(AttributeName);
-          };
-          llvm::for_each(getLangOpts().NoBuiltinFuncs, AddNoBuiltinAttr);
-          if (NBA)
-            llvm::for_each(NBA->builtinNames(), AddNoBuiltinAttr);
-        }
+        addNonCallSiteNoBuiltinAttributes(FuncAttrs, getLangOpts(), NBA);
       }
----------------
thegameg wrote:
> efriedma wrote:
> > What happens if we have a TargetDecl that isn't a FunctionDecl?  (I think this happens in some cases, but not completely sure which, exactly.)
> It looks like that can be triggered by indirect calls:
> 
> ```
> typedef void T(void);
> void test3(T f) {
>   f();
> }
> ```
> 
> Since this adds the attribute to definitions and not to call sites, we should never need that.
> 
> This patch is for the case where `CreateGlobalInitOrDestructFunction` ends up re-using the same function to attach the attributes.
> 
> I'll update the description to make it more clear.
Are you sure that's the only case where the TargetDecl isn't a FunctionDecl?  I'm afraid there might be some weird case where the TargetDecl defines something like a function, but isn't technically a FunctionDecl. Maybe an ObjC method or something like that.  And then we end up in a situation where addNonCallSiteNoBuiltinAttributes is never called.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73495/new/

https://reviews.llvm.org/D73495