Refactor ClangUserExpression::GetLanguageForExpr

[teemperor]

Summary

The ClangUserExpression::GetLanguageForExpr method is currently a big
source of sadness, as it's name implies that it's an accessor method, but it actually
is also initializing some variables that we need for parsing. This caused that we
currently call this getter just for it's side effects while ignoring it's return value,
which is confusing for the reader.

This patch renames it to UpdateLanguageForExpr and merges all calls to the
method into a single call in ClangUserExpression::PrepareForParsing (as calling
this method is anyway mandatory for parsing to succeed)

While looking at the code, I also found that we actually have two language
variables in this class hierarchy. The normal Language from the UserExpression
class and the LanguageForExpr that we implemented in this subclass. Both
don't seem to actually contain the same value, so we probably should look at this
next.

[xbolva00]

looks fine, thanks :)

source/Plugins/ExpressionParser/Clang/ClangUserExpression.h

private:
 
   void ScanContext(ExecutionContext &exe_ctx,
                    lldb_private::Status &err) override;
 
   bool AddArguments(ExecutionContext &exe_ctx, std::vector<lldb::addr_t> &args,
                     lldb::addr_t struct_address,
                     DiagnosticManager &diagnostic_manager) override;
 
-  llvm::Optional<lldb::LanguageType> GetLanguageForExpr(
-      DiagnosticManager &diagnostic_manager, ExecutionContext &exe_ctx);
+  void UpdateLanguageForExpr(DiagnosticManager &diagnostic_manager,
+                             ExecutionContext &exe_ctx);
   bool SetupPersistentState(DiagnosticManager &diagnostic_manager,
                                    ExecutionContext &exe_ctx);
   bool PrepareForParsing(DiagnosticManager &diagnostic_manager,
                          ExecutionContext &exe_ctx);
 
   ClangUserExpressionHelper m_type_system_helper;
 
   class ResultDelegate : public Materializer::PersistentVariableDelegate {
   public:
     ResultDelegate(lldb::TargetSP target) : m_target_sp(target) {}
     ConstString GetName() override;
     void DidDematerialize(lldb::ExpressionVariableSP &variable) override;
 
     void RegisterPersistentState(PersistentExpressionState *persistent_state);
     lldb::ExpressionVariableSP &GetVariable();
 
   private:
     PersistentExpressionState *m_persistent_state;
     lldb::ExpressionVariableSP m_variable;
     lldb::TargetSP m_target_sp;
   };
 
+  /// The language type of the current expression.
+  lldb::LanguageType m_expr_lang = lldb::eLanguageTypeUnknown;
+
   /// The absolute character position in the transformed source code where the
   /// user code (as typed by the user) starts. If the variable is empty, then we
   /// were not able to calculate this position.
   llvm::Optional<size_t> m_user_expression_start_pos;
   ResultDelegate m_result_delegate;
 };
 
 } // namespace lldb_private
 
 #endif // liblldb_ClangUserExpression_h_

source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp

if (target->GetEnableAutoImportClangModules()) {
                 *sc.comp_unit, modules_for_macros, error_stream);
           }
         }
       }
     }
   }
 }
 
-llvm::Optional<lldb::LanguageType> ClangUserExpression::GetLanguageForExpr(
+void ClangUserExpression::UpdateLanguageForExpr(
     DiagnosticManager &diagnostic_manager, ExecutionContext &exe_ctx) {
-  lldb::LanguageType lang_type = lldb::LanguageType::eLanguageTypeUnknown;
+  m_expr_lang = lldb::LanguageType::eLanguageTypeUnknown;
 
   std::string prefix = m_expr_prefix;
 
   if (m_options.GetExecutionPolicy() == eExecutionPolicyTopLevel) {
     m_transformed_text = m_expr_text;
   } else {
     std::unique_ptr<ExpressionSourceCode> source_code(
         ExpressionSourceCode::CreateWrapped(prefix.c_str(),
                                             m_expr_text.c_str()));
 
     if (m_in_cplusplus_method)
-      lang_type = lldb::eLanguageTypeC_plus_plus;
+      m_expr_lang = lldb::eLanguageTypeC_plus_plus;
     else if (m_in_objectivec_method)
-      lang_type = lldb::eLanguageTypeObjC;
+      m_expr_lang = lldb::eLanguageTypeObjC;
     else
-      lang_type = lldb::eLanguageTypeC;
+      m_expr_lang = lldb::eLanguageTypeC;
 
-    if (!source_code->GetText(m_transformed_text, lang_type, m_in_static_method,
-                              exe_ctx)) {
+    if (!source_code->GetText(m_transformed_text, m_expr_lang,
+                              m_in_static_method, exe_ctx)) {
       diagnostic_manager.PutString(eDiagnosticSeverityError,
                                    "couldn't construct expression body");
-      return llvm::Optional<lldb::LanguageType>();
+      return;
     }
 
     // Find and store the start position of the original code inside the
     // transformed code. We need this later for the code completion.
     std::size_t original_start;
     std::size_t original_end;
     bool found_bounds = source_code->GetOriginalBodyBounds(
-        m_transformed_text, lang_type, original_start, original_end);
+        m_transformed_text, m_expr_lang, original_start, original_end);
     if (found_bounds) {
       m_user_expression_start_pos = original_start;
     }
   }
-  return lang_type;
 }
 
 bool ClangUserExpression::PrepareForParsing(
     DiagnosticManager &diagnostic_manager, ExecutionContext &exe_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());
   }
 
   ////////////////////////////////////
   // Generate the expression
   //
 
   ApplyObjcCastHack(m_expr_text);
 
   SetupDeclVendor(exe_ctx, m_target);
+
+  UpdateLanguageForExpr(diagnostic_manager, exe_ctx);
   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))
     return false;
 
-  lldb::LanguageType lang_type = lldb::LanguageType::eLanguageTypeUnknown;
-  if (auto new_lang = GetLanguageForExpr(diagnostic_manager, exe_ctx)) {
-    lang_type = new_lang.getValue();
-  }
-
   if (log)
     log->Printf("Parsing the following code:\n%s", m_transformed_text.c_str());
 
   ////////////////////////////////////
   // Set up the target and compiler
   //
 
   Target *target = exe_ctx.GetTargetPtr();
bool ClangUserExpression::Parse(DiagnosticManager &diagnostic_manager,
   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))
+                fixed_expression, m_expr_lang, fixed_start, fixed_end))
           m_fixed_text =
               fixed_expression.substr(fixed_start, fixed_end - fixed_start);
       }
     }
     return false;
   }
 
   //////////////////////////////////////////////////////////////////////////////////////////
bool ClangUserExpression::Complete(ExecutionContext &exe_ctx,
   // We don't want any visible feedback when completing an expression. Mostly
   // because the results we get from an incomplete invocation are probably not
   // correct.
   DiagnosticManager diagnostic_manager;
 
   if (!PrepareForParsing(diagnostic_manager, exe_ctx))
     return false;
 
-  GetLanguageForExpr(diagnostic_manager, exe_ctx);
-
   if (log)
     log->Printf("Parsing the following code:\n%s", m_transformed_text.c_str());
 
   //////////////////////////
   // Parse the expression
   //
 
   m_materializer_ap.reset(new Materializer());