[libcxx-commits] [PATCH] D104366: [Demangle][Rust] Parse non-ASCII identifiers

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

tmiasko added reviewers: erik.pilkington, ldionne.
tmiasko added subscribers: erik.pilkington, ldionne.
tmiasko added a comment.

Thanks for looking David!

In addition to the test cases here, I was also compiling source code with randomly generated non-ASCII identifiers and subsequently cross-validating demangled symbols with other implementations. The only issue I observed at the time, was that compilation time is quadratic with respect to the number of multibyte characters in the source code ...

@ldionne @erik.pilkington could you take a look at the new insert method added to demangling Utility.h? (I synchronized changes with a copy in libcxxabi).



================
Comment at: libcxxabi/src/demangle/Utility.h:129
 
+  void insert(size_t Pos, const char *S, size_t N) {
+    assert(Pos <= CurrentPosition);
----------------
dblaikie wrote:
> Could use an `Arrayref<const char>` (Or `StringRef`) instead of separate `const char *` and `size_t` parameters. Not necessary, there's some value in the symmetry with standard library functions (though I guess they're more likely to use begin/end rather than begin/length), etc.
The API is based on the standard `std::string::insert` with the same type signature. I would prefer to leave this as is.


================
Comment at: llvm/lib/Demangle/RustDemangle.cpp:547-548
       Identifier Ident = parseIdentifier();
+      if (Ident.Punycode)
+        Error = true;
       for (char C : Ident.Name) {
----------------
dblaikie wrote:
> Is this error path tested?
This error code path is covered by:

```
; Invalid ABI with punycode.

CHECK: _RIC8functionFKu3n3hEuE
       _RIC8functionFKu3n3hEuE
```

If Punycode were to be supported in this position, it would demangle to `function::<unsafe extern "☃" fn()>`.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104366