#include "llvm/ADT/DenseSet.h"
#include "llvm/ADT/Optional.h"
#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/ScopeExit.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/ADT/Twine.h"
return halfOpenToRange(M, R);
}
-// Returns whether the \p D is modified.
-bool adjustDiagFromHeader(Diag &D, const clang::Diagnostic &Info,
- const LangOptions &LangOpts) {
- // We only report diagnostics with at least error severity from headers.
- // Use default severity to avoid noise with -Werror.
- if (!Info.getDiags()->getDiagnosticIDs()->isDefaultMappingAsError(
- Info.getID()))
- return false;
-
- const SourceManager &SM = Info.getSourceManager();
- const SourceLocation &DiagLoc = SM.getExpansionLoc(Info.getLocation());
- SourceLocation IncludeInMainFile;
+// Try to find a location in the main-file to report the diagnostic D.
+// Returns a description like "in included file", or nullptr on failure.
+const char *getMainFileRange(const Diag &D, const SourceManager &SM,
+ SourceLocation DiagLoc, Range &R) {
+ // Look for a note in the main file indicating template instantiation.
+ for (const auto &N : D.Notes) {
+ if (N.InsideMainFile) {
+ switch (N.ID) {
+ case diag::note_template_class_instantiation_was_here:
+ case diag::note_template_class_explicit_specialization_was_here:
+ case diag::note_template_class_instantiation_here:
+ case diag::note_template_member_class_here:
+ case diag::note_template_member_function_here:
+ case diag::note_function_template_spec_here:
+ case diag::note_template_static_data_member_def_here:
+ case diag::note_template_variable_def_here:
+ case diag::note_template_enum_def_here:
+ case diag::note_template_nsdmi_here:
+ case diag::note_template_type_alias_instantiation_here:
+ case diag::note_template_exception_spec_instantiation_here:
+ case diag::note_template_requirement_instantiation_here:
+ case diag::note_evaluating_exception_spec_here:
+ case diag::note_default_arg_instantiation_here:
+ case diag::note_default_function_arg_instantiation_here:
+ case diag::note_explicit_template_arg_substitution_here:
+ case diag::note_function_template_deduction_instantiation_here:
+ case diag::note_deduced_template_arg_substitution_here:
+ case diag::note_prior_template_arg_substitution:
+ case diag::note_template_default_arg_checking:
+ case diag::note_concept_specialization_here:
+ case diag::note_nested_requirement_here:
+ case diag::note_checking_constraints_for_template_id_here:
+ case diag::note_checking_constraints_for_var_spec_id_here:
+ case diag::note_checking_constraints_for_class_spec_id_here:
+ case diag::note_checking_constraints_for_function_here:
+ case diag::note_constraint_substitution_here:
+ case diag::note_constraint_normalization_here:
+ case diag::note_parameter_mapping_substitution_here:
+ R = N.Range;
+ return "in template";
+ default:
+ break;
+ }
+ }
+ }
+ // Look for where the file with the error was #included.
auto GetIncludeLoc = [&SM](SourceLocation SLoc) {
return SM.getIncludeLoc(SM.getFileID(SLoc));
};
- for (auto IncludeLocation = GetIncludeLoc(DiagLoc); IncludeLocation.isValid();
+ for (auto IncludeLocation = GetIncludeLoc(SM.getExpansionLoc(DiagLoc));
+ IncludeLocation.isValid();
IncludeLocation = GetIncludeLoc(IncludeLocation)) {
if (clangd::isInsideMainFile(IncludeLocation, SM)) {
- IncludeInMainFile = IncludeLocation;
- break;
+ R.start = sourceLocToPosition(SM, IncludeLocation);
+ R.end = sourceLocToPosition(
+ SM,
+ Lexer::getLocForEndOfToken(IncludeLocation, 0, SM, LangOptions()));
+ return "in included file";
}
}
- if (IncludeInMainFile.isInvalid())
- return false;
+ return nullptr;
+}
- // Update diag to point at include inside main file.
- D.File = SM.getFileEntryForID(SM.getMainFileID())->getName().str();
- D.Range.start = sourceLocToPosition(SM, IncludeInMainFile);
- D.Range.end = sourceLocToPosition(
- SM, Lexer::getLocForEndOfToken(IncludeInMainFile, 0, SM, LangOpts));
- D.InsideMainFile = true;
+// Place the diagnostic the main file, rather than the header, if possible:
+// - for errors in included files, use the #include location
+// - for errors in template instantiation, use the instantation location
+// In both cases, add the original header location as a note.
+bool tryMoveToMainFile(Diag &D, FullSourceLoc DiagLoc) {
+ const SourceManager &SM = DiagLoc.getManager();
+ DiagLoc = DiagLoc.getExpansionLoc();
+ Range R;
+ const char *Prefix = getMainFileRange(D, SM, DiagLoc, R);
+ if (!Prefix)
+ return false;
// Add a note that will point to real diagnostic.
const auto *FE = SM.getFileEntryForID(SM.getFileID(DiagLoc));
- D.Notes.emplace_back();
- Note &N = D.Notes.back();
+ D.Notes.emplace(D.Notes.begin());
+ Note &N = D.Notes.front();
N.AbsFile = std::string(FE->tryGetRealPathName());
N.File = std::string(FE->getName());
N.Message = "error occurred here";
- N.Range = diagnosticRange(Info, LangOpts);
+ N.Range = D.Range;
+ // Update diag to point at include inside main file.
+ D.File = SM.getFileEntryForID(SM.getMainFileID())->getName().str();
+ D.Range = std::move(R);
+ D.InsideMainFile = true;
// Update message to mention original file.
- D.Message = llvm::Twine("in included file: ", D.Message).str();
+ D.Message = llvm::formatv("{0}: {1}", Prefix, D.Message);
return true;
}
}
void StoreDiags::EndSourceFile() {
+ flushLastDiag();
LangOpts = None;
}
void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
const clang::Diagnostic &Info) {
DiagnosticConsumer::HandleDiagnostic(DiagLevel, Info);
+ bool OriginallyError =
+ Info.getDiags()->getDiagnosticIDs()->isDefaultMappingAsError(
+ Info.getID());
if (Info.getLocation().isInvalid()) {
// Handle diagnostics coming from command-line arguments. The source manager
// is *not* available at this point, so we cannot use it.
- if (!Info.getDiags()->getDiagnosticIDs()->isDefaultMappingAsError(
- Info.getID())) {
+ if (!OriginallyError) {
IgnoreDiagnostics::log(DiagLevel, Info);
return; // non-errors add too much noise, do not show them.
}
flushLastDiag();
LastDiag = Diag();
+ LastDiagLoc.reset();
+ LastDiagOriginallyError = OriginallyError;
LastDiag->ID = Info.getID();
fillNonLocationData(DiagLevel, Info, *LastDiag);
LastDiag->InsideMainFile = true;
D.File = std::string(SM.getFilename(Info.getLocation()));
D.AbsFile = getCanonicalPath(
SM.getFileEntryForID(SM.getFileID(Info.getLocation())), SM);
+ D.ID = Info.getID();
return D;
};
LastPrimaryDiagnosticWasSuppressed = false;
LastDiag = Diag();
- LastDiag->ID = Info.getID();
FillDiagBase(*LastDiag);
- if (!InsideMainFile)
- LastDiagWasAdjusted = adjustDiagFromHeader(*LastDiag, Info, *LangOpts);
+ LastDiagLoc.emplace(Info.getLocation(), Info.getSourceManager());
+ LastDiagOriginallyError = OriginallyError;
if (!Info.getFixItHints().empty())
AddFix(true /* try to invent a message instead of repeating the diag */);
void StoreDiags::flushLastDiag() {
if (!LastDiag)
return;
- if (!isBlacklisted(*LastDiag) && mentionsMainFile(*LastDiag) &&
- (!LastDiagWasAdjusted ||
- // Only report the first diagnostic coming from each particular header.
- IncludeLinesWithErrors.insert(LastDiag->Range.start.line).second)) {
- Output.push_back(std::move(*LastDiag));
- } else {
- vlog("Dropped diagnostic: {0}: {1}", LastDiag->File, LastDiag->Message);
+ auto Finish = llvm::make_scope_exit([&, NDiags(Output.size())] {
+ if (Output.size() == NDiags) // No new diag emitted.
+ vlog("Dropped diagnostic: {0}: {1}", LastDiag->File, LastDiag->Message);
+ LastDiag.reset();
+ });
+
+ if (isBlacklisted(*LastDiag))
+ return;
+ // Move errors that occur from headers into main file.
+ if (!LastDiag->InsideMainFile && LastDiagLoc && LastDiagOriginallyError) {
+ if (tryMoveToMainFile(*LastDiag, *LastDiagLoc)) {
+ // Suppress multiple errors from the same inclusion.
+ if (!IncludedErrorLocations
+ .insert({LastDiag->Range.start.line,
+ LastDiag->Range.start.character})
+ .second)
+ return;
+ }
}
- LastDiag.reset();
- LastDiagWasAdjusted = false;
+ if (!mentionsMainFile(*LastDiag))
+ return;
+ Output.push_back(std::move(*LastDiag));
}
} // namespace clangd
return Field(&Diag::Fixes, UnorderedElementsAre(FixMatcher1, FixMatcher2));
}
-::testing::Matcher<const Diag &>
-WithNote(::testing::Matcher<Note> NoteMatcher) {
- return Field(&Diag::Notes, ElementsAre(NoteMatcher));
+template <typename... NoteMatcherTypes>
+::testing::Matcher<const Diag &> WithNote(NoteMatcherTypes... NoteMatcher) {
+ return Field(&Diag::Notes, ElementsAre(NoteMatcher...));
}
MATCHER_P2(Diag, Range, Message,
"use a trailing return type for this function")))));
}
+TEST(DiagnosticTest, TemplatesInHeaders) {
+ // Diagnostics from templates defined in headers are placed at the expansion.
+ Annotations Main(R"cpp(
+ Derived<int> [[y]]; // error-ok
+ )cpp");
+ Annotations Header(R"cpp(
+ template <typename T>
+ struct Derived : [[T]] {};
+ )cpp");
+ TestTU TU = TestTU::withCode(Main.code());
+ TU.HeaderCode = Header.code().str();
+ EXPECT_THAT(
+ TU.build().getDiagnostics(),
+ ElementsAre(AllOf(
+ Diag(Main.range(), "in template: base specifier must name a class"),
+ WithNote(Diag(Header.range(), "error occurred here"),
+ Diag(Main.range(), "in instantiation of template class "
+ "'Derived<int>' requested here")))));
+}
+
+TEST(DiagnosticTest, MakeUnique) {
+ // We usually miss diagnostics from header functions as we don't parse them.
+ // std::make_unique is an exception.
+ Annotations Main(R"cpp(
+ struct S { S(char*); };
+ auto x = std::[[make_unique]]<S>(42); // error-ok
+ )cpp");
+ TestTU TU = TestTU::withCode(Main.code());
+ TU.HeaderCode = R"cpp(
+ namespace std {
+ // These mocks aren't quite right - we omit unique_ptr for simplicity.
+ // forward is included to show its body is not needed to get the diagnostic.
+ template <typename T> T&& forward(T& t) { return static_cast<T&&>(t); }
+ template <typename T, typename... A> T* make_unique(A&&... args) {
+ return new T(std::forward<A>(args)...);
+ }
+ }
+ )cpp";
+ EXPECT_THAT(TU.build().getDiagnostics(),
+ UnorderedElementsAre(
+ Diag(Main.range(),
+ "in template: "
+ "no matching constructor for initialization of 'S'")));
+}
+
TEST(DiagnosticTest, NoMultipleDiagnosticInFlight) {
Annotations Main(R"cpp(
template <typename T> struct Foo {