Figure out what the fixed expression is, and print it. Added another target setting to

quietly apply fixits for those who really trust clang's fixits.

Also, moved the retry into ClangUserExpression::Evaluate, where I can make a whole new ClangUserExpression 
to do the work.  Reusing any of the parts of a UserExpression in situ isn't supported at present.

<rdar://problem/25351938>

llvm-svn: 264793
This commit is contained in:
Jim Ingham 2016-03-29 22:00:08 +00:00
parent bd54f5bd25
commit e5ee6f04ab
13 changed files with 220 additions and 71 deletions

View File

@ -122,7 +122,6 @@ public:
Clear()
{
m_diagnostics.clear();
m_auto_apply_fixits = true;
m_fixed_expression.clear();
}
@ -204,21 +203,9 @@ public:
fixed_expression.clear();
}
void
SetAutoApplyFixIts(bool auto_apply)
{
m_auto_apply_fixits = auto_apply;
}
bool ShouldAutoApplyFixIts()
{
return m_auto_apply_fixits;
}
protected:
DiagnosticList m_diagnostics;
std::string m_fixed_expression;
bool m_auto_apply_fixits = true;
};
}

View File

@ -58,6 +58,14 @@ public:
bool static_method,
ExecutionContext &exe_ctx) const;
// Given a string returned by GetText, find the beginning and end of the body passed to CreateWrapped.
// Return true if the bounds could be found. This will also work on text with FixItHints applied.
static bool
GetOriginalBodyBounds(std::string transformed_text,
lldb::LanguageType wrapping_language,
size_t &start_loc,
size_t &end_loc);
private:
ExpressionSourceCode (const char *name,
const char *prefix,

View File

@ -281,6 +281,9 @@ public:
/// @param[in] line_offset
/// The offset of the first line of the expression from the "beginning" of a virtual source file used for error reporting and debug info.
///
/// @param[out] fixed_expression
/// If non-nullptr, the fixed expression is copied into the provided string.
///
/// @param[out] jit_module_sp_ptr
/// If non-nullptr, used to persist the generated IR module.
///
@ -295,9 +298,18 @@ public:
lldb::ValueObjectSP &result_valobj_sp,
Error &error,
uint32_t line_offset = 0,
std::string *fixed_expression = nullptr,
lldb::ModuleSP *jit_module_sp_ptr = nullptr);
static const Error::ValueType kNoResult = 0x1001; ///< ValueObject::GetError() returns this if there is no result from the expression.
const char *
GetFixedText()
{
if (m_fixed_text.empty())
return nullptr;
return m_fixed_text.c_str();
}
protected:
static lldb::addr_t
@ -321,6 +333,7 @@ protected:
Address m_address; ///< The address the process is stopped in.
std::string m_expr_text; ///< The text of the expression, as typed by the user
std::string m_expr_prefix; ///< The text of the translation-level definitions, as provided by the user
std::string m_fixed_text; ///< The text of the expression with fixits applied - this won't be set if the fixed text doesn't parse.
lldb::LanguageType m_language; ///< The language to use when parsing (eLanguageTypeUnknown means use defaults)
ResultType m_desired_type; ///< The type to coerce the expression's result to. If eResultTypeAny, inferred from the expression.
EvaluateExpressionOptions m_options; ///< Additional options provided by the user.

View File

@ -151,6 +151,9 @@ public:
bool
GetEnableAutoApplyFixIts () const;
bool
GetEnableNotifyAboutFixIts () const;
bool
GetEnableSyntheticValue () const;
@ -1375,7 +1378,8 @@ public:
EvaluateExpression (const char *expression,
ExecutionContextScope *exe_scope,
lldb::ValueObjectSP &result_valobj_sp,
const EvaluateExpressionOptions& options = EvaluateExpressionOptions());
const EvaluateExpressionOptions& options = EvaluateExpressionOptions(),
std::string *fixed_expression = nullptr);
lldb::ExpressionVariableSP
GetPersistentVariable(const ConstString &name);

View File

@ -70,8 +70,12 @@ class ExprCommandWithFixits(TestBase):
# Now turn off the fixits, and the expression should fail:
options.SetAutoApplyFixIts(False)
value = frame.EvaluateExpression("two_error_expression", options)
value = frame.EvaluateExpression(two_error_expression, options)
self.assertTrue(value.IsValid())
self.assertTrue(value.GetError().Fail())
error_string = value.GetError().GetCString()
self.assertTrue(error_string.find("fixed expression suggested:") != -1, "Fix was suggested")
self.assertTrue(error_string.find("my_pointer->second.a") != -1, "Fix was right")

View File

