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

[Soumi Manna via Phabricator via cfe-commits]

Manna marked 4 inline comments as done.
Manna added inline comments.


================
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) {
----------------
tahonermann wrote:
> This looks like a good change; `CallArgList` derives from `SmallVector<CallArg, 8>` and holds a few other `SmallVector` specializations as data members.
>>CallArgList derives from SmallVector<CallArg, 8> and holds a few other SmallVector specializations as data members.

Yes


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


================
Comment at: clang/lib/Sema/SemaType.cpp:4556
 
-static bool IsNoDerefableChunk(DeclaratorChunk Chunk) {
+static bool IsNoDerefableChunk(const DeclaratorChunk &Chunk) {
   return (Chunk.Kind == DeclaratorChunk::Pointer ||
----------------
tahonermann wrote:
> 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.
Thanks @tahonermann for reviews and feedbacks. 

>>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. 

Yes, this is what i saw in same file (https://clang.llvm.org/doxygen/SemaType_8cpp_source.html) while investigating this bug - it is usually passing by const references.


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

https://reviews.llvm.org/D149163