Set the default language to use when evaluating to that of the frame's CU.

[dawn]

Summary

The main purpose of this patch is to be able to get the frame's context (instead of just the target's) when evaluating, so that the language of the frame's CU can be used to select the compiler and/or compiler options to use when parsing the expression. This allows for modules built with mixed languages to be parsed in the context of their frame.

Other associated changes:

• Enable C++ language options when language is C.
• Enable C++ language options when language is C.
• Disable C++11 language options when language is C++.
• Add all C and C++ language variants when determining the language options to set.
• Add test TestMixedLanguages.py to check that the language being used for evaluation is that of the frame.
• Remove expr --language test for ObjC from TestExprOptions.py (it is redundant).
• Fix test TestExprOptions.py to check for C++11 instead of C++ since C++ has to be enabled for C.
• Fix TestPersistentPtrUpdate.py to not require C++11 in C.

[clayborg]

I will defer to Sean Callanan.

[dawn]

Please review? Thanks.

[dawn]

Added more reviewers in the hopes that someone will review this patch. Please?

[dawn]

See comments added in patch.

source/Commands/CommandObjectExpression.cpp

311 ↗

(On Diff #29454)

Ignore this part of the patch - the language will instead be controlled by an option as discussed in http://reviews.llvm.org/D11119. It is useful for testing purposes however, to show where test cases are using expressions outside the scope of the CU's language.

source/Expression/ClangExpressionParser.cpp

219 ↗

(On Diff #29454)

Do you want to be able to parse ObjC++ when the language is ObjC? If so, you'll want this.

[dawn]

I'll update this patch to what I believe Greg wants in http://reviews.llvm.org/D11482, and merge in http://reviews.llvm.org/D11173 which is also needed in that case.

source/Commands/CommandObjectExpression.cpp

311 ↗

(On Diff #29454)

In http://reviews.llvm.org/D11482, Greg says he doesn't want this to be controlled be an option, so this part of the patch stays.

[dawn]

This patch has been modified to incorporate the discussions in http://reviews.llvm.org/D11482.

Additional changes to the language options settings were added to take care of the "ask for C++, for now get ObjC++" part (the "ask for ObjC, get ObjC++" part was in the previous patch). Changes to the frame API were also added as suggested by Greg.

[dawn]

Please review? Thank you.

[zturner]

Sorry, I don't have much to add here. I guess you need to wait for Sean or
Jim to respond.

[clayborg]

I will let Jim and Sean handle this one.

[dawn]

Please review? Thank you.

[dawn]

Hi guys, I'd still really like to get this into tot. May I commit it please?

[clayborg]

Looks fine as long as the test suite tests are passing.

[clayborg]

Please wait for the OK from Jim and Sean.

[dawn]

In D11102#234450, @clayborg wrote:

Please wait for the OK from Jim and Sean.

Jim/Sean, you guys OK with this? All tests pass - we've been running with this patch locally since it was proposed and have had no issues with it.

[jingham]

This seems fine. The only things you should fix (see inline comments) are the name of GetDefaultExpressionLanguage and the comment on why the upgrading is necessary. If you do those two things, then this can go in. Sorry for the delay.

include/lldb/Target/StackFrame.h

475–476 ↗

(On Diff #31098)

This name seems confusing. This is just the language of the current stack frame. Deciding what language to use for the expression is just one possible use of this information. It might also be useful to have the frame format print the language of the frame you've just stopped in. So this should have a more generic name. How about "GetLanguage".

source/Expression/ClangExpressionParser.cpp

220–228 ↗

(On Diff #31098)

The contents of this comment are fine, but just state the reason rather than describing how you found out about the reason...

[spyffe]

I looked at this with Jim, and once his suggestions are applied this patch is fine. Thanks for working with us on it.
I want to follow up and make non-Apple platforms not force ObjC (it doesn't matter for making the expression parser work).

[dawn]

Thanks so much for your reviews! I'll post a revised patch Monday.

include/lldb/Target/StackFrame.h

475–476 ↗

(On Diff #31098)

Sure, GetLanguage is fine. I went with GetDefaultExpressionLanguage because that is what Greg suggested in http://reviews.llvm.org/D11482.

source/Expression/ClangExpressionParser.cpp

220–228 ↗

(On Diff #31098)

will do.

[dawn]

Just an update to let you know I'm still working on this (and a hundred other things)...

source/Expression/ClangExpressionParser.cpp

227 ↗

(On Diff #31098)

In the process of rewriting this patch, I realized I hadn't handled the "Want C -> get ObjC++" case, which, if the switch statement case for C is fixed to handle all the C variants, turns out to be needed. When that is fixed, we get a few new failures, including the test for the expr --language option. I'm trying to figure out how to rework those tests, but with everything enabling ObjC++ now, that's not so easy.

[dawn]

HI Jim/Sean,
Did you guys want to have a final review or can I go ahead and commit? Basically, this patch includes the changes you requested, but it also 1) fixes how the languages are recognized (the switch statement was incomplete) and 2) reworks the language-based test cases to check C/C++03 vs. C++11 (instead of C++ vs. ObjC) since C++ now enables ObjC.

[spyffe]

Looks good to me. Thanks, Dawn.

[dawn]

In D11102#234730, @spyffe wrote:

I want to follow up and make non-Apple platforms not force ObjC (it doesn't matter for making the expression parser work).

Hi Sean,

I have one question about this that still bugs me... I understand that C++ is needed in C and ObjC because of implementation details of expression evaluation, but why is ObjC needed for C++?

It will be awesome if/when you can follow up on this and remove this requirement. Let me know when that's done and I'll remove the workarounds that force ObjC and give it a try.

Thanks!

[jingham]

It isn't technically necessary, but on OS X it is very common for people to want to examine global app state from wherever and that is always ObjC, so we'd tick off our users if we forced them to explicitly dial up ObjC in other contexts. For other platforms it would be fine to choose just C++. And if the user explicitly dials up C++ (by passing --language) it should be fine to not add ObjC. But if we're setting it automatically on OS X there's a pretty strong expectation that you can just run an expression and ObjC types will be available.

Jim

lldb/trunk/include/lldb/Target/StackFrame.h

public:
     ///
     /// @return
     //    A ValueObject for this variable.
     //------------------------------------------------------------------
     lldb::ValueObjectSP
     TrackGlobalVariable (const lldb::VariableSP &variable_sp, lldb::DynamicValueType use_dynamic);
 
     //------------------------------------------------------------------
+    /// Query this frame to determine what the default language should be
+    /// when parsing expressions given the execution context.
+    ///
+    /// @return
+    ///   The language of the frame if known, else lldb::eLanguageTypeUnknown.
+    //------------------------------------------------------------------
+    lldb::LanguageType
+    GetLanguage ();
+
+    //------------------------------------------------------------------
     // lldb::ExecutionContextScope pure virtual functions
     //------------------------------------------------------------------
     lldb::TargetSP
     CalculateTarget() override;
 
     lldb::ProcessSP
     CalculateProcess() override;
 

lldb/trunk/source/Commands/CommandObjectExpression.cpp

)
     Target *target = exe_ctx.GetTargetPtr();
     
     if (!target)
         target = GetDummyTarget();
     
     if (target)
     {
         lldb::ValueObjectSP result_valobj_sp;
-
         bool keep_in_memory = true;
+        StackFrame *frame = exe_ctx.GetFramePtr();
 
         EvaluateExpressionOptions options;
         options.SetCoerceToId(m_varobj_options.use_objc);
         options.SetUnwindOnError(m_command_options.unwind_on_error);
         options.SetIgnoreBreakpoints (m_command_options.ignore_breakpoints);
         options.SetKeepInMemory(keep_in_memory);
         options.SetUseDynamic(m_varobj_options.use_dynamic);
         options.SetTryAllThreads(m_command_options.try_all_threads);
         options.SetDebug(m_command_options.debug);
         
-        // If the language was not specified, set it from target's properties
+        // If the language was not specified in the expression command,
+        // set it to the language in the target's properties if
+        // specified, else default to the langage for 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 (frame)
+            options.SetLanguage(frame->GetLanguage());
 
         // If there is any chance we are going to stop and want to see
         // what went wrong with our expression, we should generate debug info
         if (!m_command_options.ignore_breakpoints ||
             !m_command_options.unwind_on_error)
             options.SetGenerateDebugInfo(true);
         
         if (m_command_options.timeout > 0)
             options.SetTimeoutUsec(m_command_options.timeout);
         else
             options.SetTimeoutUsec(0);
 
-        target->EvaluateExpression(expr, exe_ctx.GetFramePtr(),
-                                   result_valobj_sp, options);
+        target->EvaluateExpression(expr, frame, result_valobj_sp, options);
 
         if (result_valobj_sp)
         {
             Format format = m_format_options.GetFormat();
 
             if (result_valobj_sp->GetError().Success())
             {
                 if (format != eFormatVoid)

lldb/trunk/source/Expression/ClangExpressionParser.cpp

ClangExpressionParser::ClangExpressionParser (ExecutionContextScope *exe_scope,
 
     // 3. Set options.
 
     lldb::LanguageType language = expr.Language();
 
     switch (language)
     {
     case lldb::eLanguageTypeC:
+    case lldb::eLanguageTypeC89:
+    case lldb::eLanguageTypeC99:
+    case lldb::eLanguageTypeC11:
+        // FIXME: the following language option is a temporary workaround,
+        // to "ask for C, get C++."
+        // For now, the expression parser must use C++ anytime the
+        // language is a C family language, because the expression parser
+        // uses features of C++ to capture values.
+        m_compiler->getLangOpts().CPlusPlus = true;
         break;
     case lldb::eLanguageTypeObjC:
         m_compiler->getLangOpts().ObjC1 = true;
         m_compiler->getLangOpts().ObjC2 = true;
+        // FIXME: the following language option is a temporary workaround,
+        // to "ask for ObjC, get ObjC++" (see comment above).
+        m_compiler->getLangOpts().CPlusPlus = true;
         break;
     case lldb::eLanguageTypeC_plus_plus:
-        m_compiler->getLangOpts().CPlusPlus = true;
+    case lldb::eLanguageTypeC_plus_plus_11:
+    case lldb::eLanguageTypeC_plus_plus_14:
         m_compiler->getLangOpts().CPlusPlus11 = true;
         m_compiler->getHeaderSearchOpts().UseLibcxx = true;
+        // fall thru ...
+    case lldb::eLanguageTypeC_plus_plus_03:
+        m_compiler->getLangOpts().CPlusPlus = true;
+        // FIXME: the following language option is a temporary workaround,
+        // to "ask for C++, get ObjC++".  Apple hopes to remove this requirement
+        // on non-Apple platforms, but for now it is needed.
+        m_compiler->getLangOpts().ObjC1 = true;
         break;
     case lldb::eLanguageTypeObjC_plus_plus:
+    case lldb::eLanguageTypeUnknown:
     default:
         m_compiler->getLangOpts().ObjC1 = true;
         m_compiler->getLangOpts().ObjC2 = true;
         m_compiler->getLangOpts().CPlusPlus = true;
         m_compiler->getLangOpts().CPlusPlus11 = true;
         m_compiler->getHeaderSearchOpts().UseLibcxx = true;
         break;
     }

lldb/trunk/source/Target/StackFrame.cpp

 {
     if (m_sc.block == NULL)
         GetSymbolContext (eSymbolContextBlock);
     if (m_sc.block)
         return m_sc.block->GetContainingInlinedBlock() != NULL;
     return false;
 }
 
+lldb::LanguageType
+StackFrame::GetLanguage ()
+{
+    CompileUnit *cu = GetSymbolContext(eSymbolContextCompUnit).comp_unit;
+    if (cu)
+        return cu->GetLanguage();
+    return lldb::eLanguageTypeUnknown;
+}
+
 TargetSP
 StackFrame::CalculateTarget ()
 {
     TargetSP target_sp;
     ThreadSP thread_sp(GetThread());
     if (thread_sp)
     {
         ProcessSP process_sp (thread_sp->CalculateProcess());

lldb/trunk/test/expression_command/options/TestExprOptions.py

def setUp(self):
         # Call super's setUp().
         TestBase.setUp(self)
 
         self.main_source = "main.cpp"
         self.main_source_spec = lldb.SBFileSpec (self.main_source)
         self.line = line_number('main.cpp', '// breakpoint_in_main')
         self.exe = os.path.join(os.getcwd(), "a.out")
 
-    @skipUnlessDarwin
-    def test_expr_options_objc_cpp(self):
-        self.expr_options(test_objc = True, test_cpp = True)
-
-    @skipIfDarwin # Already covered by test_expr_options_objc_cpp
-    def test_expr_options_cpp(self):
-        self.expr_options(test_objc = False, test_cpp = True)
-
-    def expr_options(self, test_objc, test_cpp):
+    def test_expr_options(self):
         """These expression command options should work as expected."""
         self.buildDefault()
 
         # Set debugger into synchronous mode
         self.dbg.SetAsync(False)
 
         # Create a target by the debugger.
         target = self.dbg.CreateTarget(self.exe)
         self.assertTrue(target, VALID_TARGET)
 
         # Set breakpoints inside main.
         breakpoint = target.BreakpointCreateBySourceRegex('// breakpoint_in_main', self.main_source_spec)
         self.assertTrue(breakpoint)
 
         # Now launch the process, and do not stop at entry point.
         process = target.LaunchSimple(None, None, self.get_process_working_directory())
         self.assertTrue(process, PROCESS_IS_VALID)
 
         threads = lldbutil.get_threads_stopped_at_breakpoint(process, breakpoint)
         self.assertEqual(len(threads), 1)
 
         frame = threads[0].GetFrameAtIndex(0)
         options = lldb.SBExpressionOptions()
 
-        if test_objc:
-            # -- test --language on ObjC builtin type using the SB API's --
-            # Make sure we can evaluate the ObjC builtin type 'id':
-            val = frame.EvaluateExpression('id my_id = 0; my_id')
-            self.assertTrue(val.IsValid())
-            self.assertTrue(val.GetError().Success())
-            self.assertEqual(val.GetValueAsUnsigned(0), 0)
-            self.DebugSBValue(val)
-
-            # Make sure it still works if language is set to ObjC++:
-            options.SetLanguage(lldb.eLanguageTypeObjC_plus_plus)
-            val = frame.EvaluateExpression('id my_id = 0; my_id', options)
-            self.assertTrue(val.IsValid())
-            self.assertTrue(val.GetError().Success())
-            self.assertEqual(val.GetValueAsUnsigned(0), 0)
-            self.DebugSBValue(val)
-
-            # Make sure it fails if language is set to C:
-            options.SetLanguage(lldb.eLanguageTypeC)
-            val = frame.EvaluateExpression('id my_id = 0; my_id', options)
-            self.assertTrue(val.IsValid())
-            self.assertFalse(val.GetError().Success())
+        # test --language on C++ expression using the SB API's
 
-        if test_cpp:
-            # -- test --language on C++ expression using the SB API's --
-            # Make sure we can evaluate 'ns::func'.
-            val = frame.EvaluateExpression('ns::func')
-            self.assertTrue(val.IsValid())
-            self.assertTrue(val.GetError().Success())
-            self.DebugSBValue(val)
+        val = frame.EvaluateExpression('foo != nullptr')
 
-            # Make sure it still works if language is set to C++:
-            options.SetLanguage(lldb.eLanguageTypeC_plus_plus)
-            val = frame.EvaluateExpression('ns::func', options)
-            self.assertTrue(val.IsValid())
-            self.assertTrue(val.GetError().Success())
-            self.DebugSBValue(val)
+        # Make sure it still works if language is set to C++11:
+        options.SetLanguage(lldb.eLanguageTypeC_plus_plus_11)
+        val = frame.EvaluateExpression('foo != nullptr', options)
 
-            # Make sure it fails if language is set to C:
-            options.SetLanguage(lldb.eLanguageTypeC)
-            val = frame.EvaluateExpression('ns::func', options)
-            self.assertTrue(val.IsValid())
-            self.assertFalse(val.GetError().Success())
+        val = frame.EvaluateExpression('foo != nullptr', options)
 
 if __name__ == '__main__':
     import atexit
     lldb.SBDebugger.Initialize()
     atexit.register(lambda: lldb.SBDebugger.Terminate())
     unittest2.main()

lldb/trunk/test/expression_command/persistent_ptr_update/TestPersistentPtrUpdate.py

def do_my_test(self):
         self.addTearDownHook(cleanup)
 
         self.runCmd("file a.out", CURRENT_EXECUTABLE_SET)
         
         self.runCmd('break set -p here')
 
         self.runCmd("run", RUN_SUCCEEDED)
         
-        self.runCmd("expr void* $foo = nullptr")
+        self.runCmd("expr void* $foo = 0")
         
         self.runCmd("continue")
         
         self.expect("expr $foo", substrs=['$foo','0x0'])
 
 if __name__ == '__main__':
     import atexit
     lldb.SBDebugger.Initialize()
     atexit.register(lambda: lldb.SBDebugger.Terminate())
     unittest2.main()

lldb/trunk/test/lang/mixed/Makefile

+LEVEL = ../../make
+
+CXX_SOURCES := foo.cpp
+C_SOURCES := main.c
+
+include $(LEVEL)/Makefile.rules

lldb/trunk/test/lang/mixed/TestMixedLanguages.py

+"""Test that lldb works correctly on compile units form different languages."""
+
+import os, time, re
+import unittest2
+import lldb
+from lldbtest import *
+
+class MixedLanguagesTestCase(TestBase):
+
+    mydir = TestBase.compute_mydir(__file__)
+
+    def test_language_of_frame(self):
+        """Test that the language defaults to the language of the current frame."""
+        self.buildDefault()
+        exe = os.path.join(os.getcwd(), "a.out")
+        self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
+
+        # Execute the cleanup function during test case tear down
+        # to restore the frame format.
+        def cleanup():
+            self.runCmd("settings set frame-format %s" % self.format_string, check=False)
+        self.addTearDownHook(cleanup)
+        self.runCmd("settings show frame-format")
+        m = re.match(
+                '^frame-format \(format-string\) = "(.*)\"$',
+                self.res.GetOutput())
+        self.assertTrue(m, "Bad settings string")
+        self.format_string = m.group(1)
+
+        # Change the default format to print the language.
+        format_string = "frame #${frame.index}: ${frame.pc}{ ${module.file.basename}`${function.name}{${function.pc-offset}}}{, lang=${language}}\n"
+        self.runCmd("settings set frame-format %s" % format_string)
+        self.expect("settings show frame-format", SETTING_MSG("frame-format"),
+            substrs = [format_string])
+
+        # Run to BP at main (in main.c) and test that the language is C.
+        self.runCmd("breakpoint set -n main")
+        self.runCmd("run")
+        self.expect("thread backtrace",
+            substrs = ["`main", "lang=c"])
+        # Make sure evaluation of C++11 fails.
+        self.expect("expr foo != nullptr", error=True,
+            startstr = "error")
+
+        # Run to BP at foo (in foo.cpp) and test that the language is C++.
+        self.runCmd("breakpoint set -n foo")
+        self.runCmd("continue")
+        self.expect("thread backtrace",
+            substrs = ["`::foo()", "lang=c++"])
+        # Make sure we can evaluate an expression requiring C++11
+        # (note: C++11 is enabled by default for C++).
+        self.expect("expr foo != nullptr",
+            patterns = ["true"])
+
+if __name__ == '__main__':
+    import atexit
+    lldb.SBDebugger.Initialize()
+    atexit.register(lambda: lldb.SBDebugger.Terminate())
+    unittest2.main()

lldb/trunk/test/lang/mixed/foo.cpp

+namespace ns {
+    int func(void)
+    {
+        return 0;
+    }
+}
+
+extern "C" int foo(void)
+{
+    return ns::func();
+}

lldb/trunk/test/lang/mixed/main.c

+int foo(void);
+static int static_value = 0;
+
+int
+bar()
+{
+    static_value++;
+    return static_value;
+}
+
+int main (int argc, char const *argv[])
+{
+    bar(); // breakpoint_in_main
+    return foo();
+}