[PATCH] D103151: [Demangle][Rust] Parse dyn-bounds

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: llvm/lib/Demangle/RustDemangle.cpp:485-492
+    if (consumeIf('L')) {
+      if (auto Lifetime = parseBase62Number()) {
+        print(" + ");
+        printLifetime(Lifetime);
+      }
+    } else {
+      Error = true;
----------------
keep thinking it'd be nice if this were wrapped up in some (albeit parameterized for different situations you mentioned) "demangleLifetime" to match the syntax description more closely


================
Comment at: llvm/lib/Demangle/RustDemangle.cpp:555
 //
 // <binder> = ["G" <base-62-number>]
 void Demangler::demangleBinder() {
----------------
According to the grammar/spec this is `<binder> = "G" <base-62-number>` rather than `<binder> = ["G" <base-62-number>]`?


================
Comment at: llvm/lib/Demangle/RustDemangle.cpp:556
 // <binder> = ["G" <base-62-number>]
 void Demangler::demangleBinder() {
   uint64_t Binder = parseOptionalBase62Number('G');
----------------
Not sure how consistently this is done throughout the code - but maybe including "optional" when a feature is optional (so it's clear it's implementing `[<binder>]` not `<binder>`) would be good?


================
Comment at: llvm/test/Demangle/rust.test:243-244
+
+CHECK: trait::<for<'a> fn(dyn for<'b> + 'a)>
+       _RIC5traitFG_DG_EL0_EuE
+
----------------
Could this test be simplified down to:
```
CHECK: trait::<dyn for<'b> + 'a>
       _RIC5traitDG_EL_E
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103151