@ -336,7 +336,14 @@ CommandObjectExpression::EvaluateExpression(const char *expr,
else
options.SetTimeoutUsec(0);
target->EvaluateExpression(expr, frame, result_valobj_sp, options);
ExpressionResults success = target->EvaluateExpression(expr, frame, result_valobj_sp, options, &m_fixed_expression);
// We only tell you about the FixIt if we applied it. The compiler errors will suggest the FixIt if it parsed.
if (error_stream && !m_fixed_expression.empty() && target->GetEnableNotifyAboutFixIts())
{
if (success == eExpressionCompleted)
error_stream->Printf (" Fixit applied, fixed expression was: \n %s\n", m_fixed_expression.c_str());
}
if (result_valobj_sp)
{
@ -477,6 +484,7 @@ bool
CommandObjectExpression::DoExecute(const char *command,
CommandReturnObject &result)
{
m_fixed_expression.clear();
m_option_group.NotifyOptionParsingStarting();
const char * expr = nullptr;
@ -598,7 +606,26 @@ CommandObjectExpression::DoExecute(const char *command,
expr = command;
if (EvaluateExpression (expr, &(result.GetOutputStream()), &(result.GetErrorStream()), &result))
{
Target *target = m_interpreter.GetExecutionContext().GetTargetPtr();
if (!m_fixed_expression.empty() && target->GetEnableNotifyAboutFixIts())
{
CommandHistory &history = m_interpreter.GetCommandHistory();
// FIXME: Can we figure out what the user actually typed (e.g. some alias for expr???)
// If we can it would be nice to show that.
std::string fixed_command("expression ");
if (expr == command)
fixed_command.append(m_fixed_expression);
else
{
// Add in any options that might have been in the original command:
fixed_command.append(command, expr - command);
fixed_command.append(m_fixed_expression);
}
history.AppendString(fixed_command);
}
return true;
}
result.SetStatus (eReturnStatusFailed);
return false;

View File

@ -108,6 +108,7 @@ protected:
CommandOptions m_command_options;
uint32_t m_expr_line_count;
std::string m_expr_lines; // Multi-line expression support
std::string m_fixed_expression; // Holds the current expression's fixed text.
};
} // namespace lldb_private

View File

@ -61,6 +61,9 @@ extern "C"
}
)";
static const char *c_start_marker = " /*LLDB_BODY_START*/\n ";
static const char *c_end_marker = ";\n /*LLDB_BODY_END*/\n";
namespace {
class AddMacroState
@ -301,6 +304,22 @@ bool ExpressionSourceCode::GetText (std::string &text, lldb::LanguageType wrappi
target_specific_defines,
m_prefix.c_str());
// First construct a tagged form of the user expression so we can find it later:
std::string tagged_body;
switch (wrapping_language)
{
default:
tagged_body = m_body;
break;
case lldb::eLanguageTypeC:
case lldb::eLanguageTypeC_plus_plus:
case lldb::eLanguageTypeObjC:
tagged_body.append(c_start_marker);
tagged_body.append(m_body);
tagged_body.append(c_end_marker);
break;
}
switch (wrapping_language)
{
default:
@ -310,23 +329,23 @@ bool ExpressionSourceCode::GetText (std::string &text, lldb::LanguageType wrappi
"%s(void *$__lldb_arg) \n"
"{ \n"
" %s; \n"
" %s; \n"
"%s"
"} \n",
m_name.c_str(),
lldb_local_var_decls.GetData(),
m_body.c_str());
tagged_body.c_str());
break;
case lldb::eLanguageTypeC_plus_plus:
wrap_stream.Printf("void \n"
"$__lldb_class::%s(void *$__lldb_arg) %s\n"
"{ \n"
" %s; \n"
" %s; \n"
"%s"
"} \n",
m_name.c_str(),
(const_object ? "const" : ""),
lldb_local_var_decls.GetData(),
m_body.c_str());
tagged_body.c_str());
break;
case lldb::eLanguageTypeObjC:
if (static_method)
@ -337,12 +356,12 @@ bool ExpressionSourceCode::GetText (std::string &text, lldb::LanguageType wrappi
"@implementation $__lldb_objc_class ($__lldb_category) \n"
"+(void)%s:(void *)$__lldb_arg \n"
"{ \n"
" %s; \n"
"%s"
"} \n"
"@end \n",
m_name.c_str(),
m_name.c_str(),
m_body.c_str());
tagged_body.c_str());
}
else
{
@ -352,12 +371,12 @@ bool ExpressionSourceCode::GetText (std::string &text, lldb::LanguageType wrappi
"@implementation $__lldb_objc_class ($__lldb_category) \n"
"-(void)%s:(void *)$__lldb_arg \n"
"{ \n"
" %s; \n"
"%s"
"} \n"
"@end \n",
m_name.c_str(),
m_name.c_str(),
m_body.c_str());
tagged_body.c_str());
}
break;
}
@ -371,3 +390,35 @@ bool ExpressionSourceCode::GetText (std::string &text, lldb::LanguageType wrappi
return true;
}
bool
ExpressionSourceCode::GetOriginalBodyBounds(std::string transformed_text,
lldb::LanguageType wrapping_language,
size_t &start_loc,
size_t &end_loc)
{
const char *start_marker;
const char *end_marker;
switch (wrapping_language)
{
default:
return false;
case lldb::eLanguageTypeC:
case lldb::eLanguageTypeC_plus_plus:
case lldb::eLanguageTypeObjC:
start_marker = c_start_marker;
end_marker = c_end_marker;
break;
}
start_loc = transformed_text.find(start_marker);
if (start_loc == std::string::npos)
return false;
start_loc += strlen(start_marker);
end_loc = transformed_text.find(end_marker);
if (end_loc == std::string::npos)
return false;
return true;
}

