[PATCH] D31778: [Modules] Implement ODR-like semantics for tag types in C/ObjC

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

bruno marked 6 inline comments as done.
bruno added inline comments.


================
Comment at: lib/Parse/ParseDecl.cpp:4236-4237
 
   Sema::SkipBodyInfo SkipBody;
+  Sema::CheckCompatTagInfo CheckCompatTag;
   if (!Name && TUK == Sema::TUK_Definition && Tok.is(tok::l_brace) &&
----------------
rsmith wrote:
> Do we really need both of these? The new stuff seems to be a natural extension of `SkipBodyInfo`, to say "parse it, check it's the same as the previous definition, then throw the new one away".
No specific reason. I'll augment `SkipBodyInfo` with two new fields then (a) a flag for the check and (b) the new tag.


================
Comment at: lib/Sema/SemaExpr.cpp:2198-2209
+  if (R.isAmbiguous()) {
+    if (!IgnorePrevDef)
+      return ExprError();
+    // We already know that there's a hidden decl included in the lookup,
+    // ignore it and only use the first found (the local) one...
+    auto RF = R.makeFilter();
+    NamedDecl *ND = R.getRepresentativeDecl();
----------------
rsmith wrote:
> This is gross. In order to make this work, you'd need to propagate the `IgnorePrevDef` flag through the entire expression parsing machinery.
> 
> Instead of this, how about deferring making the old definition visible until after you complete parsing the new one and do the structural comparison?
Thanks for pointing it out, I totally missed this approach. Your suggestion works and I'll change the patch accordingly. However, `IgnorePrevDef` still needs to be threaded in `ParseEnumBody` and `ActOnEnumConstant` in order to prevent the latter to emit `err_redefinition_of_enumerator`-like diagnostics.


https://reviews.llvm.org/D31778