[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 ↗

(On Diff #511398)

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 ↗

(On Diff #511398)

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 ↗

(On Diff #511398)

Same here.

clang/lib/Frontend/ASTUnit.cpp

2067 ↗

(On Diff #511398)

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 ↗

(On Diff #511398)

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 ↗

(On Diff #511398)

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 ↗

(On Diff #511398)

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/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/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,
+                                      const 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)
+                        const 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(const NormalForm &PDNF, const 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