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

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Build failures seem relevant.



================
Comment at: clang/lib/CodeGen/CGGPUBuiltin.cpp:128
 
-bool containsNonScalarVarargs(CodeGenFunction *CGF, CallArgList Args) {
+bool containsNonScalarVarargs(CodeGenFunction *CGF, CallArgList &Args) {
   return llvm::any_of(llvm::drop_begin(Args), [&](const CallArg &A) {
----------------
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.


================
Comment at: clang/lib/CodeGen/CGNonTrivialStruct.cpp:326
                                             std::array<CharUnits, N> Alignments,
-                                            FunctionArgList Args,
+                                            FunctionArgList &Args,
                                             CodeGenFunction *CGF) {
----------------
Same comment here, if this doesn't need to modify the Args, it should be an `ArrayRef`, else a `SmallVectorImpl`.


================
Comment at: clang/lib/CodeGen/CodeGenAction.cpp:89
   static void reportOptRecordError(Error E, DiagnosticsEngine &Diags,
-                                   const CodeGenOptions CodeGenOpts) {
+                                   const CodeGenOptions &CodeGenOpts) {
     handleAllErrors(
----------------
This one makes sense, contains a couple std::strings.


================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:993
 void CodeGenModule::EmitBackendOptionsMetadata(
-    const CodeGenOptions CodeGenOpts) {
+    const CodeGenOptions &CodeGenOpts) {
   if (getTriple().isRISCV()) {
----------------
Same here.


================
Comment at: clang/lib/Frontend/ASTUnit.cpp:2067
 void AugmentedCodeCompleteConsumer::ProcessCodeCompleteResults(Sema &S,
-                                            CodeCompletionContext Context,
+                                            CodeCompletionContext &Context,
                                             CodeCompletionResult *Results,
----------------
Should this be const-ref?  Since we couldn't modify the original before, and now this allows us to modify the original.


================
Comment at: clang/lib/Sema/CodeCompleteConsumer.cpp:641
 void PrintingCodeCompleteConsumer::ProcessCodeCompleteResults(
-    Sema &SemaRef, CodeCompletionContext Context, CodeCompletionResult *Results,
-    unsigned NumResults) {
+    Sema &SemaRef, CodeCompletionContext &Context,
+    CodeCompletionResult *Results, unsigned NumResults) {
----------------
Same above re: const.


================
Comment at: clang/lib/Sema/SemaCodeComplete.cpp:4151
                                       CodeCompleteConsumer *CodeCompleter,
-                                      CodeCompletionContext Context,
+                                      CodeCompletionContext &Context,
                                       CodeCompletionResult *Results,
----------------
Same above re: cosnt.


================
Comment at: clang/lib/Sema/SemaConcept.cpp:163
   SatisfactionStackRAII(Sema &SemaRef, const NamedDecl *ND,
-                        llvm::FoldingSetNodeID FSNID)
+                        llvm::FoldingSetNodeID &FSNID)
       : SemaRef(SemaRef) {
----------------
const.


================
Comment at: clang/lib/Sema/SemaConcept.cpp:1373
 template<typename AtomicSubsumptionEvaluator>
-static bool subsumes(NormalForm PDNF, NormalForm QCNF,
+static bool subsumes(NormalForm &PDNF, NormalForm &QCNF,
                      AtomicSubsumptionEvaluator E) {
----------------
const on these.


================
Comment at: clang/lib/Sema/SemaInit.cpp:5290
   // initialize.
-  auto ProcessEntities = [&](auto Range) -> bool {
+  auto ProcessEntities = [&](auto &Range) -> bool {
     bool IsUnionType = Entity.getType()->isUnionType();
----------------
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`.


================
Comment at: clang/lib/Serialization/GlobalModuleIndex.cpp:130
     std::unique_ptr<llvm::MemoryBuffer> IndexBuffer,
-    llvm::BitstreamCursor Cursor)
+    llvm::BitstreamCursor &Cursor)
     : Buffer(std::move(IndexBuffer)), IdentifierIndex(), NumIdentifierLookups(),
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147708