rsmith added a comment.
Thanks! I assume there's still no measurable performance impact?
================
Comment at: include/clang/AST/ODRHash.h:54
+ // in Pointers.
+ size_type NextFreeIndex;
+
----------------
Is this always the same as `Pointers.size()`?
================
Comment at: lib/AST/ODRHash.cpp:35
+ for (auto base : Record->bases()) {
+ ID.AddInteger(base.isVirtual());
+ AddQualType(base.getType());
----------------
AddBoolean for clarity maybe?
================
Comment at: lib/AST/ODRHash.cpp:335-337
+ llvm::SmallVector<const Decl *, 16> Decls(DC->decls_begin(),
+ DC->decls_end());
+ ID.AddInteger(Decls.size());
----------------
You will presumably need to filter out the implicit declarations before you
emit the size of the list, otherwise a `DeclContext` with an implicit
declaration will hash differently from one without.
================
Comment at: lib/AST/ODRHash.cpp:493-495
+ ID.AddBoolean(hasDefaultArgument);
+ if (hasDefaultArgument)
+ AddStmt(D->getDefaultArgument());
----------------
Given that `AddStmt` already writes out an "is null" flag, could you use
`AddStmt(hasDefaultArgument ? D->getDefaultArgument() : nullptr)` here?
================
Comment at: lib/Serialization/ASTReader.cpp:9015-9027
+ if (FirstEnum->getName() != SecondEnum->getName()) {
+ Diag(FirstEnum->getLocStart(),
+ diag::err_module_odr_violation_mismatch_decl_diff)
+ << Merge.first << FirstModule.empty()
+ << FirstEnum->getSourceRange() << FirstModule << EnumName
+ << FirstEnum;
+ Diag(SecondEnum->getLocStart(),
----------------
Can you factor this out into a local lambda or helper class? These dozen lines
are repeated quite a lot of times here with only small variations.
https://reviews.llvm.org/D21675
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
