[Demangle][Rust] Parse integer constants

[dblaikie]

llvm/lib/Demangle/RustDemangle.cpp

203–204

Previous code didn't have an error path for this missing piece - any particular reason to add it now (& thus have to test it) compared to not supporting this & having const or type as the only options (& if it's not const, parse it as a type - as the previous code did, parsing everything as a type regardless)

288

Somewhat surprising that though this production uses <basic-type> but the implementation doesn't call parseBasicType - any chance of sharing that implementation detail, trivial as it is?

319

grammar production comment for this?

327–332

switching between int and decimal for the demangling seems like it could be problematic for readers/parsers/etc, maybe? (what does llvm-cxxfilt do for equivalent C++ numbered values (like integer template parameters))

Maybe APSInt could be used which supports bigger (though not arbitrarily big, I don't think... - I think you have to pick the size, though I guess you could pick it dynamically based on the number of hex digits) integers.

llvm/test/Demangle/rust.test

151–185

Do these benefit from the second parameter that shows the type? Or is the type testing redundant with the existing type testing added in the previous patch?

[pirama]

@tmiasko Thanks for adding this support to llvm. We're already using your work in Android. Can you comment on the status of this support? For e.g., I see that the following name is not demangled:

_RNvYNtNtNtCs7WMZDM8jTtQ_10env_logger3fmt6writer6TargetNtNtCsfOHkQPwunBC_4core3cmp9PartialEq2neB8_.

This symbol is generated with rust 1.51 with -Z symbol-mangling-version=v0

[tmiasko]

Address review comments

• Add an basic type enum & reuse basic type parsing.
• Use APInt to format all const integers as decimals.

llvm/lib/Demangle/RustDemangle.cpp

203–204

No particular reason. I reverted the changes so that code falls back to the type production as before.

288

I introduced an enum for a basic types and reused the parsing code. What do you think?

327–332

I used APInt to consistently format values as decimals. This matches rustc pretty printing which consistently uses decimal as well.

In itanium this is non-issue since values have decimal mangling.

llvm/test/Demangle/rust.test

151–185

I included a type argument in addition to a const integer value, so that demangled from clearly indicates what type is being tested, and to visually distinguish checks from each other (in case one of them fails for example).

[tmiasko]

@pirama path productions which are necessary to demangle most symbols are not implemented yet. I can let you know the implementation is complete. At that point any reports about not demangled symbols would be quite valuable.

[dblaikie]

llvm/lib/Demangle/RustDemangle.cpp

288

Looks pretty good - if you like, you could commit that as a preliminary refactoring-only commit. (might be a good point to follow the steps to get commit access for changes like this)

299–301

Hmm - I'm just realizing that it looks like the demangle library has no dependencies on the rest of LLVM (the StringView type was a hint I saw earlier - but now looking more closely).

I'm guessing that's intentional & I'm surprised this code links successfully - I'd expect some part of SmallString or APInt would have some link-time dependence on the rest of LLVM, which I think (looking at the CMake file at least) there's no dependency on from the demangle library? Hmm, maybe llvm-cxxfilt has such a dependency and it's being picked up by accident (are you using lld or a windows linker, by chance? Those will be a bit more flexible about fulfilling library dependencies, perhaps).

So maybe we'll have to stick with the previous implementation you had here :/ (if so, might be worth a comment explaining why - though I don't immediately know where this constraint for libDemangle is documented/comes from - could you check around a bit in the library/version control history, maybe?)

llvm/test/Demangle/rust.test

151–185

perhaps rather than relying on the type encoding to test the integer encoding, the type could be rendered in the name (replacing the "integer" string) instead? (though that may be harder to read in the mangling - maybe not though)

[tmiasko]

llvm/lib/Demangle/RustDemangle.cpp

299–301

The code of Itanium demangler (ItaniumDemangle.h, DemangleConfig.h, StringView.h, and Utility.h) is shared with libcxxabi. The initial commit that introduced demangler to LLVM, comments:

The code also has no dependencies on anything else in LLVM. To enforce
that I added it as another library. That way a BUILD_SHARED_LIBS will
fail if anyone adds an use of StringRef for example.

