[PATCH] D102142: [Demangle][Rust] Parse basic types

[David Blaikie via Phabricator via llvm-commits]

dblaikie accepted this revision.
dblaikie added a comment.
This revision is now accepted and ready to land.

Looks good - maybe some patch reordering, maybe the generic rule is as good as the "M" or "Y" rule to test `<type>` - your call.



================
Comment at: llvm/lib/Demangle/RustDemangle.cpp:119
 // <path> = "C" <identifier>               // crate root
 //        | "M" <impl-path> <type>         // <T> (inherent impl)
 //        | "X" <impl-path> <type> <path>  // <T as Trait> (trait impl)
----------------
Maybe it'd be more direct to implement/test `type` here (or with "Y" below) (or is there some even more direct production/reference to types?) rather than as parameters to generic types?

Then the generic type testing can include one simple nested type without using generic types as the means to test all the basic types?



================
Comment at: llvm/lib/Demangle/RustDemangle.cpp:236
+
+static const char *parseBasicType(char C) {
+  if (isLower(C))
----------------
Spec comment for `<basic-type>` (I realize it's not much more than the table of implementation - but when looking around for the `<basic-type>` rule, it'd be nice if it were written somewhere/here)


================
Comment at: llvm/lib/Demangle/RustDemangle.cpp:239-240
+    return BasicTypes[C - 'a'];
+  else
+    return nullptr;
+}
----------------
Skip the else after return ( https://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return )



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102142