diff --git a/clang-tools-extra/clangd/Diagnostics.cpp b/clang-tools-extra/clangd/Diagnostics.cpp --- a/clang-tools-extra/clangd/Diagnostics.cpp +++ b/clang-tools-extra/clangd/Diagnostics.cpp @@ -419,6 +419,7 @@ OS << Sep << Fix; Sep = ", "; } + OS << "}"; } return OS; } diff --git a/clang-tools-extra/clangd/IncludeFixer.cpp b/clang-tools-extra/clangd/IncludeFixer.cpp --- a/clang-tools-extra/clangd/IncludeFixer.cpp +++ b/clang-tools-extra/clangd/IncludeFixer.cpp @@ -193,6 +193,14 @@ 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 @@ // 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 @@ 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 --- 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 #include #include @@ -412,6 +405,7 @@ ast_matchers::MatchFinder CTFinder; llvm::Optional CTContext; llvm::Optional FixIncludes; + llvm::DenseMap OverriddenSeverity; // No need to run clang-tidy or IncludeFixerif we are not going to surface // diagnostics. if (PreserveDiags) { @@ -434,12 +428,30 @@ 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 --- a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp +++ b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp @@ -1285,6 +1285,31 @@ "Include 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 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -15002,7 +15002,24 @@ 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 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 @@ 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 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;