[CodeGen] Add ParmVarDecls to FunctionDecls that are created to generate ObjC property getter/setter functions

[ahatanak]

Summary

This is needed to prevent clang from crashing when we make the changes proposed in https://reviews.llvm.org/D98799.

[ahatanak]

clang crashes if I remove the fake FunctionDecls as I did in https://reviews.llvm.org/D104082.

[aprantl]

If args.push_back(Params[1] = SrcDecl); doesn't trigger a warning, this seems reasonable.

clang/lib/CodeGen/CGObjC.cpp

3706

Does this compile without warnings?

[dblaikie]

Ah - this is an alternative to D104082? OK. Yeah, looks like it'd address the thing we were discussing in D98799. But do you have some more details on why D104082 wasn't the right direction? I thought we found other places that used a default-constructed/null GlobalDecl() for StartFunction` correctly, so I'm wondering why this would be a problem in this particular case(s)?

[ahatanak]

I see assert(DC && "This decl is not contained in a translation unit!"); fail in Decl::getTranslationUnitDecl when DeclRefExpr is constructed. That's because the ImplicitParamDecl passed to DeclRefExpr's constructor doesn't have a decl context if I delete FD. So it looks like FD is needed in these cases.

clang/lib/CodeGen/CGObjC.cpp

3706

Yes, clang compiles this without any warnings. I don't know whether other compilers warn.

[dblaikie]

In D104883#2841931, @ahatanak wrote:

I see assert(DC && "This decl is not contained in a translation unit!"); fail in Decl::getTranslationUnitDecl when DeclRefExpr is constructed. That's because the ImplicitParamDecl passed to DeclRefExpr's constructor doesn't have a decl context if I delete FD. So it looks like FD is needed in these cases.

Hmm, OK - I think I'm following enough. I'll leave the rest to you & @aprantl - appreciate the work!

[ahatanak]

I'll wait a day or two in case someone else has feedback.

clang/lib/CodeGen/CGObjC.cpp

CodeGenFunction::GenerateObjCAtomicSetterCopyHelperFunction(
   ArgTys.push_back(SrcTy);
   QualType FunctionTy = C.getFunctionType(ReturnTy, ArgTys, {});
 
   FunctionDecl *FD = FunctionDecl::Create(
       C, C.getTranslationUnitDecl(), SourceLocation(), SourceLocation(), II,
       FunctionTy, nullptr, SC_Static, false, false);
 
   FunctionArgList args;
-  ImplicitParamDecl DstDecl(C, FD, SourceLocation(), /*Id=*/nullptr, DestTy,
-                            ImplicitParamDecl::Other);
-  args.push_back(&DstDecl);
-  ImplicitParamDecl SrcDecl(C, FD, SourceLocation(), /*Id=*/nullptr, SrcTy,
-                            ImplicitParamDecl::Other);
-  args.push_back(&SrcDecl);
+  ParmVarDecl *Params[2];
+  ParmVarDecl *DstDecl = ParmVarDecl::Create(
+      C, FD, SourceLocation(), SourceLocation(), nullptr, DestTy,
+      C.getTrivialTypeSourceInfo(DestTy, SourceLocation()), SC_None,
+      /*DefArg=*/nullptr);
+  args.push_back(Params[0] = DstDecl);
+  ParmVarDecl *SrcDecl = ParmVarDecl::Create(
+      C, FD, SourceLocation(), SourceLocation(), nullptr, SrcTy,
+      C.getTrivialTypeSourceInfo(SrcTy, SourceLocation()), SC_None,
+      /*DefArg=*/nullptr);
+  args.push_back(Params[1] = SrcDecl);
+  FD->setParams(Params);
 
   const CGFunctionInfo &FI =
       CGM.getTypes().arrangeBuiltinFunctionDeclaration(ReturnTy, args);
 
   llvm::FunctionType *LTy = CGM.getTypes().GetFunctionType(FI);
 
   llvm::Function *Fn =
     llvm::Function::Create(LTy, llvm::GlobalValue::InternalLinkage,
                            "__assign_helper_atomic_property_",
                            &CGM.getModule());
 
   CGM.SetInternalFunctionAttributes(GlobalDecl(), Fn, FI);
 
   StartFunction(FD, ReturnTy, Fn, FI, args);
 
-  DeclRefExpr DstExpr(C, &DstDecl, false, DestTy, VK_PRValue, SourceLocation());
+  DeclRefExpr DstExpr(C, DstDecl, false, DestTy, VK_PRValue, SourceLocation());
   UnaryOperator *DST = UnaryOperator::Create(
       C, &DstExpr, UO_Deref, DestTy->getPointeeType(), VK_LValue, OK_Ordinary,
       SourceLocation(), false, FPOptionsOverride());
 
-  DeclRefExpr SrcExpr(C, &SrcDecl, false, SrcTy, VK_PRValue, SourceLocation());
+  DeclRefExpr SrcExpr(C, SrcDecl, false, SrcTy, VK_PRValue, SourceLocation());
   UnaryOperator *SRC = UnaryOperator::Create(
       C, &SrcExpr, UO_Deref, SrcTy->getPointeeType(), VK_LValue, OK_Ordinary,
       SourceLocation(), false, FPOptionsOverride());
 
   Expr *Args[2] = {DST, SRC};
   CallExpr *CalleeExp = cast<CallExpr>(PID->getSetterCXXAssignment());
   CXXOperatorCallExpr *TheCall = CXXOperatorCallExpr::Create(
       C, OO_Equal, CalleeExp->getCallee(), Args, DestTy->getPointeeType(),
CodeGenFunction::GenerateObjCAtomicGetterCopyHelperFunction(
   ArgTys.push_back(SrcTy);
   QualType FunctionTy = C.getFunctionType(ReturnTy, ArgTys, {});
 
   FunctionDecl *FD = FunctionDecl::Create(
       C, C.getTranslationUnitDecl(), SourceLocation(), SourceLocation(), II,
       FunctionTy, nullptr, SC_Static, false, false);
 
   FunctionArgList args;
-  ImplicitParamDecl DstDecl(C, FD, SourceLocation(), /*Id=*/nullptr, DestTy,
-                            ImplicitParamDecl::Other);
-  args.push_back(&DstDecl);
-  ImplicitParamDecl SrcDecl(C, FD, SourceLocation(), /*Id=*/nullptr, SrcTy,
-                            ImplicitParamDecl::Other);
-  args.push_back(&SrcDecl);
+  ParmVarDecl *Params[2];
+  ParmVarDecl *DstDecl = ParmVarDecl::Create(
+      C, FD, SourceLocation(), SourceLocation(), nullptr, DestTy,
+      C.getTrivialTypeSourceInfo(DestTy, SourceLocation()), SC_None,
+      /*DefArg=*/nullptr);
+  args.push_back(Params[0] = DstDecl);
+  ParmVarDecl *SrcDecl = ParmVarDecl::Create(
+      C, FD, SourceLocation(), SourceLocation(), nullptr, SrcTy,
+      C.getTrivialTypeSourceInfo(SrcTy, SourceLocation()), SC_None,
+      /*DefArg=*/nullptr);
+  args.push_back(Params[1] = SrcDecl);
+  FD->setParams(Params);
 
   const CGFunctionInfo &FI =
       CGM.getTypes().arrangeBuiltinFunctionDeclaration(ReturnTy, args);
 
   llvm::FunctionType *LTy = CGM.getTypes().GetFunctionType(FI);
 
   llvm::Function *Fn = llvm::Function::Create(
       LTy, llvm::GlobalValue::InternalLinkage, "__copy_helper_atomic_property_",
       &CGM.getModule());
 
   CGM.SetInternalFunctionAttributes(GlobalDecl(), Fn, FI);
 
   StartFunction(FD, ReturnTy, Fn, FI, args);
 
-  DeclRefExpr SrcExpr(getContext(), &SrcDecl, false, SrcTy, VK_PRValue,
+  DeclRefExpr SrcExpr(getContext(), SrcDecl, false, SrcTy, VK_PRValue,
                       SourceLocation());
 
   UnaryOperator *SRC = UnaryOperator::Create(
       C, &SrcExpr, UO_Deref, SrcTy->getPointeeType(), VK_LValue, OK_Ordinary,
       SourceLocation(), false, FPOptionsOverride());
 
   CXXConstructExpr *CXXConstExpr =
     cast<CXXConstructExpr>(PID->getGetterCXXConstructor());
CXXConstructExpr::Create(C, Ty, SourceLocation(),
                              ConstructorArgs,
                              CXXConstExpr->hadMultipleCandidates(),
                              CXXConstExpr->isListInitialization(),
                              CXXConstExpr->isStdInitListInitialization(),
                              CXXConstExpr->requiresZeroInitialization(),
                              CXXConstExpr->getConstructionKind(),
                              SourceRange());
 
-  DeclRefExpr DstExpr(getContext(), &DstDecl, false, DestTy, VK_PRValue,
+  DeclRefExpr DstExpr(getContext(), DstDecl, false, DestTy, VK_PRValue,
                       SourceLocation());
 
   RValue DV = EmitAnyExpr(&DstExpr);
   CharUnits Alignment
     = getContext().getTypeAlignInChars(TheCXXConstructExpr->getType());
   EmitAggExpr(TheCXXConstructExpr,
               AggValueSlot::forAddr(Address(DV.getScalarVal(), Alignment),
                                     Qualifiers(),