steakhal added a comment.
I very much like the code.
L1364 is uncovered according to my coverage results.
================
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;
----------------
Shouldn't you also check the name as well?
================
Comment at: clang/lib/AST/ASTStructuralEquivalence.cpp:1379
+
+ DC1 = DC1->getParent()->getNonTransparentContext();
+ DC2 = DC2->getParent()->getNonTransparentContext();
----------------
So, the previous `DC1->isTranslationUnit()` guards the `DC1->getParent()` to
never be `NULL`; same for `DC2`.
Pretty neat.
================
Comment at: clang/unittests/AST/StructuralEquivalenceTest.cpp:982-988
+TEST_F(StructuralEquivalenceRecordContextTest, NamespaceNamedVsInline) {
+ auto Decls = makeDecls<CXXRecordDecl>(
+ "namespace A { namespace A { class X; } }",
+ "namespace A { inline namespace A { class X; } }", Lang_CXX17,
+ cxxRecordDecl());
+ EXPECT_FALSE(testStructuralMatch(Decls));
+}
----------------
The outer-most namespace could also be //inline//.
I'd love to see a test about that as well.
================
Comment at: clang/unittests/AST/StructuralEquivalenceTest.cpp:1004-1009
+TEST_F(StructuralEquivalenceRecordContextTest, TransparentContextNE) {
+ auto Decls = makeDecls<CXXRecordDecl>("extern \"C\" { class X; }",
+ "namespace { class X; }", Lang_CXX17,
+ cxxRecordDecl());
+ EXPECT_FALSE(testStructuralMatch(Decls));
+}
----------------
`extern "C"` might be nested within other namespaces.
Probably worth testing that as well.
================
Comment at: clang/unittests/AST/StructuralEquivalenceTest.cpp:1025-1032
+TEST_F(StructuralEquivalenceTest, StructDefinitionInPrototype) {
+ auto Decls = makeDecls<ParmVarDecl>(
+ "struct Param { int a; }; void f(struct Param *p);",
+ "void f(struct Param { int a; } *p);", Lang_C89,
+ parmVarDecl(hasName("p")));
+ EXPECT_TRUE(testStructuralMatch(Decls));
+}
----------------
Please, also test if the name of the structs is mismatched.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D113118/new/
https://reviews.llvm.org/D113118
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
