Base v0 mangling grammar #747
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
philberty
reviewed
Oct 18, 2021
gcc/rust/backend/rust-mangle.cc
Outdated
| @@ -154,6 +155,85 @@ v0_simple_type_prefix (const TyTy::BaseType *ty) | |||
| gcc_unreachable (); | |||
| } | |||
|
|
|||
| // FIXME: Is this present somewhere in libbiberty already? | |||
| static std::string | |||
There was a problem hiding this comment.
I think generic things like this base62 implementation method should really belong to its own file over in https://github.com/Rust-GCC/gccrs/tree/master/gcc/rust/util
There was a problem hiding this comment.
I added the file in f425f31. I think this implementation is used throughout rustc and with more bases, so I'm assuming the function might grow bigger. I also don't really know if this implementation is rustc-specific so for now I'm assuming it is and calling it rust-base62.h :D
philberty
approved these changes
Oct 18, 2021
philberty
requested changes
Oct 18, 2021
CohenArthur
force-pushed
the
base-v0-mangling-grammar
branch
from
f425f31 to
fd9d37c
Compare
philberty
approved these changes
Oct 18, 2021
|
bors r+ |
|
nit: we are now in 2021, not 2020 :)
|
|
@dkm oh, right. We should update all of the copyright headers throughout the project |
|
Build succeeded: |
bors bot
referenced
this pull request
Oct 22, 2021
764: Update GCC/Rust files per 'contrib/update-copyright.py --this-year' r=philberty a=tschwinge As noted by `@dkm/@CohenArthur` in <https://github.com/Rust-GCC/gccrs/pull/747#issuecomment-945581716>/<https://github.com/Rust-GCC/gccrs/issues/747#issuecomment-945585735>. Co-authored-by: Thomas Schwinge <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds base functions to deal with the v0 mangling grammar, found here.
I have a few questions regarding this implementation:
1/ Is there any existing implementation for the base62 algorithm used here? This is directly adapted from rustc's base_n module which I'm assuming is relatively standard and might already exist in the compiler. I haven't been able to find it however.
2/ gccrs cannot yet deal with unicode identifiers, as pointed out by @bjorn3 in #418. This means that a big chunk of the
v0_add_identifierimplementation is missing. Should it be added in this PR too?3/ As mentionned in zulip, it would be great to be able to create unit tests for this piece of code. It would be quite easy to generate a bunch of base62 strings and ensure that the algorithm here matches with them.