[clang-tools-extra] 1ab1379 - [clangd] Include fixer for missing functions in C
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Mon Jan 10 03:17:25 PST 2022
Author: Sam McCall
Date: 2022-01-10T12:17:19+01:00
New Revision: 1ab13793beafd1db0159a410560b3ce998b15f5e
URL: https://github.com/llvm/llvm-project/commit/1ab13793beafd1db0159a410560b3ce998b15f5e
DIFF: https://github.com/llvm/llvm-project/commit/1ab13793beafd1db0159a410560b3ce998b15f5e.diff
LOG: [clangd] Include fixer for missing functions in C
A function call `unresolved()` in C will generate an implicit declaration
of the missing function and warn `ext_implicit_function_decl` or so.
(Compared to in C++ where we get `err_undeclared_var_use`).
We want to try to resolve these names.
Unfortunately typo correction is disabled in sema for performance
reasons unless this warning is promoted to error.
(We need typo correction for include-fixer.)
It's not clear to me where a switch to force this correction on should
go, include-fixer is kind of a hack. So hack more by telling sema we're
promoting them to error.
Fixes https://github.com/clangd/clangd/issues/937
Differential Revision: https://reviews.llvm.org/D115490
Added:
Modified:
clang-tools-extra/clangd/Diagnostics.cpp
clang-tools-extra/clangd/IncludeFixer.cpp
clang-tools-extra/clangd/ParsedAST.cpp
clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
clang/lib/Sema/SemaDecl.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/Diagnostics.cpp b/clang-tools-extra/clangd/Diagnostics.cpp
index b90a6a8fa945..126f4c3e50ad 100644
--- a/clang-tools-extra/clangd/Diagnostics.cpp
+++ b/clang-tools-extra/clangd/Diagnostics.cpp
@@ -419,6 +419,7 @@ llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const Diag &D) {
OS << Sep << Fix;
Sep = ", ";
}
+ OS << "}";
}
return OS;
}
diff --git a/clang-tools-extra/clangd/IncludeFixer.cpp b/clang-tools-extra/clangd/IncludeFixer.cpp
index 8c020f2486f0..1f0515c1df70 100644
--- a/clang-tools-extra/clangd/IncludeFixer.cpp
+++ b/clang-tools-extra/clangd/IncludeFixer.cpp
@@ -193,6 +193,14 @@ std::vector<Fix> IncludeFixer::fix(DiagnosticsEngine::Level DiagLevel,
case diag::err_no_member_suggest:
case diag::err_no_member_template:
case diag::err_no_member_template_suggest:
+ case diag::warn_implicit_function_decl:
+ case diag::ext_implicit_function_decl:
+ case diag::err_opencl_implicit_function_decl:
+ dlog("Unresolved name at {0}, last typo was {1}",
+ Info.getLocation().printToString(Info.getSourceManager()),
+ LastUnresolvedName
+ ? LastUnresolvedName->Loc.printToString(Info.getSourceManager())
+ : "none");
if (LastUnresolvedName) {
// Try to fix unresolved name caused by missing declaration.
// E.g.
@@ -205,8 +213,7 @@ std::vector<Fix> IncludeFixer::fix(DiagnosticsEngine::Level DiagLevel,
// UnresolvedName
// We only attempt to recover a diagnostic if it has the same location as
// the last seen unresolved name.
- if (DiagLevel >= DiagnosticsEngine::Error &&
- LastUnresolvedName->Loc == Info.getLocation())
+ if (LastUnresolvedName->Loc == Info.getLocation())
return fixUnresolvedName();
}
break;
@@ -481,6 +488,7 @@ class IncludeFixer::UnresolvedNameRecorder : public ExternalSemaSource {
CorrectionCandidateCallback &CCC,
DeclContext *MemberContext, bool EnteringContext,
const ObjCObjectPointerType *OPT) override {
+ dlog("CorrectTypo: {0}", Typo.getAsString());
assert(SemaPtr && "Sema must have been set.");
if (SemaPtr->isSFINAEContext())
return TypoCorrection();
diff --git a/clang-tools-extra/clangd/ParsedAST.cpp b/clang-tools-extra/clangd/ParsedAST.cpp
index 732c36813800..42552e9831a0 100644
--- a/clang-tools-extra/clangd/ParsedAST.cpp
+++ b/clang-tools-extra/clangd/ParsedAST.cpp
@@ -30,6 +30,7 @@
#include "clang/AST/ASTContext.h"
#include "clang/AST/Decl.h"
#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/DiagnosticSema.h"
#include "clang/Basic/LangOptions.h"
#include "clang/Basic/SourceLocation.h"
#include "clang/Basic/SourceManager.h"
@@ -37,17 +38,10 @@
#include "clang/Frontend/CompilerInstance.h"
#include "clang/Frontend/CompilerInvocation.h"
#include "clang/Frontend/FrontendActions.h"
-#include "clang/Frontend/Utils.h"
-#include "clang/Index/IndexDataConsumer.h"
-#include "clang/Index/IndexingAction.h"
#include "clang/Lex/Lexer.h"
-#include "clang/Lex/MacroInfo.h"
#include "clang/Lex/PPCallbacks.h"
#include "clang/Lex/Preprocessor.h"
-#include "clang/Lex/PreprocessorOptions.h"
-#include "clang/Sema/Sema.h"
#include "clang/Serialization/ASTWriter.h"
-#include "clang/Serialization/PCHContainerOperations.h"
#include "clang/Tooling/CompilationDatabase.h"
#include "clang/Tooling/Syntax/Tokens.h"
#include "llvm/ADT/ArrayRef.h"
@@ -55,7 +49,6 @@
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringRef.h"
-#include "llvm/Support/raw_ostream.h"
#include <algorithm>
#include <memory>
#include <vector>
@@ -412,6 +405,7 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs,
ast_matchers::MatchFinder CTFinder;
llvm::Optional<tidy::ClangTidyContext> CTContext;
llvm::Optional<IncludeFixer> FixIncludes;
+ llvm::DenseMap<diag::kind, DiagnosticsEngine::Level> OverriddenSeverity;
// No need to run clang-tidy or IncludeFixerif we are not going to surface
// diagnostics.
if (PreserveDiags) {
@@ -434,12 +428,30 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs,
Check->registerMatchers(&CTFinder);
}
+ // Clang only corrects typos for use of undeclared functions in C if that
+ // use is an error. Include fixer relies on typo correction, so pretend
+ // this is an error. (The actual typo correction is nice too).
+ // 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,
+ diag::warn_implicit_function_decl}) {
+ OverriddenSeverity.try_emplace(
+ ID, Clang->getDiagnostics().getDiagnosticLevel(ID, SourceLocation()));
+ Clang->getDiagnostics().setSeverity(ID, diag::Severity::Error,
+ SourceLocation());
+ }
+
const Config &Cfg = Config::current();
ASTDiags.setLevelAdjuster([&](DiagnosticsEngine::Level DiagLevel,
const clang::Diagnostic &Info) {
if (Cfg.Diagnostics.SuppressAll ||
isBuiltinDiagnosticSuppressed(Info.getID(), Cfg.Diagnostics.Suppress))
return DiagnosticsEngine::Ignored;
+
+ auto It = OverriddenSeverity.find(Info.getID());
+ if (It != OverriddenSeverity.end())
+ DiagLevel = It->second;
+
if (!CTChecks.empty()) {
std::string CheckName = CTContext->getCheckName(Info.getID());
bool IsClangTidyDiag = !CheckName.empty();
diff --git a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
index 3d4bc1cea87c..40a5d557d720 100644
--- a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -1285,6 +1285,31 @@ TEST(IncludeFixerTest, HeaderNamedInDiag) {
"Include <stdio.h> for symbol printf")))));
}
+TEST(IncludeFixerTest, CImplicitFunctionDecl) {
+ Annotations Test("void x() { [[foo]](); }");
+ auto TU = TestTU::withCode(Test.code());
+ TU.Filename = "test.c";
+
+ Symbol Sym = func("foo");
+ Sym.Flags |= Symbol::IndexedForCodeCompletion;
+ Sym.CanonicalDeclaration.FileURI = "unittest:///foo.h";
+ Sym.IncludeHeaders.emplace_back("\"foo.h\"", 1);
+
+ SymbolSlab::Builder Slab;
+ Slab.insert(Sym);
+ auto Index =
+ MemIndex::build(std::move(Slab).build(), RefSlab(), RelationSlab());
+ TU.ExternalIndex = Index.get();
+
+ EXPECT_THAT(
+ *TU.build().getDiagnostics(),
+ ElementsAre(AllOf(
+ Diag(Test.range(),
+ "implicit declaration of function 'foo' is invalid in C99"),
+ WithFix(Fix(Range{}, "#include \"foo.h\"\n",
+ "Include \"foo.h\" for symbol foo")))));
+}
+
TEST(DiagsInHeaders, DiagInsideHeader) {
Annotations Main(R"cpp(
#include [["a.h"]]
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index fa156d615447..d39d010f5eae 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -15002,7 +15002,24 @@ NamedDecl *Sema::ImplicitlyDefineFunction(SourceLocation Loc,
diag_id = diag::ext_implicit_function_decl;
else
diag_id = diag::warn_implicit_function_decl;
+
+ TypoCorrection Corrected;
+ // Because typo correction is expensive, only do it if the implicit
+ // function declaration is going to be treated as an error.
+ //
+ // Perform the corection before issuing the main diagnostic, as some consumers
+ // use typo-correction callbacks to enhance the main diagnostic.
+ if (S && !ExternCPrev &&
+ (Diags.getDiagnosticLevel(diag_id, Loc) >= DiagnosticsEngine::Error)) {
+ DeclFilterCCC<FunctionDecl> CCC{};
+ Corrected = CorrectTypo(DeclarationNameInfo(&II, Loc), LookupOrdinaryName,
+ S, nullptr, CCC, CTK_NonError);
+ }
+
Diag(Loc, diag_id) << &II;
+ if (Corrected)
+ diagnoseTypo(Corrected, PDiag(diag::note_function_suggestion),
+ /*ErrorRecovery*/ false);
// If we found a prior declaration of this function, don't bother building
// another one. We've already pushed that one into scope, so there's nothing
@@ -15010,18 +15027,6 @@ NamedDecl *Sema::ImplicitlyDefineFunction(SourceLocation Loc,
if (ExternCPrev)
return ExternCPrev;
- // Because typo correction is expensive, only do it if the implicit
- // function declaration is going to be treated as an error.
- if (Diags.getDiagnosticLevel(diag_id, Loc) >= DiagnosticsEngine::Error) {
- TypoCorrection Corrected;
- DeclFilterCCC<FunctionDecl> CCC{};
- if (S && (Corrected =
- CorrectTypo(DeclarationNameInfo(&II, Loc), LookupOrdinaryName,
- S, nullptr, CCC, CTK_NonError)))
- diagnoseTypo(Corrected, PDiag(diag::note_function_suggestion),
- /*ErrorRecovery*/false);
- }
-
// Set a Declarator for the implicit definition: int foo();
const char *Dummy;
AttributeFactory attrFactory;
More information about the cfe-commits
mailing list