[clang] [clang][ASTImporter] Improve structural equivalence of overloadable operators. (PR #72242)
Balázs Kéri via cfe-commits
cfe-commits at lists.llvm.org
Mon Nov 27 03:39:10 PST 2023
https://github.com/balazske updated https://github.com/llvm/llvm-project/pull/72242
>From 5300f979c96eb2f88c298872f0519e274c155cfe Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.keri at ericsson.com>
Date: Tue, 14 Nov 2023 11:53:20 +0100
Subject: [PATCH 1/2] [clang][ASTImporter] Improve structural equivalence of
overloadable operators.
Operators that are overloadable may be parsed as `CXXOperatorCallExpr`
or as `UnaryOperator` (or `BinaryOperator`). This depends on the context
and can be different if a similar construct is imported into an existing AST.
The two "forms" of the operator call AST nodes should be detected as
equivalent to allow AST import of these cases.
This fix has probably other consequences because if a structure is imported
that has `CXXOperatorCallExpr` into an AST with an existing similar structure
that has `UnaryOperator` (or binary), the additional data in the
`CXXOperatorCallExpr` node is lost at the import (because the existing node
will be used). I am not sure if this can cause problems.
---
clang/lib/AST/ASTStructuralEquivalence.cpp | 57 ++++++
.../AST/StructuralEquivalenceTest.cpp | 170 ++++++++++++++++++
2 files changed, 227 insertions(+)
diff --git a/clang/lib/AST/ASTStructuralEquivalence.cpp b/clang/lib/AST/ASTStructuralEquivalence.cpp
index 6bb4bf14b873d7b..b937ff0579ca02d 100644
--- a/clang/lib/AST/ASTStructuralEquivalence.cpp
+++ b/clang/lib/AST/ASTStructuralEquivalence.cpp
@@ -98,6 +98,8 @@ static bool IsStructurallyEquivalent(StructuralEquivalenceContext &Context,
QualType T1, QualType T2);
static bool IsStructurallyEquivalent(StructuralEquivalenceContext &Context,
Decl *D1, Decl *D2);
+static bool IsStructurallyEquivalent(StructuralEquivalenceContext &Context,
+ const Stmt *S1, const Stmt *S2);
static bool IsStructurallyEquivalent(StructuralEquivalenceContext &Context,
const TemplateArgument &Arg1,
const TemplateArgument &Arg2);
@@ -437,12 +439,67 @@ class StmtComparer {
};
} // namespace
+static bool IsStructurallyEquivalent(StructuralEquivalenceContext &Context,
+ const UnaryOperator *E1,
+ const CXXOperatorCallExpr *E2) {
+ return UnaryOperator::getOverloadedOperator(E1->getOpcode()) ==
+ E2->getOperator() &&
+ IsStructurallyEquivalent(Context, E1->getSubExpr(), E2->getArg(0));
+}
+
+static bool IsStructurallyEquivalent(StructuralEquivalenceContext &Context,
+ const CXXOperatorCallExpr *E1,
+ const UnaryOperator *E2) {
+ return E1->getOperator() ==
+ UnaryOperator::getOverloadedOperator(E2->getOpcode()) &&
+ IsStructurallyEquivalent(Context, E1->getArg(0), E2->getSubExpr());
+}
+
+static bool IsStructurallyEquivalent(StructuralEquivalenceContext &Context,
+ const BinaryOperator *E1,
+ const CXXOperatorCallExpr *E2) {
+ return BinaryOperator::getOverloadedOperator(E1->getOpcode()) ==
+ E2->getOperator() &&
+ IsStructurallyEquivalent(Context, E1->getLHS(), E2->getArg(0)) &&
+ IsStructurallyEquivalent(Context, E1->getRHS(), E2->getArg(1));
+}
+
+static bool IsStructurallyEquivalent(StructuralEquivalenceContext &Context,
+ const CXXOperatorCallExpr *E1,
+ const BinaryOperator *E2) {
+ return E1->getOperator() ==
+ BinaryOperator::getOverloadedOperator(E2->getOpcode()) &&
+ IsStructurallyEquivalent(Context, E1->getArg(0), E2->getLHS()) &&
+ IsStructurallyEquivalent(Context, E1->getArg(1), E2->getRHS());
+}
+
/// Determine structural equivalence of two statements.
static bool IsStructurallyEquivalent(StructuralEquivalenceContext &Context,
const Stmt *S1, const Stmt *S2) {
if (!S1 || !S2)
return S1 == S2;
+ // Check for statements with similar syntax but different AST.
+ // The unary and binary operators (like '+', '&') can be parsed as
+ // CXXOperatorCall too (and UnaryOperator or BinaryOperator).
+ // This depends on arguments with unresolved type and on the name lookup.
+ // The lookup results can be different in a "From" and "To" AST even if the
+ // compared structure is otherwise equivalent. For this reason we must treat
+ // these operators as equivalent even if they are represented by different AST
+ // nodes.
+ if (const auto *E2CXXOperatorCall = dyn_cast<CXXOperatorCallExpr>(S2)) {
+ if (const auto *E1Unary = dyn_cast<UnaryOperator>(S1))
+ return IsStructurallyEquivalent(Context, E1Unary, E2CXXOperatorCall);
+ if (const auto *E1Binary = dyn_cast<BinaryOperator>(S1))
+ return IsStructurallyEquivalent(Context, E1Binary, E2CXXOperatorCall);
+ }
+ if (const auto *E1CXXOperatorCall = dyn_cast<CXXOperatorCallExpr>(S1)) {
+ if (const auto *E2Unary = dyn_cast<UnaryOperator>(S2))
+ return IsStructurallyEquivalent(Context, E1CXXOperatorCall, E2Unary);
+ if (const auto *E2Binary = dyn_cast<BinaryOperator>(S2))
+ return IsStructurallyEquivalent(Context, E1CXXOperatorCall, E2Binary);
+ }
+
// Compare the statements itself.
StmtComparer Comparer(Context);
if (!Comparer.IsEquivalent(S1, S2))
diff --git a/clang/unittests/AST/StructuralEquivalenceTest.cpp b/clang/unittests/AST/StructuralEquivalenceTest.cpp
index 44d950cfe758f14..539e3a4a2c01dca 100644
--- a/clang/unittests/AST/StructuralEquivalenceTest.cpp
+++ b/clang/unittests/AST/StructuralEquivalenceTest.cpp
@@ -2252,6 +2252,176 @@ TEST_F(StructuralEquivalenceStmtTest, UnaryOperatorDifferentOps) {
EXPECT_FALSE(testStructuralMatch(t));
}
+TEST_F(StructuralEquivalenceStmtTest,
+ CXXOperatorCallExprVsUnaryBinaryOperator) {
+ auto t = makeNamedDecls(
+ R"(
+ template <typename T, T x>
+ class A;
+ template <typename T, T x, T y>
+ void foo(
+ A<T, x + y>,
+ A<T, x - y>,
+ A<T, -x>,
+ A<T, x * y>,
+ A<T, *x>,
+ A<T, x / y>,
+ A<T, x % y>,
+ A<T, x ^ y>,
+ A<T, x & y>,
+ A<T, &x>,
+ A<T, x | y>,
+ A<T, ~x>,
+ A<T, !x>,
+ A<T, x < y>,
+ A<T, (x > y)>,
+ A<T, x << y>,
+ A<T, (x >> y)>,
+ A<T, x == y>,
+ A<T, x != y>,
+ A<T, x <= y>,
+ A<T, x >= y>,
+ A<T, x <=> y>,
+ A<T, x && y>,
+ A<T, x || y>,
+ A<T, ++x>,
+ A<T, --x>,
+ A<T, (x , y)>,
+ A<T, x ->* y>,
+ A<T, x -> y>
+ );
+ )",
+ R"(
+ struct Bar {
+ Bar& operator=(Bar&);
+ Bar& operator->();
+ };
+
+ Bar& operator+(Bar&, Bar&);
+ Bar& operator+(Bar&);
+ Bar& operator-(Bar&, Bar&);
+ Bar& operator-(Bar&);
+ Bar& operator*(Bar&, Bar&);
+ Bar& operator*(Bar&);
+ Bar& operator/(Bar&, Bar&);
+ Bar& operator%(Bar&, Bar&);
+ Bar& operator^(Bar&, Bar&);
+ Bar& operator&(Bar&, Bar&);
+ Bar& operator&(Bar&);
+ Bar& operator|(Bar&, Bar&);
+ Bar& operator~(Bar&);
+ Bar& operator!(Bar&);
+ Bar& operator<(Bar&, Bar&);
+ Bar& operator>(Bar&, Bar&);
+ Bar& operator+=(Bar&, Bar&);
+ Bar& operator-=(Bar&, Bar&);
+ Bar& operator*=(Bar&, Bar&);
+ Bar& operator/=(Bar&, Bar&);
+ Bar& operator%=(Bar&, Bar&);
+ Bar& operator^=(Bar&, Bar&);
+ Bar& operator&=(Bar&, Bar&);
+ Bar& operator|=(Bar&, Bar&);
+ Bar& operator<<(Bar&, Bar&);
+ Bar& operator>>(Bar&, Bar&);
+ Bar& operator<<=(Bar&, Bar&);
+ Bar& operator>>=(Bar&, Bar&);
+ Bar& operator==(Bar&, Bar&);
+ Bar& operator!=(Bar&, Bar&);
+ Bar& operator<=(Bar&, Bar&);
+ Bar& operator>=(Bar&, Bar&);
+ Bar& operator<=>(Bar&, Bar&);
+ Bar& operator&&(Bar&, Bar&);
+ Bar& operator||(Bar&, Bar&);
+ Bar& operator++(Bar&);
+ Bar& operator--(Bar&);
+ Bar& operator,(Bar&, Bar&);
+ Bar& operator->*(Bar&, Bar&);
+
+ template <typename T, T x>
+ class A;
+ template <typename T, T x, T y>
+ void foo(
+ A<T, x + y>,
+ A<T, x - y>,
+ A<T, -x>,
+ A<T, x * y>,
+ A<T, *x>,
+ A<T, x / y>,
+ A<T, x % y>,
+ A<T, x ^ y>,
+ A<T, x & y>,
+ A<T, &x>,
+ A<T, x | y>,
+ A<T, ~x>,
+ A<T, !x>,
+ A<T, x < y>,
+ A<T, (x > y)>,
+ A<T, x << y>,
+ A<T, (x >> y)>,
+ A<T, x == y>,
+ A<T, x != y>,
+ A<T, x <= y>,
+ A<T, x >= y>,
+ A<T, x <=> y>,
+ A<T, x && y>,
+ A<T, x || y>,
+ A<T, ++x>,
+ A<T, --x>,
+ A<T, (x , y)>,
+ A<T, x ->* y>,
+ A<T, x -> y>
+ );
+ )",
+ Lang_CXX20);
+ EXPECT_TRUE(testStructuralMatch(t));
+}
+
+TEST_F(StructuralEquivalenceStmtTest,
+ CXXOperatorCallExprVsUnaryBinaryOperatorNe) {
+ auto t = makeNamedDecls(
+ R"(
+ template <typename T, T x>
+ class A;
+ template <typename T, T x, T y>
+ void foo(
+ A<T, x + y>
+ );
+ )",
+ R"(
+ struct Bar;
+
+ Bar& operator+(Bar&, Bar&);
+
+ template <typename T, T x>
+ class A;
+ template <typename T, T x, T y>
+ void foo(
+ A<T, x - y>
+ );
+ )",
+ Lang_CXX11);
+ EXPECT_FALSE(testStructuralMatch(t));
+}
+
+TEST_F(StructuralEquivalenceStmtTest, NonTypeTemplateParm) {
+ auto t = makeNamedDecls(
+ R"(
+ template <typename T, T x>
+ class A;
+ template <typename T, T x, T y>
+ void foo(A<T, x>);
+ )",
+ R"(
+ template <typename T, T x>
+ class A;
+ template <typename T, T x, T y>
+ void foo(A<T, y>);
+ )",
+ Lang_CXX11);
+ // FIXME: These should not match,
+ EXPECT_TRUE(testStructuralMatch(t));
+}
+
TEST_F(StructuralEquivalenceStmtTest, UnresolvedLookupDifferentName) {
auto t = makeStmts(
R"(
>From 0c34b2d8812574a02b492dc65c6e7c340864750f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.keri at ericsson.com>
Date: Mon, 27 Nov 2023 12:38:15 +0100
Subject: [PATCH 2/2] updated code comment
---
clang/lib/AST/ASTStructuralEquivalence.cpp | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/clang/lib/AST/ASTStructuralEquivalence.cpp b/clang/lib/AST/ASTStructuralEquivalence.cpp
index b937ff0579ca02d..414cd3d079dd2ba 100644
--- a/clang/lib/AST/ASTStructuralEquivalence.cpp
+++ b/clang/lib/AST/ASTStructuralEquivalence.cpp
@@ -480,13 +480,13 @@ static bool IsStructurallyEquivalent(StructuralEquivalenceContext &Context,
return S1 == S2;
// Check for statements with similar syntax but different AST.
- // The unary and binary operators (like '+', '&') can be parsed as
- // CXXOperatorCall too (and UnaryOperator or BinaryOperator).
- // This depends on arguments with unresolved type and on the name lookup.
- // The lookup results can be different in a "From" and "To" AST even if the
- // compared structure is otherwise equivalent. For this reason we must treat
- // these operators as equivalent even if they are represented by different AST
- // nodes.
+ // A UnaryOperator node is more lightweight than a CXXOperatorCallExpr node.
+ // The more heavyweight node is only created if the definition-time name
+ // lookup had any results. The lookup results are stored CXXOperatorCallExpr
+ // only. The lookup results can be different in a "From" and "To" AST even if
+ // the compared structure is otherwise equivalent. For this reason we must
+ // treat a similar unary/binary operator node and CXXOperatorCall node as
+ // equivalent.
if (const auto *E2CXXOperatorCall = dyn_cast<CXXOperatorCallExpr>(S2)) {
if (const auto *E1Unary = dyn_cast<UnaryOperator>(S1))
return IsStructurallyEquivalent(Context, E1Unary, E2CXXOperatorCall);
More information about the cfe-commits
mailing list