[Expr] Check the language before ignoring Objective C keywords

[aleksandr.urakov]

Summary

This patch adds the check of the language before ignoring names like id or Class, which are reserved in Objective C, but allowed in C++. It is needed to make it possible to evaluate expressions in a C++ program containing names like id or Class.

There was a discussion of this at the end of October 2018 in lldb-dev.

[aleksandr.urakov]

source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp

401–404 ↗

(On Diff #175090)

Is it still necessary? We can do here something like #ifdef __APPLE__, but then the test will fail on Apple platforms. Can we somehow specify a platform requirement in the test?

[labath]

source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp

401–404 ↗

(On Diff #175090)

Instead of ifdef it would be better to do a target->GetArchitecture().getTriple().getVendor() == llvm::Triple::Apple and then @skipIfDarwin in the test. But it would be certainly better to remove this altogether :)

[aleksandr.urakov]

source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp

401–404 ↗

(On Diff #175090)

Yes, your variant is better! I'll do it in the way you have described if there will be objections against removing it at all.

[clayborg]

source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp

404 ↗

(On Diff #175090)

A better way would be to try and grab the Objective C runtime from the process. There are some variants of objective C that might run under non-apple targets:

ProcessSP process_sp = target->GetProcess();
if (process_sp) 
  m_compiler->getLangOpts().ObjC = process_sp->GetLanguageRuntime(eLanguageTypeObjC) != nullptr;

Then C and C++ programs on Mac will be able to use "id" and other reserved words in their expressions again.

[aleksandr.urakov]

source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp

404 ↗

(On Diff #175090)

Thanks for the idea! I've updated the patch.

[aleksandr.urakov]

Ping! Can you look at this, please?

[clayborg]

Looks fine to me. Jim Ingham should ok this as well.

[jingham]

I think the ideal behavior would be "if we don't have debug info for the frame, choose ObjC++". But if you are in frame with debug info, obey that frame's language (except that we have to use C++ because the expression parser uses C++ references to play some of the games it plays.) The reasoning behind that is because in the first case, you won' have local names and are calling out to global objects and having the widest language available is the most convenient. If we did it that way, we could also make the user's choice of language always hold (except again you can't use "class" anywhere because of the use of C++...).

But I don't see an easy way to implement that behavior. You'd have to plumb through another bit - has debug info - which is orthogonal to the language, but have that affect the language choice.

However, this change is strictly better for non-objc programs, and preserves the same behavior as currently holds when there is ObjC. So I think this change is fine for now.

[aleksandr.urakov]

Ok, thank you!

[aleksandr.urakov]

The test was broken after this commit. It seems that on MacOS X the process of the compiled test application always contains Objective C Runtime, so the part of the test, which assumes that there is no ObjC option enabled, fails. To fix it I've skip this part for Darwin here: https://reviews.llvm.org/rLLDB348250 Please, tell me if you have any objections on this.

lldb/trunk/packages/Python/lldbsuite/test/expression_command/options/TestExprOptions.py

def test_expr_options(self):
         self.assertTrue(val.GetError().Success())
         self.DebugSBValue(val)
 
         # Make sure it fails if language is set to C:
         options.SetLanguage(lldb.eLanguageTypeC)
         val = frame.EvaluateExpression('foo != nullptr', options)
         self.assertTrue(val.IsValid())
         self.assertFalse(val.GetError().Success())
+
+        # Make sure we can retrieve `id` variable if language is set to C++11:
+        options.SetLanguage(lldb.eLanguageTypeC_plus_plus_11)
+        val = frame.EvaluateExpression('id == 0', options)
+        self.assertTrue(val.IsValid())
+        self.assertTrue(val.GetError().Success())
+        self.DebugSBValue(val)
+
+        # Make sure we can't retrieve `id` variable if language is set to ObjC:
+        options.SetLanguage(lldb.eLanguageTypeObjC)
+        val = frame.EvaluateExpression('id == 0', options)
+        self.assertTrue(val.IsValid())
+        self.assertFalse(val.GetError().Success())

lldb/trunk/packages/Python/lldbsuite/test/expression_command/options/main.cpp

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

lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp

void ClangASTSource::FindExternalVisibleDecls(NameSearchContext &context) {
   }
 }
 
 bool ClangASTSource::IgnoreName(const ConstString name,
                                 bool ignore_all_dollar_names) {
   static const ConstString id_name("id");
   static const ConstString Class_name("Class");
 
+  if (m_ast_context->getLangOpts().ObjC)
     if (name == id_name || name == Class_name)
       return true;
 
   StringRef name_string_ref = name.GetStringRef();
 
   // The ClangASTSource is not responsible for finding $-names.
   if (name_string_ref.empty() ||
       (ignore_all_dollar_names && name_string_ref.startswith("$")) ||
       name_string_ref.startswith("_$"))
     return true;

lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp

ClangExpressionParser::ClangExpressionParser(ExecutionContextScope *exe_scope,
   case lldb::eLanguageTypeC_plus_plus:
   case lldb::eLanguageTypeC_plus_plus_11:
   case lldb::eLanguageTypeC_plus_plus_14:
     m_compiler->getLangOpts().CPlusPlus11 = true;
     m_compiler->getHeaderSearchOpts().UseLibcxx = true;
     LLVM_FALLTHROUGH;
   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().ObjC = true;
+    if (process_sp)
+      m_compiler->getLangOpts().ObjC =
+          process_sp->GetLanguageRuntime(lldb::eLanguageTypeObjC) != nullptr;
     break;
   case lldb::eLanguageTypeObjC_plus_plus:
   case lldb::eLanguageTypeUnknown:
   default:
     m_compiler->getLangOpts().ObjC = true;
     m_compiler->getLangOpts().CPlusPlus = true;
     m_compiler->getLangOpts().CPlusPlus11 = true;
     m_compiler->getHeaderSearchOpts().UseLibcxx = true;