[Lldb-commits] [PATCH] D77055: [lldb] Also apply Fix-Its in "note:" diagnostics that belong to an error diagnostic
Raphael Isemann via Phabricator via lldb-commits
lldb-commits at lists.llvm.org
Mon Apr 6 02:08:43 PDT 2020
This revision was automatically updated to reflect the committed changes.
Closed by commit rG3c2dc28d812c: [lldb] Also apply Fix-Its in "note:" diagnostics that belong to an error… (authored by teemperor).
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77055/new/
https://reviews.llvm.org/D77055
Files:
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
lldb/test/API/commands/expression/fixits/TestFixIts.py
Index: lldb/test/API/commands/expression/fixits/TestFixIts.py
===================================================================
--- lldb/test/API/commands/expression/fixits/TestFixIts.py
+++ lldb/test/API/commands/expression/fixits/TestFixIts.py
@@ -61,6 +61,14 @@
self.assertTrue(value.GetError().Success())
self.assertEquals(value.GetValueAsUnsigned(), 20)
+ # Try a Fix-It that is stored in the 'note:' diagnostic of an error.
+ # The Fix-It here is adding parantheses around the ToStr parameters.
+ fixit_in_note_expr ="#define ToStr(x) #x\nToStr(0 {, })"
+ value = frame.EvaluateExpression(fixit_in_note_expr, options)
+ self.assertTrue(value.IsValid())
+ self.assertTrue(value.GetError().Success(), value.GetError())
+ self.assertEquals(value.GetSummary(), '"(0 {, })"')
+
# Now turn off the fixits, and the expression should fail:
options.SetAutoApplyFixIts(False)
value = frame.EvaluateExpression(two_error_expression, options)
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
===================================================================
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -147,6 +147,14 @@
llvm::StringRef getErrorString() { return m_error_stream.GetString(); }
};
+static void AddAllFixIts(ClangDiagnostic *diag, const clang::Diagnostic &Info) {
+ for (auto &fix_it : Info.getFixItHints()) {
+ if (fix_it.isNull())
+ continue;
+ diag->AddFixitHint(fix_it);
+ }
+}
+
class ClangDiagnosticManagerAdapter : public clang::DiagnosticConsumer {
public:
ClangDiagnosticManagerAdapter(DiagnosticOptions &opts) {
@@ -162,6 +170,17 @@
m_manager = manager;
}
+ /// Returns the last ClangDiagnostic message that the DiagnosticManager
+ /// received or a nullptr if the DiagnosticMangager hasn't seen any
+ /// Clang diagnostics yet.
+ ClangDiagnostic *MaybeGetLastClangDiag() const {
+ if (m_manager->Diagnostics().empty())
+ return nullptr;
+ lldb_private::Diagnostic *diag = m_manager->Diagnostics().back().get();
+ ClangDiagnostic *clang_diag = dyn_cast<ClangDiagnostic>(diag);
+ return clang_diag;
+ }
+
void HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
const clang::Diagnostic &Info) override {
if (!m_manager) {
@@ -204,6 +223,23 @@
case DiagnosticsEngine::Level::Note:
m_manager->AppendMessageToDiagnostic(m_output);
make_new_diagnostic = false;
+
+ // 'note:' diagnostics for errors and warnings can also contain Fix-Its.
+ // We add these Fix-Its to the last error diagnostic to make sure
+ // that we later have all Fix-Its related to an 'error' diagnostic when
+ // we apply them to the user expression.
+ auto *clang_diag = MaybeGetLastClangDiag();
+ // If we don't have a previous diagnostic there is nothing to do.
+ // If the previous diagnostic already has its own Fix-Its, assume that
+ // the 'note:' Fix-It is just an alternative way to solve the issue and
+ // ignore these Fix-Its.
+ if (!clang_diag || clang_diag->HasFixIts())
+ break;
+ // Ignore all Fix-Its that are not associated with an error.
+ if (clang_diag->GetSeverity() != eDiagnosticSeverityError)
+ break;
+ AddAllFixIts(clang_diag, Info);
+ break;
}
if (make_new_diagnostic) {
// ClangDiagnostic messages are expected to have no whitespace/newlines
@@ -218,13 +254,8 @@
// enough context in an expression for the warning to be useful.
// FIXME: Should we try to filter out FixIts that apply to our generated
// code, and not the user's expression?
- if (severity == eDiagnosticSeverityError) {
- for (const clang::FixItHint &fixit : Info.getFixItHints()) {
- if (fixit.isNull())
- continue;
- new_diagnostic->AddFixitHint(fixit);
- }
- }
+ if (severity == eDiagnosticSeverityError)
+ AddAllFixIts(new_diagnostic.get(), Info);
m_manager->AddDiagnostic(std::move(new_diagnostic));
}
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D77055.255255.patch
Type: text/x-patch
Size: 4228 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20200406/aa57e3cb/attachment.bin>
More information about the lldb-commits
mailing list