[PATCH] D21675: New ODR checker for modules

[Richard Smith via cfe-commits]

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