[PATCH] D72355: [clangd] Assert that the testcases in FindExplicitReferencesTest.All have no diagnostics
merge_guards_bot added a comment.
{icon check-circle color=green} Unit tests: pass. 61771 tests passed, 0 failed
and 780 were skipped.
{icon check-circle color=green} clang-tidy: pass.
{icon check-circle color=green} clang-format: pass.
Build artifacts
:
diff.json
,
clang-tidy.txt
,
clang-format.patch
,
CMakeCache.txt
,
console-log.txt
,
test-results.xml
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D72355/new/
https://reviews.llvm.org/D72355
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D72355: [clangd] Assert that the testcases in FindExplicitReferencesTest.All have no diagnostics
This revision was automatically updated to reflect the committed changes.
Closed by commit rG1ad1308b69b8: [clangd] Assert that the testcases in
FindExplicitReferencesTest.All have no… (authored by nridge).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D72355/new/
https://reviews.llvm.org/D72355
Files:
clang-tools-extra/clangd/unittests/FindTargetTests.cpp
Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -566,6 +566,10 @@
TU.ExtraArgs.push_back("-std=c++17");
auto AST = TU.build();
+for (auto &D : AST.getDiagnostics()) {
+ if (D.Severity > DiagnosticsEngine::Warning)
+ADD_FAILURE() << D << Code;
+}
auto *TestDecl = &findDecl(AST, "foo");
if (auto *T = llvm::dyn_cast(TestDecl))
@@ -718,7 +722,7 @@
"3: targets = {vb}, decl\n"},
// MemberExpr should know their using declaration.
{R"cpp(
-struct X { void func(int); }
+struct X { void func(int); };
struct Y : X {
using X::func;
};
@@ -824,7 +828,7 @@
void foo() {
$0^TT $1^x;
$2^foo<$3^TT>();
- $4^foo<$5^vector>()
+ $4^foo<$5^vector>();
$6^foo<$7^TP...>();
}
)cpp",
@@ -924,7 +928,7 @@
// Namespace aliases should be handled properly.
{
R"cpp(
-namespace ns { struct Type {} }
+namespace ns { struct Type {}; }
namespace alias = ns;
namespace rec_alias = alias;
Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -566,6 +566,10 @@
TU.ExtraArgs.push_back("-std=c++17");
auto AST = TU.build();
+for (auto &D : AST.getDiagnostics()) {
+ if (D.Severity > DiagnosticsEngine::Warning)
+ADD_FAILURE() << D << Code;
+}
auto *TestDecl = &findDecl(AST, "foo");
if (auto *T = llvm::dyn_cast(TestDecl))
@@ -718,7 +722,7 @@
"3: targets = {vb}, decl\n"},
// MemberExpr should know their using declaration.
{R"cpp(
-struct X { void func(int); }
+struct X { void func(int); };
struct Y : X {
using X::func;
};
@@ -824,7 +828,7 @@
void foo() {
$0^TT $1^x;
$2^foo<$3^TT>();
- $4^foo<$5^vector>()
+ $4^foo<$5^vector>();
$6^foo<$7^TP...>();
}
)cpp",
@@ -924,7 +928,7 @@
// Namespace aliases should be handled properly.
{
R"cpp(
-namespace ns { struct Type {} }
+namespace ns { struct Type {}; }
namespace alias = ns;
namespace rec_alias = alias;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D72355: [clangd] Assert that the testcases in FindExplicitReferencesTest.All have no diagnostics
nridge added inline comments.
Comment at: clang-tools-extra/clangd/unittests/FindTargetTests.cpp:898
class $10^Foo {
- $11^Foo(int);
+ $11^Foo(int) {}
$12^Foo(): $13^Foo(111) {}
ilya-biryukov wrote:
> NIT: is this change redundant now? This was probably a warning, not an error.
You're right, reverted this change.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D72355/new/
https://reviews.llvm.org/D72355
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D72355: [clangd] Assert that the testcases in FindExplicitReferencesTest.All have no diagnostics
nridge updated this revision to Diff 237570.
nridge marked 2 inline comments as done.
nridge added a comment.
Address nit
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D72355/new/
https://reviews.llvm.org/D72355
Files:
clang-tools-extra/clangd/unittests/FindTargetTests.cpp
Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -566,6 +566,10 @@
TU.ExtraArgs.push_back("-std=c++17");
auto AST = TU.build();
+for (auto &D : AST.getDiagnostics()) {
+ if (D.Severity > DiagnosticsEngine::Warning)
+ADD_FAILURE() << D << Code;
+}
auto *TestDecl = &findDecl(AST, "foo");
if (auto *T = llvm::dyn_cast(TestDecl))
@@ -718,7 +722,7 @@
"3: targets = {vb}, decl\n"},
// MemberExpr should know their using declaration.
{R"cpp(
-struct X { void func(int); }
+struct X { void func(int); };
struct Y : X {
using X::func;
};
@@ -824,7 +828,7 @@
void foo() {
$0^TT $1^x;
$2^foo<$3^TT>();
- $4^foo<$5^vector>()
+ $4^foo<$5^vector>();
$6^foo<$7^TP...>();
}
)cpp",
@@ -924,7 +928,7 @@
// Namespace aliases should be handled properly.
{
R"cpp(
-namespace ns { struct Type {} }
+namespace ns { struct Type {}; }
namespace alias = ns;
namespace rec_alias = alias;
Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -566,6 +566,10 @@
TU.ExtraArgs.push_back("-std=c++17");
auto AST = TU.build();
+for (auto &D : AST.getDiagnostics()) {
+ if (D.Severity > DiagnosticsEngine::Warning)
+ADD_FAILURE() << D << Code;
+}
auto *TestDecl = &findDecl(AST, "foo");
if (auto *T = llvm::dyn_cast(TestDecl))
@@ -718,7 +722,7 @@
"3: targets = {vb}, decl\n"},
// MemberExpr should know their using declaration.
{R"cpp(
-struct X { void func(int); }
+struct X { void func(int); };
struct Y : X {
using X::func;
};
@@ -824,7 +828,7 @@
void foo() {
$0^TT $1^x;
$2^foo<$3^TT>();
- $4^foo<$5^vector>()
+ $4^foo<$5^vector>();
$6^foo<$7^TP...>();
}
)cpp",
@@ -924,7 +928,7 @@
// Namespace aliases should be handled properly.
{
R"cpp(
-namespace ns { struct Type {} }
+namespace ns { struct Type {}; }
namespace alias = ns;
namespace rec_alias = alias;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D72355: [clangd] Assert that the testcases in FindExplicitReferencesTest.All have no diagnostics
ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.
LGTM, thanks!
Comment at: clang-tools-extra/clangd/unittests/FindTargetTests.cpp:898
class $10^Foo {
- $11^Foo(int);
+ $11^Foo(int) {}
$12^Foo(): $13^Foo(111) {}
NIT: is this change redundant now? This was probably a warning, not an error.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D72355/new/
https://reviews.llvm.org/D72355
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D72355: [clangd] Assert that the testcases in FindExplicitReferencesTest.All have no diagnostics
merge_guards_bot added a comment.
{icon check-circle color=green} Unit tests: pass. 61726 tests passed, 0 failed
and 779 were skipped.
{icon times-circle color=red} clang-tidy: fail. Please fix clang-tidy findings
.
{icon check-circle color=green} clang-format: pass.
Build artifacts
:
diff.json
,
clang-tidy.txt
,
clang-format.patch
,
CMakeCache.txt
,
console-log.txt
,
test-results.xml
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D72355/new/
https://reviews.llvm.org/D72355
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D72355: [clangd] Assert that the testcases in FindExplicitReferencesTest.All have no diagnostics
nridge updated this revision to Diff 237193.
nridge added a comment.
Address review comments
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D72355/new/
https://reviews.llvm.org/D72355
Files:
clang-tools-extra/clangd/unittests/FindTargetTests.cpp
Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -566,6 +566,10 @@
TU.ExtraArgs.push_back("-std=c++17");
auto AST = TU.build();
+for (auto &D : AST.getDiagnostics()) {
+ if (D.Severity > DiagnosticsEngine::Warning)
+ADD_FAILURE() << D << Code;
+}
auto *TestDecl = &findDecl(AST, "foo");
if (auto *T = llvm::dyn_cast(TestDecl))
@@ -718,7 +722,7 @@
"3: targets = {vb}, decl\n"},
// MemberExpr should know their using declaration.
{R"cpp(
-struct X { void func(int); }
+struct X { void func(int); };
struct Y : X {
using X::func;
};
@@ -824,7 +828,7 @@
void foo() {
$0^TT $1^x;
$2^foo<$3^TT>();
- $4^foo<$5^vector>()
+ $4^foo<$5^vector>();
$6^foo<$7^TP...>();
}
)cpp",
@@ -891,7 +895,7 @@
};
// delegating initializer
class $10^Foo {
- $11^Foo(int);
+ $11^Foo(int) {}
$12^Foo(): $13^Foo(111) {}
};
}
@@ -924,7 +928,7 @@
// Namespace aliases should be handled properly.
{
R"cpp(
-namespace ns { struct Type {} }
+namespace ns { struct Type {}; }
namespace alias = ns;
namespace rec_alias = alias;
Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -566,6 +566,10 @@
TU.ExtraArgs.push_back("-std=c++17");
auto AST = TU.build();
+for (auto &D : AST.getDiagnostics()) {
+ if (D.Severity > DiagnosticsEngine::Warning)
+ADD_FAILURE() << D << Code;
+}
auto *TestDecl = &findDecl(AST, "foo");
if (auto *T = llvm::dyn_cast(TestDecl))
@@ -718,7 +722,7 @@
"3: targets = {vb}, decl\n"},
// MemberExpr should know their using declaration.
{R"cpp(
-struct X { void func(int); }
+struct X { void func(int); };
struct Y : X {
using X::func;
};
@@ -824,7 +828,7 @@
void foo() {
$0^TT $1^x;
$2^foo<$3^TT>();
- $4^foo<$5^vector>()
+ $4^foo<$5^vector>();
$6^foo<$7^TP...>();
}
)cpp",
@@ -891,7 +895,7 @@
};
// delegating initializer
class $10^Foo {
- $11^Foo(int);
+ $11^Foo(int) {}
$12^Foo(): $13^Foo(111) {}
};
}
@@ -924,7 +928,7 @@
// Namespace aliases should be handled properly.
{
R"cpp(
-namespace ns { struct Type {} }
+namespace ns { struct Type {}; }
namespace alias = ns;
namespace rec_alias = alias;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D72355: [clangd] Assert that the testcases in FindExplicitReferencesTest.All have no diagnostics
ilya-biryukov requested changes to this revision.
ilya-biryukov added inline comments.
This revision now requires changes to proceed.
Comment at: clang-tools-extra/clangd/unittests/FindTargetTests.cpp:571
+ ADD_FAILURE() << D << Code;
+assert(AST.getDiagnostics().empty());
`ADD_FAILURE()` should be enough to indicate there are errors to the users.
No need to crash additionally.
Could you remove this assert?
Comment at: clang-tools-extra/clangd/unittests/FindTargetTests.cpp:750
void foo(int a, int b) {
- $0^FOO+$1^BAR;
+ (void)($0^FOO+$1^BAR);
}
Could we assert there are no errors instead?
Having warnings is totally fine and allows to avoid changing testcases (like
this one)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D72355/new/
https://reviews.llvm.org/D72355
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D72355: [clangd] Assert that the testcases in FindExplicitReferencesTest.All have no diagnostics
merge_guards_bot added a comment.
{icon question-circle color=gray} Unit tests: unknown.
{icon question-circle color=gray} clang-tidy: unknown.
{icon question-circle color=gray} clang-format: unknown.
Build artifacts
:
diff.json
,
console-log.txt
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D72355/new/
https://reviews.llvm.org/D72355
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D72355: [clangd] Assert that the testcases in FindExplicitReferencesTest.All have no diagnostics
nridge created this revision.
nridge added a reviewer: kadircet.
Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, MaskRay,
ilya-biryukov.
Herald added a project: clang.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D72355
Files:
clang-tools-extra/clangd/unittests/FindTargetTests.cpp
Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -566,6 +566,9 @@
TU.ExtraArgs.push_back("-std=c++17");
auto AST = TU.build();
+for (auto &D : AST.getDiagnostics())
+ ADD_FAILURE() << D << Code;
+assert(AST.getDiagnostics().empty());
auto *TestDecl = &findDecl(AST, "foo");
if (auto *T = llvm::dyn_cast(TestDecl))
@@ -663,7 +666,7 @@
void foo() {
$0^Struct $1^x;
$2^Typedef $3^y;
- static_cast<$4^Struct*>(0);
+ (void)static_cast<$4^Struct*>(0);
}
)cpp",
"0: targets = {Struct}\n"
@@ -718,7 +721,7 @@
"3: targets = {vb}, decl\n"},
// MemberExpr should know their using declaration.
{R"cpp(
-struct X { void func(int); }
+struct X { void func(int); };
struct Y : X {
using X::func;
};
@@ -744,7 +747,7 @@
#define BAR b
void foo(int a, int b) {
- $0^FOO+$1^BAR;
+ (void)($0^FOO+$1^BAR);
}
)cpp",
"0: targets = {a}\n"
@@ -824,7 +827,7 @@
void foo() {
$0^TT $1^x;
$2^foo<$3^TT>();
- $4^foo<$5^vector>()
+ $4^foo<$5^vector>();
$6^foo<$7^TP...>();
}
)cpp",
@@ -891,7 +894,7 @@
};
// delegating initializer
class $10^Foo {
- $11^Foo(int);
+ $11^Foo(int) {}
$12^Foo(): $13^Foo(111) {}
};
}
@@ -924,7 +927,7 @@
// Namespace aliases should be handled properly.
{
R"cpp(
-namespace ns { struct Type {} }
+namespace ns { struct Type {}; }
namespace alias = ns;
namespace rec_alias = alias;
Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -566,6 +566,9 @@
TU.ExtraArgs.push_back("-std=c++17");
auto AST = TU.build();
+for (auto &D : AST.getDiagnostics())
+ ADD_FAILURE() << D << Code;
+assert(AST.getDiagnostics().empty());
auto *TestDecl = &findDecl(AST, "foo");
if (auto *T = llvm::dyn_cast(TestDecl))
@@ -663,7 +666,7 @@
void foo() {
$0^Struct $1^x;
$2^Typedef $3^y;
- static_cast<$4^Struct*>(0);
+ (void)static_cast<$4^Struct*>(0);
}
)cpp",
"0: targets = {Struct}\n"
@@ -718,7 +721,7 @@
"3: targets = {vb}, decl\n"},
// MemberExpr should know their using declaration.
{R"cpp(
-struct X { void func(int); }
+struct X { void func(int); };
struct Y : X {
using X::func;
};
@@ -744,7 +747,7 @@
#define BAR b
void foo(int a, int b) {
- $0^FOO+$1^BAR;
+ (void)($0^FOO+$1^BAR);
}
)cpp",
"0: targets = {a}\n"
@@ -824,7 +827,7 @@
void foo() {
$0^TT $1^x;
$2^foo<$3^TT>();
- $4^foo<$5^vector>()
+ $4^foo<$5^vector>();
$6^foo<$7^TP...>();
}
)cpp",
@@ -891,7 +894,7 @@
};
// delegating initializer
class $10^Foo {
- $11^Foo(int);
+ $11^Foo(int) {}
$12^Foo(): $13^Foo(111) {}
};
}
@@ -924,7 +927,7 @@
// Namespace aliases should be handled properly.
{
R"cpp(
-namespace ns { struct Type {} }
+namespace ns { struct Type {}; }
namespace alias = ns;
namespace rec_alias = alias;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
