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