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; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