The build with BUILD_SHARED_LIBS does indeed fail.

The implementation of the Itanium demangler itself cannot any dependencies, but it seems to me that we could add them to the demangler component. I don't know if we want to do that. Functionally wise, the difference is rather minor. Apart from APInt used here, I was potentially looking into using unicode::isPrintable when demangling char constants.

[tmiasko]

• Remove dependencies from implementation.
• Show integer constants wider than 64-bits as hexadecimal numbers.
• Include integer types in non-mangled form in tests

[dblaikie]

Seems good, thanks!

llvm/lib/Demangle/RustDemangle.cpp

446–448

Might be worth a more formal description here, something like:

The return value is unspecified if HexDigits.size() > 16.

(so you can't even rely on the return value being zero, maximum, or some other default value - it's arbitrary/meaningless)

llvm/include/llvm/Demangle/RustDemangle.h

public:
   Demangler(size_t MaxRecursionLevel = 500);
 
   bool demangle(StringView MangledName);
 
 private:
   void demanglePath();
   void demangleGenericArg();
   void demangleType();
+  void demangleConst();
+  void demangleConstInt();
 
   Identifier parseIdentifier();
   uint64_t parseOptionalBase62Number(char Tag);
   uint64_t parseBase62Number();
   uint64_t parseDecimalNumber();
+  uint64_t parseHexNumber(StringView &HexDigits);
 
   void print(char C) {
     if (Error)
       return;
 
     Output += C;
   }
 

llvm/lib/Demangle/RustDemangle.cpp

