Skip to content

Commit 0bb0e26

Browse files
authored
[lldb][TypeSystemClang] Set SuppressInlineNamespace to 'All' (#171138)
We used to set it to `true` up until recently, see [here](#170802). 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.
1 parent 0fbb45e commit 0bb0e26

File tree

4 files changed

+48
-3
lines changed

4 files changed

+48
-3
lines changed

lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3871,9 +3871,8 @@ TypeSystemClang::GetDisplayTypeName(lldb::opaque_compiler_type_t type) {
38713871
printing_policy.SuppressTagKeyword = true;
38723872
printing_policy.SuppressScope = false;
38733873
printing_policy.SuppressUnwrittenScope = true;
3874-
// FIXME: should we suppress "All" inline namespaces?
3875-
printing_policy.SuppressInlineNamespace = llvm::to_underlying(
3876-
PrintingPolicy::SuppressInlineNamespaceMode::Redundant);
3874+
printing_policy.SuppressInlineNamespace =
3875+
llvm::to_underlying(PrintingPolicy::SuppressInlineNamespaceMode::All);
38773876
return ConstString(qual_type.getAsString(printing_policy));
38783877
}
38793878

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
CXX_SOURCES := main.cpp
2+
3+
include Makefile.rules
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
import lldb
2+
from lldbsuite.test.lldbtest import *
3+
from lldbsuite.test import lldbutil
4+
5+
6+
class TestInlineNamespaceInTypename(TestBase):
7+
def test(self):
8+
"""
9+
Tests that we correctly omit the inline namespace when printing
10+
the type name for "display", even if omitting the inline namespace
11+
would be ambiguous in the current context.
12+
"""
13+
self.build()
14+
target = self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
15+
16+
t1 = target.FindGlobalVariables("t1", 1)
17+
self.assertTrue(len(t1), 1)
18+
self.assertEqual(t1[0].GetDisplayTypeName(), "foo::Duplicate")
19+
20+
# 'foo::Duplicate' would be an ambiguous reference, but we still
21+
# omit the inline namespace when displaying the type.
22+
t2 = target.FindGlobalVariables("t2", 1)
23+
self.assertTrue(len(t2), 1)
24+
self.assertEqual(t2[0].GetDisplayTypeName(), "foo::Duplicate")
25+
self.assertEqual(t2[0].GetTypeName(), "foo::bar::Duplicate")
26+
27+
t3 = target.FindGlobalVariables("t3", 1)
28+
self.assertTrue(len(t3), 1)
29+
self.assertEqual(t3[0].GetDisplayTypeName(), "foo::Unique")
30+
self.assertEqual(t3[0].GetTypeName(), "foo::bar::Unique")
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
namespace foo {
2+
struct Duplicate {
3+
} t1;
4+
5+
inline namespace bar {
6+
struct Duplicate {
7+
} t2;
8+
struct Unique {
9+
} t3;
10+
} // namespace bar
11+
} // namespace foo
12+
13+
int main() { return 0; }

0 commit comments

Comments
 (0)