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

Reply via email to