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

10 files changed, 263 insertions, 16 deletions

diff --git a/lldb/source/API/SBExpressionOptions.cpp b/lldb/source/API/SBExpressionOptions.cpp
index 43b7d03064f..a7dd721b90a 100644
--- a/lldb/source/API/SBExpressionOptions.cpp
+++ b/lldb/source/API/SBExpressionOptions.cpp
@@ -197,6 +197,18 @@ SBExpressionOptions::SetPrefix (const char *prefix)
     return m_opaque_ap->SetPrefix(prefix);
 }
 
+bool
+SBExpressionOptions::GetAutoApplyFixIts ()
+{
+    return m_opaque_ap->GetAutoApplyFixIts ();
+}
+
+void
+SBExpressionOptions::SetAutoApplyFixIts (bool b)
+{
+    return m_opaque_ap->SetAutoApplyFixIts (b);
+}
+
 EvaluateExpressionOptions *
 SBExpressionOptions::get() const
 {
diff --git a/lldb/source/Commands/CommandObjectExpression.cpp b/lldb/source/Commands/CommandObjectExpression.cpp
index 7bf94ed1583..fa2be2c9efb 100644
--- a/lldb/source/Commands/CommandObjectExpression.cpp
+++ b/lldb/source/Commands/CommandObjectExpression.cpp
@@ -61,6 +61,7 @@ CommandObjectExpression::CommandOptions::g_option_table[] =
     { LLDB_OPT_SET_1 | LLDB_OPT_SET_2, false, "unwind-on-error",    'u', OptionParser::eRequiredArgument, nullptr, nullptr, 0, eArgTypeBoolean,    "Clean up program state if the expression causes a crash, or raises a signal.  Note, unlike gdb hitting a breakpoint is controlled by another option (-i)."},
     { LLDB_OPT_SET_1 | LLDB_OPT_SET_2, false, "debug",              'g', OptionParser::eNoArgument      , nullptr, nullptr, 0, eArgTypeNone,       "When specified, debug the JIT code by setting a breakpoint on the first instruction and forcing breakpoints to not be ignored (-i0) and no unwinding to happen on error (-u0)."},
     { LLDB_OPT_SET_1 | LLDB_OPT_SET_2, false, "language",           'l', OptionParser::eRequiredArgument, nullptr, nullptr, 0, eArgTypeLanguage,   "Specifies the Language to use when parsing the expression.  If not set the target.language setting is used." },
+    { LLDB_OPT_SET_1 | LLDB_OPT_SET_2, false, "apply-fixits",       'X', OptionParser::eRequiredArgument, nullptr, nullptr, 0, eArgTypeLanguage,   "If true, simple FixIt hints will be automatically applied to the expression." },
     { LLDB_OPT_SET_1, false, "description-verbosity", 'v', OptionParser::eOptionalArgument, nullptr, g_description_verbosity_type, 0, eArgTypeDescriptionVerbosity,        "How verbose should the output of this expression be, if the object description is asked for."},
 };
 
