[PATCH] D55646: [ASTImporter] Make ODR diagnostics warning by default

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In general, I agree that it doesn't make sense for ODR violations (with false positives) to trigger error diagnostics while performing AST merging, so downgrading this to warnings is a good idea. However, I do worry about impacting modules and C compatibility with the changes. I'm curious what @rsmith thinks the answer should be here. AFAIK, modules do not *require* ODR violations checks (I believe that's still ill-formed, no diagnostic required), but confirmation would be good.



================
Comment at: lib/Sema/SemaType.cpp:7609
+      StructuralEquivalenceKind::Default, false /*StrictTypeSpelling*/,
+      true /*Complain*/);
   return Ctx.IsEquivalent(D, Suggested);
----------------
Are we sure this isn't breaking C compatibility? IIRC, C has structural equivalence rules because it doesn't have quite the same notion of an ODR as C++ does regarding things like tag redeclarations. I worry that we'll be downgrading valid errors into warnings here because of that.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55646/new/

https://reviews.llvm.org/D55646