[Lldb-commits] [PATCH] D13350: [lldb] Fix evaluation of qualified global variables

[Dawn Perchik via lldb-commits]

dawn added a subscriber: dawn.
dawn added a comment.

This patch fixes bugs:

- https://llvm.org/bugs/show_bug.cgi?id=24994 (::val gets NS::val inside NS since r247746)
- https://llvm.org/bugs/show_bug.cgi?id=24995 (shadowed var gets ambiguity since r247836)

See inline comments.  Mostly had concerns about indentation - please always run patches through clang-format (both clang and lldb have their own config file in the root dir).


================
Comment at: clang/include/clang/AST/DeclBase.h:1769
@@ +1768,3 @@
+      return old_value;
+  }
+
----------------
Indenting in clang is 2 spaces.

================
Comment at: lldb/include/lldb/Symbol/CompilerDeclContext.h:17
@@ -16,2 +16,3 @@
 #include "lldb/Core/ConstString.h"
+#include "clang/AST/DeclBase.h"
 
----------------
clang includes should precede lldb ones.

================
Comment at: lldb/include/lldb/Symbol/TypeSystem.h:121
@@ -120,1 +120,3 @@
 
+    // extra is language dependant
+    virtual std::vector<void *>
----------------
I don't understand this comment - please clarify?  Also, please start comments with capital letters.

================
Comment at: lldb/include/lldb/Symbol/TypeSystem.h:123
@@ +122,3 @@
+    virtual std::vector<void *>
+    DeclContextFindDeclByNameEx (void *opaque_decl_ctx, ConstString name, void* extra)
+    {
----------------
It would be cleaner to have a default argument instead of this second overload.  What are the guidelines on using default arguments in lldb?  I've seen them both used and not used.

================
Comment at: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp:1370
@@ -1370,1 +1369,3 @@
+                std::vector<CompilerDecl> found_decls =
+                        compiler_decl_context.FindDeclByName(name, context.m_decl_context);
                 
----------------
Looks like indentation was accidentally changed here?

================
Comment at: lldb/source/Symbol/ClangASTContext.cpp:8984
@@ +8983,3 @@
+    return nd->getLexicalDeclContext()->getDeclKind() == lookupCtx->getDeclKind();
+}
+
----------------
Please run patch through clang-format to fix indentation issues.


http://reviews.llvm.org/D13350