@@ -149,6 +150,17 @@ CommandObjectExpression::CommandOptions::SetOptionValue (CommandInterpreter &int
         ignore_breakpoints = false;
         break;
 
+    case 'X':
+        {
+            bool success;
+            bool tmp_value = Args::StringToBoolean(option_arg, true, &success);
+            if (success)
+                auto_apply_fixits = tmp_value ? eLazyBoolYes : eLazyBoolNo;
+            else
+                error.SetErrorStringWithFormat("could not convert \"%s\" to a boolean value.", option_arg);
+            break;
+        }
+            
     default:
         error.SetErrorStringWithFormat("invalid short option character '%c'", short_option);
         break;
@@ -178,6 +190,7 @@ CommandObjectExpression::CommandOptions::OptionParsingStarting (CommandInterpret
     debug = false;
     language = eLanguageTypeUnknown;
     m_verbosity = eLanguageRuntimeDescriptionDisplayVerbosityCompact;
+    auto_apply_fixits = eLazyBoolCalculate;
 }
 
 const OptionDefinition*
@@ -294,6 +307,14 @@ CommandObjectExpression::EvaluateExpression(const char *expr,
         options.SetTryAllThreads(m_command_options.try_all_threads);
         options.SetDebug(m_command_options.debug);
         options.SetLanguage(m_command_options.language);
+        
+        bool auto_apply_fixits;
+        if (m_command_options.auto_apply_fixits == eLazyBoolCalculate)
+            auto_apply_fixits = target->GetEnableAutoApplyFixIts();
+        else
+            auto_apply_fixits = m_command_options.auto_apply_fixits == eLazyBoolYes ? true : false;
+        
+        options.SetAutoApplyFixIts(auto_apply_fixits);
 
         // If there is any chance we are going to stop and want to see
         // what went wrong with our expression, we should generate debug info
diff --git a/lldb/source/Commands/CommandObjectExpression.h b/lldb/source/Commands/CommandObjectExpression.h
index 7103675f399..9bd0497d27e 100644
--- a/lldb/source/Commands/CommandObjectExpression.h
+++ b/lldb/source/Commands/CommandObjectExpression.h
@@ -14,13 +14,13 @@
 // C++ Includes
 // Other libraries and framework includes
 // Project includes
+#include "lldb/lldb-private-enumerations.h"
 #include "lldb/Core/IOHandler.h"
 #include "lldb/Interpreter/CommandObject.h"
 #include "lldb/Interpreter/OptionGroupBoolean.h"
 #include "lldb/Interpreter/OptionGroupFormat.h"
 #include "lldb/Interpreter/OptionGroupValueObjectDisplay.h"
 #include "lldb/Target/ExecutionContext.h"
-
 namespace lldb_private {
 
 class CommandObjectExpression :
@@ -63,6 +63,7 @@ public:
         bool        try_all_threads;
         lldb::LanguageType language;
         LanguageRuntimeDescriptionDisplayVerbosity m_verbosity;
+        LazyBool        auto_apply_fixits;
     };
 
     CommandObjectExpression (CommandInterpreter &interpreter);
diff --git a/lldb/source/Expression/DiagnosticManager.cpp b/lldb/source/Expression/DiagnosticManager.cpp
index 8b788680936..e0a5c492068 100644
--- a/lldb/source/Expression/DiagnosticManager.cpp
+++ b/lldb/source/Expression/DiagnosticManager.cpp
@@ -52,10 +52,10 @@ DiagnosticManager::GetString(char separator)
 {
     std::string ret;
 
-    for (const Diagnostic &diagnostic : Diagnostics())
+    for (const Diagnostic *diagnostic : Diagnostics())
     {
-        ret.append(StringForSeverity(diagnostic.severity));
-        ret.append(diagnostic.message);
+        ret.append(StringForSeverity(diagnostic->GetSeverity()));
+        ret.append(diagnostic->GetMessage());
         ret.push_back(separator);
     }
 
diff --git a/lldb/source/Expression/UserExpression.cpp b/lldb/source/Expression/UserExpression.cpp
index 6f637b1db92..2d1336b3ef5 100644
--- a/lldb/source/Expression/UserExpression.cpp
+++ b/lldb/source/Expression/UserExpression.cpp
@@ -253,6 +253,7 @@ UserExpression::Evaluate (ExecutionContext &exe_ctx,
     }
 
     DiagnosticManager diagnostic_manager;
+    diagnostic_manager.SetAutoApplyFixIts(options.GetAutoApplyFixIts());
 
     if (!user_expression_sp->Parse(diagnostic_manager, exe_ctx, execution_policy, keep_expression_in_memory,
                                    generate_debug_info))
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangDiagnostic.h b/lldb/source/Plugins/ExpressionParser/Clang/ClangDiagnostic.h
new file mode 100644
index 00000000000..7d4c74cc82e
--- /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 f372c23d8c5..763fc738598 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 @@ public:
             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 @@ ClangExpressionParser::Parse(DiagnosticManager &diagnostic_manager)
     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 cb72daaaa88..115067b4307 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.h
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.h
@@ -73,6 +73,9 @@ public:
     //------------------------------------------------------------------
     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 3066384a1a5..9bf8cd5f295 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 @@ ClangUserExpression::Parse(DiagnosticManager &diagnostic_manager, ExecutionConte
         diagnostic_manager.PutCString(eDiagnosticSeverityWarning, err.AsCString());
     }
 
-    StreamString m_transformed_stream;
-
     ////////////////////////////////////
     // Generate the expression
     //
@@ -489,10 +488,38 @@ ClangUserExpression::Parse(DiagnosticManager &diagnostic_manager, ExecutionConte
     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 @@ ClangUserExpression::Parse(DiagnosticManager &diagnostic_manager, ExecutionConte
     //
 
     {
-        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 @@ ClangUserExpression::Parse(DiagnosticManager &diagnostic_manager, ExecutionConte
 
     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())
         {
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index a96e4c98361..2360afd23c2 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -3425,6 +3425,7 @@ g_properties[] =
     { "debug-file-search-paths"            , OptionValue::eTypeFileSpecList, false, 0                       , nullptr, nullptr, "List of directories to be searched when locating debug symbol files." },
     { "clang-module-search-paths"          , OptionValue::eTypeFileSpecList, false, 0                       , nullptr, nullptr, "List of directories to be searched when locating modules for Clang." },
     { "auto-import-clang-modules"          , OptionValue::eTypeBoolean   , false, true                      , nullptr, nullptr, "Automatically load Clang modules referred to by the program." },
+    { "auto-apply-fixits"                  , OptionValue::eTypeBoolean   , false, true                      , nullptr, nullptr, "Automatically apply fixit hints to expressions." },
     { "max-children-count"                 , OptionValue::eTypeSInt64    , false, 256                       , nullptr, nullptr, "Maximum number of children to expand in any level of depth." },
     { "max-string-summary-length"          , OptionValue::eTypeSInt64    , false, 1024                      , nullptr, nullptr, "Maximum number of characters to show when using %s in summary strings." },
     { "max-memory-read-size"               , OptionValue::eTypeSInt64    , false, 1024                      , nullptr, nullptr, "Maximum number of bytes that 'memory read' will fetch before --force must be specified." },
@@ -3481,6 +3482,7 @@ enum
     ePropertyDebugFileSearchPaths,
     ePropertyClangModuleSearchPaths,
     ePropertyAutoImportClangModules,
+    ePropertyAutoApplyFixIts,
     ePropertyMaxChildrenCount,
     ePropertyMaxSummaryLength,
     ePropertyMaxMemReadSize,
@@ -3854,6 +3856,13 @@ TargetProperties::GetEnableAutoImportClangModules() const
 }
 
 bool
+TargetProperties::GetEnableAutoApplyFixIts() const
+{
+    const uint32_t idx = ePropertyAutoApplyFixIts;
+    return m_collection_sp->GetPropertyAtIndexAsBoolean(nullptr, idx, g_properties[idx].default_uint_value != 0);
+}
+
+bool
 TargetProperties::GetEnableSyntheticValue () const
 {
     const uint32_t idx = ePropertyEnableSynthetic;