[PATCH] D71734: [Modules] Handle tag types and complain about bad merges in C/Objective-C mode

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

Think I'll need to make another review pass but here are my comments so far:

- Why are you adding ODR hash support for `RecordDecl` and not `TagDecl`? We already have support for `EnumDecl`, so `TagDecl` seems like a good candidate to cover both. Honestly, I don't know if it is possible or a good idea but it looks plausible enough to consider.
- Are anonymous structs working? Worth adding test cases.
- Are unions working? Didn't notice any code specifically for them but `RecordDecl` covers both structs and unions, so they should be working and we need to test that.
- Few testing additions. These cases might be already covered or might be low value, so take these suggestions with a grain of salt:
  - test self-referential structs like `struct Node { struct Node *next; };`
  - test comparing structs and forward declarations, e.g., `struct S;` and `struct S { ... };`, and another couple `struct S { ... };` and `struct S; struct S { ... };` The motivation is to make sure we aren't stumped when we cannot find struct definition or when the definition is in unexpected place.



================
Comment at: clang/lib/AST/Decl.cpp:4519
+  Hash.AddRecordDecl(this);
+  setHasODRHash();
+  ODRHash = Hash.CalculateHash();
----------------
To be consistent with the existing code, it is better to call it as `setHasODRHash(true)`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71734