This revision was automatically updated to reflect the committed changes.
Closed by commit rG958608285eb4: [lldb] Allow LLDB to automatically retry a
failed expression with an imported… (authored by teemperor).
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D92784/new/
https://reviews.llvm.org/D92784
Files:
lldb/include/lldb/Target/Target.h
lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.h
lldb/source/Target/Target.cpp
lldb/source/Target/TargetProperties.td
lldb/test/API/commands/expression/import-std-module/retry-with-std-module/Makefile
lldb/test/API/commands/expression/import-std-module/retry-with-std-module/TestRetryWithStdModule.py
lldb/test/API/commands/expression/import-std-module/retry-with-std-module/main.cpp
Index: lldb/test/API/commands/expression/import-std-module/retry-with-std-module/main.cpp
===================================================================
--- /dev/null
+++ lldb/test/API/commands/expression/import-std-module/retry-with-std-module/main.cpp
@@ -0,0 +1,7 @@
+#include <vector>
+
+int main(int argc, char **argv) {
+ std::vector<int> a = {3, 1, 2};
+ int local = 3;
+ return 0; // Set break point at this line.
+}
Index: lldb/test/API/commands/expression/import-std-module/retry-with-std-module/TestRetryWithStdModule.py
===================================================================
--- /dev/null
+++ lldb/test/API/commands/expression/import-std-module/retry-with-std-module/TestRetryWithStdModule.py
@@ -0,0 +1,76 @@
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestCase(TestBase):
+
+ mydir = TestBase.compute_mydir(__file__)
+
+ @add_test_categories(["libc++"])
+ @skipIf(compiler=no_match("clang"))
+ def test(self):
+ self.build()
+
+ lldbutil.run_to_source_breakpoint(self,
+ "// Set break point at this line.",
+ lldb.SBFileSpec("main.cpp"))
+
+ # Test printing the vector before enabling any C++ module setting.
+ self.expect_expr("a", result_type="std::vector<int, std::allocator<int> >")
+
+ # Set loading the import-std-module to 'fallback' which loads the module
+ # and retries when an expression fails to parse.
+ self.runCmd("settings set target.import-std-module fallback")
+
+ # Printing the vector still works. This should return the same type
+ # as before as this shouldn't use a C++ module type (the C++ module type
+ # is hiding the second template parameter as it's equal to the default
+ # argument which the C++ module has type info for).
+ self.expect_expr("a", result_type="std::vector<int, std::allocator<int> >")
+
+ # This expression can only parse with a C++ module. LLDB should
+ # automatically fall back to import the C++ module to get this working.
+ self.expect_expr("std::max<std::size_t>(0U, a.size())", result_value="3")
+
+
+ # The 'a' and 'local' part can be parsed without loading a C++ module and will
+ # load type/runtime information. The 'std::max...' part will fail to
+ # parse without a C++ module. Make sure we reset all the relevant parts of
+ # the C++ parser so that we don't end up with for example a second
+ # definition of 'local' when retrying.
+ self.expect_expr("a; local; std::max<std::size_t>(0U, a.size())", result_value="3")
+
+
+ # Try to declare top-level declarations that require a C++ module to parse.
+ # Top-level expressions don't support importing the C++ module (yet), so
+ # this should still fail as before.
+ self.expect("expr --top-level -- int i = std::max(1, 2);", error=True,
+ substrs=["no member named 'max' in namespace 'std'"])
+
+ # Check that diagnostics from the first parse attempt don't show up
+ # in the C++ module parse attempt. In the expression below, we first
+ # fail to parse 'std::max'. Then we retry with a loaded C++ module
+ # and succeed to parse the 'std::max' part. However, the
+ # trailing 'unknown_identifier' will fail to parse even with the
+ # loaded module. The 'std::max' diagnostic from the first attempt
+ # however should not be shown to the user.
+ self.expect("expr std::max(1, 2); unknown_identifier", error=True,
+ matching=False,
+ substrs=["no member named 'max'"])
+ # The proper diagnostic however should be shown on the retry.
+ self.expect("expr std::max(1, 2); unknown_identifier", error=True,
+ substrs=["use of undeclared identifier 'unknown_identifier'"])
+
+ # Turn on the 'import-std-module' setting and make sure we import the
+ # C++ module.
+ self.runCmd("settings set target.import-std-module true")
+ # This is still expected to work.
+ self.expect_expr("std::max<std::size_t>(0U, a.size())", result_value="3")
+
+ # Turn of the 'import-std-module' setting and make sure we don't load
+ # the module (which should prevent parsing the expression involving
+ # 'std::max').
+ self.runCmd("settings set target.import-std-module false")
+ self.expect("expr std::max(1, 2);", error=True,
+ substrs=["no member named 'max' in namespace 'std'"])
Index: lldb/test/API/commands/expression/import-std-module/retry-with-std-module/Makefile
===================================================================
--- /dev/null
+++ lldb/test/API/commands/expression/import-std-module/retry-with-std-module/Makefile
@@ -0,0 +1,3 @@
+USE_LIBCPP := 1
+CXX_SOURCES := main.cpp
+include Makefile.rules
Index: lldb/source/Target/TargetProperties.td
===================================================================
--- lldb/source/Target/TargetProperties.td
+++ lldb/source/Target/TargetProperties.td
@@ -49,9 +49,11 @@
def AutoImportClangModules: Property<"auto-import-clang-modules", "Boolean">,
DefaultTrue,
Desc<"Automatically load Clang modules referred to by the program.">;
- def ImportStdModule: Property<"import-std-module", "Boolean">,
- DefaultFalse,
- Desc<"Import the C++ std module to improve debugging STL containers.">;
+ def ImportStdModule: Property<"import-std-module", "Enum">,
+ DefaultEnumValue<"eImportStdModuleFalse">,
+ EnumValues<"OptionEnumValues(g_import_std_module_value_types)">,
+ Desc<"Import the 'std' C++ module to improve expression parsing involving "
+ " C++ standard library types.">;
def AutoApplyFixIts: Property<"auto-apply-fixits", "Boolean">,
DefaultTrue,
Desc<"Automatically apply fix-it hints to expressions.">;
Index: lldb/source/Target/Target.cpp
===================================================================
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -3516,6 +3516,28 @@
},
};
+static constexpr OptionEnumValueElement g_import_std_module_value_types[] = {
+ {
+ eImportStdModuleFalse,
+ "false",
+ "Never import the 'std' C++ module in the expression parser.",
+ },
+ {
+ eImportStdModuleFallback,
+ "fallback",
+ "Retry evaluating expressions with an imported 'std' C++ module if they"
+ " failed to parse without the module. This allows evaluating more "
+ "complex expressions involving C++ standard library types."
+ },
+ {
+ eImportStdModuleTrue,
+ "true",
+ "Always import the 'std' C++ module. This allows evaluating more "
+ "complex expressions involving C++ standard library types. This feature"
+ " is experimental."
+ },
+};
+
static constexpr OptionEnumValueElement g_hex_immediate_style_values[] = {
{
Disassembler::eHexStyleC,
@@ -3969,10 +3991,10 @@
nullptr, idx, g_target_properties[idx].default_uint_value != 0);
}
-bool TargetProperties::GetEnableImportStdModule() const {
+ImportStdModule TargetProperties::GetImportStdModule() const {
const uint32_t idx = ePropertyImportStdModule;
- return m_collection_sp->GetPropertyAtIndexAsBoolean(
- nullptr, idx, g_target_properties[idx].default_uint_value != 0);
+ return (ImportStdModule)m_collection_sp->GetPropertyAtIndexAsEnumeration(
+ nullptr, idx, g_target_properties[idx].default_uint_value);
}
bool TargetProperties::GetEnableAutoApplyFixIts() const {
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.h
===================================================================
--- lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.h
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.h
@@ -168,12 +168,23 @@
lldb::ExpressionVariableSP
GetResultAfterDematerialization(ExecutionContextScope *exe_scope) override;
- bool DidImportCxxModules() const { return m_imported_cpp_modules; }
+ /// Returns true iff this expression is using any imported C++ modules.
+ bool DidImportCxxModules() const { return !m_imported_cpp_modules.empty(); }
private:
/// Populate m_in_cplusplus_method and m_in_objectivec_method based on the
/// environment.
+ /// Contains the actual parsing implementation.
+ /// The parameter have the same meaning as in ClangUserExpression::Parse.
+ /// \see ClangUserExpression::Parse
+ bool TryParse(DiagnosticManager &diagnostic_manager,
+ ExecutionContextScope *exe_scope, ExecutionContext &exe_ctx,
+ lldb_private::ExecutionPolicy execution_policy, bool keep_result_in_memory,
+ bool generate_debug_info);
+
+ void SetupCppModuleImports(ExecutionContext &exe_ctx);
+
void ScanContext(ExecutionContext &exe_ctx,
lldb_private::Status &err) override;
@@ -219,6 +230,8 @@
ResultDelegate m_result_delegate;
ClangPersistentVariables *m_clang_state;
std::unique_ptr<ClangExpressionSourceCode> m_source_code;
+ /// The parser instance we used to parse the expression.
+ std::unique_ptr<ClangExpressionParser> m_parser;
/// File name used for the expression.
std::string m_filename;
@@ -226,8 +239,9 @@
/// See the comment to `UserExpression::Evaluate` for details.
ValueObject *m_ctx_obj;
- /// True iff this expression explicitly imported C++ modules.
- bool m_imported_cpp_modules = false;
+ /// A list of module names that should be imported when parsing.
+ /// \see CppModuleConfiguration::GetImportedModules
+ std::vector<std::string> m_imported_cpp_modules;
/// True if the expression parser should enforce the presence of a valid class
/// pointer in order to generate the expression as a method.
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
===================================================================
--- lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
@@ -417,7 +417,6 @@
DiagnosticManager &diagnostic_manager, ExecutionContext &exe_ctx,
std::vector<std::string> modules_to_import, bool for_completion) {
- m_filename = m_clang_state->GetNextExprFileName();
std::string prefix = m_expr_prefix;
if (m_options.GetExecutionPolicy() == eExecutionPolicyTopLevel) {
@@ -477,9 +476,6 @@
if (!target)
return LogConfigError("No target");
- if (!target->GetEnableImportStdModule())
- return LogConfigError("Importing std module not enabled in settings");
-
StackFrame *frame = exe_ctx.GetFramePtr();
if (!frame)
return LogConfigError("No frame");
@@ -529,8 +525,6 @@
bool ClangUserExpression::PrepareForParsing(
DiagnosticManager &diagnostic_manager, ExecutionContext &exe_ctx,
bool for_completion) {
- Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS));
-
InstallContext(exe_ctx);
if (!SetupPersistentState(diagnostic_manager, exe_ctx))
@@ -551,50 +545,20 @@
SetupDeclVendor(exe_ctx, m_target, diagnostic_manager);
- CppModuleConfiguration module_config = GetModuleConfig(m_language, exe_ctx);
- llvm::ArrayRef<std::string> imported_modules =
- module_config.GetImportedModules();
- m_imported_cpp_modules = !imported_modules.empty();
- m_include_directories = module_config.GetIncludeDirs();
+ m_filename = m_clang_state->GetNextExprFileName();
- LLDB_LOG(log, "List of imported modules in expression: {0}",
- llvm::make_range(imported_modules.begin(), imported_modules.end()));
- LLDB_LOG(log, "List of include directories gathered for modules: {0}",
- llvm::make_range(m_include_directories.begin(),
- m_include_directories.end()));
+ if (m_target->GetImportStdModule() == eImportStdModuleTrue)
+ SetupCppModuleImports(exe_ctx);
- CreateSourceCode(diagnostic_manager, exe_ctx, imported_modules,
+ CreateSourceCode(diagnostic_manager, exe_ctx, m_imported_cpp_modules,
for_completion);
return true;
}
-bool ClangUserExpression::Parse(DiagnosticManager &diagnostic_manager,
- ExecutionContext &exe_ctx,
- lldb_private::ExecutionPolicy execution_policy,
- bool keep_result_in_memory,
- bool generate_debug_info) {
- Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS));
-
- if (!PrepareForParsing(diagnostic_manager, exe_ctx, /*for_completion*/ false))
- return false;
-
- LLDB_LOGF(log, "Parsing the following code:\n%s", m_transformed_text.c_str());
-
- ////////////////////////////////////
- // Set up the target and compiler
- //
-
- Target *target = exe_ctx.GetTargetPtr();
-
- if (!target) {
- diagnostic_manager.PutString(eDiagnosticSeverityError, "invalid target");
- return false;
- }
-
- //////////////////////////
- // Parse the expression
- //
-
+bool ClangUserExpression::TryParse(
+ DiagnosticManager &diagnostic_manager, ExecutionContextScope *exe_scope,
+ ExecutionContext &exe_ctx, lldb_private::ExecutionPolicy execution_policy,
+ bool keep_result_in_memory, bool generate_debug_info) {
m_materializer_up = std::make_unique<Materializer>();
ResetDeclMap(exe_ctx, m_result_delegate, keep_result_in_memory);
@@ -612,26 +576,16 @@
DeclMap()->SetLookupsEnabled(true);
}
- Process *process = exe_ctx.GetProcessPtr();
- ExecutionContextScope *exe_scope = process;
-
- if (!exe_scope)
- exe_scope = exe_ctx.GetTargetPtr();
-
- // 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.
-
- ClangExpressionParser parser(exe_scope, *this, generate_debug_info,
- m_include_directories, m_filename);
+ m_parser = std::make_unique<ClangExpressionParser>(
+ exe_scope, *this, generate_debug_info, m_include_directories, m_filename);
- unsigned num_errors = parser.Parse(diagnostic_manager);
+ unsigned num_errors = m_parser->Parse(diagnostic_manager);
// 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)) {
+ if (m_parser->RewriteExpression(diagnostic_manager)) {
size_t fixed_start;
size_t fixed_end;
m_fixed_text = diagnostic_manager.GetFixedExpression();
@@ -652,7 +606,7 @@
//
{
- Status jit_error = parser.PrepareForExecution(
+ Status jit_error = m_parser->PrepareForExecution(
m_jit_start_addr, m_jit_end_addr, m_execution_unit_sp, exe_ctx,
m_can_interpret, execution_policy);
@@ -666,10 +620,91 @@
return false;
}
}
+ return true;
+}
+
+void ClangUserExpression::SetupCppModuleImports(ExecutionContext &exe_ctx) {
+ Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS));
+
+ CppModuleConfiguration module_config = GetModuleConfig(m_language, exe_ctx);
+ m_imported_cpp_modules = module_config.GetImportedModules();
+ m_include_directories = module_config.GetIncludeDirs();
+
+ LLDB_LOG(log, "List of imported modules in expression: {0}",
+ llvm::make_range(m_imported_cpp_modules.begin(),
+ m_imported_cpp_modules.end()));
+ LLDB_LOG(log, "List of include directories gathered for modules: {0}",
+ llvm::make_range(m_include_directories.begin(),
+ m_include_directories.end()));
+}
+
+static bool shouldRetryWithCppModule(Target &target, ExecutionPolicy exe_policy) {
+ // Top-level expression don't yet support importing C++ modules.
+ if (exe_policy == ExecutionPolicy::eExecutionPolicyTopLevel)
+ return false;
+ return target.GetImportStdModule() == eImportStdModuleFallback;
+}
+
+bool ClangUserExpression::Parse(DiagnosticManager &diagnostic_manager,
+ ExecutionContext &exe_ctx,
+ lldb_private::ExecutionPolicy execution_policy,
+ bool keep_result_in_memory,
+ bool generate_debug_info) {
+ Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS));
+
+ if (!PrepareForParsing(diagnostic_manager, exe_ctx, /*for_completion*/ false))
+ return false;
+
+ LLDB_LOGF(log, "Parsing the following code:\n%s", m_transformed_text.c_str());
+
+ ////////////////////////////////////
+ // Set up the target and compiler
+ //
+
+ Target *target = exe_ctx.GetTargetPtr();
+
+ if (!target) {
+ diagnostic_manager.PutString(eDiagnosticSeverityError, "invalid target");
+ return false;
+ }
+
+ //////////////////////////
+ // Parse the expression
+ //
+
+ Process *process = exe_ctx.GetProcessPtr();
+ ExecutionContextScope *exe_scope = process;
+
+ if (!exe_scope)
+ exe_scope = exe_ctx.GetTargetPtr();
+
+ bool parse_success = TryParse(diagnostic_manager, exe_scope, exe_ctx,
+ execution_policy, keep_result_in_memory,
+ generate_debug_info);
+ // If the expression failed to parse, check if retrying parsing with a loaded
+ // C++ module is possible.
+ if (!parse_success && shouldRetryWithCppModule(*target, execution_policy)) {
+ // Load the loaded C++ modules.
+ SetupCppModuleImports(exe_ctx);
+ // If we did load any modules, then retry parsing.
+ if (!m_imported_cpp_modules.empty()) {
+ // The module imports are injected into the source code wrapper,
+ // so recreate those.
+ CreateSourceCode(diagnostic_manager, exe_ctx, m_imported_cpp_modules,
+ /*for_completion*/ false);
+ // Clear the error diagnostics from the previous parse attempt.
+ diagnostic_manager.Clear();
+ parse_success = TryParse(diagnostic_manager, exe_scope, exe_ctx,
+ execution_policy, keep_result_in_memory,
+ generate_debug_info);
+ }
+ }
+ if (!parse_success)
+ return false;
if (exe_ctx.GetProcessPtr() && execution_policy == eExecutionPolicyTopLevel) {
Status static_init_error =
- parser.RunStaticInitializers(m_execution_unit_sp, exe_ctx);
+ m_parser->RunStaticInitializers(m_execution_unit_sp, exe_ctx);
if (!static_init_error.Success()) {
const char *error_cstr = static_init_error.AsCString();
Index: lldb/include/lldb/Target/Target.h
===================================================================
--- lldb/include/lldb/Target/Target.h
+++ lldb/include/lldb/Target/Target.h
@@ -65,6 +65,12 @@
eLoadDependentsNo,
};
+enum ImportStdModule {
+ eImportStdModuleFalse,
+ eImportStdModuleFallback,
+ eImportStdModuleTrue,
+};
+
class TargetExperimentalProperties : public Properties {
public:
TargetExperimentalProperties();
@@ -135,7 +141,7 @@
bool GetEnableAutoImportClangModules() const;
- bool GetEnableImportStdModule() const;
+ ImportStdModule GetImportStdModule() const;
bool GetEnableAutoApplyFixIts() const;
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
