[lldb][TypeSystemClang] Set SuppressInlineNamespace to 'All'

[Michael137]

We used to set it to true up until recently, see here. That's incorrect because SuppressInlineNamespace is actually an enum. What probably happened is that SuppressInlineNamespace used to be a boolean but got turned into an enum. But the assignment in LLDB wasn't updated. But because the bitfield is an unsigned, the compiler never complained.

This meant that ever since SuppressInlineNamespace became an enum, we've been setting it to SuppressInlineNamespaceMode::Redundant. Which means we would only omit the inline namespace when displaying typenames if Clang deemed it unambiguous. This is probably a rare situtation but the attached test-case is one such scenario. Here, target var t1 followed by target var t2 would print the inline namespace for t2, because in that context, the type is otherwise ambiguous. But because LLDB's context is lazily constructed, evaluating t2 first would omit the inline namespace, because t1 isn't in the context yet to make it ambiguous.

This patch sets the SuppressInlineNamespace to SuppressInlineNamespaceMode::All, which is most likely what was intended in the first place, and also removes the above-mentioned non-determinism from our typename printing.

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Michael Buch (Michael137)

Changes

We used to set it to true up until recently, see TBD. That's incorrect because SuppressInlineNamespace is actually an enum. What probably happened is that SuppressInlineNamespace used to be a boolean but got turned into an enum. But the assignment in LLDB wasn't updated. But because the bitfield is an unsigned, the compiler never complained.

This meant that ever since SuppressInlineNamespace became an enum, we've been setting it to SuppressInlineNamespaceMode::Redundant. Which means we would only omit the inline namespace when displaying typenames if Clang deemed it unambiguous. This is probably a rare situtation but the attached test-case is one such scenario. Here, target var t1 followed by target var t2 would print the inline namespace for t2, because in that context, the type is otherwise ambiguous. But because LLDB's context is lazily constructed, evaluating t2 first would omit the inline namespace, because t1 isn't in the context yet to make it ambiguous.

This patch sets the SuppressInlineNamespace to SuppressInlineNamespaceMode::All, which is most likely what was intended in the first place, and also removes the above-mentioned non-determinism from our typename printing.

---

Full diff: https://github.com/llvm/llvm-project/pull/171138.diff

4 Files Affected:

• (modified) lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp (+2-3)
• (added) lldb/test/API/lang/cpp/inline-namespace-in-typename/Makefile (+3)
• (added) lldb/test/API/lang/cpp/inline-namespace-in-typename/TestInlineNamespaceInTypename.py (+30)
• (added) lldb/test/API/lang/cpp/inline-namespace-in-typename/main.cpp (+13)

diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index 2cb4a46130c84..625d0e546ad3b 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -3871,9 +3871,8 @@ TypeSystemClang::GetDisplayTypeName(lldb::opaque_compiler_type_t type) {
   printing_policy.SuppressTagKeyword = true;
   printing_policy.SuppressScope = false;
   printing_policy.SuppressUnwrittenScope = true;
-  // FIXME: should we suppress "All" inline namespaces?
-  printing_policy.SuppressInlineNamespace = llvm::to_underlying(
-      PrintingPolicy::SuppressInlineNamespaceMode::Redundant);
+  printing_policy.SuppressInlineNamespace =
+      llvm::to_underlying(PrintingPolicy::SuppressInlineNamespaceMode::All);
   return ConstString(qual_type.getAsString(printing_policy));
 }
 
diff --git a/lldb/test/API/lang/cpp/inline-namespace-in-typename/Makefile b/lldb/test/API/lang/cpp/inline-namespace-in-typename/Makefile
new file mode 100644
index 0000000000000..99998b20bcb05
--- /dev/null
+++ b/lldb/test/API/lang/cpp/inline-namespace-in-typename/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
diff --git a/lldb/test/API/lang/cpp/inline-namespace-in-typename/TestInlineNamespaceInTypename.py b/lldb/test/API/lang/cpp/inline-namespace-in-typename/TestInlineNamespaceInTypename.py
new file mode 100644
index 0000000000000..19681364466ce
--- /dev/null
+++ b/lldb/test/API/lang/cpp/inline-namespace-in-typename/TestInlineNamespaceInTypename.py
@@ -0,0 +1,30 @@
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestInlineNamespaceInTypename(TestBase):
+    def test(self):
+        """
+        Tests that we correctly omit the inline namespace when printing
+        the type name for "display", even if omitting the inline namespace
+        would be ambiguous in the current context.
+        """
+        self.build()
+        target = self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
+
+        t1 = target.FindGlobalVariables("t1", 1)
+        self.assertTrue(len(t1), 1)
+        self.assertEqual(t1[0].GetDisplayTypeName(), "foo::Duplicate")
+
+        # 'foo::Duplicate' would be an ambiguous reference, but we still
+        # omit the inline namespace when displaying the type.
+        t2 = target.FindGlobalVariables("t2", 1)
+        self.assertTrue(len(t2), 1)
+        self.assertEqual(t2[0].GetDisplayTypeName(), "foo::Duplicate")
+        self.assertEqual(t2[0].GetTypeName(), "foo::bar::Duplicate")
+
+        t3 = target.FindGlobalVariables("t3", 1)
+        self.assertTrue(len(t3), 1)
+        self.assertEqual(t3[0].GetDisplayTypeName(), "foo::Unique")
+        self.assertEqual(t3[0].GetTypeName(), "foo::bar::Unique")
diff --git a/lldb/test/API/lang/cpp/inline-namespace-in-typename/main.cpp b/lldb/test/API/lang/cpp/inline-namespace-in-typename/main.cpp
new file mode 100644
index 0000000000000..eabd93c050e7a
--- /dev/null
+++ b/lldb/test/API/lang/cpp/inline-namespace-in-typename/main.cpp
@@ -0,0 +1,13 @@
+namespace foo {
+struct Duplicate {
+} t1;
+
+inline namespace bar {
+struct Duplicate {
+} t2;
+struct Unique {
+} t3;
+} // namespace bar
+} // namespace foo
+
+int main() { return 0; }