[PATCH] D55534: [AST] Store "UsesADL" information in CallExpr.
This revision was automatically updated to reflect the committed changes.
Closed by commit rC348977: [AST] Store UsesADL information in
CallExpr. (authored by EricWF, committed by ).
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55534/new/
https://reviews.llvm.org/D55534
Files:
docs/LibASTMatchersReference.html
include/clang/AST/Expr.h
include/clang/AST/ExprCXX.h
include/clang/AST/Stmt.h
include/clang/ASTMatchers/ASTMatchers.h
include/clang/Sema/Overload.h
include/clang/Sema/Sema.h
lib/AST/ASTDumper.cpp
lib/AST/ASTImporter.cpp
lib/AST/Expr.cpp
lib/ASTMatchers/Dynamic/Registry.cpp
lib/Sema/SemaExpr.cpp
lib/Sema/SemaOverload.cpp
lib/Serialization/ASTReaderStmt.cpp
lib/Serialization/ASTWriterStmt.cpp
test/AST/ast-dump-expr.cpp
test/Import/call-expr/Inputs/F.cpp
test/Import/call-expr/test.cpp
unittests/ASTMatchers/ASTMatchersNodeTest.cpp
Index: docs/LibASTMatchersReference.html
===
--- docs/LibASTMatchersReference.html
+++ docs/LibASTMatchersReference.html
@@ -2562,6 +2562,28 @@
+Matcherhttps://clang.llvm.org/doxygen/classclang_1_1CallExpr.html;>CallExprusesADL
+Matches call expressions which were resolved using ADL.
+
+Example matches y(x) but not y(42) or NS::y(x).
+ namespace NS {
+struct X {};
+void y(X);
+ }
+
+ void y(...);
+
+ void test() {
+NS::X x;
+y(x); // Matches
+NS::y(x); // Doesn't match
+y(42); // Doesn't match
+using NS::y;
+y(x); // Found by both unqualified lookup and ADL, doesn't match
+ }
+
+
+
Matcherhttps://clang.llvm.org/doxygen/classclang_1_1CastExpr.html;>CastExprhasCastKindCastKind Kind
Matches casts that has a given cast kind.
Index: include/clang/AST/Stmt.h
===
--- include/clang/AST/Stmt.h
+++ include/clang/AST/Stmt.h
@@ -430,6 +430,9 @@
unsigned : NumExprBits;
unsigned NumPreArgs : 1;
+
+/// True if the callee of the call expression was found using ADL.
+unsigned UsesADL : 1;
};
class MemberExprBitfields {
Index: include/clang/AST/Expr.h
===
--- include/clang/AST/Expr.h
+++ include/clang/AST/Expr.h
@@ -2412,14 +2412,20 @@
void updateDependenciesFromArg(Expr *Arg);
+public:
+ enum class ADLCallKind : bool { NotADL, UsesADL };
+ static constexpr ADLCallKind NotADL = ADLCallKind::NotADL;
+ static constexpr ADLCallKind UsesADL = ADLCallKind::UsesADL;
+
protected:
// These versions of the constructor are for derived classes.
CallExpr(const ASTContext , StmtClass SC, Expr *fn,
ArrayRef preargs, ArrayRef args, QualType t,
- ExprValueKind VK, SourceLocation rparenloc, unsigned MinNumArgs = 0);
+ ExprValueKind VK, SourceLocation rparenloc, unsigned MinNumArgs = 0,
+ ADLCallKind UsesADL = NotADL);
CallExpr(const ASTContext , StmtClass SC, Expr *fn, ArrayRef args,
QualType t, ExprValueKind VK, SourceLocation rparenloc,
- unsigned MinNumArgs = 0);
+ unsigned MinNumArgs = 0, ADLCallKind UsesADL = NotADL);
CallExpr(const ASTContext , StmtClass SC, unsigned NumPreArgs,
unsigned NumArgs, EmptyShell Empty);
@@ -2443,7 +2449,8 @@
/// arguments. The actual number of arguments will be the greater of
/// args.size() and MinNumArgs.
CallExpr(const ASTContext , Expr *fn, ArrayRef args, QualType t,
- ExprValueKind VK, SourceLocation rparenloc, unsigned MinNumArgs = 0);
+ ExprValueKind VK, SourceLocation rparenloc, unsigned MinNumArgs = 0,
+ ADLCallKind UsesADL = NotADL);
/// Build an empty call expression.
CallExpr(const ASTContext , unsigned NumArgs, EmptyShell Empty);
@@ -2452,6 +2459,14 @@
Expr *getCallee() { return cast(SubExprs[FN]); }
void setCallee(Expr *F) { SubExprs[FN] = F; }
+ ADLCallKind getADLCallKind() const {
+return static_cast(CallExprBits.UsesADL);
+ }
+ void setADLCallKind(ADLCallKind V = UsesADL) {
+CallExprBits.UsesADL = static_cast(V);
+ }
+ bool usesADL() const { return getADLCallKind() == UsesADL; }
+
Decl *getCalleeDecl();
const Decl *getCalleeDecl() const {
return const_cast(this)->getCalleeDecl();
Index: include/clang/AST/ExprCXX.h
===
--- include/clang/AST/ExprCXX.h
+++ include/clang/AST/ExprCXX.h
@@ -90,10 +90,12 @@
friend class ASTStmtReader;
friend class ASTStmtWriter;
- CXXOperatorCallExpr(ASTContext& C, OverloadedOperatorKind Op, Expr *fn,
- ArrayRef args, QualType t, ExprValueKind VK,
- SourceLocation operatorloc, FPOptions FPFeatures)
- : CallExpr(C, CXXOperatorCallExprClass, fn, args, t, VK, operatorloc),
+ CXXOperatorCallExpr(ASTContext , OverloadedOperatorKind Op, Expr *fn,
+ ArrayRef args, QualType t,
[PATCH] D55534: [AST] Store "UsesADL" information in CallExpr.
EricWF updated this revision to Diff 177907.
EricWF marked an inline comment as done.
EricWF added a comment.
Address final review comments.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55534/new/
https://reviews.llvm.org/D55534
Files:
docs/LibASTMatchersReference.html
include/clang/AST/Expr.h
include/clang/AST/ExprCXX.h
include/clang/AST/Stmt.h
include/clang/ASTMatchers/ASTMatchers.h
include/clang/Sema/Overload.h
include/clang/Sema/Sema.h
lib/AST/ASTDumper.cpp
lib/AST/ASTImporter.cpp
lib/AST/Expr.cpp
lib/ASTMatchers/Dynamic/Registry.cpp
lib/Sema/SemaExpr.cpp
lib/Sema/SemaOverload.cpp
lib/Serialization/ASTReaderStmt.cpp
lib/Serialization/ASTWriterStmt.cpp
test/AST/ast-dump-expr.cpp
test/Import/call-expr/Inputs/F.cpp
test/Import/call-expr/test.cpp
unittests/ASTMatchers/ASTMatchersNodeTest.cpp
Index: unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -199,6 +199,40 @@
"-fno-delayed-template-parsing"));
}
+TEST(Matcher, ADLCall) {
+ StatementMatcher ADLMatch = callExpr(usesADL());
+ StatementMatcher ADLMatchOper = cxxOperatorCallExpr(usesADL());
+ auto NS_Str = R"cpp(
+ namespace NS {
+struct X {};
+void f(X);
+void operator+(X, X);
+ }
+ struct MyX {};
+ void f(...);
+ void operator+(MyX, MyX);
+)cpp";
+
+ auto MkStr = [&](std::string Body) -> std::string {
+std::string S = NS_Str;
+S += "void test_fn() { " + Body + " }";
+return S;
+ };
+
+ EXPECT_TRUE(matches(MkStr("NS::X x; f(x);"), ADLMatch));
+ EXPECT_TRUE(notMatches(MkStr("NS::X x; NS::f(x);"), ADLMatch));
+ EXPECT_TRUE(notMatches(MkStr("MyX x; f(x);"), ADLMatch));
+ EXPECT_TRUE(notMatches(MkStr("NS::X x; using NS::f; f(x);"), ADLMatch));
+
+ // Operator call expressions
+ EXPECT_TRUE(matches(MkStr("NS::X x; x + x;"), ADLMatch));
+ EXPECT_TRUE(matches(MkStr("NS::X x; x + x;"), ADLMatchOper));
+ EXPECT_TRUE(notMatches(MkStr("MyX x; x + x;"), ADLMatch));
+ EXPECT_TRUE(notMatches(MkStr("MyX x; x + x;"), ADLMatchOper));
+ EXPECT_TRUE(matches(MkStr("NS::X x; operator+(x, x);"), ADLMatch));
+ EXPECT_TRUE(notMatches(MkStr("NS::X x; NS::operator+(x, x);"), ADLMatch));
+}
+
TEST(Matcher, Call) {
// FIXME: Do we want to overload Call() to directly take
// Matcher, too?
Index: test/Import/call-expr/test.cpp
===
--- /dev/null
+++ test/Import/call-expr/test.cpp
@@ -0,0 +1,8 @@
+// RUN: clang-import-test -dump-ast -import %S/Inputs/F.cpp -expression %s | FileCheck %s
+void expr() {
+ f();
+}
+
+// CHECK: FunctionDecl 0x{{[^ ]*}} <{{[^>]*}}> line:{{.*}}:{{[^ ]*}} used f 'void ()'
+// CHECK: -CallExpr 0x{{[^ ]*}} <{{[^>]*}}> 'void' adl
+// CHECK: -CXXOperatorCallExpr 0x{{[^ ]*}} <{{[^>]*}}> 'void' adl
Index: test/Import/call-expr/Inputs/F.cpp
===
--- /dev/null
+++ test/Import/call-expr/Inputs/F.cpp
@@ -0,0 +1,10 @@
+namespace NS {
+struct X {};
+void f(X) {}
+void operator+(X, X) {}
+} // namespace NS
+void f() {
+ NS::X x;
+ f(x);
+ x + x;
+}
Index: test/AST/ast-dump-expr.cpp
===
--- test/AST/ast-dump-expr.cpp
+++ test/AST/ast-dump-expr.cpp
@@ -508,3 +508,46 @@
// CHECK-NEXT: DeclRefExpr 0x{{[^ ]*}} 'Ts...' lvalue ParmVar 0x{{[^ ]*}} 'a' 'Ts...'
// CHECK-NEXT: DeclRefExpr 0x{{[^ ]*}} 'int' lvalue Var 0x{{[^ ]*}} 'b' 'int'
}
+
+
+namespace NS {
+struct X {};
+void f(X);
+void y(...);
+} // namespace NS
+
+// CHECK-LABEL: FunctionDecl 0x{{[^ ]*}} {{.*}}ADLCall 'void ()'
+void ADLCall() {
+ NS::X x;
+ // CHECK: CallExpr 0x{{[^ ]*}} ]+}}> 'void' adl{{$}}
+ f(x);
+ // CHECK: CallExpr 0x{{[^ ]*}} ]+}}> 'void' adl{{$}}
+ y(x);
+}
+
+// CHECK-LABEL: FunctionDecl 0x{{[^ ]*}} {{.*}}NonADLCall 'void ()'
+void NonADLCall() {
+ NS::X x;
+ // CHECK: CallExpr 0x{{[^ ]*}} ]+}}> 'void'{{$}}
+ NS::f(x);
+}
+
+// CHECK-LABEL: FunctionDecl 0x{{[^ ]*}} {{.*}}NonADLCall2 'void ()'
+void NonADLCall2() {
+ NS::X x;
+ using NS::f;
+ // CHECK: CallExpr 0x{{[^ ]*}} ]+}}> 'void'{{$}}
+ f(x);
+ // CHECK: CallExpr 0x{{[^ ]*}} ]+}}> 'void' adl{{$}}
+ y(x);
+}
+
+namespace test_adl_call_three {
+using namespace NS;
+// CHECK-LABEL: FunctionDecl 0x{{[^ ]*}} {{.*}}NonADLCall3 'void ()'
+void NonADLCall3() {
+ X x;
+ // CHECK: CallExpr 0x{{[^ ]*}} ]+}}> 'void'{{$}}
+ f(x);
+}
+} // namespace test_adl_call_three
\ No newline at end of file
Index: lib/Serialization/ASTWriterStmt.cpp
===
--- lib/Serialization/ASTWriterStmt.cpp
+++ lib/Serialization/ASTWriterStmt.cpp
@@ -651,6 +651,7 @@
for (CallExpr::arg_iterator Arg = E->arg_begin(), ArgEnd = E->arg_end();
Arg != ArgEnd; ++Arg)
Record.AddStmt(*Arg);
+
[PATCH] D55534: [AST] Store "UsesADL" information in CallExpr.
rsmith accepted this revision. rsmith added inline comments. This revision is now accepted and ready to land. Comment at: include/clang/ASTMatchers/ASTMatchers.h:1276 +/// y(42); // Doesn't match +/// } +/// \endcode I think it would be useful to add to the example: ``` using NS::y; y(x); // Found by both unqualified lookup and ADL, doesn't match ``` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55534/new/ https://reviews.llvm.org/D55534 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D55534: [AST] Store "UsesADL" information in CallExpr.
EricWF marked an inline comment as done. EricWF added inline comments. Comment at: docs/LibASTMatchersReference.html:2579-2581 +y(x); Matches +NS::y(x); Doesn't match +y(42); Doesn't match. aaron.ballman wrote: > EricWF wrote: > > aaron.ballman wrote: > > > This is not your bug to fix, but it seems the documentation generator is > > > stripping the comment markers. This impacts other matchers as well, such > > > as `throughUsingDecl()`. > > I wonder if it strips `/**/` comments. I don't want to get blocked on this. > > > Don't let it block you -- I'm poking at a fix currently, but this is not your > bug. Thanks for the fix! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55534/new/ https://reviews.llvm.org/D55534 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D55534: [AST] Store "UsesADL" information in CallExpr.
EricWF marked an inline comment as done. EricWF added a comment. I think this is good to go. Any more comments? Comment at: include/clang/Sema/Sema.h:2758 bool AllowExplicit = false, +bool IsADLCandidate = false, ConversionSequenceList EarlyConversions = None); EricWF wrote: > rsmith wrote: > > Long lists of bool arguments make me nervous, especially with default > > arguments. Can you introduce an enum for the new param at least? > Ack. I was thinking the same thing. And you were right to be nervous. The change found bugs. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55534/new/ https://reviews.llvm.org/D55534 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D55534: [AST] Store "UsesADL" information in CallExpr.
EricWF updated this revision to Diff 177752.
EricWF marked 3 inline comments as done.
EricWF added a comment.
Address review comments.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55534/new/
https://reviews.llvm.org/D55534
Files:
docs/LibASTMatchersReference.html
include/clang/AST/Expr.h
include/clang/AST/ExprCXX.h
include/clang/AST/Stmt.h
include/clang/ASTMatchers/ASTMatchers.h
include/clang/Sema/Overload.h
include/clang/Sema/Sema.h
lib/AST/ASTDumper.cpp
lib/AST/ASTImporter.cpp
lib/AST/Expr.cpp
lib/ASTMatchers/Dynamic/Registry.cpp
lib/Sema/SemaExpr.cpp
lib/Sema/SemaOverload.cpp
lib/Serialization/ASTReaderStmt.cpp
lib/Serialization/ASTWriterStmt.cpp
test/AST/ast-dump-expr.cpp
test/Import/call-expr/Inputs/F.cpp
test/Import/call-expr/test.cpp
unittests/ASTMatchers/ASTMatchersNodeTest.cpp
Index: unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -199,6 +199,40 @@
"-fno-delayed-template-parsing"));
}
+TEST(Matcher, ADLCall) {
+ StatementMatcher ADLMatch = callExpr(usesADL());
+ StatementMatcher ADLMatchOper = cxxOperatorCallExpr(usesADL());
+ auto NS_Str = R"cpp(
+ namespace NS {
+struct X {};
+void f(X);
+void operator+(X, X);
+ }
+ struct MyX {};
+ void f(...);
+ void operator+(MyX, MyX);
+)cpp";
+
+ auto MkStr = [&](std::string Body) -> std::string {
+std::string S = NS_Str;
+S += "void test_fn() { " + Body + " }";
+return S;
+ };
+
+ EXPECT_TRUE(matches(MkStr("NS::X x; f(x);"), ADLMatch));
+ EXPECT_TRUE(notMatches(MkStr("NS::X x; NS::f(x);"), ADLMatch));
+ EXPECT_TRUE(notMatches(MkStr("MyX x; f(x);"), ADLMatch));
+ EXPECT_TRUE(notMatches(MkStr("NS::X x; using NS::f; f(x);"), ADLMatch));
+
+ // Operator call expressions
+ EXPECT_TRUE(matches(MkStr("NS::X x; x + x;"), ADLMatch));
+ EXPECT_TRUE(matches(MkStr("NS::X x; x + x;"), ADLMatchOper));
+ EXPECT_TRUE(notMatches(MkStr("MyX x; x + x;"), ADLMatch));
+ EXPECT_TRUE(notMatches(MkStr("MyX x; x + x;"), ADLMatchOper));
+ EXPECT_TRUE(matches(MkStr("NS::X x; operator+(x, x);"), ADLMatch));
+ EXPECT_TRUE(notMatches(MkStr("NS::X x; NS::operator+(x, x);"), ADLMatch));
+}
+
TEST(Matcher, Call) {
// FIXME: Do we want to overload Call() to directly take
// Matcher, too?
Index: test/Import/call-expr/test.cpp
===
--- /dev/null
+++ test/Import/call-expr/test.cpp
@@ -0,0 +1,8 @@
+// RUN: clang-import-test -dump-ast -import %S/Inputs/F.cpp -expression %s | FileCheck %s
+void expr() {
+ f();
+}
+
+// CHECK: FunctionDecl 0x{{[^ ]*}} <{{[^>]*}}> line:{{.*}}:{{[^ ]*}} used f 'void ()'
+// CHECK: -CallExpr 0x{{[^ ]*}} <{{[^>]*}}> 'void' adl
+// CHECK: -CXXOperatorCallExpr 0x{{[^ ]*}} <{{[^>]*}}> 'void' adl
Index: test/Import/call-expr/Inputs/F.cpp
===
--- /dev/null
+++ test/Import/call-expr/Inputs/F.cpp
@@ -0,0 +1,10 @@
+namespace NS {
+struct X {};
+void f(X) {}
+void operator+(X, X) {}
+} // namespace NS
+void f() {
+ NS::X x;
+ f(x);
+ x + x;
+}
Index: test/AST/ast-dump-expr.cpp
===
--- test/AST/ast-dump-expr.cpp
+++ test/AST/ast-dump-expr.cpp
@@ -508,3 +508,46 @@
// CHECK-NEXT: DeclRefExpr 0x{{[^ ]*}} 'Ts...' lvalue ParmVar 0x{{[^ ]*}} 'a' 'Ts...'
// CHECK-NEXT: DeclRefExpr 0x{{[^ ]*}} 'int' lvalue Var 0x{{[^ ]*}} 'b' 'int'
}
+
+
+namespace NS {
+struct X {};
+void f(X);
+void y(...);
+} // namespace NS
+
+// CHECK-LABEL: FunctionDecl 0x{{[^ ]*}} {{.*}}ADLCall 'void ()'
+void ADLCall() {
+ NS::X x;
+ // CHECK: CallExpr 0x{{[^ ]*}} ]+}}> 'void' adl{{$}}
+ f(x);
+ // CHECK: CallExpr 0x{{[^ ]*}} ]+}}> 'void' adl{{$}}
+ y(x);
+}
+
+// CHECK-LABEL: FunctionDecl 0x{{[^ ]*}} {{.*}}NonADLCall 'void ()'
+void NonADLCall() {
+ NS::X x;
+ // CHECK: CallExpr 0x{{[^ ]*}} ]+}}> 'void'{{$}}
+ NS::f(x);
+}
+
+// CHECK-LABEL: FunctionDecl 0x{{[^ ]*}} {{.*}}NonADLCall2 'void ()'
+void NonADLCall2() {
+ NS::X x;
+ using NS::f;
+ // CHECK: CallExpr 0x{{[^ ]*}} ]+}}> 'void'{{$}}
+ f(x);
+ // CHECK: CallExpr 0x{{[^ ]*}} ]+}}> 'void' adl{{$}}
+ y(x);
+}
+
+namespace test_adl_call_three {
+using namespace NS;
+// CHECK-LABEL: FunctionDecl 0x{{[^ ]*}} {{.*}}NonADLCall3 'void ()'
+void NonADLCall3() {
+ X x;
+ // CHECK: CallExpr 0x{{[^ ]*}} ]+}}> 'void'{{$}}
+ f(x);
+}
+} // namespace test_adl_call_three
\ No newline at end of file
Index: lib/Serialization/ASTWriterStmt.cpp
===
--- lib/Serialization/ASTWriterStmt.cpp
+++ lib/Serialization/ASTWriterStmt.cpp
@@ -651,6 +651,7 @@
for (CallExpr::arg_iterator Arg = E->arg_begin(), ArgEnd = E->arg_end();
Arg != ArgEnd; ++Arg)
Record.AddStmt(*Arg);
+
[PATCH] D55534: [AST] Store "UsesADL" information in CallExpr.
EricWF marked 13 inline comments as done.
EricWF added inline comments.
Comment at: include/clang/Sema/Sema.h:2758
bool AllowExplicit = false,
+bool IsADLCandidate = false,
ConversionSequenceList EarlyConversions = None);
rsmith wrote:
> Long lists of bool arguments make me nervous, especially with default
> arguments. Can you introduce an enum for the new param at least?
Ack. I was thinking the same thing.
Comment at: lib/AST/ASTImporter.cpp:7387
return new (Importer.getToContext()) CallExpr(
Importer.getToContext(), ToCallee, ToArgs, ToType, E->getValueKind(),
ToRParenLoc);
rsmith wrote:
> Do we need to pass through the usesADL flag here too?
Seems like it. I'll add tests as well.
Comment at: lib/AST/Expr.cpp:1267
: Expr(SC, Empty), NumArgs(NumArgs) {
+ CallExprBits.UsesADL = false;
CallExprBits.NumPreArgs = NumPreArgs;
riccibruno wrote:
> riccibruno wrote:
> > I believe that msan can cope with bit level operations just fine.
> > What I am afraid is that initializing it here will hide the
> > fact that it is not initialized during deserialization if the
> > `E->setUsesADL(Record.readInt());` is removed by mistake.
> >
> > At least not initializing it here will leave a chance for msan
> > to catch this.
> I don't think they do.
Regardless. I'll remove the initialization.
I think potentially getting a random value will make it easier to spot bugs.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55534/new/
https://reviews.llvm.org/D55534
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D55534: [AST] Store "UsesADL" information in CallExpr.
riccibruno added inline comments.
Comment at: lib/AST/Expr.cpp:1267
: Expr(SC, Empty), NumArgs(NumArgs) {
+ CallExprBits.UsesADL = false;
CallExprBits.NumPreArgs = NumPreArgs;
riccibruno wrote:
> I believe that msan can cope with bit level operations just fine.
> What I am afraid is that initializing it here will hide the
> fact that it is not initialized during deserialization if the
> `E->setUsesADL(Record.readInt());` is removed by mistake.
>
> At least not initializing it here will leave a chance for msan
> to catch this.
I don't think they do.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55534/new/
https://reviews.llvm.org/D55534
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D55534: [AST] Store "UsesADL" information in CallExpr.
rsmith added inline comments.
Comment at: include/clang/Sema/Sema.h:2758
bool AllowExplicit = false,
+bool IsADLCandidate = false,
ConversionSequenceList EarlyConversions = None);
Long lists of bool arguments make me nervous, especially with default
arguments. Can you introduce an enum for the new param at least?
Comment at: lib/AST/ASTImporter.cpp:7387
return new (Importer.getToContext()) CallExpr(
Importer.getToContext(), ToCallee, ToArgs, ToType, E->getValueKind(),
ToRParenLoc);
Do we need to pass through the usesADL flag here too?
Comment at: lib/AST/Expr.cpp:1267
: Expr(SC, Empty), NumArgs(NumArgs) {
+ CallExprBits.UsesADL = false;
CallExprBits.NumPreArgs = NumPreArgs;
EricWF wrote:
> riccibruno wrote:
> > It do not really matter but there is not point initializing this bit here.
> I'm a but nervous MSAN won't catch an uninitialized read because it's a
> bitfield, so I would rather be safe than sorry?
IIRC all the ExprBits get initialized by the Expr(EmptyShell) ctor.
Comment at: lib/Sema/SemaExpr.cpp:5673
+ if (Config) {
TheCall = new (Context)
CUDAKernelCallExpr(Context, Fn, cast(Config), Args, ResultTy,
If you believe that CUDA kernel calls can't find functions with ADL, please add
an assert here.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55534/new/
https://reviews.llvm.org/D55534
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D55534: [AST] Store "UsesADL" information in CallExpr.
riccibruno added inline comments.
Comment at: lib/AST/Expr.cpp:1267
: Expr(SC, Empty), NumArgs(NumArgs) {
+ CallExprBits.UsesADL = false;
CallExprBits.NumPreArgs = NumPreArgs;
I believe that msan can cope with bit level operations just fine.
What I am afraid is that initializing it here will hide the
fact that it is not initialized during deserialization if the
`E->setUsesADL(Record.readInt());` is removed by mistake.
At least not initializing it here will leave a chance for msan
to catch this.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55534/new/
https://reviews.llvm.org/D55534
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D55534: [AST] Store "UsesADL" information in CallExpr.
aaron.ballman added inline comments. Comment at: docs/LibASTMatchersReference.html:2579-2581 +y(x); Matches +NS::y(x); Doesn't match +y(42); Doesn't match. EricWF wrote: > aaron.ballman wrote: > > This is not your bug to fix, but it seems the documentation generator is > > stripping the comment markers. This impacts other matchers as well, such as > > `throughUsingDecl()`. > I wonder if it strips `/**/` comments. I don't want to get blocked on this. > Don't let it block you -- I'm poking at a fix currently, but this is not your bug. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55534/new/ https://reviews.llvm.org/D55534 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D55534: [AST] Store "UsesADL" information in CallExpr.
EricWF marked 3 inline comments as done.
EricWF added a comment.
Address more inline comments.
Comment at: docs/LibASTMatchersReference.html:2579-2581
+y(x); Matches
+NS::y(x); Doesn't match
+y(42); Doesn't match.
aaron.ballman wrote:
> This is not your bug to fix, but it seems the documentation generator is
> stripping the comment markers. This impacts other matchers as well, such as
> `throughUsingDecl()`.
I wonder if it strips `/**/` comments. I don't want to get blocked on this.
Comment at: lib/AST/Expr.cpp:1267
: Expr(SC, Empty), NumArgs(NumArgs) {
+ CallExprBits.UsesADL = false;
CallExprBits.NumPreArgs = NumPreArgs;
riccibruno wrote:
> It do not really matter but there is not point initializing this bit here.
I'm a but nervous MSAN won't catch an uninitialized read because it's a
bitfield, so I would rather be safe than sorry?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55534/new/
https://reviews.llvm.org/D55534
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D55534: [AST] Store "UsesADL" information in CallExpr.
aaron.ballman marked an inline comment as done.
aaron.ballman added a comment.
A few more minor nits.
Comment at: docs/LibASTMatchersReference.html:2579-2581
+y(x); Matches
+NS::y(x); Doesn't match
+y(42); Doesn't match.
This is not your bug to fix, but it seems the documentation generator is
stripping the comment markers. This impacts other matchers as well, such as
`throughUsingDecl()`.
Comment at: include/clang/Sema/Overload.h:831
+ private:
+// Only the OverloadCandidate set is allowed to construct
OverloadCandidates.
+friend class OverloadCandidateSet;
OverloadCandidate set -> OverloadCandidateSet ? (I'd also be fine with
removing the comment -- seems somewhat obvious from context.)
Comment at: lib/Sema/SemaExpr.cpp:5672
CallExpr *TheCall;
- if (Config)
+ if (Config) {
TheCall = new (Context)
No need to add the braces here (alternatively, add them to the `else` clause).
Comment at: unittests/ASTMatchers/ASTMatchersNodeTest.cpp:204
+ StatementMatcher ADLMatch = callExpr(usesADL());
+ auto NS_Str = R"DELIM(
+ namespace NS {
EricWF wrote:
> fowles wrote:
> > if you use cc or cpp as your delimiter, I think clang-format will recurse
> > correctly
> That's super cool. Thanks!
Wow, that is really neat!
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55534/new/
https://reviews.llvm.org/D55534
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D55534: [AST] Store "UsesADL" information in CallExpr.
EricWF updated this revision to Diff 177720.
EricWF marked 2 inline comments as done.
EricWF added a comment.
Register matcher and regenerate docs.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55534/new/
https://reviews.llvm.org/D55534
Files:
docs/LibASTMatchersReference.html
include/clang/AST/Expr.h
include/clang/AST/ExprCXX.h
include/clang/AST/Stmt.h
include/clang/ASTMatchers/ASTMatchers.h
include/clang/Sema/Overload.h
include/clang/Sema/Sema.h
lib/AST/ASTDumper.cpp
lib/AST/ASTImporter.cpp
lib/AST/Expr.cpp
lib/ASTMatchers/Dynamic/Registry.cpp
lib/Sema/SemaExpr.cpp
lib/Sema/SemaOverload.cpp
lib/Serialization/ASTReaderStmt.cpp
lib/Serialization/ASTWriterStmt.cpp
test/AST/ast-dump-expr.cpp
unittests/ASTMatchers/ASTMatchersNodeTest.cpp
Index: unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -199,6 +199,40 @@
"-fno-delayed-template-parsing"));
}
+TEST(Matcher, ADLCall) {
+ StatementMatcher ADLMatch = callExpr(usesADL());
+ StatementMatcher ADLMatchOper = cxxOperatorCallExpr(usesADL());
+ auto NS_Str = R"cpp(
+ namespace NS {
+struct X {};
+void f(X);
+void operator+(X, X);
+ }
+ struct MyX {};
+ void f(...);
+ void operator+(MyX, MyX);
+)cpp";
+
+ auto MkStr = [&](std::string Body) -> std::string {
+std::string S = NS_Str;
+S += "void test_fn() { " + Body + " }";
+return S;
+ };
+
+ EXPECT_TRUE(matches(MkStr("NS::X x; f(x);"), ADLMatch));
+ EXPECT_TRUE(notMatches(MkStr("NS::X x; NS::f(x);"), ADLMatch));
+ EXPECT_TRUE(notMatches(MkStr("MyX x; f(x);"), ADLMatch));
+ EXPECT_TRUE(notMatches(MkStr("NS::X x; using NS::f; f(x);"), ADLMatch));
+
+ // Operator call expressions
+ EXPECT_TRUE(matches(MkStr("NS::X x; x + x;"), ADLMatch));
+ EXPECT_TRUE(matches(MkStr("NS::X x; x + x;"), ADLMatchOper));
+ EXPECT_TRUE(notMatches(MkStr("MyX x; x + x;"), ADLMatch));
+ EXPECT_TRUE(notMatches(MkStr("MyX x; x + x;"), ADLMatchOper));
+ EXPECT_TRUE(matches(MkStr("NS::X x; operator+(x, x);"), ADLMatch));
+ EXPECT_TRUE(notMatches(MkStr("NS::X x; NS::operator+(x, x);"), ADLMatch));
+}
+
TEST(Matcher, Call) {
// FIXME: Do we want to overload Call() to directly take
// Matcher, too?
Index: test/AST/ast-dump-expr.cpp
===
--- /dev/null
+++ test/AST/ast-dump-expr.cpp
@@ -0,0 +1,44 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -Wno-unused-value -std=c++14 -ast-dump %s \
+// RUN:| FileCheck -strict-whitespace %s
+
+namespace NS {
+struct X {};
+void f(X);
+void y(...);
+} // namespace NS
+
+// CHECK-LABEL: FunctionDecl 0x{{[^ ]*}} {{.*}}ADLCall 'void ()'
+void ADLCall() {
+ NS::X x;
+ // CHECK: CallExpr 0x{{[^ ]*}} ]+}}> 'void' adl{{$}}
+ f(x);
+ // CHECK: CallExpr 0x{{[^ ]*}} ]+}}> 'void' adl{{$}}
+ y(x);
+}
+
+// CHECK-LABEL: FunctionDecl 0x{{[^ ]*}} {{.*}}NonADLCall 'void ()'
+void NonADLCall() {
+ NS::X x;
+ // CHECK: CallExpr 0x{{[^ ]*}} ]+}}> 'void'{{$}}
+ NS::f(x);
+}
+
+// CHECK-LABEL: FunctionDecl 0x{{[^ ]*}} {{.*}}NonADLCall2 'void ()'
+void NonADLCall2() {
+ NS::X x;
+ using NS::f;
+ // CHECK: CallExpr 0x{{[^ ]*}} ]+}}> 'void'{{$}}
+ f(x);
+ // CHECK: CallExpr 0x{{[^ ]*}} ]+}}> 'void' adl{{$}}
+ y(x);
+}
+
+namespace test_adl_call_three {
+using namespace NS;
+// CHECK-LABEL: FunctionDecl 0x{{[^ ]*}} {{.*}}NonADLCall3 'void ()'
+void NonADLCall3() {
+ X x;
+ // CHECK: CallExpr 0x{{[^ ]*}} ]+}}> 'void'{{$}}
+ f(x);
+}
+} // namespace test_adl_call_three
Index: lib/Serialization/ASTWriterStmt.cpp
===
--- lib/Serialization/ASTWriterStmt.cpp
+++ lib/Serialization/ASTWriterStmt.cpp
@@ -651,6 +651,7 @@
for (CallExpr::arg_iterator Arg = E->arg_begin(), ArgEnd = E->arg_end();
Arg != ArgEnd; ++Arg)
Record.AddStmt(*Arg);
+ Record.push_back(E->usesADL());
Code = serialization::EXPR_CALL;
}
Index: lib/Serialization/ASTReaderStmt.cpp
===
--- lib/Serialization/ASTReaderStmt.cpp
+++ lib/Serialization/ASTReaderStmt.cpp
@@ -737,6 +737,7 @@
E->setCallee(Record.readSubExpr());
for (unsigned I = 0; I != NumArgs; ++I)
E->setArg(I, Record.readSubExpr());
+ E->setUsesADL(Record.readInt());
}
void ASTStmtReader::VisitCXXMemberCallExpr(CXXMemberCallExpr *E) {
Index: lib/Sema/SemaOverload.cpp
===
--- lib/Sema/SemaOverload.cpp
+++ lib/Sema/SemaOverload.cpp
@@ -5946,15 +5946,13 @@
/// \param PartialOverloading true if we are performing "partial" overloading
/// based on an incomplete set of function arguments. This feature is used by
/// code completion.
-void
-Sema::AddOverloadCandidate(FunctionDecl *Function,
-
[PATCH] D55534: [AST] Store "UsesADL" information in CallExpr.
EricWF updated this revision to Diff 177716.
EricWF added a comment.
More tests.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55534/new/
https://reviews.llvm.org/D55534
Files:
include/clang/AST/Expr.h
include/clang/AST/ExprCXX.h
include/clang/AST/Stmt.h
include/clang/ASTMatchers/ASTMatchers.h
include/clang/Sema/Overload.h
include/clang/Sema/Sema.h
lib/AST/ASTDumper.cpp
lib/AST/ASTImporter.cpp
lib/AST/Expr.cpp
lib/Sema/SemaExpr.cpp
lib/Sema/SemaOverload.cpp
lib/Serialization/ASTReaderStmt.cpp
lib/Serialization/ASTWriterStmt.cpp
test/AST/ast-dump-expr.cpp
unittests/ASTMatchers/ASTMatchersNodeTest.cpp
Index: unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -199,6 +199,40 @@
"-fno-delayed-template-parsing"));
}
+TEST(Matcher, ADLCall) {
+ StatementMatcher ADLMatch = callExpr(usesADL());
+ StatementMatcher ADLMatchOper = cxxOperatorCallExpr(usesADL());
+ auto NS_Str = R"cpp(
+ namespace NS {
+struct X {};
+void f(X);
+void operator+(X, X);
+ }
+ struct MyX {};
+ void f(...);
+ void operator+(MyX, MyX);
+)cpp";
+
+ auto MkStr = [&](std::string Body) -> std::string {
+std::string S = NS_Str;
+S += "void test_fn() { " + Body + " }";
+return S;
+ };
+
+ EXPECT_TRUE(matches(MkStr("NS::X x; f(x);"), ADLMatch));
+ EXPECT_TRUE(notMatches(MkStr("NS::X x; NS::f(x);"), ADLMatch));
+ EXPECT_TRUE(notMatches(MkStr("MyX x; f(x);"), ADLMatch));
+ EXPECT_TRUE(notMatches(MkStr("NS::X x; using NS::f; f(x);"), ADLMatch));
+
+ // Operator call expressions
+ EXPECT_TRUE(matches(MkStr("NS::X x; x + x;"), ADLMatch));
+ EXPECT_TRUE(matches(MkStr("NS::X x; x + x;"), ADLMatchOper));
+ EXPECT_TRUE(notMatches(MkStr("MyX x; x + x;"), ADLMatch));
+ EXPECT_TRUE(notMatches(MkStr("MyX x; x + x;"), ADLMatchOper));
+ EXPECT_TRUE(matches(MkStr("NS::X x; operator+(x, x);"), ADLMatch));
+ EXPECT_TRUE(notMatches(MkStr("NS::X x; NS::operator+(x, x);"), ADLMatch));
+}
+
TEST(Matcher, Call) {
// FIXME: Do we want to overload Call() to directly take
// Matcher, too?
Index: test/AST/ast-dump-expr.cpp
===
--- /dev/null
+++ test/AST/ast-dump-expr.cpp
@@ -0,0 +1,44 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -Wno-unused-value -std=c++14 -ast-dump %s \
+// RUN:| FileCheck -strict-whitespace %s
+
+namespace NS {
+struct X {};
+void f(X);
+void y(...);
+} // namespace NS
+
+// CHECK-LABEL: FunctionDecl 0x{{[^ ]*}} {{.*}}ADLCall 'void ()'
+void ADLCall() {
+ NS::X x;
+ // CHECK: CallExpr 0x{{[^ ]*}} ]+}}> 'void' adl{{$}}
+ f(x);
+ // CHECK: CallExpr 0x{{[^ ]*}} ]+}}> 'void' adl{{$}}
+ y(x);
+}
+
+// CHECK-LABEL: FunctionDecl 0x{{[^ ]*}} {{.*}}NonADLCall 'void ()'
+void NonADLCall() {
+ NS::X x;
+ // CHECK: CallExpr 0x{{[^ ]*}} ]+}}> 'void'{{$}}
+ NS::f(x);
+}
+
+// CHECK-LABEL: FunctionDecl 0x{{[^ ]*}} {{.*}}NonADLCall2 'void ()'
+void NonADLCall2() {
+ NS::X x;
+ using NS::f;
+ // CHECK: CallExpr 0x{{[^ ]*}} ]+}}> 'void'{{$}}
+ f(x);
+ // CHECK: CallExpr 0x{{[^ ]*}} ]+}}> 'void' adl{{$}}
+ y(x);
+}
+
+namespace test_adl_call_three {
+using namespace NS;
+// CHECK-LABEL: FunctionDecl 0x{{[^ ]*}} {{.*}}NonADLCall3 'void ()'
+void NonADLCall3() {
+ X x;
+ // CHECK: CallExpr 0x{{[^ ]*}} ]+}}> 'void'{{$}}
+ f(x);
+}
+} // namespace test_adl_call_three
Index: lib/Serialization/ASTWriterStmt.cpp
===
--- lib/Serialization/ASTWriterStmt.cpp
+++ lib/Serialization/ASTWriterStmt.cpp
@@ -651,6 +651,7 @@
for (CallExpr::arg_iterator Arg = E->arg_begin(), ArgEnd = E->arg_end();
Arg != ArgEnd; ++Arg)
Record.AddStmt(*Arg);
+ Record.push_back(E->usesADL());
Code = serialization::EXPR_CALL;
}
Index: lib/Serialization/ASTReaderStmt.cpp
===
--- lib/Serialization/ASTReaderStmt.cpp
+++ lib/Serialization/ASTReaderStmt.cpp
@@ -737,6 +737,7 @@
E->setCallee(Record.readSubExpr());
for (unsigned I = 0; I != NumArgs; ++I)
E->setArg(I, Record.readSubExpr());
+ E->setUsesADL(Record.readInt());
}
void ASTStmtReader::VisitCXXMemberCallExpr(CXXMemberCallExpr *E) {
Index: lib/Sema/SemaOverload.cpp
===
--- lib/Sema/SemaOverload.cpp
+++ lib/Sema/SemaOverload.cpp
@@ -5946,15 +5946,13 @@
/// \param PartialOverloading true if we are performing "partial" overloading
/// based on an incomplete set of function arguments. This feature is used by
/// code completion.
-void
-Sema::AddOverloadCandidate(FunctionDecl *Function,
- DeclAccessPair FoundDecl,
- ArrayRef Args,
- OverloadCandidateSet ,
-
[PATCH] D55534: [AST] Store "UsesADL" information in CallExpr.
aaron.ballman added inline comments. Comment at: include/clang/ASTMatchers/ASTMatchers.h:1260 +/// Matches call expressions which were resolved using ADL. +/// Don't forget to regenerate the documentation and update Registry.cpp to add clang-query support. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55534/new/ https://reviews.llvm.org/D55534 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D55534: [AST] Store "UsesADL" information in CallExpr.
riccibruno added inline comments.
Comment at: lib/AST/Expr.cpp:1267
: Expr(SC, Empty), NumArgs(NumArgs) {
+ CallExprBits.UsesADL = false;
CallExprBits.NumPreArgs = NumPreArgs;
It do not really matter but there is not point initializing this bit here.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55534/new/
https://reviews.llvm.org/D55534
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D55534: [AST] Store "UsesADL" information in CallExpr.
EricWF updated this revision to Diff 177705.
EricWF marked an inline comment as done.
EricWF added a comment.
Update with fixes for review comments.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55534/new/
https://reviews.llvm.org/D55534
Files:
include/clang/AST/Expr.h
include/clang/AST/ExprCXX.h
include/clang/AST/Stmt.h
include/clang/ASTMatchers/ASTMatchers.h
include/clang/Sema/Overload.h
include/clang/Sema/Sema.h
lib/AST/ASTDumper.cpp
lib/AST/ASTImporter.cpp
lib/AST/Expr.cpp
lib/Sema/SemaExpr.cpp
lib/Sema/SemaOverload.cpp
lib/Serialization/ASTReaderStmt.cpp
lib/Serialization/ASTWriterStmt.cpp
test/AST/ast-dump-expr.cpp
unittests/ASTMatchers/ASTMatchersNodeTest.cpp
Index: unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -199,6 +199,32 @@
"-fno-delayed-template-parsing"));
}
+TEST(Matcher, ADLCall) {
+ StatementMatcher ADLMatch = callExpr(usesADL());
+ auto NS_Str = R"cpp(
+ namespace NS {
+struct X {};
+struct Y {};
+void f(X);
+template
+void g(T);
+ }
+ struct MyX {};
+ void f(...);
+ void g(...);
+)cpp";
+
+ auto MkStr = [&](std::string Body) -> std::string {
+std::string S = NS_Str;
+S += "void test_fn() { " + Body + " }";
+return S;
+ };
+
+ EXPECT_TRUE(matches(MkStr("NS::X x; f(x);"), ADLMatch));
+ EXPECT_TRUE(notMatches(MkStr("NS::X x; NS::f(x);"), ADLMatch));
+ EXPECT_TRUE(notMatches(MkStr("MyX x; f(x);"), ADLMatch));
+}
+
TEST(Matcher, Call) {
// FIXME: Do we want to overload Call() to directly take
// Matcher, too?
Index: test/AST/ast-dump-expr.cpp
===
--- /dev/null
+++ test/AST/ast-dump-expr.cpp
@@ -0,0 +1,44 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -Wno-unused-value -std=c++14 -ast-dump %s \
+// RUN:| FileCheck -strict-whitespace %s
+
+namespace NS {
+struct X {};
+void f(X);
+void y(...);
+} // namespace NS
+
+// CHECK-LABEL: FunctionDecl 0x{{[^ ]*}} {{.*}}ADLCall 'void ()'
+void ADLCall() {
+ NS::X x;
+ // CHECK: CallExpr 0x{{[^ ]*}} ]+}}> 'void' adl{{$}}
+ f(x);
+ // CHECK: CallExpr 0x{{[^ ]*}} ]+}}> 'void' adl{{$}}
+ y(x);
+}
+
+// CHECK-LABEL: FunctionDecl 0x{{[^ ]*}} {{.*}}NonADLCall 'void ()'
+void NonADLCall() {
+ NS::X x;
+ // CHECK: CallExpr 0x{{[^ ]*}} ]+}}> 'void'{{$}}
+ NS::f(x);
+}
+
+// CHECK-LABEL: FunctionDecl 0x{{[^ ]*}} {{.*}}NonADLCall2 'void ()'
+void NonADLCall2() {
+ NS::X x;
+ using NS::f;
+ // CHECK: CallExpr 0x{{[^ ]*}} ]+}}> 'void'{{$}}
+ f(x);
+ // CHECK: CallExpr 0x{{[^ ]*}} ]+}}> 'void' adl{{$}}
+ y(x);
+}
+
+namespace test_adl_call_three {
+using namespace NS;
+// CHECK-LABEL: FunctionDecl 0x{{[^ ]*}} {{.*}}NonADLCall3 'void ()'
+void NonADLCall3() {
+ X x;
+ // CHECK: CallExpr 0x{{[^ ]*}} ]+}}> 'void'{{$}}
+ f(x);
+}
+} // namespace test_adl_call_three
Index: lib/Serialization/ASTWriterStmt.cpp
===
--- lib/Serialization/ASTWriterStmt.cpp
+++ lib/Serialization/ASTWriterStmt.cpp
@@ -651,6 +651,7 @@
for (CallExpr::arg_iterator Arg = E->arg_begin(), ArgEnd = E->arg_end();
Arg != ArgEnd; ++Arg)
Record.AddStmt(*Arg);
+ Record.push_back(E->usesADL());
Code = serialization::EXPR_CALL;
}
Index: lib/Serialization/ASTReaderStmt.cpp
===
--- lib/Serialization/ASTReaderStmt.cpp
+++ lib/Serialization/ASTReaderStmt.cpp
@@ -737,6 +737,7 @@
E->setCallee(Record.readSubExpr());
for (unsigned I = 0; I != NumArgs; ++I)
E->setArg(I, Record.readSubExpr());
+ E->setUsesADL(Record.readInt());
}
void ASTStmtReader::VisitCXXMemberCallExpr(CXXMemberCallExpr *E) {
Index: lib/Sema/SemaOverload.cpp
===
--- lib/Sema/SemaOverload.cpp
+++ lib/Sema/SemaOverload.cpp
@@ -5946,15 +5946,13 @@
/// \param PartialOverloading true if we are performing "partial" overloading
/// based on an incomplete set of function arguments. This feature is used by
/// code completion.
-void
-Sema::AddOverloadCandidate(FunctionDecl *Function,
- DeclAccessPair FoundDecl,
- ArrayRef Args,
- OverloadCandidateSet ,
- bool SuppressUserConversions,
- bool PartialOverloading,
- bool AllowExplicit,
- ConversionSequenceList EarlyConversions) {
+void Sema::AddOverloadCandidate(FunctionDecl *Function,
+DeclAccessPair FoundDecl, ArrayRef Args,
+OverloadCandidateSet ,
+bool SuppressUserConversions,
+bool
[PATCH] D55534: [AST] Store "UsesADL" information in CallExpr.
EricWF marked 12 inline comments as done. EricWF added a comment. Address more review comments. Update incoming. Comment at: include/clang/AST/Expr.h:2425 CallExpr(const ASTContext , StmtClass SC, unsigned NumPreArgs, - unsigned NumArgs, EmptyShell Empty); + unsigned NumArgs, bool UsesADL, EmptyShell Empty); riccibruno wrote: > There is no need to pass this flag to the empty constructor > since it is going to be deserialized in `ASTReaderStmt`. > Only what is strictly needed to create the empty `CallExpr` > is passed here. In fact if you wanted to pass the flag when creating > the empty `CallExpr` you would have to update what is > under `case EXPR_CALL:` in `ASTReader::ReadStmtFromStream`. Ack. Thanks for the explanation. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55534/new/ https://reviews.llvm.org/D55534 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D55534: [AST] Store "UsesADL" information in CallExpr.
riccibruno added inline comments.
Comment at: include/clang/AST/Expr.h:2425
CallExpr(const ASTContext , StmtClass SC, unsigned NumPreArgs,
- unsigned NumArgs, EmptyShell Empty);
+ unsigned NumArgs, bool UsesADL, EmptyShell Empty);
There is no need to pass this flag to the empty constructor
since it is going to be deserialized in `ASTReaderStmt`.
Only what is strictly needed to create the empty `CallExpr`
is passed here. In fact if you wanted to pass the flag when creating
the empty `CallExpr` you would have to update what is
under `case EXPR_CALL:` in `ASTReader::ReadStmtFromStream`.
Comment at: include/clang/AST/ExprCXX.h:106
: CallExpr(C, CXXOperatorCallExprClass, /*NumPreArgs=*/0, NumArgs,
- Empty) {}
+ /*UsesADL=*/false, Empty) {}
same
Comment at: include/clang/AST/ExprCXX.h:178
+ : CallExpr(C, CXXMemberCallExprClass, /*NumPreArgs=*/0, NumArgs,
+ /*UsesADL=*/false, Empty) {}
same
Comment at: include/clang/AST/ExprCXX.h:224
: CallExpr(C, CUDAKernelCallExprClass, /*NumPreArgs=*/END_PREARG,
NumArgs,
- Empty) {}
+ /*UsesADL=*/false, Empty) {}
same
Comment at: include/clang/AST/ExprCXX.h:505
: CallExpr(C, UserDefinedLiteralClass, /*NumPreArgs=*/0, NumArgs,
- Empty) {}
+ /*UsesADL=*/false, Empty) {}
same
Comment at: lib/Serialization/ASTReaderStmt.cpp:741
E->setArg(I, Record.readSubExpr());
+ E->setUsesADL(UsesADL);
}
`E->setUsesADL(Record.readInt())` with the
` bool UsesADL = Record.readInt();` removed ?
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55534/new/
https://reviews.llvm.org/D55534
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D55534: [AST] Store "UsesADL" information in CallExpr.
EricWF marked 14 inline comments as done.
EricWF added a comment.
Address review comments. Fixes incoming.
Comment at: include/clang/AST/ExprCXX.h:218
+ SourceLocation RP, unsigned MinNumArgs = 0,
+ bool UsesADL = false)
: CallExpr(C, CUDAKernelCallExprClass, fn, Config, args, t, VK, RP,
fowles wrote:
> Can CUDAKernelCalls ever use ADL?
Yeah, after further investigation, I don't think so. I'll revert this change.
Comment at: include/clang/AST/ExprCXX.h:499
+ : CallExpr(C, UserDefinedLiteralClass, Fn, Args, T, VK, LitEndLoc,
+ /*MinNumArgs=*/0, /*UsesADL=*/false),
UDSuffixLoc(SuffixLoc) {}
fowles wrote:
> Same question, I assume user defined literals can only be called as string
> suffixes and thus never get ADL
Right. Their argument is a literal of some sort.
Comment at: lib/Sema/SemaOverload.cpp:8946
+ ExplicitTemplateArgs, Args, CandidateSet,
+ /*SuppressUserConversions=*/false,
+ PartialOverloading,
/*IsADLCandidate=*/true);
fowles wrote:
> should this fix be in a separate change?
I'll commit the fix as a separate patch.
Comment at: lib/Serialization/ASTReaderStmt.cpp:735
unsigned NumArgs = Record.readInt();
+ bool UsesADL = Record.readInt();
assert((NumArgs == E->getNumArgs()) && "Wrong NumArgs!");
fowles wrote:
> readInt into a bool?
Yeah. You serialize and serialize all integer types using `readInt()`
Comment at: unittests/ASTMatchers/ASTMatchersNodeTest.cpp:204
+ StatementMatcher ADLMatch = callExpr(usesADL());
+ auto NS_Str = R"DELIM(
+ namespace NS {
fowles wrote:
> if you use cc or cpp as your delimiter, I think clang-format will recurse
> correctly
That's super cool. Thanks!
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55534/new/
https://reviews.llvm.org/D55534
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D55534: [AST] Store "UsesADL" information in CallExpr.
fowles added inline comments.
Comment at: include/clang/AST/ExprCXX.h:218
+ SourceLocation RP, unsigned MinNumArgs = 0,
+ bool UsesADL = false)
: CallExpr(C, CUDAKernelCallExprClass, fn, Config, args, t, VK, RP,
Can CUDAKernelCalls ever use ADL?
Comment at: include/clang/AST/ExprCXX.h:499
+ : CallExpr(C, UserDefinedLiteralClass, Fn, Args, T, VK, LitEndLoc,
+ /*MinNumArgs=*/0, /*UsesADL=*/false),
UDSuffixLoc(SuffixLoc) {}
Same question, I assume user defined literals can only be called as string
suffixes and thus never get ADL
Comment at: include/clang/ASTMatchers/ASTMatchers.h:1275
+/// NS::y(x); // Doesn't match
+/// y(42);
+/// }
y(42); // Doesn't match
Comment at: include/clang/Sema/Overload.h:775
+/// True if the candidate was found using ADL.
+bool IsADLCandidate;
+
maybe bit pack these? I think overload sets for operator<< actually get pretty
big
Comment at: lib/Sema/SemaOverload.cpp:8946
+ ExplicitTemplateArgs, Args, CandidateSet,
+ /*SuppressUserConversions=*/false,
+ PartialOverloading,
/*IsADLCandidate=*/true);
should this fix be in a separate change?
Comment at: lib/Sema/SemaOverload.cpp:12024
return SemaRef.BuildResolvedCallExpr(Fn, FDecl, LParenLoc, Args, RParenLoc,
- ExecConfig);
+ ExecConfig, /*IsExecConfig*/ false,
+ (*Best)->IsADLCandidate);
/*IsExecConfig=*/
Comment at: lib/Sema/SemaOverload.cpp:12077
return SemaRef.BuildResolvedCallExpr(Fn, FDecl, LParenLoc, Args, RParenLoc,
- ExecConfig);
+ ExecConfig, /*IsExecConfig*/ false,
+ (*Best)->IsADLCandidate);
here too
Comment at: lib/Serialization/ASTReaderStmt.cpp:735
unsigned NumArgs = Record.readInt();
+ bool UsesADL = Record.readInt();
assert((NumArgs == E->getNumArgs()) && "Wrong NumArgs!");
readInt into a bool?
Comment at: unittests/ASTMatchers/ASTMatchersNodeTest.cpp:204
+ StatementMatcher ADLMatch = callExpr(usesADL());
+ auto NS_Str = R"DELIM(
+ namespace NS {
if you use cc or cpp as your delimiter, I think clang-format will recurse
correctly
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55534/new/
https://reviews.llvm.org/D55534
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D55534: [AST] Store "UsesADL" information in CallExpr.
EricWF created this revision.
EricWF added reviewers: fowles, rsmith, klimek.
Herald added a reviewer: shafik.
Currently the Clang AST doesn't store information about how the callee of a
CallExpr was found. Specifically if it was found using ADL.
However, this information is invaluable to tooling. Consider a tool which
renames usages of a function. If the originally CallExpr was formed using ADL,
then the tooling may need to additionally qualify the replacement.
Without information about how the callee was found, the tooling is left
scratching it's head. Additionally, we want to be able to match ADL calls as
quickly as possible, which means avoiding computing the answer on the fly.
This patch changes `CallExpr` to store whether it's callee was found using ADL.
It does not change the size of any AST nodes.
Repository:
rC Clang
https://reviews.llvm.org/D55534
Files:
include/clang/AST/Expr.h
include/clang/AST/ExprCXX.h
include/clang/AST/Stmt.h
include/clang/ASTMatchers/ASTMatchers.h
include/clang/Sema/Overload.h
include/clang/Sema/Sema.h
lib/AST/ASTDumper.cpp
lib/AST/ASTImporter.cpp
lib/AST/Expr.cpp
lib/Sema/SemaExpr.cpp
lib/Sema/SemaOverload.cpp
lib/Serialization/ASTReaderStmt.cpp
lib/Serialization/ASTWriterStmt.cpp
test/AST/ast-dump-expr.cpp
unittests/ASTMatchers/ASTMatchersNodeTest.cpp
Index: unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -199,6 +199,32 @@
"-fno-delayed-template-parsing"));
}
+TEST(Matcher, ADLCall) {
+ StatementMatcher ADLMatch = callExpr(usesADL());
+ auto NS_Str = R"DELIM(
+ namespace NS {
+struct X {};
+struct Y {};
+void f(X);
+template
+void g(T);
+ }
+ struct MyX {};
+ void f(...);
+ void g(...);
+)DELIM";
+
+ auto MkStr = [&](std::string Body) -> std::string {
+std::string S = NS_Str;
+S += "void test_fn() { " + Body + " }";
+return S;
+ };
+
+ EXPECT_TRUE(matches(MkStr("NS::X x; f(x);"), ADLMatch));
+ EXPECT_TRUE(notMatches(MkStr("NS::X x; NS::f(x);"), ADLMatch));
+ EXPECT_TRUE(notMatches(MkStr("MyX x; f(x);"), ADLMatch));
+}
+
TEST(Matcher, Call) {
// FIXME: Do we want to overload Call() to directly take
// Matcher, too?
Index: test/AST/ast-dump-expr.cpp
===
--- /dev/null
+++ test/AST/ast-dump-expr.cpp
@@ -0,0 +1,44 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -Wno-unused-value -std=c++14 -ast-dump %s \
+// RUN:| FileCheck -strict-whitespace %s
+
+namespace NS {
+struct X {};
+void f(X);
+void y(...);
+} // namespace NS
+
+// CHECK-LABEL: FunctionDecl 0x{{[^ ]*}} {{.*}}ADLCall 'void ()'
+void ADLCall() {
+ NS::X x;
+ // CHECK: CallExpr 0x{{[^ ]*}} ]+}}> 'void' adl{{$}}
+ f(x);
+ // CHECK: CallExpr 0x{{[^ ]*}} ]+}}> 'void' adl{{$}}
+ y(x);
+}
+
+// CHECK-LABEL: FunctionDecl 0x{{[^ ]*}} {{.*}}NonADLCall 'void ()'
+void NonADLCall() {
+ NS::X x;
+ // CHECK: CallExpr 0x{{[^ ]*}} ]+}}> 'void'{{$}}
+ NS::f(x);
+}
+
+// CHECK-LABEL: FunctionDecl 0x{{[^ ]*}} {{.*}}NonADLCall2 'void ()'
+void NonADLCall2() {
+ NS::X x;
+ using NS::f;
+ // CHECK: CallExpr 0x{{[^ ]*}} ]+}}> 'void'{{$}}
+ f(x);
+ // CHECK: CallExpr 0x{{[^ ]*}} ]+}}> 'void' adl{{$}}
+ y(x);
+}
+
+namespace test_adl_call_three {
+using namespace NS;
+// CHECK-LABEL: FunctionDecl 0x{{[^ ]*}} {{.*}}NonADLCall3 'void ()'
+void NonADLCall3() {
+ X x;
+ // CHECK: CallExpr 0x{{[^ ]*}} ]+}}> 'void'{{$}}
+ f(x);
+}
+} // namespace test_adl_call_three
Index: lib/Serialization/ASTWriterStmt.cpp
===
--- lib/Serialization/ASTWriterStmt.cpp
+++ lib/Serialization/ASTWriterStmt.cpp
@@ -646,6 +646,7 @@
void ASTStmtWriter::VisitCallExpr(CallExpr *E) {
VisitExpr(E);
Record.push_back(E->getNumArgs());
+ Record.push_back(E->usesADL());
Record.AddSourceLocation(E->getRParenLoc());
Record.AddStmt(E->getCallee());
for (CallExpr::arg_iterator Arg = E->arg_begin(), ArgEnd = E->arg_end();
Index: lib/Serialization/ASTReaderStmt.cpp
===
--- lib/Serialization/ASTReaderStmt.cpp
+++ lib/Serialization/ASTReaderStmt.cpp
@@ -732,11 +732,13 @@
void ASTStmtReader::VisitCallExpr(CallExpr *E) {
VisitExpr(E);
unsigned NumArgs = Record.readInt();
+ bool UsesADL = Record.readInt();
assert((NumArgs == E->getNumArgs()) && "Wrong NumArgs!");
E->setRParenLoc(ReadSourceLocation());
E->setCallee(Record.readSubExpr());
for (unsigned I = 0; I != NumArgs; ++I)
E->setArg(I, Record.readSubExpr());
+ E->setUsesADL(UsesADL);
}
void ASTStmtReader::VisitCXXMemberCallExpr(CXXMemberCallExpr *E) {
Index: lib/Sema/SemaOverload.cpp
===
---
