clayborg updated this revision to Diff 254352.
clayborg added a comment.
Fixed patch reviewer suggestions and added a test for "id".
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D76964/new/
https://reviews.llvm.org/D76964
Files:
lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
lldb/test/API/commands/expression/ignore/Makefile
lldb/test/API/commands/expression/ignore/TestIgnoreName.py
lldb/test/API/commands/expression/ignore/main.cpp
Index: lldb/test/API/commands/expression/ignore/main.cpp
===================================================================
--- /dev/null
+++ lldb/test/API/commands/expression/ignore/main.cpp
@@ -0,0 +1,12 @@
+namespace a {
+ struct Class {
+ };
+ struct id {
+ };
+}
+
+int main(int argc, char **argv) {
+ a::Class c;
+ a::id i;
+ return 0;
+}
Index: lldb/test/API/commands/expression/ignore/TestIgnoreName.py
===================================================================
--- /dev/null
+++ lldb/test/API/commands/expression/ignore/TestIgnoreName.py
@@ -0,0 +1,33 @@
+"""
+Test calling user an expression that uses an Objective C keyword "id" or
+"Class" when this identifier is inside of a namespace, class or structure.
+ClangASTSource::FindExternalVisibleDecls(...) might try to find one of these
+keywords inside of a non root decl context and this should succeed.
+"""
+
+
+import lldb
+
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestIgnoreName(TestBase):
+ mydir = TestBase.compute_mydir(__file__)
+
+ def test(self):
+ """Test that we can evaluate an expression that finds something inside
+ a namespace that uses an Objective C keyword.
+ """
+ self.build()
+ exe_path = self.getBuildArtifact("a.out")
+ error = lldb.SBError()
+ triple = None
+ platform = None
+ add_dependent_modules = False
+ target = self.dbg.CreateTarget(exe_path, triple, platform,
+ add_dependent_modules, error)
+ self.assertTrue(target, VALID_TARGET)
+ self.expect_expr("a::Class x; x", result_type="a::Class")
+ self.expect_expr("a::id i; i", result_type="a::id")
Index: lldb/test/API/commands/expression/ignore/Makefile
===================================================================
--- /dev/null
+++ lldb/test/API/commands/expression/ignore/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
===================================================================
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
@@ -1321,7 +1321,7 @@
Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS));
const ConstString name(context.m_decl_name.getAsString().c_str());
- if (IgnoreName(name, false))
+ if (IgnoreName(name, namespace_decl, false))
return;
// Only look for functions by name out in our symbols if the function doesn't
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h
===================================================================
--- lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h
@@ -325,13 +325,19 @@
/// \param[in] name
/// The name to be considered.
///
+ /// \param[in] namespace_decl
+ /// The namespace decl that this name is being searched. This can be
+ /// invalid if we are searching the root namespace.
+ ///
/// \param[in] ignore_all_dollar_names
/// True if $-names of all sorts should be ignored.
///
/// \return
/// True if the name is one of a class of names that are ignored by
/// global lookup for performance reasons.
- bool IgnoreName(const ConstString name, bool ignore_all_dollar_names);
+ bool IgnoreName(const ConstString name,
+ const CompilerDeclContext &namespace_decl,
+ bool ignore_all_dollar_names);
public:
/// Copies a single Decl into the parser's AST context.
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
===================================================================
--- lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
@@ -574,13 +574,21 @@
}
bool ClangASTSource::IgnoreName(const ConstString name,
+ const CompilerDeclContext &namespace_decl,
bool ignore_all_dollar_names) {
- static const ConstString id_name("id");
- static const ConstString Class_name("Class");
-
- if (m_ast_context->getLangOpts().ObjC)
- if (name == id_name || name == Class_name)
+ if (m_ast_context->getLangOpts().ObjC) {
+ if (name == "id" || name == "Class") {
+ // Only disallow using "id" and "Class" if we are searching from the root
+ // namespace or translation unit level. If namespace_decl is valid, then
+ // this is not the root namespace.
+ if (namespace_decl.IsValid())
+ return false;
+ LLDB_LOG(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS),
+ " CAS::IgnoreName(name=\"{0}\") Ignoring reserved keryword",
+ name);
return true;
+ }
+ }
StringRef name_string_ref = name.GetStringRef();
@@ -600,7 +608,7 @@
SymbolContextList sc_list;
const ConstString name(context.m_decl_name.getAsString().c_str());
- if (IgnoreName(name, true))
+ if (IgnoreName(name, namespace_decl, true))
return;
if (!m_target)
@@ -663,7 +671,7 @@
NameSearchContext &context, lldb::ModuleSP module_sp,
const CompilerDeclContext &namespace_decl) {
const ConstString name(context.m_decl_name.getAsString().c_str());
- if (IgnoreName(name, true))
+ if (IgnoreName(name, namespace_decl, true))
return;
Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS));
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
