[clang] Cleanup unneeded Function nullptr checks [NFC]

[mikerice]

Summary

Add an assert and avoid unneeded nullptr checks of Fn in CodeGenFunction::GenerateCode.

Unless I am missing something, Fn is always non-null here. If Fn could be null then the code in StartFunction seems seriously broken since it is used unchecked there.

Once there is one "if (Fn)" in the code, static analyzers start complaining about any unchecked uses of Fn.

[aaron.ballman]

clang/lib/CodeGen/CodeGenFunction.cpp

1299–1302

How obnoxious would it be if I asked to make Fn a llvm::Function & instead of a pointer? Then we don't need the assert at all and the interface will enforce the requirement for us.

[mikerice]

clang/lib/CodeGen/CodeGenFunction.cpp

1299–1302

Good idea, but there is an assignment to it (Fn = Clone) which would be a problem right?

[rjmccall]

LGTM

[aaron.ballman]

LG as-is, but feel free to go with the reference approach if you think it has value.

clang/lib/CodeGen/CodeGenFunction.cpp

1299–1302

Good idea, but there is an assignment to it (Fn = Clone) which would be a problem right?

I was thinking we'd be cheap and do something like:

void CodeGenFunction::GenerateCode(GlobalDecl GD, llvm::Function &Func,
                                   const CGFunctionInfo &FnInfo) {
  llvm::Function *Fn = &Func;
  ....

under the assumption that the static analysis tool should be smart enough to see that the pointer comes from a reference and thus cannot be null. But I may be making a faulty assumption there, so I don't insist.

clang/lib/CodeGen/CodeGenFunction.cpp

QualType CodeGenFunction::BuildFunctionArgList(GlobalDecl GD,
   }
 
   if (MD && (isa<CXXConstructorDecl>(MD) || isa<CXXDestructorDecl>(MD)))
     CGM.getCXXABI().addImplicitStructorParams(*this, ResTy, Args);
 
   return ResTy;
 }
 
 void CodeGenFunction::GenerateCode(GlobalDecl GD, llvm::Function *Fn,
                                    const CGFunctionInfo &FnInfo) {
+  assert(Fn && "generating code for null Function");
   const FunctionDecl *FD = cast<FunctionDecl>(GD.getDecl());
   CurGD = GD;
 
   FunctionArgList Args;
   QualType ResTy = BuildFunctionArgList(GD, Args);
 
-  // When generating code for a builtin with an inline declaration, use a
-  // mangled name to hold the actual body, while keeping an external definition
-  // in case the function pointer is referenced somewhere.
-  if (Fn) {
-    if (FD->isInlineBuiltinDeclaration()) {
-      std::string FDInlineName = (Fn->getName() + ".inline").str();
-      llvm::Module *M = Fn->getParent();
-      llvm::Function *Clone = M->getFunction(FDInlineName);
-      if (!Clone) {
-        Clone = llvm::Function::Create(Fn->getFunctionType(),
-                                       llvm::GlobalValue::InternalLinkage,
-                                       Fn->getAddressSpace(), FDInlineName, M);
-        Clone->addFnAttr(llvm::Attribute::AlwaysInline);
-      }
-      Fn->setLinkage(llvm::GlobalValue::ExternalLinkage);
-      Fn = Clone;
-    }
-
+    // When generating code for a builtin with an inline declaration, use a
+    // mangled name to hold the actual body, while keeping an external
+    // definition in case the function pointer is referenced somewhere.
+  } else {
     // Detect the unusual situation where an inline version is shadowed by a
     // non-inline version. In that case we should pick the external one
     // everywhere. That's GCC behavior too. Unfortunately, I cannot find a way
     // to detect that situation before we reach codegen, so do some late
     // replacement.
-    else {
-      for (const FunctionDecl *PD = FD->getPreviousDecl(); PD;
-           PD = PD->getPreviousDecl()) {
-        if (LLVM_UNLIKELY(PD->isInlineBuiltinDeclaration())) {
-          std::string FDInlineName = (Fn->getName() + ".inline").str();
-          llvm::Module *M = Fn->getParent();
-          if (llvm::Function *Clone = M->getFunction(FDInlineName)) {
-            Clone->replaceAllUsesWith(Fn);
-            Clone->eraseFromParent();
-          }
-          break;
-        }
-      }
-    }
-  }
 
   // Check if we should generate debug info for this function.
   if (FD->hasAttr<NoDebugAttr>()) {
     // Clear non-distinct debug info that was possibly attached to the function
     // due to an earlier declaration without the nodebug attribute
-    if (Fn)
-      Fn->setSubprogram(nullptr);
     // Disable debug info indefinitely for this function
     DebugInfo = nullptr;
   }
 
   // The function might not have a body if we're generating thunks for a
   // function declaration.
   SourceRange BodyRange;
   if (Stmt *Body = FD->getBody())