[PATCH] D101821: [Demangle][Rust] Print special namespaces

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: llvm/test/Demangle/rust.test:14-25
+CHECK: ns::f::{closure#0}
+       _RNCNvCs21hi0yVfW1J_2ns1f0
+
+CHECK: ns::f::{closure#1}
+       _RNCNvCs21hi0yVfW1J_2ns1fs_0
+
+CHECK: ns::f::{closure#1}::{closure#0}
----------------
Do these benefit from being inside two namespaces? I think it'd be great if features were tested as much in isolation as possible, with maybe a single test demonstrating that a feature works in a particular context (which is really testing the outer production - and should used the simplest instance of the inner production as possible).

There's also a rather large and, so far as I can tell, arbitrary, disambiguator ('s21hi0yVfW1J') - is there a particular reason for that being present, and having such a long/complicated value?

ie, maybe these tests could be:
```
_RNCC5crate0
_RNCC5crates_0
_RNCNCC5crates_00
_RNCNCC5crates_01g
```

& I'm still thinking maybe some kind of comment would be useful to describe these things, though I'm not sure exactly what.
```
_R N [ C [ C 5crate ] 0 ]
_R N [ C [ C 5crate ] [ s _0 ] ]
_R N [ C [ N [ C [ C 5crate ] [ s _0 ] ] ] 0 ]
```

In any case - could you simplify down the test cases to make them as narrow and legible/obvious as possible? Maybe add some comments about the invalid/mangled cases - where the invalidity is interesting, for instance? But hopefully if they're simplified way down, the invalidity will be more clear/obvious?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101821/new/

https://reviews.llvm.org/D101821