dblaikie added a comment.
In D81347#2080217 <https://reviews.llvm.org/D81347#2080217>, @aprantl wrote:
> I was going to ask why make this change, but looking at the patch, it's
> pretty obvious :-)
Might be worth writing it down for everyone else - isn't exactly obvious to me
(though not a part of the codebase I'm particularly familiar with).
================
Comment at: clang/include/clang/Basic/Module.h:57
+struct ASTFileSignature : std::array<uint8_t, 20> {
+ using BaseT = std::array<uint8_t, 20>;
+
----------------
Looks like "Base" is the more common name for this purpose (rather than
"BaseT") across the LLVM project. BaseT makes me think (when I'm reading the
rest of the code) that's it's a template parameter.
================
Comment at: clang/include/clang/Basic/Module.h:61
+
+ ASTFileSignature(BaseT S = {{0}}) : BaseT(std::move(S)) {}
+
----------------
Bit of a big structure to pass by value - should it be by const ref instead?
(existing problem with the old code anyway, so no big deal either way)
(also the std::move is a no-op here because std::array<uint32_t> has a move
that's the same as copy, though I don't mind it out of habit/consistency)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D81347/new/
https://reviews.llvm.org/D81347
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
