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 _______________________________________________ lldb-commits mailing list lldb-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits