[StructuralEquivalence] fix crash & improve comparing on GenericSelectionExpr

[danix800]

1. fix crash on 'default' assoc which has no TypeSourceInfo (nullptr);
2. improve equivalency comparison on control expr & associated exprs.

[llvmbot]

@llvm/pr-subscribers-clang

Changes

1. fix crash on 'default' assoc which has no TypeSourceInfo (nullptr);
2. improve equivalency comparison on control expr & associated exprs.

---

Full diff: https://github.com/llvm/llvm-project/pull/67458.diff

2 Files Affected:

• (modified) clang/lib/AST/ASTStructuralEquivalence.cpp (+20)
• (modified) clang/unittests/AST/StructuralEquivalenceTest.cpp (+22)

diff --git a/clang/lib/AST/ASTStructuralEquivalence.cpp b/clang/lib/AST/ASTStructuralEquivalence.cpp
index 5b98d14dd3d9104..d9b2f045552d52c 100644
--- a/clang/lib/AST/ASTStructuralEquivalence.cpp
+++ b/clang/lib/AST/ASTStructuralEquivalence.cpp
@@ -247,6 +247,20 @@ class StmtComparer {
 
   bool IsStmtEquivalent(const GenericSelectionExpr *E1,
                         const GenericSelectionExpr *E2) {
+    if (!IsStructurallyEquivalent(Context,
+                                  const_cast<Expr *>(E1->getControllingExpr()),
+                                  const_cast<Expr *>(E2->getControllingExpr())))
+      return false;
+
+    for (auto Pair : zip_longest(E1->getAssocExprs(), E2->getAssocExprs())) {
+      std::optional<Expr *> Child1 = std::get<0>(Pair);
+      std::optional<Expr *> Child2 = std::get<1>(Pair);
+      if (!Child1 || !Child2)
+        return false;
+      if (!IsStructurallyEquivalent(Context, *Child1, *Child2))
+        return false;
+    }
+
     for (auto Pair : zip_longest(E1->getAssocTypeSourceInfos(),
                                  E2->getAssocTypeSourceInfos())) {
       std::optional<TypeSourceInfo *> Child1 = std::get<0>(Pair);
@@ -255,6 +269,12 @@ class StmtComparer {
       if (!Child1 || !Child2)
         return false;
 
+      if (!(*Child1) != !(*Child2))
+        return false;
+
+      if (!(*Child1))
+        continue;
+
       if (!IsStructurallyEquivalent(Context, (*Child1)->getType(),
                                     (*Child2)->getType()))
         return false;
diff --git a/clang/unittests/AST/StructuralEquivalenceTest.cpp b/clang/unittests/AST/StructuralEquivalenceTest.cpp
index 44d950cfe758f14..35c1385e73fb389 100644
--- a/clang/unittests/AST/StructuralEquivalenceTest.cpp
+++ b/clang/unittests/AST/StructuralEquivalenceTest.cpp
@@ -2091,6 +2091,28 @@ TEST_F(StructuralEquivalenceStmtTest, GenericSelectionExprOrderDiffers) {
   EXPECT_FALSE(testStructuralMatch(t));
 }
 
+TEST_F(StructuralEquivalenceStmtTest, GenericSelectionExprControlDiffers) {
+  auto t = makeWrappedStmts("_Generic(0u, unsigned int: 0, float: 1)",
+                            "_Generic(1u, unsigned int: 0, float: 1)", Lang_C99,
+                            genericSelectionExpr());
+  EXPECT_FALSE(testStructuralMatch(t));
+}
+
+TEST_F(StructuralEquivalenceStmtTest, GenericSelectionExprAssocExprDiffers) {
+  auto t = makeWrappedStmts("_Generic(0u, unsigned int: 0, float: 1)",
+                            "_Generic(0u, unsigned int: 1u, float: 1)", Lang_C99,
+                            genericSelectionExpr());
+  EXPECT_FALSE(testStructuralMatch(t));
+}
+
+TEST_F(StructuralEquivalenceStmtTest,
+       GenericSelectionExprDefaultNoTypeSourceInfo) {
+  auto t = makeWrappedStmts("_Generic(0u, unsigned int: 0, default: 1)",
+                            "_Generic(0u, unsigned int: 0, default: 1)",
+                            Lang_C99, genericSelectionExpr());
+  EXPECT_TRUE(testStructuralMatch(t));
+}
+
 TEST_F(StructuralEquivalenceStmtTest, GenericSelectionExprDependentResultSame) {
   auto t = makeStmts(
       R"(

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[tmroeder]

Could you please add comments to the new checks that explains why they are needed?

I don't doubt that they are important, but I haven't touched this code in a very long time, and I'm sure that other readers of this code would appreciate it.

[cor3ntin]

@AaronBallman ping

[AaronBallman]

Oh wow, this seems to have fallen off my radar for quite some time, thank you for the ping!

@danix800 are you planning to continue working on this?