[Lldb-commits] [PATCH] D48339: Refactor ClangUserExpression::Parse [NFC]

[Raphael Isemann via Phabricator via lldb-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL336734: Refactor ClangUserExpression::Parse [NFC] (authored 
by teemperor, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D48339?vs=151990=154897#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D48339

Files:
  lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
  lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangUserExpression.h

Index: lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangUserExpression.h
===
--- lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangUserExpression.h
+++ lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangUserExpression.h
@@ -174,6 +174,13 @@
 lldb::addr_t struct_address,
 DiagnosticManager _manager) override;
 
+  llvm::Optional GetLanguageForExpr(
+  DiagnosticManager _manager, ExecutionContext _ctx);
+  bool SetupPersistentState(DiagnosticManager _manager,
+   ExecutionContext _ctx);
+  bool PrepareForParsing(DiagnosticManager _manager,
+ ExecutionContext _ctx);
+
   ClangUserExpressionHelper m_type_system_helper;
 
   class ResultDelegate : public Materializer::PersistentVariableDelegate {
Index: lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
===
--- lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
+++ lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
@@ -322,17 +322,8 @@
 };
 } // namespace
 
-bool ClangUserExpression::Parse(DiagnosticManager _manager,
-ExecutionContext _ctx,
-lldb_private::ExecutionPolicy execution_policy,
-bool keep_result_in_memory,
-bool generate_debug_info) {
-  Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS));
-
-  Status err;
-
-  InstallContext(exe_ctx);
-
+bool ClangUserExpression::SetupPersistentState(DiagnosticManager _manager,
+ ExecutionContext _ctx) {
   if (Target *target = exe_ctx.GetTargetPtr()) {
 if (PersistentExpressionState *persistent_state =
 target->GetPersistentExpressionStateForLanguage(
@@ -349,35 +340,24 @@
  "error: couldn't start parsing (no target)");
 return false;
   }
+  return true;
+}
 
-  ScanContext(exe_ctx, err);
-
-  if (!err.Success()) {
-diagnostic_manager.PutString(eDiagnosticSeverityWarning, err.AsCString());
-  }
-
-  
-  // Generate the expression
-  //
-
-  ApplyObjcCastHack(m_expr_text);
-
-  std::string prefix = m_expr_prefix;
-
+static void SetupDeclVendor(ExecutionContext _ctx, Target *target) {
   if (ClangModulesDeclVendor *decl_vendor =
-  m_target->GetClangModulesDeclVendor()) {
+  target->GetClangModulesDeclVendor()) {
 const ClangModulesDeclVendor::ModuleVector _imported_modules =
 llvm::cast(
-m_target->GetPersistentExpressionStateForLanguage(
+target->GetPersistentExpressionStateForLanguage(
 lldb::eLanguageTypeC))
 ->GetHandLoadedClangModules();
 ClangModulesDeclVendor::ModuleVector modules_for_macros;
 
 for (ClangModulesDeclVendor::ModuleID module : hand_imported_modules) {
   modules_for_macros.push_back(module);
 }
 
-if (m_target->GetEnableAutoImportClangModules()) {
+if (target->GetEnableAutoImportClangModules()) {
   if (StackFrame *frame = exe_ctx.GetFramePtr()) {
 if (Block *block = frame->GetFrameBlock()) {
   SymbolContext sc;
@@ -394,8 +374,13 @@
   }
 }
   }
+}
+
+llvm::Optional ClangUserExpression::GetLanguageForExpr(
+DiagnosticManager _manager, ExecutionContext _ctx) {
+  lldb::LanguageType lang_type = lldb::LanguageType::eLanguageTypeUnknown;
 
-  lldb::LanguageType lang_type = lldb::eLanguageTypeUnknown;
+  std::string prefix = m_expr_prefix;
 
   if (m_options.GetExecutionPolicy() == eExecutionPolicyTopLevel) {
 m_transformed_text = m_expr_text;
@@ -415,9 +400,50 @@
   exe_ctx)) {
   diagnostic_manager.PutString(eDiagnosticSeverityError,
"couldn't construct expression body");
-  return false;
+  return llvm::Optional();
 }
   }
+  return lang_type;
+}
+
+bool ClangUserExpression::PrepareForParsing(
+DiagnosticManager _manager, ExecutionContext _ctx) {
+  InstallContext(exe_ctx);
+
+  if (!SetupPersistentState(diagnostic_manager, exe_ctx))
+return false;
+
+  Status err;
+  ScanContext(exe_ctx, err);
+
+  if 

[Lldb-commits] [PATCH] D48339: Refactor ClangUserExpression::Parse [NFC]

[Raphael Isemann via Phabricator via lldb-commits]

teemperor created this revision.

This patch splits out functionality from the `Parse` method into different 
methods.
This benefits the code completion work (which should reuse those methods) and 
makes the
code a bit more readable.

Note that this patch is as minimal as possible. Some of the code in the new 
methods definitely
needs more refactoring.


https://reviews.llvm.org/D48339

Files:
  source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
  source/Plugins/ExpressionParser/Clang/ClangUserExpression.h

Index: source/Plugins/ExpressionParser/Clang/ClangUserExpression.h
===
--- source/Plugins/ExpressionParser/Clang/ClangUserExpression.h
+++ source/Plugins/ExpressionParser/Clang/ClangUserExpression.h
@@ -174,6 +174,13 @@
 lldb::addr_t struct_address,
 DiagnosticManager _manager) override;
 
+  llvm::Optional GetLanguageForExpr(
+  DiagnosticManager _manager, ExecutionContext _ctx);
+  bool SetupPersistentState(DiagnosticManager _manager,
+   ExecutionContext _ctx);
+  bool PrepareForParsing(DiagnosticManager _manager,
+ ExecutionContext _ctx);
+
   ClangUserExpressionHelper m_type_system_helper;
 
   class ResultDelegate : public Materializer::PersistentVariableDelegate {
Index: source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
===
--- source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
+++ source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
@@ -322,17 +322,8 @@
 };
 } // namespace
 
-bool ClangUserExpression::Parse(DiagnosticManager _manager,
-ExecutionContext _ctx,
-lldb_private::ExecutionPolicy execution_policy,
-bool keep_result_in_memory,
-bool generate_debug_info) {
-  Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS));
-
-  Status err;
-
-  InstallContext(exe_ctx);
-
+bool ClangUserExpression::SetupPersistentState(DiagnosticManager _manager,
+ ExecutionContext _ctx) {
   if (Target *target = exe_ctx.GetTargetPtr()) {
 if (PersistentExpressionState *persistent_state =
 target->GetPersistentExpressionStateForLanguage(
@@ -349,35 +340,24 @@
  "error: couldn't start parsing (no target)");
 return false;
   }
+  return true;
+}
 
-  ScanContext(exe_ctx, err);
-
-  if (!err.Success()) {
-diagnostic_manager.PutString(eDiagnosticSeverityWarning, err.AsCString());
-  }
-
-  
-  // Generate the expression
-  //
-
-  ApplyObjcCastHack(m_expr_text);
-
-  std::string prefix = m_expr_prefix;
-
+static void SetupDeclVendor(ExecutionContext _ctx, Target *target) {
   if (ClangModulesDeclVendor *decl_vendor =
-  m_target->GetClangModulesDeclVendor()) {
+  target->GetClangModulesDeclVendor()) {
 const ClangModulesDeclVendor::ModuleVector _imported_modules =
 llvm::cast(
-m_target->GetPersistentExpressionStateForLanguage(
+target->GetPersistentExpressionStateForLanguage(
 lldb::eLanguageTypeC))
 ->GetHandLoadedClangModules();
 ClangModulesDeclVendor::ModuleVector modules_for_macros;
 
 for (ClangModulesDeclVendor::ModuleID module : hand_imported_modules) {
   modules_for_macros.push_back(module);
 }
 
-if (m_target->GetEnableAutoImportClangModules()) {
+if (target->GetEnableAutoImportClangModules()) {
   if (StackFrame *frame = exe_ctx.GetFramePtr()) {
 if (Block *block = frame->GetFrameBlock()) {
   SymbolContext sc;
@@ -394,8 +374,13 @@
   }
 }
   }
+}
+
+llvm::Optional ClangUserExpression::GetLanguageForExpr(
+DiagnosticManager _manager, ExecutionContext _ctx) {
+  lldb::LanguageType lang_type = lldb::LanguageType::eLanguageTypeUnknown;
 
-  lldb::LanguageType lang_type = lldb::eLanguageTypeUnknown;
+  std::string prefix = m_expr_prefix;
 
   if (m_options.GetExecutionPolicy() == eExecutionPolicyTopLevel) {
 m_transformed_text = m_expr_text;
@@ -415,9 +400,50 @@
   exe_ctx)) {
   diagnostic_manager.PutString(eDiagnosticSeverityError,
"couldn't construct expression body");
-  return false;
+  return llvm::Optional();
 }
   }
+  return lang_type;
+}
+
+bool ClangUserExpression::PrepareForParsing(
+DiagnosticManager _manager, ExecutionContext _ctx) {
+  InstallContext(exe_ctx);
+
+  if (!SetupPersistentState(diagnostic_manager, exe_ctx))
+return false;
+
+  Status err;
+  ScanContext(exe_ctx, err);
+
+  if (!err.Success()) {
+diagnostic_manager.PutString(eDiagnosticSeverityWarning, err.AsCString());
+  }
+
+  
+  //