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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
