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

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

Thanks!
Added a couple of superficial coding style comments.

I think there should also be a test for C99 and one for C11 (and potentially ObjC2).



================
Comment at: include/clang/AST/ASTStructuralEquivalence.h:52
 
+  /// \brief Whether warn or error on tag type mismatches.
+  bool ErrorOnTagTypeMismatch;
----------------
No need to use \brief in new code any more. We are compiling with the Doxygen autobrief option.


================
Comment at: lib/Parse/ParseDecl.cpp:4316
+        !Actions.hasStructuralCompatLayout(TagDecl, CheckCompatTag.NewDef)) {
+      DS.SetTypeSpecError(); // Bail out...
+      return;
----------------
I think the LLVM coding style wants this to be 
```
{
    // Bail out.
    DS.SetTypeSpecError();
    return;
}
```

or
```
// Bail out.
return DS.SetTypeSpecError();
```


================
Comment at: lib/Parse/ParseDeclCXX.cpp:1913
+                    : TagOrTempResult.get();
+      // Parse the definition body
+      ParseStructUnionBody(StartLoc, TagType, D);
----------------
`.`


================
Comment at: lib/Sema/SemaDecl.cpp:13386
+              bool AllowODR = getLangOpts().CPlusPlus || getLangOpts().ObjC1 ||
+                              getLangOpts().C11;
               NamedDecl *Hidden = nullptr;
----------------
Does `ObjC2` imply `ObjC1`?


================
Comment at: lib/Sema/SemaType.cpp:7085
 
+/// \brief Determine if \p D abd \p Suggested have a structurally compatibale
+///        layout as described by C11 6.2.7/1.
----------------
Comment should go on the declaration.


https://reviews.llvm.org/D31778