[lldb][Expression] Add --cpp-ignore-context-qualifiers expression evaluation option#177926
[lldb][Expression] Add --cpp-ignore-context-qualifiers expression evaluation option#177926Michael137 wants to merge 10 commits intollvm:mainfrom
Conversation
|
@llvm/pr-subscribers-lldb Author: Michael Buch (Michael137) ChangesDepends on:
In #177922 we make expressions run in C++ member functions honor the function qualifiers of the current stop context. E.g., this means we can no longer run non-const member functions when stopped in a const-member function. To ensure users can still do this if they really need/want to, we provide an option to not honor the qualifiers at all, leaving the We may want to come up with a better name because this doesn't only apply to Patch is 55.24 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/177926.diff 39 Files Affected:
diff --git a/lldb/include/lldb/API/SBExpressionOptions.h b/lldb/include/lldb/API/SBExpressionOptions.h
index a9e929a4c0bd9..2b922b97f75b9 100644
--- a/lldb/include/lldb/API/SBExpressionOptions.h
+++ b/lldb/include/lldb/API/SBExpressionOptions.h
@@ -107,6 +107,10 @@ class LLDB_API SBExpressionOptions {
// Sets whether we will JIT an expression if it cannot be interpreted
void SetAllowJIT(bool allow);
+ bool GetIgnoreConstContext();
+
+ void SetIgnoreConstContext(bool b = true);
+
protected:
lldb_private::EvaluateExpressionOptions *get() const;
diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h
index 812a638910b3b..77caccdbcc74d 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -481,6 +481,10 @@ class EvaluateExpressionOptions {
void SetIsForUtilityExpr(bool b) { m_running_utility_expression = b; }
+ bool GetIgnoreConstContext() const { return m_ignore_const_contexts; }
+
+ void SetIgnoreConstContext(bool val) { m_ignore_const_contexts = val; }
+
private:
ExecutionPolicy m_execution_policy = default_execution_policy;
SourceLanguage m_language;
@@ -518,6 +522,8 @@ class EvaluateExpressionOptions {
/// used for symbol/function lookup before any other context (except for
/// the module corresponding to the current frame).
SymbolContextList m_preferred_lookup_contexts;
+
+ bool m_ignore_const_contexts = false;
};
// Target
diff --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py b/lldb/packages/Python/lldbsuite/test/lldbtest.py
index 6bb4516948da5..6034eca3b93f2 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -2575,6 +2575,7 @@ def expect_expr(
result_value=None,
result_type=None,
result_children=None,
+ options=None,
):
"""
Evaluates the given expression and verifies the result.
@@ -2584,6 +2585,7 @@ def expect_expr(
:param result_type: The type that the expression result should have. None if the type should not be checked.
:param result_children: The expected children of the expression result
as a list of ValueChecks. None if the children shouldn't be checked.
+ :param options: Expression evaluation options. None if a default set of options should be used.
"""
self.assertTrue(
expr.strip() == expr,
@@ -2591,13 +2593,15 @@ def expect_expr(
)
frame = self.frame()
- options = lldb.SBExpressionOptions()
- # Disable fix-its that tests don't pass by accident.
- options.SetAutoApplyFixIts(False)
+ if not options:
+ options = lldb.SBExpressionOptions()
- # Set the usual default options for normal expressions.
- options.SetIgnoreBreakpoints(True)
+ # Disable fix-its that tests don't pass by accident.
+ options.SetAutoApplyFixIts(False)
+
+ # Set the usual default options for normal expressions.
+ options.SetIgnoreBreakpoints(True)
if self.frame().IsValid():
options.SetLanguage(frame.GuessLanguage())
diff --git a/lldb/source/API/SBExpressionOptions.cpp b/lldb/source/API/SBExpressionOptions.cpp
index 15ed403eaaea1..84f23df47b0cb 100644
--- a/lldb/source/API/SBExpressionOptions.cpp
+++ b/lldb/source/API/SBExpressionOptions.cpp
@@ -256,6 +256,18 @@ void SBExpressionOptions::SetAllowJIT(bool allow) {
: eExecutionPolicyNever);
}
+bool SBExpressionOptions::GetIgnoreConstContext() {
+ LLDB_INSTRUMENT_VA(this);
+
+ return m_opaque_up->GetIgnoreConstContext();
+}
+
+void SBExpressionOptions::SetIgnoreConstContext(bool b) {
+ LLDB_INSTRUMENT_VA(this, b);
+
+ m_opaque_up->SetIgnoreConstContext(b);
+}
+
EvaluateExpressionOptions *SBExpressionOptions::get() const {
return m_opaque_up.get();
}
diff --git a/lldb/source/Commands/CommandObjectExpression.cpp b/lldb/source/Commands/CommandObjectExpression.cpp
index 4919bd3639d3e..14dea55164c6e 100644
--- a/lldb/source/Commands/CommandObjectExpression.cpp
+++ b/lldb/source/Commands/CommandObjectExpression.cpp
@@ -25,6 +25,7 @@
#include "lldb/lldb-enumerations.h"
#include "lldb/lldb-forward.h"
#include "lldb/lldb-private-enumerations.h"
+#include <tuple>
using namespace lldb;
using namespace lldb_private;
@@ -44,6 +45,9 @@ Status CommandObjectExpression::CommandOptions::SetOptionValue(
const int short_option = GetDefinitions()[option_idx].short_option;
switch (short_option) {
+ case 'K':
+ ignore_const_context = true;
+ break;
case 'l':
language = Language::GetLanguageTypeFromString(option_arg);
if (language == eLanguageTypeUnknown) {
@@ -191,6 +195,7 @@ void CommandObjectExpression::CommandOptions::OptionParsingStarting(
top_level = false;
allow_jit = true;
suppress_persistent_result = eLazyBoolCalculate;
+ ignore_const_context = false;
}
llvm::ArrayRef<OptionDefinition>
@@ -213,6 +218,7 @@ CommandObjectExpression::CommandOptions::GetEvaluateExpressionOptions(
options.SetExecutionPolicy(
allow_jit ? EvaluateExpressionOptions::default_execution_policy
: lldb_private::eExecutionPolicyNever);
+ options.SetIgnoreConstContext(ignore_const_context);
bool auto_apply_fixits;
if (this->auto_apply_fixits == eLazyBoolCalculate)
diff --git a/lldb/source/Commands/CommandObjectExpression.h b/lldb/source/Commands/CommandObjectExpression.h
index 6fccf10e5dbc1..6bbe02f584b21 100644
--- a/lldb/source/Commands/CommandObjectExpression.h
+++ b/lldb/source/Commands/CommandObjectExpression.h
@@ -57,6 +57,7 @@ class CommandObjectExpression : public CommandObjectRaw,
LanguageRuntimeDescriptionDisplayVerbosity m_verbosity;
LazyBool auto_apply_fixits;
LazyBool suppress_persistent_result;
+ bool ignore_const_context;
};
CommandObjectExpression(CommandInterpreter &interpreter);
diff --git a/lldb/source/Commands/Options.td b/lldb/source/Commands/Options.td
index d96354a39b8b8..c82a029ae95ce 100644
--- a/lldb/source/Commands/Options.td
+++ b/lldb/source/Commands/Options.td
@@ -778,6 +778,9 @@ let Command = "expression" in {
Desc<"Persist expression result in a variable for subsequent use. "
"Expression results will be labeled with $-prefixed variables, "
"e.g. $0, $1, etc.">;
+ def ignore_const_context : Option<"ignore-const-context", "K">,
+ Groups<[1, 2]>,
+ Desc<"TODO">;
}
let Command = "frame diag" in {
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
index 24a5dc5362964..52a77fbbac66a 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
@@ -88,10 +88,12 @@ ClangExpressionDeclMap::ClangExpressionDeclMap(
bool keep_result_in_memory,
Materializer::PersistentVariableDelegate *result_delegate,
const lldb::TargetSP &target,
- const std::shared_ptr<ClangASTImporter> &importer, ValueObject *ctx_obj)
+ const std::shared_ptr<ClangASTImporter> &importer, ValueObject *ctx_obj,
+ bool ignore_const_context)
: ClangASTSource(target, importer), m_found_entities(), m_struct_members(),
m_keep_result_in_memory(keep_result_in_memory),
- m_result_delegate(result_delegate), m_ctx_obj(ctx_obj), m_parser_vars(),
+ m_result_delegate(result_delegate), m_ctx_obj(ctx_obj),
+ m_ignore_const_context(ignore_const_context), m_parser_vars(),
m_struct_vars() {
EnableStructVars();
}
@@ -844,6 +846,12 @@ void ClangExpressionDeclMap::LookUpLldbClass(NameSearchContext &context) {
QualType class_qual_type = m_ast_context->getCanonicalTagType(class_decl);
+ // The synthesized __lldb_expr will adopt the qualifiers from this class
+ // type. Make sure we use the qualifiers of the method that we're currently
+ // stopped in.
+ class_qual_type.addFastQualifiers(
+ method_decl->getMethodQualifiers().getFastQualifiers());
+
TypeFromUser class_user_type(
class_qual_type.getAsOpaquePtr(),
function_decl_ctx.GetTypeSystem()->weak_from_this());
@@ -1991,7 +1999,8 @@ void ClangExpressionDeclMap::AddContextClassType(NameSearchContext &context,
std::array<CompilerType, 1> args{void_clang_type.GetPointerType()};
CompilerType method_type = m_clang_ast_context->CreateFunctionType(
- void_clang_type, args, false, 0);
+ void_clang_type, args, false,
+ m_ignore_const_context ? 0 : ut.GetTypeQualifiers());
const bool is_virtual = false;
const bool is_static = false;
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h
index dddc5a06c9051..3a5f702f6c674 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h
@@ -82,7 +82,8 @@ class ClangExpressionDeclMap : public ClangASTSource {
bool keep_result_in_memory,
Materializer::PersistentVariableDelegate *result_delegate,
const lldb::TargetSP &target,
- const std::shared_ptr<ClangASTImporter> &importer, ValueObject *ctx_obj);
+ const std::shared_ptr<ClangASTImporter> &importer, ValueObject *ctx_obj,
+ bool ignore_const_context);
/// Destructor
~ClangExpressionDeclMap() override;
@@ -306,6 +307,8 @@ class ClangExpressionDeclMap : public ClangASTSource {
///For details see the comment to
///`UserExpression::Evaluate`.
+ bool m_ignore_const_context = false;
+
/// The following values should not live beyond parsing
class ParserVars {
public:
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
index cfe187ffc4114..2c764d250d465 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
@@ -10,6 +10,7 @@
#include "ClangExpressionUtil.h"
+#include "clang/AST/TypeBase.h"
#include "clang/Basic/CharInfo.h"
#include "clang/Basic/FileManager.h"
#include "clang/Basic/SourceManager.h"
@@ -189,6 +190,28 @@ static void AddMacros(const DebugMacros *dm, CompileUnit *comp_unit,
}
}
+/// Return qualifers of the current C++ method.
+static clang::Qualifiers GetFrameCVQualifiers(StackFrame *frame) {
+ if (!frame)
+ return {};
+
+ auto this_sp = frame->FindVariable(ConstString("this"));
+ if (!this_sp)
+ return {};
+
+ // Lambdas that capture 'this' have a member variable called 'this'. The class
+ // context of __lldb_expr for a lambda is the class type of the 'this' capture
+ // (not the anonymous lambda structure). So use the qualifiers of the captured
+ // 'this'.
+ if (auto this_this_sp = this_sp->GetChildMemberWithName("this"))
+ return clang::Qualifiers::fromCVRMask(
+ this_this_sp->GetCompilerType().GetPointeeType().GetTypeQualifiers());
+
+ // Not in a lambda. Return 'this' qualifiers.
+ return clang::Qualifiers::fromCVRMask(
+ this_sp->GetCompilerType().GetPointeeType().GetTypeQualifiers());
+}
+
lldb_private::ClangExpressionSourceCode::ClangExpressionSourceCode(
llvm::StringRef filename, llvm::StringRef name, llvm::StringRef prefix,
llvm::StringRef body, Wrapping wrap, WrapKind wrap_kind)
@@ -340,9 +363,12 @@ void ClangExpressionSourceCode::AddLocalVariableDecls(StreamString &stream,
}
}
-bool ClangExpressionSourceCode::GetText(
- std::string &text, ExecutionContext &exe_ctx, bool add_locals,
- bool force_add_all_locals, llvm::ArrayRef<std::string> modules) const {
+bool ClangExpressionSourceCode::GetText(std::string &text,
+ ExecutionContext &exe_ctx,
+ bool add_locals,
+ bool force_add_all_locals,
+ llvm::ArrayRef<std::string> modules,
+ bool ignore_const_context) const {
const char *target_specific_defines = "typedef signed char BOOL;\n";
std::string module_macros;
llvm::raw_string_ostream module_macros_stream(module_macros);
@@ -464,13 +490,18 @@ bool ClangExpressionSourceCode::GetText(
break;
case WrapKind::CppMemberFunction:
wrap_stream.Printf("%s"
- "void \n"
- "$__lldb_class::%s(void *$__lldb_arg) \n"
- "{ \n"
- " %s; \n"
+ "void \n"
+ "$__lldb_class::%s(void *$__lldb_arg) %s \n"
+ "{ \n"
+ " %s; \n"
"%s"
- "} \n",
+ "} \n",
module_imports.c_str(), m_name.c_str(),
+ ignore_const_context
+ ? ""
+ : GetFrameCVQualifiers(exe_ctx.GetFramePtr())
+ .getAsString()
+ .c_str(),
lldb_local_var_decls.GetData(), tagged_body.c_str());
break;
case WrapKind::ObjCInstanceMethod:
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.h b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.h
index f721bb2f319e1..02090b1aa9b12 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.h
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.h
@@ -63,8 +63,8 @@ class ClangExpressionSourceCode : public ExpressionSourceCode {
///
/// \return true iff the source code was successfully generated.
bool GetText(std::string &text, ExecutionContext &exe_ctx, bool add_locals,
- bool force_add_all_locals,
- llvm::ArrayRef<std::string> modules) const;
+ bool force_add_all_locals, llvm::ArrayRef<std::string> modules,
+ bool ignore_const_context) 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
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
index 2cbbae11bd18a..3b49eb4716a63 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
@@ -416,7 +416,8 @@ void ClangUserExpression::CreateSourceCode(
m_filename, prefix, m_expr_text, GetWrapKind()));
if (!m_source_code->GetText(m_transformed_text, exe_ctx, !m_ctx_obj,
- for_completion, modules_to_import)) {
+ for_completion, modules_to_import,
+ m_options.GetIgnoreConstContext())) {
diagnostic_manager.PutString(lldb::eSeverityError,
"couldn't construct expression body");
return;
@@ -950,8 +951,8 @@ char ClangUserExpression::ClangUserExpressionHelper::ID;
void ClangUserExpression::ClangUserExpressionHelper::ResetDeclMap(
ExecutionContext &exe_ctx,
Materializer::PersistentVariableDelegate &delegate,
- bool keep_result_in_memory,
- ValueObject *ctx_obj) {
+ bool keep_result_in_memory, ValueObject *ctx_obj,
+ bool ignore_const_context) {
std::shared_ptr<ClangASTImporter> ast_importer;
auto *state = exe_ctx.GetTargetSP()->GetPersistentExpressionStateForLanguage(
lldb::eLanguageTypeC);
@@ -961,7 +962,7 @@ void ClangUserExpression::ClangUserExpressionHelper::ResetDeclMap(
}
m_expr_decl_map_up = std::make_unique<ClangExpressionDeclMap>(
keep_result_in_memory, &delegate, exe_ctx.GetTargetSP(), ast_importer,
- ctx_obj);
+ ctx_obj, ignore_const_context);
}
clang::ASTConsumer *
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.h b/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.h
index 7c0c6a0147e2a..2ac50b8152f33 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.h
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.h
@@ -71,8 +71,8 @@ class ClangUserExpression : public LLVMUserExpression {
void ResetDeclMap(ExecutionContext &exe_ctx,
Materializer::PersistentVariableDelegate &result_delegate,
- bool keep_result_in_memory,
- ValueObject *ctx_obj);
+ bool keep_result_in_memory, ValueObject *ctx_obj,
+ bool ignore_const_context);
/// Return the object that the parser should allow to access ASTs. May be
/// NULL if the ASTs do not need to be transformed.
@@ -167,8 +167,8 @@ class ClangUserExpression : public LLVMUserExpression {
Materializer::PersistentVariableDelegate &result_delegate,
bool keep_result_in_memory) {
m_type_system_helper.ResetDeclMap(exe_ctx, result_delegate,
- keep_result_in_memory,
- m_ctx_obj);
+ keep_result_in_memory, m_ctx_obj,
+ m_options.GetIgnoreConstContext());
}
lldb::ExpressionVariableSP
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangUtilityFunction.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangUtilityFunction.cpp
index e6983066a12fa..ea9a4d815a14d 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangUtilityFunction.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangUtilityFunction.cpp
@@ -187,5 +187,5 @@ void ClangUtilityFunction::ClangUtilityFunctionHelper::ResetDeclMap(
}
m_expr_decl_map_up = std::make_unique<ClangExpressionDeclMap>(
keep_result_in_memory, nullptr, exe_ctx.GetTargetSP(), ast_importer,
- nullptr);
+ nullptr, /*ignore_const_context=*/false);
}
diff --git a/lldb/test/API/commands/expression/expr_inside_lambda/TestExprInsideLambdas.py b/lldb/test/API/commands/expression/expr_inside_lambda/TestExprInsideLambdas.py
index e35cfa6a289a7..0a7683b310f43 100644
--- a/lldb/test/API/commands/expression/expr_inside_lambda/TestExprInsideLambdas.py
+++ b/lldb/test/API/commands/expression/expr_inside_lambda/TestExprInsideLambdas.py
@@ -127,8 +127,8 @@ def test_expr_inside_lambda(self):
# Inside non_capturing_method
lldbutil.continue_to_breakpoint(process, bkpt)
- self.expect_expr("local", result_type="int", result_value="5")
- self.expect_expr("local2", result_type="int", result_value="10")
+ self.expect_expr("local", result_type="const int", result_value="5")
+ self.expect_expr("local2", result_type="const int", result_value="10")
self.expect_expr("local2 * local", result_type="int", result_value="50")
self.expectExprError(
diff --git a/lldb/test/API/lang/cpp/const_this/TestConstThis.py b/lldb/test/API/lang/cpp/const_this/TestConstThis.py
index 4b7d3aadb62ab..c2df61fde2b58 100644
--- a/lldb/test/API/lang/cpp/const_this/TestConstThis.py
+++ b/lldb/test/API/lang/cpp/const_this/TestConstThis.py
@@ -11,10 +11,9 @@ def run_class_tests(self):
# Expression not referencing context class.
self.expect_expr("1 + 1", result_type="int", result_value="2")
# Referencing context class.
- # FIXME: This and the expression below should return const types.
- self.expect_expr("member", result_type="int", result_value="3")
+ self.expect_expr("member", result_type="const int", result_value="3")
# Check t...
[truncated]
|
🐧 Linux x64 Test Results
✅ The build succeeded and all tests passed. |
2edd1f4 to
6be6655
Compare
Depends on: * llvm#177921 * llvm#177926 (only last commit is relevant for this review)
Depends on: * llvm#177921 * llvm#177926 (only last commit is relevant for this review)
Depends on: * llvm#177921 * llvm#177926 (only last commit is relevant for this review)
6be6655 to
51f47b5
Compare
Depends on: * llvm#177921 * llvm#177926 (only last commit is relevant for this review)
2738237 to
e0d43f9
Compare
Depends on: * llvm#177921 * llvm#177926 (only last commit is relevant for this review)
Depends on: * llvm#177921 * llvm#177926 (only last commit is relevant for this review)
| %feature("docstring", "Gets whether to JIT an expression if it cannot be interpreted." | ||
| ) lldb::SBExpressionOptions::GetAllowJIT; | ||
|
|
||
| %feature("docstring", "Sets whether to ignore the CV-qualifiers of the scope during expression evaluation." |
There was a problem hiding this comment.
One thing I don't like about this is that this is using terminology from the C language family. Is there any way to either make the wording more neutral, or to explicitly call out that this is for C(++)?
There was a problem hiding this comment.
How about:
ignore function qualifiers of the scope that the expression is evaluated in.
E.g., running an expression in a C++ member function marked `const`, will ignore
the fact that it's running in a `const` function.
e0d43f9 to
5e3a7af
Compare
Depends on: * llvm#177921 * llvm#177926 (only last commit is relevant for this review)
Depends on: * llvm#177921 * llvm#177926 (only last commit is relevant for this review)
lldb/include/lldb/Target/Target.h
Outdated
| /// CV-qualifiers of the scope. E.g., this would permit calling a | ||
| /// non-const C++ method when stopped in a const-method (which would be | ||
| /// disallowed by C++ language rules). | ||
| bool m_ignore_context_qualifierss = false; |
There was a problem hiding this comment.
typo: m_ignore_context_qualifierss
There was a problem hiding this comment.
I'm wondering if this should be a dictionary of language(plugin)-specific options that all start with the language name?
There was a problem hiding this comment.
I added a FIXME for this and renamed the option to have a cpp- prefix for now
|
✅ With the latest revision this PR passed the Python code formatter. |
Depends on: * llvm#177921 * llvm#177926 (only last commit is relevant for this review)
Depends on: * llvm#177921 * llvm#177926 (only last commit is relevant for this review)
7ce3b2e to
7fde8f6
Compare
Depends on: * llvm#177921 * llvm#177926 (only last commit is relevant for this review)
Depends on: * llvm#177921 * llvm#177926 (only last commit is relevant for this review)
…tions The motivation here is that we don't want to pollute the SBAPI with getters/setters for expression evaluation options that only apply to a single language. The ultimate goal would be to have plugins register additional options to the `expression` command when the plugin is loaded. This patch only provides the minimal `SBExpressionOptions` interface to set an option with an arbitrary name, which the language plugin knows how to interpret. The underlying options dictionary is an `StructuredData::Dictionary` so we can map strings to values of any type. But the SBAPI just exposes setting a boolean value. Future overloads of `SetLanguageOption` can provide setters for more types. The boolean setter/getter will be used for the C++-specific option being introduced in: llvm#177926
273d23d to
fb76048
Compare
Depends on: * llvm#177921 * llvm#177926 (only last commit is relevant for this review)
Depends on: * llvm#177921 * llvm#177926 (only last commit is relevant for this review)
Depends on: * llvm#177921 * llvm#177926 (only last commit is relevant for this review)
lldb/source/Commands/Options.td
Outdated
| @@ -778,6 +778,13 @@ let Command = "expression" in { | |||
| Desc<"Persist expression result in a variable for subsequent use. " | |||
| "Expression results will be labeled with $-prefixed variables, " | |||
| "e.g. $0, $1, etc.">; | |||
| def ignore_context_qualifiers | |||
| : Option<"cpp-ignore-context-qualifiers", "Q">, | |||
There was a problem hiding this comment.
expr -l calls it c++, should we use the same name for consistency as the prefix?
There was a problem hiding this comment.
FWIW, but the C++ plugin name is cplusplus. We'll have to choose some prefix eventually. I don't mind c++
Allows API tests to pass `SBExpressionOptions` when testing a successful expression evaluation with `expect_expr`. Currently one would have to use `SBFrame::EvaluateExpression` or pass the option as an argument to the raw command (via `expect()` or `HandleCommand()`). Chose not to do the `SetIgnoreBreakpoints`/`SetAutoApplyFixIts` with the assumption that most expression evaluation tests don't actually need to care about these. If the options are passed explicitly, lets use them as-is. Otherwise default to the old options. First usage of this new parameter would be in #177926
…tions (#179208) The motivation here is that we don't want to pollute the SBAPI with getters/setters for expression evaluation options that only apply to a single language. The ultimate goal would be to have plugins register additional options to the `expression` command when the plugin is loaded. This patch only provides the minimal `SBExpressionOptions` interface to set an option with an arbitrary name, which the language plugin knows how to interpret. The underlying options dictionary is an `StructuredData::Dictionary` so we can map strings to values of any type. But the SBAPI just exposes setting a boolean value. Future overloads of `SetLanguageOption` can provide setters for more types. The boolean setter/getter will be used for the C++-specific option being introduced in: #177926
…xpr (#177920) Allows API tests to pass `SBExpressionOptions` when testing a successful expression evaluation with `expect_expr`. Currently one would have to use `SBFrame::EvaluateExpression` or pass the option as an argument to the raw command (via `expect()` or `HandleCommand()`). Chose not to do the `SetIgnoreBreakpoints`/`SetAutoApplyFixIts` with the assumption that most expression evaluation tests don't actually need to care about these. If the options are passed explicitly, lets use them as-is. Otherwise default to the old options. First usage of this new parameter would be in llvm/llvm-project#177926
…pression options (#179208) The motivation here is that we don't want to pollute the SBAPI with getters/setters for expression evaluation options that only apply to a single language. The ultimate goal would be to have plugins register additional options to the `expression` command when the plugin is loaded. This patch only provides the minimal `SBExpressionOptions` interface to set an option with an arbitrary name, which the language plugin knows how to interpret. The underlying options dictionary is an `StructuredData::Dictionary` so we can map strings to values of any type. But the SBAPI just exposes setting a boolean value. Future overloads of `SetLanguageOption` can provide setters for more types. The boolean setter/getter will be used for the C++-specific option being introduced in: llvm/llvm-project#177926
…context This was originally removed in `8bdcd522510f923185cdfaec66c4a78d0a0d38c0` under the assumption this wasn't required (i.e., LLDB should just always pretend it's in a mutable context). But since function qualifiers affect overloading in C++, this assumption can lead to unexpected expression evaluator behaviour. Instead, in this patch we try to add function qualifiers to `$__lldb_class::$__lldb_expr` that resemble the qualifiers in the method that we're stopped in.
…ption Depends on: * llvm#177920 * llvm#177922 In llvm#177922 we make expressions run in C++ member functions honor the function qualifiers of the current stop context. E.g., this means we can no longer run non-const member functions when stopped in a const-member function. To ensure users can still do this if they really need/want to, we provide an option to not honor the qualifiers at all, leaving the `__lldb_expr` as the least qualified, allowing it to call any function/mutate any members.
e539b61 to
355ef7c
Compare
Depends on: * llvm#177921 * llvm#177926 (only last commit is relevant for this review)
Depends on: * llvm#177921 * llvm#177926 (only last commit is relevant for this review)
Depends on:
(only commit d8676d0 and later are relevant for this review)
In #177922 we make expressions run in C++ member functions honor the function qualifiers of the current stop context. E.g., this means we can no longer run non-const member functions when stopped in a const-member function.
To ensure users can still do this if they really need/want to, we provide an option to not honor the qualifiers at all, leaving the
__lldb_exprminimally qualified, allowing it to call any function/mutate any members.