rsmith added a comment. There are a bunch of cases here that do this:
if (auto t = getThing()) ID.addThing(t); if (auto t = getOtherThing()) ID.addThing(t); That will result in hash collisions between objects with thing and objects with otherthing (for instance, `struct A { int n : 1; };` and `struct A { int n = 1; };` appear to hash the same). And there are some cases where you add a list of objects without first adding the size, which is liable to allow collisions. I'm not too worried about these cases, since this is just a hash anyway, and is only a best-effort attempt to check for ODR violations, but some of them look like they'd be easy enough to fix, so I figure why not :) Anyway, this looks really good. Do you know if the computation of the ODR hash has any measurable performance impact when building a large module? I'm not really expecting one, but it seems worth checking just in case. > DeclBase.cpp:1827 > + void VisitParmVarDecl(const ParmVarDecl *D) { > + VisitStmt(D->getDefaultArg()); > + Inherited::VisitParmVarDecl(D); You should not include the default argument if it was inherited from a previous declaration. That can happen in a friend declaration: // module 1 void f(int = 0); struct X { friend void f(int); }; // has (inherited) default arg // module 2 struct X { friend void f(int); }; // has no default arg https://reviews.llvm.org/D21675 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits