[NFC][clang] Fix static analyzer tool remarks about large copies by values

[Manna]

Summary

Reported by Coverity:

Big parameter passed by value
Copying large values is inefficient, consider passing by reference; Low, medium, and high size thresholds for detection can be adjusted.

1. Inside "SemaConcept.cpp" file, in subsumes<clang::​Sema::​MaybeEmitAmbiguousAtomicConstraintsDiagnostic(clang::​NamedDecl *, llvm::​ArrayRef<clang::​Expr const *>, clang::​NamedDecl *, llvm::​ArrayRef<clang::​Expr const *>)::​[lambda(clang::​AtomicConstraint const &, clang::​AtomicConstraint const &) (instance 2)]>(llvm::​SmallVector<llvm::​SmallVector<clang::​AtomicConstraint *, 2u>, 4u>, llvm::​SmallVector<llvm::​SmallVector<clang::​AtomicConstraint *, 2u>, 4u>, T1): A large function call parameter exceeding the low threshold is passed by value.
   
   i. pass_by_value: Passing parameter PDNF of type NormalForm (size 144 bytes) by value, which exceeds the low threshold of 128 bytes.

ii. pass_by_value: Passing parameter QCNF of type NormalForm (size 144 bytes) by value, which exceeds the low threshold of 128 bytes.

2. Inside "CodeGenAction.cpp" file, in clang::​reportOptRecordError(llvm::​Error, clang::​DiagnosticsEngine &, clang::​CodeGenOptions): A very large function call parameter exceeding the high threshold is passed by value.

i. pass_by_value: Passing parameter CodeGenOpts of type clang::CodeGenOptions const (size 1560 bytes) by value, which exceeds the high threshold of 512 bytes.

3. Inside "SemaCodeComplete.cpp" file, in HandleCodeCompleteResults(clang::​Sema *, clang::​CodeCompleteConsumer *, clang::​CodeCompletionContext, clang::​CodeCompletionResult *, unsigned int): A large function call parameter exceeding the low threshold is passed by value.

i. pass_by_value: Passing parameter Context of type clang::CodeCompletionContext (size 200 bytes) by value, which exceeds the low threshold of 128 bytes.

4. Inside "SemaConcept.cpp" file, in <unnamed>::​SatisfactionStackRAII::​SatisfactionStackRAII(clang::​Sema &, clang::​NamedDecl const *, llvm::​FoldingSetNodeID): A large function call parameter exceeding the low threshold is passed by value.

i. pass_by_value: Passing parameter FSNID of type llvm::FoldingSetNodeID (size 144 bytes) by value, which exceeds the low threshold of 128 bytes.

[erichkeane]

Build failures seem relevant.

clang/lib/CodeGen/CGGPUBuiltin.cpp

128

This shouldn't be the SmallVector, this should be an ArrayRef unless they need to modify it (or a llvm::SmallVectorImpl of the type if they do).

Also, anything that you're changing to be a 'reference' from a copy changes semantics unless you make it a const-ref.

clang/lib/CodeGen/CGNonTrivialStruct.cpp

326

Same comment here, if this doesn't need to modify the Args, it should be an ArrayRef, else a SmallVectorImpl.

clang/lib/CodeGen/CodeGenAction.cpp

89

This one makes sense, contains a couple std::strings.

clang/lib/CodeGen/CodeGenModule.cpp

993

Same here.

clang/lib/Frontend/ASTUnit.cpp

2067

Should this be const-ref? Since we couldn't modify the original before, and now this allows us to modify the original.

clang/lib/Sema/CodeCompleteConsumer.cpp

641

Same above re: const.

clang/lib/Sema/SemaCodeComplete.cpp

4151

Same above re: cosnt.

clang/lib/Sema/SemaConcept.cpp

163

const.

1373

const on these.

clang/lib/Sema/SemaInit.cpp

5290

This should probably do the work to be an ArrayRef with a deduced type. In fact, the deduction probably isn't necessary, since every collection is an IniitializedEntity.

clang/lib/Serialization/GlobalModuleIndex.cpp

130

This probably should NOT happen. The point of this type is to contain a state for this type, and this would be a breaking change, since it would result in the cursor in the containing type being modified.

[Manna]

Thanks @erichkeane for reviews. I have updated patch.

[Manna]

I have uploaded wrong one. I will fix it with new patch.

[Manna]

I have addressed @erichkeane's review comments.

[erichkeane]

These all LGTM.

[Manna]

Fix clang-format error.

[Manna]

Thanks @erichkeane.
I have committed the patch here: https://reviews.llvm.org/rG33cf2a39cb11

[Manna]

I am closing this revision since it's already been pushed https://reviews.llvm.org/rG33cf2a39cb11

[erichkeane]

Note, in the future, if in your commit message you do:

Differential Revision: <URL to the review>

the review will auto-close so you don't have to manually do so. AND it'll automatically link to the commit you made. It makes it easier to find your reviews as well from the git-log, so please do so!

[Manna]

Thanks @erichkeane for the suggestion. I will follow it in future commits.

In D147708#4255361, @erichkeane wrote:

Note, in the future, if in your commit message you do:

