[PATCH] D102179: [Demangle][Rust] Parse integer constants

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

tmiasko added inline comments.


================
Comment at: llvm/lib/Demangle/RustDemangle.cpp:418-420
+  APInt Value(HexDigits.size() * 4 /* NumBits */,
+              StringRef(HexDigits.begin(), HexDigits.size()), 16 /* Radix */);
+  SmallString<40> Str;
----------------
dblaikie wrote:
> 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?)
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102179