[PATCH] D113118: [clang][AST] Check context of record in structural equivalence.

[Balázs Kéri via Phabricator via cfe-commits]

balazske added a comment.

In the last case the the RecordDecl's `a` and `b` can be matched (not internal statements in a function) and this is similar to the existing test cases. The `struct a` and `struct b` already fails to match because different name.



================
Comment at: clang/lib/AST/ASTStructuralEquivalence.cpp:1360-1364
+    // Special case: We allow a struct defined in a function to be equivalent
+    // with a similar struct defined outside of a function.
+    if ((DC1->isFunctionOrMethod() && DC2->isTranslationUnit()) ||
+        (DC2->isFunctionOrMethod() && DC1->isTranslationUnit()))
+      return true;
----------------
steakhal wrote:
> Shouldn't you also check the name as well?
Here `DC1` or `DC2` is already a function or translation unit, the name of it is not relevant.


================
Comment at: clang/lib/AST/ASTStructuralEquivalence.cpp:1379
+
+    DC1 = DC1->getParent()->getNonTransparentContext();
+    DC2 = DC2->getParent()->getNonTransparentContext();
----------------
steakhal wrote:
> So, the previous `DC1->isTranslationUnit()` guards the `DC1->getParent()` to never be `NULL`; same for `DC2`.
> Pretty neat.
Change to `!DC1->getParent()` (from `DC1->isTranslationUnit()`)? (The line can be moved to here to increase readability, so the break statement is at end of the loop.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113118