[clangd] Include fixer for missing functions in C

[sammccall]

Summary

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

[kadircet]

clang/lib/Sema/SemaDecl.cpp

15012

maybe some comments around importance of the sequencing here? it would probably be nice to have that at a higher level documentation too, but unclear where. as in theory it's not just about the typo correction but also emitting diagnostics while doing so.

[sammccall]

clang/lib/Sema/SemaDecl.cpp

15012

Yeah, this is obviously pretty hacky, and I don't have great ideas to improve it.
Added a comment to leave some breadcrumbs and discourage moving this code around.

clang-tools-extra/clangd/Diagnostics.cpp

llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const Diag &D) {
   }
   if (!D.Fixes.empty()) {
     OS << ", fixes: {";
     const char *Sep = "";
     for (auto &Fix : D.Fixes) {
       OS << Sep << Fix;
       Sep = ", ";
     }
+    OS << "}";
   }
   return OS;
 }
 
 CodeAction toCodeAction(const Fix &F, const URIForFile &File) {
   CodeAction Action;
   Action.title = F.Message;
   Action.kind = std::string(CodeAction::QUICKFIX_KIND);

clang-tools-extra/clangd/IncludeFixer.cpp

std::vector<Fix> IncludeFixer::fix(DiagnosticsEngine::Level DiagLevel,
   case diag::err_undeclared_use:
   case diag::err_undeclared_use_suggest:
   case diag::err_undeclared_var_use:
   case diag::err_undeclared_var_use_suggest:
   case diag::err_no_member: // Could be no member in namespace.
   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.
       //   clang::SourceManager SM;
       //          ~~~~~~~~~~~~~
       //          UnresolvedName
       //   or
       //   namespace clang {  SourceManager SM; }
       //                      ~~~~~~~~~~~~~
       //                      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;
 
   // Cases where clang explicitly knows which header to include.
   // (There's no fix provided for boring formatting reasons).
   case diag::err_implied_std_initializer_list_not_found:
     return only(insertHeader("<initializer_list>"));
public:
 
   // Captures the latest typo and treat it as an unresolved name that can
   // potentially be fixed by adding #includes.
   TypoCorrection CorrectTypo(const DeclarationNameInfo &Typo, int LookupKind,
                              Scope *S, CXXScopeSpec *SS,
                              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();
     if (!isInsideMainFile(Typo.getLoc(), SemaPtr->SourceMgr))
       return clang::TypoCorrection();
 
     auto Extracted = extractUnresolvedNameCheaply(
         SemaPtr->SourceMgr, Typo, SS, SemaPtr->LangOpts,

clang-tools-extra/clangd/ParsedAST.cpp

 #include "TidyProvider.h"
 #include "index/CanonicalIncludes.h"
 #include "index/Index.h"
 #include "support/Logger.h"
 #include "support/Trace.h"
 #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"
 #include "clang/Basic/TokenKinds.h"
 #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"
 #include "llvm/ADT/STLExtras.h"
 #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>
 
 // Force the linker to link in Clang-tidy modules.
 // clangd doesn't support the static analyzer.
 #if CLANGD_TIDY_CHECKS
 #define CLANG_TIDY_DISABLE_STATIC_ANALYZER_CHECKS
ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs,
   //  - checks don't see all preprocessor events in the preamble
   //  - matchers run only over the main-file top-level decls (and can't see
   //    ancestors outside this scope).
   // In practice almost all checks work well without modifications.
   std::vector<std::unique_ptr<tidy::ClangTidyCheck>> CTChecks;
   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) {
     trace::Span Tracer("ClangTidyInit");
     tidy::ClangTidyCheckFactories CTFactories;
     for (const auto &E : tidy::ClangTidyModuleRegistry::entries())
       E.instantiate()->addCheckFactories(CTFactories);
     CTContext.emplace(std::make_unique<tidy::DefaultOptionsProvider>(
         tidy::ClangTidyGlobalOptions(), ClangTidyOpts));
     CTContext->setDiagnosticsEngine(&Clang->getDiagnostics());
     CTContext->setASTContext(&Clang->getASTContext());
     CTContext->setCurrentFile(Filename);
     CTChecks = CTFactories.createChecks(CTContext.getPointer());
     llvm::erase_if(CTChecks, [&](const auto &Check) {
       return !Check->isLanguageVersionSupported(CTContext->getLangOpts());
     });
     Preprocessor *PP = &Clang->getPreprocessor();
     for (const auto &Check : CTChecks) {
       Check->registerPPCallbacks(Clang->getSourceManager(), PP, PP);
       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();
         if (IsClangTidyDiag) {
           if (Cfg.Diagnostics.Suppress.contains(CheckName))
             return DiagnosticsEngine::Ignored;
           // Check for suppression comment. Skip the check for diagnostics not
           // in the main file, because we don't want that function to query the

clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp

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 <stdio.h>\n",
                       "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"]]
     void foo() {})cpp");
   Annotations Header("[[no_type_spec]]; // error-ok");
   TestTU TU = TestTU::withCode(Main.code());
   TU.AdditionalFiles = {{"a.h", std::string(Header.code())}};
   EXPECT_THAT(*TU.build().getDiagnostics(),

clang/lib/Sema/SemaDecl.cpp

if (II.getName().startswith("__builtin_"))
     diag_id = diag::warn_builtin_unknown;
   // OpenCL v2.0 s6.9.u - Implicit function declaration is not supported.
   else if (getLangOpts().OpenCL)
     diag_id = diag::err_opencl_implicit_function_decl;
   else if (getLangOpts().C99)
     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
   // more to do.
   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;
   DeclSpec DS(attrFactory);
   unsigned DiagID;
   bool Error = DS.SetTypeSpecType(DeclSpec::TST_int, Loc, Dummy, DiagID,
                                   Context.getPrintingPolicy());
   (void)Error; // Silence warning.