void Demangler::demanglePath() {
   }
 }
 
 // <generic-arg> = <lifetime>
 //               | <type>
 //               | "K" <const>
 // <lifetime> = "L" <base-62-number>
 void Demangler::demangleGenericArg() {
-  // FIXME parse remaining productions
+  if (consumeIf('L'))
+    Error = true; // FIXME demangle lifetimes.
+  else if (consumeIf('K'))
+    demangleConst();
+  else
     demangleType();
 }
 
 static const char *const BasicTypes[] = {
     "i8",    // a
     "bool",  // b
     "char",  // c
     "f64",   // d
     "str",   // e
void Demangler::demangleType() {
   if (const char *BasicType = parseBasicType(consume())) {
     print(BasicType);
   } else {
     // FIXME parse remaining productions.
     Error = true;
   }
 }
 
+// <const> = <basic-type> <const-data>
+//         | "p"                          // placeholder
+//         | <backref>
+// <const-data> = ["n"] {<hex-digit>} "_"
+void Demangler::demangleConst() {
+  char Type = consume();
+  switch (Type) {
+  case 'a': // i8
+  case 'h': // u8
+  case 'i': // isize
+  case 'j': // usize
+  case 'l': // i32
+  case 'm': // u32
+  case 'n': // i128
+  case 'o': // u128
+  case 's': // i16
+  case 't': // u16
+  case 'x': // i64
+  case 'y': // u64
+    demangleConstInt();
+    break;
+  case 'p': // placeholder
+    print('_');
+    break;
+  default:
+    // FIXME demangle backreferences, bool constants, and char constants.
+    Error = true;
+    break;
+  }
+}
+
+void Demangler::demangleConstInt() {
+  bool IsNegative = consumeIf('n');
+  StringView HexDigits;
+  uint64_t Value = parseHexNumber(HexDigits);
+
+  if (IsNegative)
+    print('-');
+
+  if (HexDigits.size() <= 16) {
+    printDecimalNumber(Value);
+  } else {
+    print("0x");
+    print(HexDigits);
+  }
+}
+
 // <undisambiguated-identifier> = ["u"] <decimal-number> ["_"] <bytes>
 Identifier Demangler::parseIdentifier() {
   bool Punycode = consumeIf('u');
   uint64_t Bytes = parseDecimalNumber();
 
   // Underscore resolves the ambiguity when identifier starts with a decimal
   // digit or another underscore.
   consumeIf('_');
while (isDigit(look())) {
 
     uint64_t D = consume() - '0';
     if (!addAssign(Value, D))
       return 0;
   }
 
   return Value;
 }
+
+// Parses a hexadecimal number with <0-9a-f> as a digits. Returns the parsed
+// value and stores hex digits in HexDigits. The resulting value might be not
+// representable in uint64_t, so check the length of hex digits (<= 16), before
+// using the returned value.
+//
+// <hex-number> = {<0-9a-f>} "_"
+uint64_t Demangler::parseHexNumber(StringView &HexDigits) {
+  size_t Start = Position;
+  uint64_t Value = 0;
+
+  while (!Error && !consumeIf('_')) {
+    char C = consume();
+    Value *= 16;
+    if (isDigit(C))
+      Value += C - '0';
+    else if ('a' <= C && C <= 'f')
+      Value += 10 + (C - 'a');
+    else
+      Error = true;
+  }
+
+  if (Error) {
+    Value = 0;
+    HexDigits = StringView();
+    return Value;
+  }
+
+  assert(Start < Position);
+  size_t N = Position - 1 - Start;
+  HexDigits = Input.substr(Start, N);
+  return Value;
+}

llvm/test/Demangle/rust.test

CHECK: generic::<_>
        _RIC7genericpE
 
 CHECK: generic::<_, _>
        _RIC7genericppE
 
 CHECK: generic::<_, _, _>
        _RIC7genericpppE
 
+; Generic const arguments
+
+CHECK: generic_const::<_>
+       _RIC13generic_constKpE
+
 ; Basic types
 
 CHECK: basic::<i8>
        _RIC5basicaE
 
 CHECK: basic::<bool>
        _RIC5basicbE
 
CHECK: basic::<i64>
        _RIC5basicxE
 
 CHECK: basic::<u64>
        _RIC5basicyE
 
 CHECK: basic::<!>
        _RIC5basiczE
 
+; Integer constants. Test value demangling.
+
+CHECK: integer::<0>
+       _RIC7integerKi0_E
+
+CHECK: integer::<1>
+       _RIC7integerKi1_E
+
+CHECK: integer::<-1>
+       _RIC7integerKin1_E
+
+CHECK: integer::<-15>
+       _RIC7integerKinf_E
+
+CHECK: integer::<-16>
+       _RIC7integerKin10_E
+
+CHECK: integer::<-17>
+       _RIC7integerKin11_E
+
+CHECK: integer::<127>
+       _RIC7integerKi7f_E
+
+CHECK: integer::<18446744073709551615>
+       _RIC7integerKoffffffffffffffff_E
+
+CHECK: integer::<0x10000000000000000>
+       _RIC7integerKo10000000000000000_E
+
+CHECK: integer::<-0x123456789abcdef01>
+       _RIC7integerKnn123456789abcdef01_E
+
+; Integer constants. Test all integer types.
+
+CHECK: integer::<0, i8>
+       _RIC7integerKa0_aE
+
+CHECK: integer::<0, u8>
+       _RIC7integerKh0_hE
+
+CHECK: integer::<0, isize>
+       _RIC7integerKi0_iE
+
+CHECK: integer::<0, usize>
+       _RIC7integerKj0_jE
+
+CHECK: integer::<0, i32>
+       _RIC7integerKl0_lE
+
+CHECK: integer::<0, u32>
+       _RIC7integerKm0_mE
+
+CHECK: integer::<0, i128>
+       _RIC7integerKn0_nE
+
+CHECK: integer::<0, u128>
+       _RIC7integerKo0_oE
+
+CHECK: integer::<0, i16>
+       _RIC7integerKs0_sE
+
+CHECK: integer::<0, u16>
+       _RIC7integerKt0_tE
+
+CHECK: integer::<0, i64>
+       _RIC7integerKx0_xE
+
+CHECK: integer::<0, u64>
+       _RIC7integerKy0_yE
+
 ; Invalid mangled characters
 
 CHECK: _RNvC2a.1c
        _RNvC2a.1c
 
 CHECK: _RNvC2a$1c
        _RNvC2a$1c