[PATCH] D149163: [NFC][CLANG] Fix coverity remarks about large copy by values

[Tom Honermann via Phabricator via cfe-commits]

tahonermann accepted this revision.
tahonermann added a comment.
This revision is now accepted and ready to land.

These changes all look good to me. For the `DeclaratorChunk` case, I noted an additional removal of a comment that I think should be considered. Assuming I haven't misread the code, `DeclaratorChunk` does not look like a small value type (at least not any more).



================
Comment at: clang/lib/CodeGen/CGGPUBuiltin.cpp:128
 
-bool containsNonScalarVarargs(CodeGenFunction *CGF, CallArgList Args) {
+bool containsNonScalarVarargs(CodeGenFunction *CGF, const CallArgList &Args) {
   return llvm::any_of(llvm::drop_begin(Args), [&](const CallArg &A) {
----------------
This looks like a good change; `CallArgList` derives from `SmallVector<CallArg, 8>` and holds a few other `SmallVector` specializations as data members.


================
Comment at: clang/lib/CodeGen/CGNonTrivialStruct.cpp:326
                                             std::array<CharUnits, N> Alignments,
-                                            FunctionArgList Args,
+                                            const FunctionArgList &Args,
                                             CodeGenFunction *CGF) {
----------------
This looks like a good change. `FunctionArgList` derives from `SmallVector<const VarDecl *, 16>`.


================
Comment at: clang/lib/CodeGen/CodeGenModule.h:1714
   /// the backend to LLVM.
-  void EmitBackendOptionsMetadata(const CodeGenOptions CodeGenOpts);
+  void EmitBackendOptionsMetadata(const CodeGenOptions &CodeGenOpts);
 
----------------
This is definitely a good change; it seems likely that the omission of the `&` was not intentional.


================
Comment at: clang/lib/Sema/SemaType.cpp:4556
 
-static bool IsNoDerefableChunk(DeclaratorChunk Chunk) {
+static bool IsNoDerefableChunk(const DeclaratorChunk &Chunk) {
   return (Chunk.Kind == DeclaratorChunk::Pointer ||
----------------
The definition of `DeclaratorChunk` has this comment:
  /// This is intended to be a small value object.

However, it contains what looks likely to be a sizable anonymous union. See `clang/include/clang/Sema/DeclSpec.h` lines 1587-1595 and the definition of `FunctionTypeInfo` in that same file. I don't see other pass-by-value uses of this type. Perhaps the above comment should be removed.


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

https://reviews.llvm.org/D149163