[Lldb-commits] [lldb] 2b37c5b - [lldb][NFC] Make ClangExpressionSourceCode's wrapping logic more consistent
Raphael Isemann via lldb-commits
lldb-commits at lists.llvm.org
Mon Jun 1 04:25:00 PDT 2020
Author: Raphael Isemann
Date: 2020-06-01T13:24:30+02:00
New Revision: 2b37c5b560584f05edf5d375d4ca86fe9c5b0173
URL: https://github.com/llvm/llvm-project/commit/2b37c5b560584f05edf5d375d4ca86fe9c5b0173
DIFF: https://github.com/llvm/llvm-project/commit/2b37c5b560584f05edf5d375d4ca86fe9c5b0173.diff
LOG: [lldb][NFC] Make ClangExpressionSourceCode's wrapping logic more consistent
Summary:
ClangExpressionSourceCode has different ways to wrap the user expression based on
which context the expression is executed in. For example, if we're in a C++ member
function we put the expression inside a fake member function of a fake class to make the
evaluation possible. Similar things are done for Objective-C instance/static methods.
There is also a default wrapping where we put the expression in a normal function
just to make it possible to execute it.
The way we currently define which kind of wrapping the expression needs is based on
the `wrapping_language` we keep passing to the ClangExpressionSourceCode
instance. We repurposed the language type enum for that variable to distinguish the
cases above with the following mapping:
* language = C_plus_plus -> member function wrapping
* language = ObjC -> instance/static method wrapping (`is_static` distinguished between those two).
* language = C -> normal function wrapping
* all other cases like C_plus_plus11, Haskell etc. make our class a no-op that does mostly nothing.
That mapping is currently not documented and just confusing as the `language`
is unrelated to the expression language (and in the ClangUserExpression we even pretend
that it *is* the actual language, but luckily never used it for anything). Some of the code
in ClangExpressionSourceCode is also obviously thinking that this is the actual language of
the expression as it checks for non-existent cases such as `ObjC_plus_plus` which is
not part of the mapping.
This patch makes a new enum to describe the four cases above (with instance/static Objective-C
methods now being their own case). It also make that enum just a member of
ClangExpressionSourceCode instead of having to pass the same value to the class repeatedly.
This gets also rid of all the switch-case-checks for 'unknown' language such as C_plus_plus11 as this
is no longer necessary.
Reviewers: labath, JDevlieghere
Reviewed By: labath
Subscribers: abidh
Differential Revision: https://reviews.llvm.org/D80793
Added:
Modified:
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.h
lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.h
Removed:
################################################################################
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
index 41d2b4adf3ca..a429963277d1 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
@@ -174,8 +174,8 @@ static void AddMacros(const DebugMacros *dm, CompileUnit *comp_unit,
lldb_private::ClangExpressionSourceCode::ClangExpressionSourceCode(
llvm::StringRef filename, llvm::StringRef name, llvm::StringRef prefix,
- llvm::StringRef body, Wrapping wrap)
- : ExpressionSourceCode(name, prefix, body, wrap) {
+ llvm::StringRef body, Wrapping wrap, WrapKind wrap_kind)
+ : ExpressionSourceCode(name, prefix, body, wrap), m_wrap_kind(wrap_kind) {
// Use #line markers to pretend that we have a single-line source file
// containing only the user expression. This will hide our wrapper code
// from the user when we render diagnostics with Clang.
@@ -261,10 +261,9 @@ TokenVerifier::TokenVerifier(std::string body) {
}
}
-static void AddLocalVariableDecls(const lldb::VariableListSP &var_list_sp,
- StreamString &stream,
- const std::string &expr,
- lldb::LanguageType wrapping_language) {
+void ClangExpressionSourceCode::AddLocalVariableDecls(
+ const lldb::VariableListSP &var_list_sp, StreamString &stream,
+ const std::string &expr) const {
TokenVerifier tokens(expr);
for (size_t i = 0; i < var_list_sp->GetSize(); i++) {
@@ -281,13 +280,12 @@ static void AddLocalVariableDecls(const lldb::VariableListSP &var_list_sp,
if (!expr.empty() && !tokens.hasToken(var_name.GetStringRef()))
continue;
- if ((var_name == "self" || var_name == "_cmd") &&
- (wrapping_language == lldb::eLanguageTypeObjC ||
- wrapping_language == lldb::eLanguageTypeObjC_plus_plus))
+ const bool is_objc = m_wrap_kind == WrapKind::ObjCInstanceMethod ||
+ m_wrap_kind == WrapKind::ObjCStaticMethod;
+ if ((var_name == "self" || var_name == "_cmd") && is_objc)
continue;
- if (var_name == "this" &&
- wrapping_language == lldb::eLanguageTypeC_plus_plus)
+ if (var_name == "this" && m_wrap_kind == WrapKind::CppMemberFunction)
continue;
stream.Printf("using $__lldb_local_vars::%s;\n", var_name.AsCString());
@@ -295,9 +293,8 @@ static void AddLocalVariableDecls(const lldb::VariableListSP &var_list_sp,
}
bool ClangExpressionSourceCode::GetText(
- std::string &text, lldb::LanguageType wrapping_language, bool static_method,
- ExecutionContext &exe_ctx, bool add_locals, bool force_add_all_locals,
- llvm::ArrayRef<std::string> modules) const {
+ std::string &text, ExecutionContext &exe_ctx, bool add_locals,
+ bool force_add_all_locals, llvm::ArrayRef<std::string> modules) const {
const char *target_specific_defines = "typedef signed char BOOL;\n";
std::string module_macros;
@@ -374,21 +371,11 @@ bool ClangExpressionSourceCode::GetText(
lldb::VariableListSP var_list_sp =
frame->GetInScopeVariableList(false, true);
AddLocalVariableDecls(var_list_sp, lldb_local_var_decls,
- force_add_all_locals ? "" : m_body,
- wrapping_language);
+ force_add_all_locals ? "" : m_body);
}
}
if (m_wrap) {
- switch (wrapping_language) {
- default:
- return false;
- case lldb::eLanguageTypeC:
- case lldb::eLanguageTypeC_plus_plus:
- case lldb::eLanguageTypeObjC:
- break;
- }
-
// Generate a list of @import statements that will import the specified
// module into our expression.
std::string module_imports;
@@ -407,22 +394,12 @@ bool ClangExpressionSourceCode::GetText(
// 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(m_start_marker);
- tagged_body.append(m_body);
- tagged_body.append(m_end_marker);
- break;
- }
- switch (wrapping_language) {
- default:
- break;
- case lldb::eLanguageTypeC:
+ tagged_body.append(m_start_marker);
+ tagged_body.append(m_body);
+ tagged_body.append(m_end_marker);
+
+ switch (m_wrap_kind) {
+ case WrapKind::Function:
wrap_stream.Printf("%s"
"void \n"
"%s(void *$__lldb_arg) \n"
@@ -433,7 +410,7 @@ bool ClangExpressionSourceCode::GetText(
module_imports.c_str(), m_name.c_str(),
lldb_local_var_decls.GetData(), tagged_body.c_str());
break;
- case lldb::eLanguageTypeC_plus_plus:
+ case WrapKind::CppMemberFunction:
wrap_stream.Printf("%s"
"void \n"
"$__lldb_class::%s(void *$__lldb_arg) \n"
@@ -444,38 +421,38 @@ bool ClangExpressionSourceCode::GetText(
module_imports.c_str(), m_name.c_str(),
lldb_local_var_decls.GetData(), tagged_body.c_str());
break;
- case lldb::eLanguageTypeObjC:
- if (static_method) {
- wrap_stream.Printf(
- "%s"
- "@interface $__lldb_objc_class ($__lldb_category) \n"
- "+(void)%s:(void *)$__lldb_arg; \n"
- "@end \n"
- "@implementation $__lldb_objc_class ($__lldb_category) \n"
- "+(void)%s:(void *)$__lldb_arg \n"
- "{ \n"
- " %s; \n"
- "%s"
- "} \n"
- "@end \n",
- module_imports.c_str(), m_name.c_str(), m_name.c_str(),
- lldb_local_var_decls.GetData(), tagged_body.c_str());
- } else {
- wrap_stream.Printf(
- "%s"
- "@interface $__lldb_objc_class ($__lldb_category) \n"
- "-(void)%s:(void *)$__lldb_arg; \n"
- "@end \n"
- "@implementation $__lldb_objc_class ($__lldb_category) \n"
- "-(void)%s:(void *)$__lldb_arg \n"
- "{ \n"
- " %s; \n"
- "%s"
- "} \n"
- "@end \n",
- module_imports.c_str(), m_name.c_str(), m_name.c_str(),
- lldb_local_var_decls.GetData(), tagged_body.c_str());
- }
+ case WrapKind::ObjCInstanceMethod:
+ wrap_stream.Printf(
+ "%s"
+ "@interface $__lldb_objc_class ($__lldb_category) \n"
+ "-(void)%s:(void *)$__lldb_arg; \n"
+ "@end \n"
+ "@implementation $__lldb_objc_class ($__lldb_category) \n"
+ "-(void)%s:(void *)$__lldb_arg \n"
+ "{ \n"
+ " %s; \n"
+ "%s"
+ "} \n"
+ "@end \n",
+ module_imports.c_str(), m_name.c_str(), m_name.c_str(),
+ lldb_local_var_decls.GetData(), tagged_body.c_str());
+ break;
+
+ case WrapKind::ObjCStaticMethod:
+ wrap_stream.Printf(
+ "%s"
+ "@interface $__lldb_objc_class ($__lldb_category) \n"
+ "+(void)%s:(void *)$__lldb_arg; \n"
+ "@end \n"
+ "@implementation $__lldb_objc_class ($__lldb_category) \n"
+ "+(void)%s:(void *)$__lldb_arg \n"
+ "{ \n"
+ " %s; \n"
+ "%s"
+ "} \n"
+ "@end \n",
+ module_imports.c_str(), m_name.c_str(), m_name.c_str(),
+ lldb_local_var_decls.GetData(), tagged_body.c_str());
break;
}
@@ -488,17 +465,7 @@ bool ClangExpressionSourceCode::GetText(
}
bool ClangExpressionSourceCode::GetOriginalBodyBounds(
- std::string transformed_text, lldb::LanguageType wrapping_language,
- size_t &start_loc, size_t &end_loc) {
- switch (wrapping_language) {
- default:
- return false;
- case lldb::eLanguageTypeC:
- case lldb::eLanguageTypeC_plus_plus:
- case lldb::eLanguageTypeObjC:
- break;
- }
-
+ std::string transformed_text, size_t &start_loc, size_t &end_loc) {
start_loc = transformed_text.find(m_start_marker);
if (start_loc == std::string::npos)
return false;
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.h b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.h
index bb2e6346a49c..9a54f0e3ad8d 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.h
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.h
@@ -28,20 +28,30 @@ class ClangExpressionSourceCode : public ExpressionSourceCode {
static const llvm::StringRef g_prefix_file_name;
static const char *g_expression_prefix;
+ /// The possible ways an expression can be wrapped.
+ enum class WrapKind {
+ /// Wrapped in a non-static member function of a C++ class.
+ CppMemberFunction,
+ /// Wrapped in an instance Objective-C method.
+ ObjCInstanceMethod,
+ /// Wrapped in a static Objective-C method.
+ ObjCStaticMethod,
+ /// Wrapped in a non-member function.
+ /// Note that this is also used for static member functions of a C++ class.
+ Function
+ };
+
static ClangExpressionSourceCode *CreateWrapped(llvm::StringRef filename,
llvm::StringRef prefix,
- llvm::StringRef body) {
+ llvm::StringRef body,
+ WrapKind wrap_kind) {
return new ClangExpressionSourceCode(filename, "$__lldb_expr", prefix, body,
- Wrap);
+ Wrap, wrap_kind);
}
/// Generates the source code that will evaluate the expression.
///
/// \param text output parameter containing the source code string.
- /// \param wrapping_language If the expression is supossed to be wrapped,
- /// then this is the language that should be used for that.
- /// \param static_method True iff the expression is valuated inside a static
- /// Objective-C method.
/// \param exe_ctx The execution context in which the expression will be
/// evaluated.
/// \param add_locals True iff local variables should be injected into the
@@ -51,8 +61,7 @@ class ClangExpressionSourceCode : public ExpressionSourceCode {
/// \param modules A list of (C++) modules that the expression should import.
///
/// \return true iff the source code was successfully generated.
- bool GetText(std::string &text, lldb::LanguageType wrapping_language,
- bool static_method, ExecutionContext &exe_ctx, bool add_locals,
+ bool GetText(std::string &text, ExecutionContext &exe_ctx, bool add_locals,
bool force_add_all_locals,
llvm::ArrayRef<std::string> modules) const;
@@ -60,19 +69,24 @@ class ClangExpressionSourceCode : public ExpressionSourceCode {
// passed to CreateWrapped. Return true if the bounds could be found. This
// will also work on text with FixItHints applied.
bool GetOriginalBodyBounds(std::string transformed_text,
- lldb::LanguageType wrapping_language,
size_t &start_loc, size_t &end_loc);
protected:
ClangExpressionSourceCode(llvm::StringRef filename, llvm::StringRef name,
llvm::StringRef prefix, llvm::StringRef body,
- Wrapping wrap);
+ Wrapping wrap, WrapKind wrap_kind);
private:
+ void AddLocalVariableDecls(const lldb::VariableListSP &var_list_sp,
+ StreamString &stream,
+ const std::string &expr) const;
+
/// String marking the start of the user expression.
std::string m_start_marker;
/// String marking the end of the user expression.
std::string m_end_marker;
+ /// How the expression has been wrapped.
+ const WrapKind m_wrap_kind;
};
} // namespace lldb_private
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
index f01357f10115..2d6de782ced2 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
@@ -397,16 +397,20 @@ static void SetupDeclVendor(ExecutionContext &exe_ctx, Target *target,
"current compilation unit.");
}
-void ClangUserExpression::UpdateLanguageForExpr() {
- m_expr_lang = lldb::LanguageType::eLanguageTypeUnknown;
- if (m_options.GetExecutionPolicy() == eExecutionPolicyTopLevel)
- return;
+ClangExpressionSourceCode::WrapKind ClangUserExpression::GetWrapKind() const {
+ assert(m_options.GetExecutionPolicy() != eExecutionPolicyTopLevel &&
+ "Top level expressions aren't wrapped.");
+ using Kind = ClangExpressionSourceCode::WrapKind;
if (m_in_cplusplus_method)
- m_expr_lang = lldb::eLanguageTypeC_plus_plus;
- else if (m_in_objectivec_method)
- m_expr_lang = lldb::eLanguageTypeObjC;
- else
- m_expr_lang = lldb::eLanguageTypeC;
+ return Kind::CppMemberFunction;
+ else if (m_in_objectivec_method) {
+ if (m_in_static_method)
+ return Kind::ObjCStaticMethod;
+ return Kind::ObjCInstanceMethod;
+ }
+ // Not in any kind of 'special' function, so just wrap it in a normal C
+ // function.
+ return Kind::Function;
}
void ClangUserExpression::CreateSourceCode(
@@ -420,10 +424,9 @@ void ClangUserExpression::CreateSourceCode(
m_transformed_text = m_expr_text;
} else {
m_source_code.reset(ClangExpressionSourceCode::CreateWrapped(
- m_filename, prefix, m_expr_text));
+ m_filename, prefix, m_expr_text, GetWrapKind()));
- if (!m_source_code->GetText(m_transformed_text, m_expr_lang,
- m_in_static_method, exe_ctx, !m_ctx_obj,
+ if (!m_source_code->GetText(m_transformed_text, exe_ctx, !m_ctx_obj,
for_completion, modules_to_import)) {
diagnostic_manager.PutString(eDiagnosticSeverityError,
"couldn't construct expression body");
@@ -435,7 +438,7 @@ void ClangUserExpression::CreateSourceCode(
std::size_t original_start;
std::size_t original_end;
bool found_bounds = m_source_code->GetOriginalBodyBounds(
- m_transformed_text, m_expr_lang, original_start, original_end);
+ m_transformed_text, original_start, original_end);
if (found_bounds)
m_user_expression_start_pos = original_start;
}
@@ -560,7 +563,6 @@ bool ClangUserExpression::PrepareForParsing(
llvm::make_range(m_include_directories.begin(),
m_include_directories.end()));
- UpdateLanguageForExpr();
CreateSourceCode(diagnostic_manager, exe_ctx, imported_modules,
for_completion);
return true;
@@ -635,9 +637,8 @@ bool ClangUserExpression::Parse(DiagnosticManager &diagnostic_manager,
m_fixed_text = diagnostic_manager.GetFixedExpression();
// Retrieve the original expression in case we don't have a top level
// expression (which has no surrounding source code).
- if (m_source_code &&
- m_source_code->GetOriginalBodyBounds(m_fixed_text, m_expr_lang,
- fixed_start, fixed_end))
+ if (m_source_code && m_source_code->GetOriginalBodyBounds(
+ m_fixed_text, fixed_start, fixed_end))
m_fixed_text =
m_fixed_text.substr(fixed_start, fixed_end - fixed_start);
}
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.h b/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.h
index d3073624cfa5..f734069655ef 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.h
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.h
@@ -185,7 +185,8 @@ class ClangUserExpression : public LLVMUserExpression {
ExecutionContext &exe_ctx,
std::vector<std::string> modules_to_import,
bool for_completion);
- void UpdateLanguageForExpr();
+ /// Defines how the current expression should be wrapped.
+ ClangExpressionSourceCode::WrapKind GetWrapKind() const;
bool SetupPersistentState(DiagnosticManager &diagnostic_manager,
ExecutionContext &exe_ctx);
bool PrepareForParsing(DiagnosticManager &diagnostic_manager,
@@ -208,8 +209,6 @@ class ClangUserExpression : public LLVMUserExpression {
lldb::TargetSP m_target_sp;
};
- /// The language type of the current expression.
- lldb::LanguageType m_expr_lang = lldb::eLanguageTypeUnknown;
/// The include directories that should be used when parsing the expression.
std::vector<std::string> m_include_directories;
More information about the lldb-commits
mailing list