[Lldb-commits] [PATCH] D11482: Add target setting to have the language follow the frame's CU.

[Greg Clayton]

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

I think it is fine to add support for looking at the target.language and using it if set, but I don't think we need a "target.language-follows-frame" setting at all. We should just do the right thing. But the right thing I think it means just asking the frame for its expression language. See inlined comments.


================
Comment at: include/lldb/Target/Target.h:175-177
@@ -174,2 +174,5 @@
 
+    bool
+    GetLanguageFollowsFrame () const;
+
     const char *
----------------
Remove this, we shouldn't need this setting.

================
Comment at: include/lldb/Target/Target.h:1443-1445
@@ -1439,2 +1442,5 @@
 
+    lldb::LanguageType
+    GetLanguageOfSelectedFrame () const;
+
     SourceManager &
----------------
This should be on lldb_private::StackFrame and should be:

```
lldb::LanguageType
StackFrame::GetDefaultExpressionLanguage();
```

================
Comment at: source/Commands/CommandObjectExpression.cpp:303-312
@@ -302,8 +302,12 @@
         options.SetDebug(m_command_options.debug);
-        
-        // If the language was not specified, set it from target's properties
+
+        // If the expression's language was not specified, check the
+        // target's properties to see if a language override was set,
+        // or if the language should match that of the frame.
         if (m_command_options.language != eLanguageTypeUnknown)
             options.SetLanguage(m_command_options.language);
-        else
+        else if (target->GetLanguage() != eLanguageTypeUnknown)
             options.SetLanguage(target->GetLanguage());
+        else if (target->GetLanguageFollowsFrame())
+            options.SetLanguage(target->GetLanguageOfSelectedFrame());
 
----------------
This should be:

```
if (m_command_options.language != eLanguageTypeUnknown)
    options.SetLanguage(m_command_options.language);
else
{
    StackFrame *frame = exe_ctx.GetFramePtr();
    if (frame)
        options.SetLanguage(frame-> GetDefaultExpressionLanguage());
}
````



================
Comment at: source/Target/Target.cpp:391-395
@@ -387,2 +390,7 @@
             language = GetLanguage();
+        // If the target's language override wasn't set but
+        // was set to follow the selected frame, use that.
+        if (language == lldb::eLanguageTypeUnknown &&
+            GetLanguageFollowsFrame())
+            language = GetLanguageOfSelectedFrame();
 
----------------
Remove this.

================
Comment at: source/Target/Target.cpp:430-434
@@ -419,2 +429,7 @@
             language = GetLanguage();
+        // If the target's language override wasn't set but
+        // was set to follow the selected frame, use that.
+        if (language == lldb::eLanguageTypeUnknown &&
+            GetLanguageFollowsFrame())
+            language = GetLanguageOfSelectedFrame();
 
----------------
Remove this.

================
Comment at: source/Target/Target.cpp:468-472
@@ -450,3 +467,7 @@
             language = GetLanguage();
-
+        // If the target's language override wasn't set but
+        // was set to follow the selected frame, use that.
+        if (language == lldb::eLanguageTypeUnknown &&
+            GetLanguageFollowsFrame())
+            language = GetLanguageOfSelectedFrame();
 
----------------
Remove this.

================
Comment at: source/Target/Target.cpp:2158-2178
@@ -2136,1 +2157,23 @@
 
+// Return the language of the selected frame's CU.
+lldb::LanguageType
+Target::GetLanguageOfSelectedFrame () const
+{
+    LanguageType language = eLanguageTypeUnknown;
+    if (m_process_sp)
+    {
+        ThreadSP sel_thread_sp(m_process_sp->GetThreadList().GetSelectedThread());
+        if (sel_thread_sp)
+        {
+            StackFrameSP sel_frame_sp = sel_thread_sp->GetSelectedFrame();
+            if (sel_frame_sp)
+            {
+                CompileUnit *cu = sel_frame_sp->GetSymbolContext(eSymbolContextCompUnit).comp_unit;
+                if (cu)
+                    language = cu->GetLanguage();
+            }
+        }
+    }
+    return language;
+}
+
----------------
This should be moved to the StackFrame as:

```
lldb::LanguageType
StackFrame::GetDefaultExpressionLanguage();
```

And it should check if the language is form of C, C++, ObjC, or ObjC ++, then return ObjC++, else return the language.

================
Comment at: source/Target/Target.cpp:2998
@@ -2955,1 +2997,3 @@
+    { "language"                           , OptionValue::eTypeLanguage  , false, eLanguageTypeUnknown      , NULL, NULL, "The language to use when interpreting expressions entered in commands." },
+    { "language-follows-frame"             , OptionValue::eTypeBoolean   , false, false                     , NULL, NULL, "If the target.language is unknown, default to the language of the selected frame." },
     { "expr-prefix"                        , OptionValue::eTypeFileSpec  , false, 0                         , NULL, NULL, "Path to a file containing expressions to be prepended to all expressions." },
----------------
I would prefer to not have this option as it doesn't really make sense and no one should have to worry about it.

================
Comment at: source/Target/Target.cpp:3058
@@ -3013,2 +3057,3 @@
     ePropertyLanguage,
+    ePropertyLanguageFollowsFrame,
     ePropertyExprPrefix,
----------------
Remove

================
Comment at: source/Target/Target.cpp:3516-3522
@@ -3470,2 +3515,9 @@
 
+bool
+TargetProperties::GetLanguageFollowsFrame () const
+{
+    const uint32_t idx = ePropertyLanguageFollowsFrame;
+    return m_collection_sp->GetPropertyAtIndexAsBoolean (NULL, idx, g_properties[idx].default_uint_value != 0);
+}
+
 const char *
----------------
Remove


Repository:
  rL LLVM

http://reviews.llvm.org/D11482