[Lldb-commits] [PATCH] D76964: Fix an issue where the IgnoreName function was not allowing "Class" to be looked up inside a namespace or other class.

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.


================
Comment at: lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp:582
+    static const ConstString Class_name("Class");
+    if (name == id_name || name == Class_name) {
+      // Only disallow using "id" and "Class" if we are searching from the root
----------------
For these tiny strings a StringRef `==` comparison is going to be more efficient than constructing and storing a pointer to a ConstString.


================
Comment at: lldb/test/API/commands/expression/ignore/TestIgnoreName.py:36
+                        "Make sure our expression evaluated without errors")
+        self.assertTrue(expr_result.GetValue() == None,
+                        'Expression value is None')
----------------
`assertEqual(expr_result.GetValue(), None,  ...)` is better here, because it will print the result of `expr_result.GetValue()` in the failure case.


================
Comment at: lldb/test/API/commands/expression/ignore/TestIgnoreName.py:38
+                        'Expression value is None')
+        self.assertTrue(expr_result.GetType().GetName() == "a::Class",
+                        'Expression result type is "a::Class"')
----------------
same here


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76964