View File

@ -363,6 +363,7 @@ REPL::IOHandlerInputComplete (IOHandler &io_handler, std::string &code)
result_valobj_sp,
error,
0, // Line offset
nullptr, // Fixed Expression
&jit_module_sp);
//CommandInterpreter &ci = debugger.GetCommandInterpreter();

View File

@ -160,6 +160,7 @@ UserExpression::Evaluate (ExecutionContext &exe_ctx,
lldb::ValueObjectSP &result_valobj_sp,
Error &error,
uint32_t line_offset,
std::string *fixed_expression,
lldb::ModuleSP *jit_module_sp_ptr)
{
Log *log(lldb_private::GetLogIfAnyCategoriesSet (LIBLLDB_LOG_EXPRESSIONS | LIBLLDB_LOG_STEP));
@ -253,18 +254,65 @@ 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))
bool parse_success = user_expression_sp->Parse(diagnostic_manager,
exe_ctx,
execution_policy,
keep_expression_in_memory,
generate_debug_info);
// Calculate the fixed expression always, since we need it for errors.
std::string tmp_fixed_expression;
if (fixed_expression == nullptr)
fixed_expression = &tmp_fixed_expression;
const char *fixed_text = user_expression_sp->GetFixedText();
if (fixed_text != nullptr)
fixed_expression->append(fixed_text);
// If there is a fixed expression, try to parse it:
if (!parse_success)
{
execution_results = lldb::eExpressionParseError;
if (!diagnostic_manager.Diagnostics().size())
error.SetExpressionError(execution_results, "expression failed to parse, unknown error");
else
error.SetExpressionError(execution_results, diagnostic_manager.GetString().c_str());
if (fixed_expression && !fixed_expression->empty() && options.GetAutoApplyFixIts())
{
lldb::UserExpressionSP fixed_expression_sp(target->GetUserExpressionForLanguage (fixed_expression->c_str(),
full_prefix,
language,
desired_type,
options,
error));
DiagnosticManager fixed_diagnostic_manager;
parse_success = fixed_expression_sp->Parse(fixed_diagnostic_manager,
exe_ctx,
execution_policy,
keep_expression_in_memory,
generate_debug_info);
if (parse_success)
{
diagnostic_manager.Clear();
user_expression_sp = fixed_expression_sp;
}
}
if (!parse_success)
{
if (!fixed_expression->empty() && target->GetEnableNotifyAboutFixIts())
{
error.SetExpressionErrorWithFormat(execution_results, "expression failed to parse, fixed expression suggested:\n %s",
fixed_expression->c_str());
}
else
{
if (!diagnostic_manager.Diagnostics().size())
error.SetExpressionError(execution_results, "expression failed to parse, unknown error");
else
error.SetExpressionError(execution_results, diagnostic_manager.GetString().c_str());
}
}
}
else
if (parse_success)
{
// If a pointer to a lldb::ModuleSP was passed in, return the JIT'ed module if one was created
if (jit_module_sp_ptr)

View File

@ -399,6 +399,8 @@ ClangUserExpression::Parse(DiagnosticManager &diagnostic_manager, ExecutionConte
}
}
lldb::LanguageType lang_type = lldb::eLanguageTypeUnknown;
if (m_options.GetExecutionPolicy() == eExecutionPolicyTopLevel)
{
m_transformed_text = m_expr_text;
@ -408,8 +410,6 @@ ClangUserExpression::Parse(DiagnosticManager &diagnostic_manager, ExecutionConte
std::unique_ptr<ExpressionSourceCode> source_code(
ExpressionSourceCode::CreateWrapped(prefix.c_str(), m_expr_text.c_str()));
lldb::LanguageType lang_type;
if (m_in_cplusplus_method)
lang_type = lldb::eLanguageTypeC_plus_plus;
else if (m_in_objectivec_method)
@ -491,37 +491,25 @@ ClangUserExpression::Parse(DiagnosticManager &diagnostic_manager, ExecutionConte
// 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));
ClangExpressionParser parser(exe_scope, *this, generate_debug_info);
unsigned num_errors = parser_sp->Parse(diagnostic_manager);
unsigned num_errors = parser.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);
}
}
}
// Check here for FixItHints. If there are any try to apply the fixits and set the fixed text in m_fixed_text
// before returning an error.
if (num_errors)
{
if (diagnostic_manager.HasFixIts())
{
if (parser.RewriteExpression(diagnostic_manager))
{
size_t fixed_start;
size_t fixed_end;
const std::string &fixed_expression = diagnostic_manager.GetFixedExpression();
if (ExpressionSourceCode::GetOriginalBodyBounds(fixed_expression, lang_type, fixed_start, fixed_end))
m_fixed_text = fixed_expression.substr(fixed_start, fixed_end - fixed_start);
}
}
diagnostic_manager.Printf(eDiagnosticSeverityError, "%u error%s parsing expression", num_errors,
num_errors == 1 ? "" : "s");
@ -535,12 +523,12 @@ ClangUserExpression::Parse(DiagnosticManager &diagnostic_manager, ExecutionConte
//
{
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);
Error jit_error = parser.PrepareForExecution(m_jit_start_addr,
m_jit_end_addr,
m_execution_unit_sp,
exe_ctx,
m_can_interpret,
execution_policy);
if (!jit_error.Success())
{
@ -555,7 +543,7 @@ ClangUserExpression::Parse(DiagnosticManager &diagnostic_manager, ExecutionConte
if (exe_ctx.GetProcessPtr() && execution_policy == eExecutionPolicyTopLevel)
{
Error static_init_error = parser_sp->RunStaticInitializers(m_execution_unit_sp, exe_ctx);
Error static_init_error = parser.RunStaticInitializers(m_execution_unit_sp, exe_ctx);
if (!static_init_error.Success())
{

View File

@ -186,9 +186,14 @@ AddressSanitizerRuntime::RetrieveReportData()
options.SetIgnoreBreakpoints(true);
options.SetTimeoutUsec(RETRIEVE_REPORT_DATA_FUNCTION_TIMEOUT_USEC);
options.SetPrefix(address_sanitizer_retrieve_report_data_prefix);
options.SetAutoApplyFixIts(false);
ValueObjectSP return_value_sp;
if (process_sp->GetTarget().EvaluateExpression(address_sanitizer_retrieve_report_data_command, frame_sp.get(), return_value_sp, options) != eExpressionCompleted)
std::string fixed_expression;
if (process_sp->GetTarget().EvaluateExpression(address_sanitizer_retrieve_report_data_command,
frame_sp.get(),
return_value_sp,
options) != eExpressionCompleted)
return StructuredData::ObjectSP();
int present = return_value_sp->GetValueForExpressionPath(".present")->GetValueAsUnsigned(0);

View File

@ -2234,7 +2234,8 @@ ExpressionResults
Target::EvaluateExpression(const char *expr_cstr,
ExecutionContextScope *exe_scope,
lldb::ValueObjectSP &result_valobj_sp,
const EvaluateExpressionOptions& options)
const EvaluateExpressionOptions& options,
std::string *fixed_expression)
{
result_valobj_sp.reset();
@ -2284,7 +2285,9 @@ Target::EvaluateExpression(const char *expr_cstr,
expr_cstr,
prefix,
result_valobj_sp,
error);
error,
0, // Line Number
fixed_expression);
}
m_suppress_stop_hooks = old_suppress_value;
@ -3426,6 +3429,7 @@ g_properties[] =
{ "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." },
{ "notify-about-fixits" , OptionValue::eTypeBoolean , false, true , nullptr, nullptr, "Print the fixed expression text." },
{ "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." },
@ -3483,6 +3487,7 @@ enum
ePropertyClangModuleSearchPaths,
ePropertyAutoImportClangModules,
ePropertyAutoApplyFixIts,
ePropertyNotifyAboutFixIts,
ePropertyMaxChildrenCount,
ePropertyMaxSummaryLength,
ePropertyMaxMemReadSize,
@ -3862,6 +3867,13 @@ TargetProperties::GetEnableAutoApplyFixIts() const
return m_collection_sp->GetPropertyAtIndexAsBoolean(nullptr, idx, g_properties[idx].default_uint_value != 0);
}
bool
TargetProperties::GetEnableNotifyAboutFixIts() const
{
const uint32_t idx = ePropertyNotifyAboutFixIts;
return m_collection_sp->GetPropertyAtIndexAsBoolean(nullptr, idx, g_properties[idx].default_uint_value != 0);
}
bool
TargetProperties::GetEnableSyntheticValue () const
{