[Lldb-commits] [PATCH] D59960: Fix for ambiguous lookup in expressions between local variable and namespace

[Frederic Riss via Phabricator via lldb-commits]

friss added inline comments.


================
Comment at: source/Expression/ExpressionSourceCode.cpp:171
     if (!var_name || var_name == ConstString("this") ||
+        var_name == ConstString("self") || var_name == ConstString("_cmd") ||
         var_name == ConstString(".block_descriptor"))
----------------
This seems dangerous. Now people having a variable called 'self' or '_cmd' in their C++ code will not be able to evaluate them anymore. This filtering needs to be per-language.


================
Comment at: source/Expression/ExpressionSourceCode.cpp:257
+    if (add_locals)
       if (Language::LanguageIsCPlusPlus(frame->GetLanguage())) {
         if (target->GetInjectLocalVariables(&exe_ctx)) {
----------------
How does this work? The locals seem to be added only in C++, I'm not sure how this patch makes any difference?


================
Comment at: source/Expression/ExpressionSourceCode.cpp:334
+            m_name.c_str(), m_name.c_str(), lldb_local_var_decls.GetData(),
+            tagged_body.c_str());
       } else {
----------------
Why only in the static_method case?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59960/new/

https://reviews.llvm.org/D59960