[Lldb-commits] [PATCH] D15527: Add ability to override JIT expr compiler options.

[Greg Clayton via lldb-commits]

clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

See inlined comments.


================
Comment at: source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp:85-97
@@ -83,1 +84,15 @@
 
+namespace {
+    void debugStringVector(Log *log, const std::vector<std::string>& vec, const char *name)
+    {
+        if(!log)
+            return;
+
+        log->Debug("Begin %s:", name);
+        for (const auto& s : vec)
+            log->Debug("%s", s.c_str());
+
+        log->Debug("End %s.", name);
+    }
+}
+
----------------
We have an internal class named StringList that is a vector of strings. It would be nice to use that class instead of a direct std::vector and place this call inside the StringList class as a new method.

================
Comment at: source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp:191
@@ -174,1 +190,3 @@
+    lldb_private::LanguageRuntime::OverrideExprOptions *target_opts_override = nullptr;
+    lldb_private::LanguageRuntime *lang_rt = nullptr;
     lldb::TargetSP target_sp;
----------------
We probably shouldn't be using LanguageRuntime, but Language. LanguageRuntime is for things that are dynamically in a process for a given langauge. For example the ObjectiveC language runtime has the dynamic class table of all classes, methods, and other info and this info can be queried from the runtime by looking in the objective C shared library and grabbing stuff from live memory. Language is more for the "this stuff is always this way for this specific language". If you are reading something from memory from the process in order to determine which language options should be enabled, then this would belong in LanguageRuntime. We used to only have lldb_private::LanguageRuntime and we recently added lldb_private::Language, so there might be some stuff that is in the wrong spot inside LangaugeRuntime, but now that we have lldb_private::Language we need to use it correctly.

================
Comment at: source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp:204
@@ -184,3 +203,3 @@
     {
-        std::string triple = target_sp->GetArchitecture().GetTriple().str();
-        m_compiler->getTargetOpts().Triple = triple;
+        lang_rt = exe_scope->CalculateProcess()->GetLanguageRuntime(frame_lang);
+        if (log)
----------------
Should we actually use the frame language if the user specified another language? The user might type:

```
(lldb) expression --language=swift -- 2+3
```

and we might be sitting in a C frame. We probably need to use the expression language from the options if it is set. 


Repository:
  rL LLVM

http://reviews.llvm.org/D15527