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):
```
lang=python
def test(self):
"""Test that we can evaluate an expression that finds something inside
a namespace that uses an Objective C keyword.
"""
self.build()
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.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D76964/new/
https://reviews.llvm.org/D76964
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
