sammccall created this revision.
sammccall added a reviewer: kadircet.
Herald added subscribers: usaxena95, arphaman.
sammccall requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added projects: clang, clang-tools-extra.
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
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D115490
Files:
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
Index: clang/lib/Sema/SemaDecl.cpp
===================================================================
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -14985,7 +14985,21 @@
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.
+ 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
@@ -14993,18 +15007,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<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;
Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -1222,6 +1222,31 @@
"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"]]
Index: clang-tools-extra/clangd/ParsedAST.cpp
===================================================================
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ 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>
@@ -344,6 +337,7 @@
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) {
@@ -370,12 +364,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();
Index: clang-tools-extra/clangd/IncludeFixer.cpp
===================================================================
--- clang-tools-extra/clangd/IncludeFixer.cpp
+++ clang-tools-extra/clangd/IncludeFixer.cpp
@@ -106,6 +106,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.
@@ -118,8 +126,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;
@@ -393,6 +400,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();
Index: clang-tools-extra/clangd/Diagnostics.cpp
===================================================================
--- clang-tools-extra/clangd/Diagnostics.cpp
+++ clang-tools-extra/clangd/Diagnostics.cpp
@@ -419,6 +419,7 @@
OS << Sep << Fix;
Sep = ", ";
}
+ OS << "}";
}
return OS;
}
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
