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

[Tomasz Miąsko via Phabricator via llvm-commits]

tmiasko 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}
----------------
dblaikie wrote:
> 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?
I used rustc to generate the test cases previously. The extra nesting is necessary at language level. The long disambiguator `s21hi0yVfW1J` is a crate hash. Together with a crate name it forms a unique global identifier of the crate (c++filt shows / omits those hashes output based on `-i` flag, and I would like to implement something similar later on).

Neither of those aspect is strictly necessary for testing, so I reduced the test cases, but they no longer correspond to valid Rust programs.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101821