[PATCH] D133043: [clangd] Fix tests for implicit C function declaration

[Aleksandr Platonov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGcc4b86cfc01c: [clangd] Fix tests for implicit C function 
declaration (authored by ArcsinX).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133043/new/

https://reviews.llvm.org/D133043

Files:
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp


Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -1465,11 +1465,12 @@
 TEST(IncludeFixerTest, HeaderNamedInDiag) {
   Annotations Test(R"cpp(
 $insert[[]]int main() {
-  [[printf]](""); // error-ok
+  [[printf]]("");
 }
   )cpp");
   auto TU = TestTU::withCode(Test.code());
-  TU.ExtraArgs = {"-xc"};
+  TU.ExtraArgs = {"-xc", "-std=c99",
+  "-Wno-error=implicit-function-declaration"};
   auto Index = buildIndexWithSymbol({});
   TU.ExternalIndex = Index.get();
 
@@ -1482,13 +1483,22 @@
  "declarations"),
   withFix(Fix(Test.range("insert"), "#include \n",
   "Include  for symbol printf");
+
+  TU.ExtraArgs = {"-xc", "-std=c89"};
+  EXPECT_THAT(
+  *TU.build().getDiagnostics(),
+  ElementsAre(AllOf(
+  Diag(Test.range(), "implicitly declaring library function 'printf' "
+ "with type 'int (const char *, ...)'"),
+  withFix(Fix(Test.range("insert"), "#include \n",
+  "Include  for symbol printf");
 }
 
 TEST(IncludeFixerTest, CImplicitFunctionDecl) {
-  Annotations Test("void x() { [[foo]](); /* error-ok */ }");
+  Annotations Test("void x() { [[foo]](); }");
   auto TU = TestTU::withCode(Test.code());
   TU.Filename = "test.c";
-  TU.ExtraArgs.push_back("-std=c99");
+  TU.ExtraArgs = {"-std=c99", "-Wno-error=implicit-function-declaration"};
 
   Symbol Sym = func("foo");
   Sym.Flags |= Symbol::IndexedForCodeCompletion;
@@ -1509,6 +1519,13 @@
"support implicit function declarations"),
   withFix(Fix(Range{}, "#include \"foo.h\"\n",
   "Include \"foo.h\" for symbol foo");
+
+  TU.ExtraArgs = {"-std=c89", "-Wall"};
+  EXPECT_THAT(*TU.build().getDiagnostics(),
+  ElementsAre(AllOf(
+  Diag(Test.range(), "implicit declaration of function 'foo'"),
+  withFix(Fix(Range{}, "#include \"foo.h\"\n",
+  "Include \"foo.h\" for symbol foo");
 }
 
 TEST(DiagsInHeaders, DiagInsideHeader) {
Index: clang-tools-extra/clangd/ParsedAST.cpp
===
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -493,6 +493,7 @@
 // We restore the original severity in the level adjuster.
 // FIXME: It would be better to have a real API for this, but what?
 for (auto ID : {diag::ext_implicit_function_decl_c99,
+diag::ext_implicit_lib_function_decl,
 diag::ext_implicit_lib_function_decl_c99,
 diag::warn_implicit_function_decl}) {
   OverriddenSeverity.try_emplace(


Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -1465,11 +1465,12 @@
 TEST(IncludeFixerTest, HeaderNamedInDiag) {
   Annotations Test(R"cpp(
 $insert[[]]int main() {
-  [[printf]](""); // error-ok
+  [[printf]]("");
 }
   )cpp");
   auto TU = TestTU::withCode(Test.code());
-  TU.ExtraArgs = {"-xc"};
+  TU.ExtraArgs = {"-xc", "-std=c99",
+  "-Wno-error=implicit-function-declaration"};
   auto Index = buildIndexWithSymbol({});
   TU.ExternalIndex = Index.get();
 
@@ -1482,13 +1483,22 @@
  "declarations"),
   withFix(Fix(Test.range("insert"), "#include \n",
   "Include  for symbol printf");
+
+  TU.ExtraArgs = {"-xc", "-std=c89"};
+  EXPECT_THAT(
+  *TU.build().getDiagnostics(),
+  ElementsAre(AllOf(
+  Diag(Test.range(), "implicitly declaring library function 'printf' "
+ "with type 'int (const char *, ...)'"),
+  withFix(Fix(Test.range("insert"), "#include \n",
+  "Include  for symbol printf");
 }
 
 TEST(IncludeFixerTest, CImplicitFunctionDecl) {
-  Annotations Test("void x() { [[foo]](); /* error-ok */ }");
+  Annotations Test("void x() { [[foo]](); }");
   auto TU = TestTU::withCode(Test.code());
   TU.Filename = "test.c";
-  TU.ExtraArgs.push_back("-std=c99");
+  TU.ExtraArgs = {"-std=c99", 

[PATCH] D133043: [clangd] Fix tests for implicit C function declaration

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

Thanks!

(And not surprising at all this regressed, the mechanism clangd is using here 
is extremely fragile, we just couldn't come up with a better one :-()


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133043/new/

https://reviews.llvm.org/D133043

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133043: [clangd] Fix tests for implicit C function declaration

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

LGTM, but someone with more clangd experience should do the final sign off 
(seeing as how I caused the issue in the first place, sorry about that!).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133043/new/

https://reviews.llvm.org/D133043

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133043: [clangd] Fix tests for implicit C function declaration

[Aleksandr Platonov via Phabricator via cfe-commits]

ArcsinX created this revision.
ArcsinX added reviewers: aaron.ballman, sammccall, kadircet.
Herald added a subscriber: arphaman.
Herald added a project: All.
ArcsinX requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

clangd code fixes at D122983  were not right.
We need to check that clangd provides IncludeFixer fixits for implicit function 
declaration even if this is not an error (e.g. implicit function declaration in 
C89).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133043

Files:
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp


Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -1465,11 +1465,12 @@
 TEST(IncludeFixerTest, HeaderNamedInDiag) {
   Annotations Test(R"cpp(
 $insert[[]]int main() {
-  [[printf]](""); // error-ok
+  [[printf]]("");
 }
   )cpp");
   auto TU = TestTU::withCode(Test.code());
-  TU.ExtraArgs = {"-xc"};
+  TU.ExtraArgs = {"-xc", "-std=c99",
+  "-Wno-error=implicit-function-declaration"};
   auto Index = buildIndexWithSymbol({});
   TU.ExternalIndex = Index.get();
 
@@ -1482,13 +1483,22 @@
  "declarations"),
   withFix(Fix(Test.range("insert"), "#include \n",
   "Include  for symbol printf");
+
+  TU.ExtraArgs = {"-xc", "-std=c89"};
+  EXPECT_THAT(
+  *TU.build().getDiagnostics(),
+  ElementsAre(AllOf(
+  Diag(Test.range(), "implicitly declaring library function 'printf' "
+ "with type 'int (const char *, ...)'"),
+  withFix(Fix(Test.range("insert"), "#include \n",
+  "Include  for symbol printf");
 }
 
 TEST(IncludeFixerTest, CImplicitFunctionDecl) {
-  Annotations Test("void x() { [[foo]](); /* error-ok */ }");
+  Annotations Test("void x() { [[foo]](); }");
   auto TU = TestTU::withCode(Test.code());
   TU.Filename = "test.c";
-  TU.ExtraArgs.push_back("-std=c99");
+  TU.ExtraArgs = {"-std=c99", "-Wno-error=implicit-function-declaration"};
 
   Symbol Sym = func("foo");
   Sym.Flags |= Symbol::IndexedForCodeCompletion;
@@ -1509,6 +1519,13 @@
"support implicit function declarations"),
   withFix(Fix(Range{}, "#include \"foo.h\"\n",
   "Include \"foo.h\" for symbol foo");
+
+  TU.ExtraArgs = {"-std=c89", "-Wall"};
+  EXPECT_THAT(*TU.build().getDiagnostics(),
+  ElementsAre(AllOf(
+  Diag(Test.range(), "implicit declaration of function 'foo'"),
+  withFix(Fix(Range{}, "#include \"foo.h\"\n",
+  "Include \"foo.h\" for symbol foo");
 }
 
 TEST(DiagsInHeaders, DiagInsideHeader) {
Index: clang-tools-extra/clangd/ParsedAST.cpp
===
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -493,6 +493,7 @@
 // We restore the original severity in the level adjuster.
 // FIXME: It would be better to have a real API for this, but what?
 for (auto ID : {diag::ext_implicit_function_decl_c99,
+diag::ext_implicit_lib_function_decl,
 diag::ext_implicit_lib_function_decl_c99,
 diag::warn_implicit_function_decl}) {
   OverriddenSeverity.try_emplace(


Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -1465,11 +1465,12 @@
 TEST(IncludeFixerTest, HeaderNamedInDiag) {
   Annotations Test(R"cpp(
 $insert[[]]int main() {
-  [[printf]](""); // error-ok
+  [[printf]]("");
 }
   )cpp");
   auto TU = TestTU::withCode(Test.code());
-  TU.ExtraArgs = {"-xc"};
+  TU.ExtraArgs = {"-xc", "-std=c99",
+  "-Wno-error=implicit-function-declaration"};
   auto Index = buildIndexWithSymbol({});
   TU.ExternalIndex = Index.get();
 
@@ -1482,13 +1483,22 @@
  "declarations"),
   withFix(Fix(Test.range("insert"), "#include \n",
   "Include  for symbol printf");
+
+  TU.ExtraArgs = {"-xc", "-std=c89"};
+  EXPECT_THAT(
+  *TU.build().getDiagnostics(),
+  ElementsAre(AllOf(
+  Diag(Test.range(), "implicitly declaring library function 'printf' "
+ "with type 'int (const char *, ...)'"),
+  withFix(Fix(Test.range("insert"), "#include \n",
+  "Include  for symbol printf");
 }
 
 TEST(IncludeFixerTest,