[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.

Raphael Isemann via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Wed Apr 1 00:31:00 PDT 2020

teemperor requested changes to this revision.
teemperor added a comment.
This revision now requires changes to proceed.

LGTM minus some stylistic changes.

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
aprantl wrote:
> For these tiny strings a StringRef `==` comparison is going to be more efficient than constructing and storing a pointer to a ConstString.
There is no need for StringRef as ConstString allows direct comparison against string literals, so this works and is much faster: `name == "id" || name == "Class"`

Comment at: lldb/test/API/commands/expression/ignore/TestIgnoreName.py:6
+Ticket: https://llvm.org/bugs/show_bug.cgi?id=26790
Left over from the original test case (TestCallUserAnonTypedef.py)

Comment at: lldb/test/API/commands/expression/ignore/TestIgnoreName.py:32
+                                       add_dependent_modules, error)
+        self.assertTrue(error.Success(), "Make sure our target got created")
+        expr_result = target.EvaluateExpression("a::Class a; a")
I don't think we usually check the error, but only if target is valid in any other test. So this whole test can just be this (at least after D77197 has landed):
    def test(self):                                                             
        """Test that we can evaluate an expression that finds something inside  
           a namespace that uses an Objective C keyword.                        
        target = self.dbg.CreateTarget(self.getBuildArtifact("a.out"))          
        self.assertTrue(target, VALID_TARGET)                                   
        self.expect_expr("a::Class x; x", result_type="a::Class")   

Comment at: lldb/test/API/commands/expression/ignore/main.cpp:7
+int main(int argc, char **argv)
+  a::Class c;
I think new test files should follow LLVM code style.

  rG LLVM Github Monorepo



More information about the lldb-commits mailing list