From a1e541bf9fe0253541e6460ed8866f1a4ce0c9dd Mon Sep 17 00:00:00 2001 From: Jim Ingham Date: Fri, 25 Mar 2016 01:57:14 +0000 Subject: [PATCH] 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. llvm-svn: 264379 --- lldb/include/lldb/API/SBExpressionOptions.h | 6 + lldb/include/lldb/Expression/DiagnosticManager.h | 145 +++++++++++++++++++-- lldb/include/lldb/Expression/ExpressionParser.h | 19 ++- lldb/include/lldb/Target/Target.h | 17 +++ lldb/lldb.xcodeproj/project.pbxproj | 2 + .../test/expression_command/fixits/Makefile | 12 ++ .../test/expression_command/fixits/TestFixIts.py | 77 +++++++++++ .../test/expression_command/fixits/main.cpp | 25 ++++ lldb/scripts/interface/SBExpressionOptions.i | 9 ++ lldb/source/API/SBExpressionOptions.cpp | 12 ++ lldb/source/Commands/CommandObjectExpression.cpp | 21 +++ lldb/source/Commands/CommandObjectExpression.h | 3 +- lldb/source/Expression/DiagnosticManager.cpp | 6 +- lldb/source/Expression/UserExpression.cpp | 1 + .../ExpressionParser/Clang/ClangDiagnostic.h | 59 +++++++++ .../Clang/ClangExpressionParser.cpp | 120 ++++++++++++++++- .../ExpressionParser/Clang/ClangExpressionParser.h | 3 + .../ExpressionParser/Clang/ClangUserExpression.cpp | 45 ++++++- lldb/source/Target/Target.cpp | 9 ++ 19 files changed, 562 insertions(+), 29 deletions(-) create mode 100644 lldb/packages/Python/lldbsuite/test/expression_command/fixits/Makefile create mode 100644 lldb/packages/Python/lldbsuite/test/expression_command/fixits/TestFixIts.py create mode 100644 lldb/packages/Python/lldbsuite/test/expression_command/fixits/main.cpp create mode 100644 lldb/source/Plugins/ExpressionParser/Clang/ClangDiagnostic.h diff --git a/lldb/include/lldb/API/SBExpressionOptions.h b/lldb/include/lldb/API/SBExpressionOptions.h index ed2f918..dc6fe23 100644 --- a/lldb/include/lldb/API/SBExpressionOptions.h +++ b/lldb/include/lldb/API/SBExpressionOptions.h @@ -110,6 +110,12 @@ public: void SetPrefix (const char *prefix); + + void + SetAutoApplyFixIts(bool b = true); + + bool + GetAutoApplyFixIts(); protected: diff --git a/lldb/include/lldb/Expression/DiagnosticManager.h b/lldb/include/lldb/Expression/DiagnosticManager.h index b1e48dd..c4dd948 100644 --- a/lldb/include/lldb/Expression/DiagnosticManager.h +++ b/lldb/include/lldb/Expression/DiagnosticManager.h @@ -38,15 +38,82 @@ enum DiagnosticSeverity const uint32_t LLDB_INVALID_COMPILER_ID = UINT32_MAX; -struct Diagnostic +class Diagnostic { - std::string message; - uint32_t compiler_id; // Compiler-specific diagnostic ID - DiagnosticSeverity severity; - DiagnosticOrigin origin; +friend class DiagnosticManager; + +public: + DiagnosticOrigin getKind() const { return m_origin; } + + static bool classof(const Diagnostic *diag) + { + DiagnosticOrigin kind = diag->getKind(); + switch (kind) + { + case eDiagnosticOriginUnknown: + case eDiagnosticOriginLLDB: + case eDiagnosticOriginGo: + case eDiagnosticOriginLLVM: + return true; + case eDiagnosticOriginClang: + case eDiagnosticOriginSwift: + return false; + } + } + + Diagnostic(const char *message, DiagnosticSeverity severity, DiagnosticOrigin origin, uint32_t compiler_id) : + m_message(message), + m_severity(severity), + m_origin(origin), + m_compiler_id(compiler_id) + { + } + + Diagnostic(const Diagnostic &rhs) : + m_message(rhs.m_message), + m_severity(rhs.m_severity), + m_origin(rhs.m_origin), + m_compiler_id(rhs.m_compiler_id) + { + } + + virtual ~Diagnostic() = default; + + virtual bool HasFixIts () const { return false; } + + DiagnosticSeverity + GetSeverity() const + { + return m_severity; + } + + uint32_t + GetCompilerID() const + { + return m_compiler_id; + } + + const char * + GetMessage() const + { + return m_message.c_str(); + } + + void AppendMessage(const char *message, bool precede_with_newline = true) + { + if (precede_with_newline) + m_message.push_back('\n'); + m_message.append(message); + } + +protected: + std::string m_message; + DiagnosticSeverity m_severity; + DiagnosticOrigin m_origin; + uint32_t m_compiler_id; // Compiler-specific diagnostic ID }; -typedef std::vector DiagnosticList; +typedef std::vector DiagnosticList; class DiagnosticManager { @@ -55,19 +122,47 @@ public: Clear() { m_diagnostics.clear(); + m_auto_apply_fixits = true; + m_fixed_expression.clear(); } + // The diagnostic manager holds a list of diagnostics, which are owned by the manager. const DiagnosticList & Diagnostics() { return m_diagnostics; } - + + ~DiagnosticManager() + { + for (Diagnostic *diag : m_diagnostics) + { + delete diag; + } + } + + bool + HasFixIts() + { + for (Diagnostic *diag : m_diagnostics) + { + if (diag->HasFixIts()) + return true; + } + return false; + } + void AddDiagnostic(const char *message, DiagnosticSeverity severity, DiagnosticOrigin origin, uint32_t compiler_id = LLDB_INVALID_COMPILER_ID) { - m_diagnostics.push_back({std::string(message), compiler_id, severity, origin}); + m_diagnostics.push_back(new Diagnostic(message, severity, origin, compiler_id)); + } + + void + AddDiagnostic(Diagnostic *diagnostic) + { + m_diagnostics.push_back(diagnostic); } size_t @@ -80,11 +175,10 @@ public: { if (m_diagnostics.size()) { - m_diagnostics.back().message.push_back('\n'); - m_diagnostics.back().message.append(cstr); + m_diagnostics.back()->AppendMessage(cstr); } } - + // Returns a string containing errors in this format: // // "error: error text\n @@ -95,9 +189,36 @@ public: void Dump(Log *log); + + const std::string & + GetFixedExpression() + { + return m_fixed_expression; + } + + // Moves fixed_expression to the internal storage. + void + SetFixedExpression(std::string fixed_expression) + { + m_fixed_expression = std::move(fixed_expression); + fixed_expression.clear(); + } + + void + SetAutoApplyFixIts(bool auto_apply) + { + m_auto_apply_fixits = auto_apply; + } + + bool ShouldAutoApplyFixIts() + { + return m_auto_apply_fixits; + } -private: +protected: DiagnosticList m_diagnostics; + std::string m_fixed_expression; + bool m_auto_apply_fixits = true; }; } diff --git a/lldb/include/lldb/Expression/ExpressionParser.h b/lldb/include/lldb/Expression/ExpressionParser.h index d5dc03d..323e515 100644 --- a/lldb/include/lldb/Expression/ExpressionParser.h +++ b/lldb/include/lldb/Expression/ExpressionParser.h @@ -58,7 +58,7 @@ public: /// wrap the expression in anything at all. /// /// @param[in] diagnostic_manager - /// The stream to print errors to. + /// The diagnostic manager in which to store the errors and warnings. /// /// @return /// The number of errors encountered during parsing. 0 means @@ -66,6 +66,23 @@ public: //------------------------------------------------------------------ virtual unsigned Parse(DiagnosticManager &diagnostic_manager) = 0; + + //------------------------------------------------------------------ + /// Try to use the FixIts in the diagnostic_manager to rewrite the + /// expression. If successful, the rewritten expression is stored + /// in the diagnostic_manager, get it out with GetFixedExpression. + /// + /// @param[in] diagnostic_manager + /// The diagnostic manager containing fixit's to apply. + /// + /// @return + /// \b true if the rewrite was successful, \b false otherwise. + //------------------------------------------------------------------ + virtual bool + RewriteExpression(DiagnosticManager &diagnostic_manager) + { + return false; + } //------------------------------------------------------------------ /// Ready an already-parsed expression for execution, possibly diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h index 94829d6..c74e4fe 100644 --- a/lldb/include/lldb/Target/Target.h +++ b/lldb/include/lldb/Target/Target.h @@ -149,6 +149,9 @@ public: GetEnableAutoImportClangModules () const; bool + GetEnableAutoApplyFixIts () const; + + bool GetEnableSyntheticValue () const; uint32_t @@ -271,6 +274,7 @@ public: m_trap_exceptions (true), m_generate_debug_info (false), m_result_is_internal (false), + m_auto_apply_fixits (true), m_use_dynamic (lldb::eNoDynamicValues), m_timeout_usec (default_timeout), m_one_thread_timeout_usec (0), @@ -541,6 +545,18 @@ public: { return m_result_is_internal; } + + void + SetAutoApplyFixIts(bool b) + { + m_auto_apply_fixits = b; + } + + bool + GetAutoApplyFixIts() const + { + return m_auto_apply_fixits; + } private: ExecutionPolicy m_execution_policy; @@ -558,6 +574,7 @@ private: bool m_generate_debug_info; bool m_ansi_color_errors; bool m_result_is_internal; + bool m_auto_apply_fixits; lldb::DynamicValueType m_use_dynamic; uint32_t m_timeout_usec; uint32_t m_one_thread_timeout_usec; diff --git a/lldb/lldb.xcodeproj/project.pbxproj b/lldb/lldb.xcodeproj/project.pbxproj index 9073c93..e38cd5b 100644 --- a/lldb/lldb.xcodeproj/project.pbxproj +++ b/lldb/lldb.xcodeproj/project.pbxproj @@ -2347,6 +2347,7 @@ 4C2479BE1BA39843009C9A7B /* ExpressionParser.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = ExpressionParser.h; path = include/lldb/Expression/ExpressionParser.h; sourceTree = ""; }; 4C29E77D1BA2403F00DFF855 /* ExpressionTypeSystemHelper.h */ = {isa = PBXFileReference; explicitFileType = sourcecode.cpp.h; name = ExpressionTypeSystemHelper.h; path = include/lldb/Expression/ExpressionTypeSystemHelper.h; sourceTree = ""; }; 4C2FAE2E135E3A70001EDE44 /* SharedCluster.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = SharedCluster.h; path = include/lldb/Utility/SharedCluster.h; sourceTree = ""; }; + 4C3DA2301CA0BFB800CEB1D4 /* ClangDiagnostic.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = ClangDiagnostic.h; path = ExpressionParser/Clang/ClangDiagnostic.h; sourceTree = ""; }; 4C43DEF9110641F300E55CBF /* ThreadPlanShouldStopHere.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = ThreadPlanShouldStopHere.h; path = include/lldb/Target/ThreadPlanShouldStopHere.h; sourceTree = ""; }; 4C43DEFA110641F300E55CBF /* ThreadPlanShouldStopHere.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = ThreadPlanShouldStopHere.cpp; path = source/Target/ThreadPlanShouldStopHere.cpp; sourceTree = ""; }; 4C43DF8511069BFD00E55CBF /* ThreadPlanStepInRange.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = ThreadPlanStepInRange.h; path = include/lldb/Target/ThreadPlanStepInRange.h; sourceTree = ""; }; @@ -5288,6 +5289,7 @@ 4984BA0C1B97620B008658D4 /* Clang */ = { isa = PBXGroup; children = ( + 4C3DA2301CA0BFB800CEB1D4 /* ClangDiagnostic.h */, 4C98D3E0118FB98F00E575D0 /* ClangFunctionCaller.h */, 4C98D3DA118FB96F00E575D0 /* ClangFunctionCaller.cpp */, 26BC7DC010F1B79500F91463 /* ClangExpressionHelper.h */, diff --git a/lldb/packages/Python/lldbsuite/test/expression_command/fixits/Makefile b/lldb/packages/Python/lldbsuite/test/expression_command/fixits/Makefile new file mode 100644 index 0000000..7df664a --- /dev/null +++ b/lldb/packages/Python/lldbsuite/test/expression_command/fixits/Makefile @@ -0,0 +1,12 @@ +LEVEL = ../../make + +CXX_SOURCES := main.cpp + +# clang-3.5+ outputs FullDebugInfo by default for Darwin/FreeBSD +# targets. Other targets do not, which causes this test to fail. +# This flag enables FullDebugInfo for all targets. +ifneq (,$(findstring clang,$(CC))) + CFLAGS_EXTRAS += -fno-limit-debug-info +endif + +include $(LEVEL)/Makefile.rules diff --git a/lldb/packages/Python/lldbsuite/test/expression_command/fixits/TestFixIts.py b/lldb/packages/Python/lldbsuite/test/expression_command/fixits/TestFixIts.py new file mode 100644 index 0000000..7f442ab --- /dev/null +++ b/lldb/packages/Python/lldbsuite/test/expression_command/fixits/TestFixIts.py @@ -0,0 +1,77 @@ +""" +Test calling an expression with errors that a FixIt can fix. +""" + +from __future__ import print_function + + + +import lldb +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil + +class ExprCommandWithFixits(TestBase): + + mydir = TestBase.compute_mydir(__file__) + + def setUp(self): + # Call super's setUp(). + TestBase.setUp(self) + + self.main_source = "main.cpp" + self.main_source_spec = lldb.SBFileSpec (self.main_source) + + @skipUnlessDarwin + def test(self): + """Test calling a function that throws and ObjC exception.""" + self.build() + self.try_expressions() + + def try_expressions(self): + """Test calling expressions with errors that can be fixed by the FixIts.""" + exe_name = "a.out" + exe = os.path.join(os.getcwd(), exe_name) + + target = self.dbg.CreateTarget(exe) + self.assertTrue(target, VALID_TARGET) + + breakpoint = target.BreakpointCreateBySourceRegex('Stop here to evaluate expressions',self.main_source_spec) + self.assertTrue(breakpoint.GetNumLocations() > 0, VALID_BREAKPOINT) + + # Launch the process, and do not stop at the entry point. + process = target.LaunchSimple (None, None, self.get_process_working_directory()) + + self.assertTrue(process, PROCESS_IS_VALID) + + # Frame #0 should be at our breakpoint. + threads = lldbutil.get_threads_stopped_at_breakpoint (process, breakpoint) + + self.assertTrue(len(threads) == 1) + self.thread = threads[0] + + options = lldb.SBExpressionOptions() + options.SetAutoApplyFixIts(True) + + frame = self.thread.GetFrameAtIndex(0) + + # Try with one error: + value = frame.EvaluateExpression("my_pointer.first", options) + self.assertTrue(value.IsValid()) + self.assertTrue(value.GetError().Success()) + self.assertTrue(value.GetValueAsUnsigned() == 10) + + # Try with two errors: + two_error_expression = "my_pointer.second->a" + value = frame.EvaluateExpression(two_error_expression, options) + self.assertTrue(value.IsValid()) + self.assertTrue(value.GetError().Success()) + self.assertTrue(value.GetValueAsUnsigned() == 20) + + # Now turn off the fixits, and the expression should fail: + options.SetAutoApplyFixIts(False) + value = frame.EvaluateExpression("two_error_expression", options) + self.assertTrue(value.IsValid()) + self.assertTrue(value.GetError().Fail()) + + diff --git a/lldb/packages/Python/lldbsuite/test/expression_command/fixits/main.cpp b/lldb/packages/Python/lldbsuite/test/expression_command/fixits/main.cpp new file mode 100644 index 0000000..371d833 --- /dev/null +++ b/lldb/packages/Python/lldbsuite/test/expression_command/fixits/main.cpp @@ -0,0 +1,25 @@ +#include + +struct SubStruct +{ + int a; + int b; +}; + +struct MyStruct +{ + int first; + struct SubStruct second; +}; + +int +main() +{ + struct MyStruct my_struct = {10, {20, 30}}; + struct MyStruct *my_pointer = &my_struct; + printf ("Stop here to evaluate expressions: %d %d %p\n", my_pointer->first, my_pointer->second.a, my_pointer); + return 0; +} + + + diff --git a/lldb/scripts/interface/SBExpressionOptions.i b/lldb/scripts/interface/SBExpressionOptions.i index 1f423cf..1e822ce 100644 --- a/lldb/scripts/interface/SBExpressionOptions.i +++ b/lldb/scripts/interface/SBExpressionOptions.i @@ -118,6 +118,15 @@ public: %feature("docstring", "Sets the prefix to use for this expression. This prefix gets inserted after the 'target.expr-prefix' prefix contents, but before the wrapped expression function body.") SetPrefix; void SetPrefix (const char *prefix); + + %feature("docstring", "Sets whether to auto-apply FixIt hints to the expression being evaluated.") SetAutoApplyFixIts; + void + SetAutoApplyFixIts(bool b = true); + + %feature("docstring", "Gets whether to auto-apply FixIt hints to an expression.") GetAutoApplyFixIts; + bool + GetAutoApplyFixIts(); + protected: diff --git a/lldb/source/API/SBExpressionOptions.cpp b/lldb/source/API/SBExpressionOptions.cpp index 43b7d03..a7dd721 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 7bf94ed..fa2be2c 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 7103675..9bd0497 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 8b78868..e0a5c49 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 6f637b1..2d1336b 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 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 + +#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 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 @@ 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(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 @@ 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 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 @@ 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 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 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 a96e4c9..2360afd 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; -- 2.7.4