Use Clang's FixItHints to correct expressions with "trivial" mistakes (e.g. "." for "->".)
This feature is controlled by an expression command option, a target property and the
SBExpressionOptions setting. FixIt's are only applied to UserExpressions, not UtilityFunctions,
those you have to get right when you make them.
This is just a first stage. At present the fixits are applied silently. The next step
is to tell the user about the applied fixit.
<rdar://problem/25351938>
llvm-svn: 264379
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangDiagnostic.h b/lldb/source/Plugins/ExpressionParser/Clang/ClangDiagnostic.h
new file mode 100644
index 0000000..7d4c74c
--- /dev/null
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangDiagnostic.h
@@ -0,0 +1,59 @@
+//===-- DiagnosticManager.h -------------------------------------*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef lldb_ClangDiagnostic_h
+#define lldb_ClangDiagnostic_h
+
+#include <vector>
+
+#include "clang/Basic/Diagnostic.h"
+
+#include "lldb/lldb-defines.h"
+#include "lldb/lldb-types.h"
+
+#include "lldb/Expression/DiagnosticManager.h"
+
+namespace lldb_private
+{
+
+typedef std::vector<clang::FixItHint> FixItList;
+
+class ClangDiagnostic : public Diagnostic
+{
+public:
+ static inline bool classof(const ClangDiagnostic *) { return true; }
+ static inline bool classof(const Diagnostic *diag) {
+ return diag->getKind() == eDiagnosticOriginClang;
+ }
+
+ ClangDiagnostic(const char *message, DiagnosticSeverity severity, uint32_t compiler_id) :
+ Diagnostic(message, severity, eDiagnosticOriginClang, compiler_id)
+ {
+ }
+
+ virtual ~ClangDiagnostic() = default;
+
+ bool HasFixIts () const override { return !m_fixit_vec.empty(); }
+
+ void
+ AddFixitHint (const clang::FixItHint &fixit)
+ {
+ m_fixit_vec.push_back(fixit);
+ }
+
+ const FixItList &
+ FixIts() const
+ {
+ return m_fixit_vec;
+ }
+ FixItList m_fixit_vec;
+};
+
+} // namespace lldb_private
+#endif /* lldb_ClangDiagnostic_h */
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
index f372c23..763fc73 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -17,9 +17,12 @@
#include "clang/Basic/FileManager.h"
#include "clang/Basic/SourceLocation.h"
#include "clang/Basic/TargetInfo.h"
-#include "clang/Basic/Version.h"
+#include "clang/Basic/Version.h"
#include "clang/CodeGen/CodeGenAction.h"
#include "clang/CodeGen/ModuleBuilder.h"
+#include "clang/Edit/Commit.h"
+#include "clang/Edit/EditsReceiver.h"
+#include "clang/Edit/EditedSource.h"
#include "clang/Frontend/CompilerInstance.h"
#include "clang/Frontend/CompilerInvocation.h"
#include "clang/Frontend/FrontendActions.h"
@@ -30,6 +33,7 @@
#include "clang/Lex/Preprocessor.h"
#include "clang/Parse/ParseAST.h"
#include "clang/Rewrite/Frontend/FrontendActions.h"
+#include "clang/Rewrite/Core/Rewriter.h"
#include "clang/Sema/SemaConsumer.h"
#include "clang/StaticAnalyzer/Frontend/FrontendActions.h"
@@ -54,6 +58,7 @@
// Project includes
#include "ClangExpressionParser.h"
+#include "ClangDiagnostic.h"
#include "ClangASTSource.h"
#include "ClangExpressionHelper.h"
@@ -175,24 +180,48 @@
diag_str.push_back('\0');
const char *data = diag_str.data();
+ DiagnosticSeverity severity;
+ bool make_new_diagnostic = true;
+
switch (DiagLevel)
{
case DiagnosticsEngine::Level::Fatal:
case DiagnosticsEngine::Level::Error:
- m_manager->AddDiagnostic(data, eDiagnosticSeverityError, eDiagnosticOriginClang, Info.getID());
+ severity = eDiagnosticSeverityError;
break;
case DiagnosticsEngine::Level::Warning:
- m_manager->AddDiagnostic(data, eDiagnosticSeverityWarning, eDiagnosticOriginClang, Info.getID());
+ severity = eDiagnosticSeverityWarning;
break;
case DiagnosticsEngine::Level::Remark:
case DiagnosticsEngine::Level::Ignored:
- m_manager->AddDiagnostic(data, eDiagnosticSeverityRemark, eDiagnosticOriginClang, Info.getID());
+ severity = eDiagnosticSeverityRemark;
break;
case DiagnosticsEngine::Level::Note:
m_manager->AppendMessageToDiagnostic(data);
+ make_new_diagnostic = false;
+ }
+ if (make_new_diagnostic)
+ {
+ ClangDiagnostic *new_diagnostic = new ClangDiagnostic(data, severity, Info.getID());
+ m_manager->AddDiagnostic(new_diagnostic);
+
+ // Don't store away warning fixits, since the compiler doesn't have 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)
+ {
+ size_t num_fixit_hints = Info.getNumFixItHints();
+ for (int i = 0; i < num_fixit_hints; i++)
+ {
+ const clang::FixItHint &fixit = Info.getFixItHint(i);
+ if (!fixit.isNull())
+ new_diagnostic->AddFixitHint(fixit);
+ }
+ }
}
}
-
+
m_passthrough->HandleDiagnostic(DiagLevel, Info);
}
@@ -666,6 +695,87 @@
return num_errors;
}
+bool
+ClangExpressionParser::RewriteExpression(DiagnosticManager &diagnostic_manager)
+{
+ clang::SourceManager &source_manager = m_compiler->getSourceManager();
+ clang::edit::EditedSource editor(source_manager, m_compiler->getLangOpts(), nullptr);
+ clang::edit::Commit commit(editor);
+ clang::Rewriter rewriter(source_manager, m_compiler->getLangOpts());
+
+ class RewritesReceiver : public edit::EditsReceiver {
+ Rewriter &rewrite;
+
+ public:
+ RewritesReceiver(Rewriter &in_rewrite) : rewrite(in_rewrite) { }
+
+ void insert(SourceLocation loc, StringRef text) override {
+ rewrite.InsertText(loc, text);
+ }
+ void replace(CharSourceRange range, StringRef text) override {
+ rewrite.ReplaceText(range.getBegin(), rewrite.getRangeSize(range), text);
+ }
+ };
+
+ RewritesReceiver rewrites_receiver(rewriter);
+
+ const DiagnosticList &diagnostics = diagnostic_manager.Diagnostics();
+ size_t num_diags = diagnostics.size();
+ if (num_diags == 0)
+ return false;
+
+ for (const Diagnostic *diag : diagnostic_manager.Diagnostics())
+ {
+ const ClangDiagnostic *diagnostic = llvm::dyn_cast<ClangDiagnostic>(diag);
+ if (diagnostic && diagnostic->HasFixIts())
+ {
+ for (const FixItHint &fixit : diagnostic->FixIts())
+ {
+ // This is cobbed from clang::Rewrite::FixItRewriter.
+ if (fixit.CodeToInsert.empty())
+ {
+ if (fixit.InsertFromRange.isValid())
+ {
+ commit.insertFromRange(fixit.RemoveRange.getBegin(),
+ fixit.InsertFromRange, /*afterToken=*/false,
+ fixit.BeforePreviousInsertions);
+ }
+ else
+ commit.remove(fixit.RemoveRange);
+ }
+ else
+ {
+ if (fixit.RemoveRange.isTokenRange() ||
+ fixit.RemoveRange.getBegin() != fixit.RemoveRange.getEnd())
+ commit.replace(fixit.RemoveRange, fixit.CodeToInsert);
+ else
+ commit.insert(fixit.RemoveRange.getBegin(), fixit.CodeToInsert,
+ /*afterToken=*/false, fixit.BeforePreviousInsertions);
+ }
+ }
+ }
+ }
+
+ // FIXME - do we want to try to propagate specific errors here?
+ if (!commit.isCommitable())
+ return false;
+ else if (!editor.commit(commit))
+ return false;
+
+ // Now play all the edits, and stash the result in the diagnostic manager.
+ editor.applyRewrites(rewrites_receiver);
+ RewriteBuffer &main_file_buffer = rewriter.getEditBuffer(source_manager.getMainFileID());
+
+ std::string fixed_expression;
+ llvm::raw_string_ostream out_stream(fixed_expression);
+
+ main_file_buffer.write(out_stream);
+ out_stream.flush();
+ diagnostic_manager.SetFixedExpression(fixed_expression);
+
+ return true;
+}
+
static bool FindFunctionInModule (ConstString &mangled_name,
llvm::Module *module,
const char *orig_name)
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.h b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.h
index cb72daa..115067b 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.h
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.h
@@ -73,6 +73,9 @@
//------------------------------------------------------------------
unsigned
Parse(DiagnosticManager &diagnostic_manager) override;
+
+ bool
+ RewriteExpression(DiagnosticManager &diagnostic_manager) override;
//------------------------------------------------------------------
/// Ready an already-parsed expression for execution, possibly
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
index 3066384..9bf8cd5 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
@@ -23,6 +23,7 @@
#include "ClangExpressionParser.h"
#include "ClangModulesDeclVendor.h"
#include "ClangPersistentVariables.h"
+#include "ClangDiagnostic.h"
#include "lldb/Core/ConstString.h"
#include "lldb/Core/Log.h"
@@ -358,8 +359,6 @@
diagnostic_manager.PutCString(eDiagnosticSeverityWarning, err.AsCString());
}
- StreamString m_transformed_stream;
-
////////////////////////////////////
// Generate the expression
//
@@ -489,10 +488,38 @@
if (!exe_scope)
exe_scope = exe_ctx.GetTargetPtr();
- ClangExpressionParser parser(exe_scope, *this, generate_debug_info);
+ // We use a shared pointer here so we can use the original parser - if it succeeds
+ // or the rewrite parser we might make if it fails. But the parser_sp will never be empty.
+
+ std::shared_ptr<ClangExpressionParser> parser_sp(new ClangExpressionParser(exe_scope, *this, generate_debug_info));
- unsigned num_errors = parser.Parse(diagnostic_manager);
+ unsigned num_errors = parser_sp->Parse(diagnostic_manager);
+ // Check here for FixItHints. If there are any try fixing the source and re-parsing...
+ if (num_errors && diagnostic_manager.HasFixIts() && diagnostic_manager.ShouldAutoApplyFixIts())
+ {
+ if (parser_sp->RewriteExpression(diagnostic_manager))
+ {
+ std::string backup_source = std::move(m_transformed_text);
+ m_transformed_text = diagnostic_manager.GetFixedExpression();
+ // Make a new diagnostic manager and parser, and try again with the rewritten expression:
+ // FIXME: It would be nice to reuse the parser we have but that doesn't seem to be possible.
+ DiagnosticManager rewrite_manager;
+ std::shared_ptr<ClangExpressionParser> rewrite_parser_sp(new ClangExpressionParser(exe_scope, *this, generate_debug_info));
+ unsigned rewrite_errors = rewrite_parser_sp->Parse(rewrite_manager);
+ if (rewrite_errors == 0)
+ {
+ diagnostic_manager.Clear();
+ parser_sp = rewrite_parser_sp;
+ num_errors = 0;
+ }
+ else
+ {
+ m_transformed_text = std::move(backup_source);
+ }
+ }
+ }
+
if (num_errors)
{
diagnostic_manager.Printf(eDiagnosticSeverityError, "%u error%s parsing expression", num_errors,
@@ -508,8 +535,12 @@
//
{
- Error jit_error = parser.PrepareForExecution(m_jit_start_addr, m_jit_end_addr, m_execution_unit_sp, exe_ctx,
- m_can_interpret, execution_policy);
+ Error jit_error = parser_sp->PrepareForExecution(m_jit_start_addr,
+ m_jit_end_addr,
+ m_execution_unit_sp,
+ exe_ctx,
+ m_can_interpret,
+ execution_policy);
if (!jit_error.Success())
{
@@ -524,7 +555,7 @@
if (exe_ctx.GetProcessPtr() && execution_policy == eExecutionPolicyTopLevel)
{
- Error static_init_error = parser.RunStaticInitializers(m_execution_unit_sp, exe_ctx);
+ Error static_init_error = parser_sp->RunStaticInitializers(m_execution_unit_sp, exe_ctx);
if (!static_init_error.Success())
{