Differential Revision: <URL to the review>

the review will auto-close so you don't have to manually do so. AND it'll automatically link to the commit you made. It makes it easier to find your reviews as well from the git-log, so please do so!

clang/lib/CodeGen/CGGPUBuiltin.cpp

for (unsigned I = 1, NumArgs = Args.size(); I < NumArgs; ++I) {
       Builder.CreateAlignedStore(Arg, P, DL.getPrefTypeAlign(Arg->getType()));
     }
     llvm::Value *BufferPtr =
         Builder.CreatePointerCast(Alloca, llvm::Type::getInt8PtrTy(Ctx));
     return {BufferPtr, DL.getTypeAllocSize(AllocaTy)};
   }
 }
 
-bool containsNonScalarVarargs(CodeGenFunction *CGF, CallArgList Args) {
+bool containsNonScalarVarargs(CodeGenFunction *CGF, CallArgList &Args) {
   return llvm::any_of(llvm::drop_begin(Args), [&](const CallArg &A) {
     return !A.getRValue(*CGF).isScalar();
   });
 }
 
 RValue EmitDevicePrintfCallExpr(const CallExpr *E, CodeGenFunction *CGF,
                                 llvm::Function *Decl, bool WithSizeArg) {
   CodeGenModule &CGM = CGF->CGM;

clang/lib/CodeGen/CGNonTrivialStruct.cpp

static const CGFunctionInfo &getFunctionInfo(CodeGenModule &CGM,
   llvm::append_range(Args, Params);
 
   return CGM.getTypes().arrangeBuiltinFunctionDeclaration(Ctx.VoidTy, Args);
 }
 
 template <size_t N, size_t... Ints>
 static std::array<Address, N> getParamAddrs(std::index_sequence<Ints...> IntSeq,
                                             std::array<CharUnits, N> Alignments,
-                                            FunctionArgList Args,
+                                            FunctionArgList &Args,
                                             CodeGenFunction *CGF) {
   return std::array<Address, N>{
       {Address(CGF->Builder.CreateLoad(CGF->GetAddrOfLocalVar(Args[Ints])),
                CGF->VoidPtrTy, Alignments[Ints], KnownNonNull)...}};
 }
 
 // Template classes that are used as bases for classes that emit special
 // functions.

clang/lib/CodeGen/CodeGenAction.cpp

public:
     }
 
   private:
     const CodeGenOptions &CodeGenOpts;
     BackendConsumer *BackendCon;
   };
 
   static void reportOptRecordError(Error E, DiagnosticsEngine &Diags,
-                                   const CodeGenOptions CodeGenOpts) {
+                                   const CodeGenOptions &CodeGenOpts) {
     handleAllErrors(
         std::move(E),
       [&](const LLVMRemarkSetupFileError &E) {
           Diags.Report(diag::err_cannot_open_file)
               << CodeGenOpts.OptRecordFile << E.message();
         },
       [&](const LLVMRemarkSetupPatternError &E) {
           Diags.Report(diag::err_drv_optimization_remark_pattern)

clang/lib/CodeGen/CodeGenModule.cpp

llvm::Metadata *OCLVerElts[] = {
           Int32Ty, (Version % 100) / 10))};
   llvm::NamedMDNode *OCLVerMD =
       TheModule.getOrInsertNamedMetadata("opencl.ocl.version");
   llvm::LLVMContext &Ctx = TheModule.getContext();
   OCLVerMD->addOperand(llvm::MDNode::get(Ctx, OCLVerElts));
 }
 
 void CodeGenModule::EmitBackendOptionsMetadata(
-    const CodeGenOptions CodeGenOpts) {
+    const CodeGenOptions &CodeGenOpts) {
   if (getTriple().isRISCV()) {
     getModule().addModuleFlag(llvm::Module::Min, "SmallDataLimit",
                               CodeGenOpts.SmallDataLimit);
   }
 }
 
 void CodeGenModule::UpdateCompletedType(const TagDecl *TD) {
   // Make sure that this type is translated.

clang/lib/Frontend/ASTUnit.cpp

for (unsigned I = 0; I != NumResults; ++I) {
     if (IdentifierInfo *Identifier = Name.getAsIdentifierInfo())
       HiddenNames.insert(Identifier->getName());
     else
       HiddenNames.insert(Name.getAsString());
   }
 }
 
 void AugmentedCodeCompleteConsumer::ProcessCodeCompleteResults(Sema &S,
-                                            CodeCompletionContext Context,
+                                            CodeCompletionContext &Context,
                                             CodeCompletionResult *Results,
                                             unsigned NumResults) {
   // Merge the results we were given with the results we cached.
   bool AddedResult = false;
   uint64_t InContexts =
       Context.getKind() == CodeCompletionContext::CCC_Recovery
         ? NormalContexts : (1LL << Context.getKind());
   // Contains the set of names that are hidden by "local" completion results.

clang/lib/Sema/CodeCompleteConsumer.cpp

bool PrintingCodeCompleteConsumer::isResultFilteredOut(
   case CodeCompletionResult::RK_Pattern:
     return !(Result.Pattern->getTypedText() &&
              StringRef(Result.Pattern->getTypedText()).startswith(Filter));
   }
   llvm_unreachable("Unknown code completion result Kind.");
 }
 
 void PrintingCodeCompleteConsumer::ProcessCodeCompleteResults(
-    Sema &SemaRef, CodeCompletionContext Context, CodeCompletionResult *Results,
-    unsigned NumResults) {
+    Sema &SemaRef, CodeCompletionContext &Context,
+    CodeCompletionResult *Results, unsigned NumResults) {
   std::stable_sort(Results, Results + NumResults);
 
   if (!Context.getPreferredType().isNull())
     OS << "PREFERRED-TYPE: " << Context.getPreferredType() << '\n';
 
   StringRef Filter = SemaRef.getPreprocessor().getCodeCompletionFilter();
   // Print the completions.
   for (unsigned I = 0; I != NumResults; ++I) {

clang/lib/Sema/SemaCodeComplete.cpp

static void AddPrettyFunctionResults(const LangOptions &LangOpts,
   Results.AddResult(Result("__FUNCTION__", CCP_Constant));
   if (LangOpts.C99 || LangOpts.CPlusPlus11)
     Results.AddResult(Result("__func__", CCP_Constant));
   Results.ExitScope();
 }
 
 static void HandleCodeCompleteResults(Sema *S,
                                       CodeCompleteConsumer *CodeCompleter,
-                                      CodeCompletionContext Context,
+                                      CodeCompletionContext &Context,
                                       CodeCompletionResult *Results,
                                       unsigned NumResults) {
   if (CodeCompleter)
     CodeCompleter->ProcessCodeCompleteResults(*S, Context, Results, NumResults);
 }
 
 static CodeCompletionContext
 mapCodeCompletionContext(Sema &S, Sema::ParserCompletionContext PCC) {

clang/lib/Sema/SemaConcept.cpp

bool Sema::CheckConstraintExpression(const Expr *ConstraintExpression,
   return true;
 }
 
 namespace {
 struct SatisfactionStackRAII {
   Sema &SemaRef;
   bool Inserted = false;
   SatisfactionStackRAII(Sema &SemaRef, const NamedDecl *ND,
-                        llvm::FoldingSetNodeID FSNID)
+                        llvm::FoldingSetNodeID &FSNID)
       : SemaRef(SemaRef) {
       if (ND) {
       SemaRef.PushSatisfactionStackEntry(ND, FSNID);
       Inserted = true;
       }
   }
   ~SatisfactionStackRAII() {
         if (Inserted)
for (auto &RConjunction : RDNF) {
                 std::back_inserter(Combined));
       Res.emplace_back(Combined);
     }
   }
   return Res;
 }
 
 template<typename AtomicSubsumptionEvaluator>
-static bool subsumes(NormalForm PDNF, NormalForm QCNF,
+static bool subsumes(NormalForm &PDNF, NormalForm &QCNF,
                      AtomicSubsumptionEvaluator E) {
   // C++ [temp.constr.order] p2
   //   Then, P subsumes Q if and only if, for every disjunctive clause Pi in the
   //   disjunctive normal form of P, Pi subsumes every conjunctive clause Qj in
   //   the conjuctive normal form of Q, where [...]
   for (const auto &Pi : PDNF) {
     for (const auto &Qj : QCNF) {
       // C++ [temp.constr.order] p2

clang/lib/Sema/SemaInit.cpp

static void TryOrBuildParenListInitialization(
   SmallVector<Expr *, 4> InitExprs;
   QualType ResultType;
   Expr *ArrayFiller = nullptr;
   FieldDecl *InitializedFieldInUnion = nullptr;
 
   // Process entities (i.e. array members, base classes, or class fields) by
   // adding an initialization expression to InitExprs for each entity to
   // initialize.
-  auto ProcessEntities = [&](auto Range) -> bool {
+  auto ProcessEntities = [&](auto &Range) -> bool {
     bool IsUnionType = Entity.getType()->isUnionType();
     for (InitializedEntity SubEntity : Range) {
       // Unions should only have one initializer expression.
       // If there are more initializers than it will be caught when we check
       // whether Index equals Args.size().
       if (ArgIndexToProcess == 1 && IsUnionType)
         return true;
 

clang/lib/Serialization/GlobalModuleIndex.cpp

 
 typedef llvm::OnDiskIterableChainedHashTable<IdentifierIndexReaderTrait>
     IdentifierIndexTable;
 
 }
 
 GlobalModuleIndex::GlobalModuleIndex(
     std::unique_ptr<llvm::MemoryBuffer> IndexBuffer,
-    llvm::BitstreamCursor Cursor)
+    llvm::BitstreamCursor &Cursor)
     : Buffer(std::move(IndexBuffer)), IdentifierIndex(), NumIdentifierLookups(),
       NumIdentifierLookupHits() {
   auto Fail = [&](llvm::Error &&Err) {
     report_fatal_error("Module index '" + Buffer->getBufferIdentifier() +
                        "' failed: " + toString(std::move(Err)));
   };
 
   llvm::TimeTraceScope TimeScope("Module LoadIndex");