Fix handling of coherence related lang_items #1970
Merged
Conversation
This file contains hidden or 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
powerboat9
force-pushed
the
int-lang
branch
3 times, most recently
from
268afa8 to
2c5b1d5
Compare
philberty
requested changes
Mar 7, 2023
There was a problem hiding this comment.
I am not sure i like this approach yet for two reasons:
- We are lossing the ability to lookup the const_ptr lang item and we still cant lookup the i32 langitem for example
- In rustc they have lang items defined on the primitive impl blocks how do we implement a check that lang item i32 has already been defined if we remove all of this?
We don't need to copy Rustc and i dont like jumping ahead when we dont have any test cases for this area either.
I would prefer that we just add the mappings for each of the lang items instead of grouping them like this because we don't know how much we dont know in order to implement the rest of libcore.
gcc/rust/ChangeLog: * util/rust-lang-item.h (RustLangItem::ItemType): New enumerators. (RustLangItem::Parse): Parse new enumerators. (RustLangItem::ToString): Handle new enumerators. gcc/testsuite/ChangeLog: * rust/compile/lang-impl.rs: New test.
|
@philberty Fixed |
philberty
approved these changes
Mar 8, 2023
|
little nit: your commit is missing the SIgned-Off-By. Not a big deal, but would probably make the task of pushing all these in gcc easier if it was systematically there :) |
|
@dkm Ah, sorry. I was trying out a new rebase-heavy workflow, and forgot to sign off on the final comment. |
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.
Fixes #1896. I don't think we have coherence checks yet, but the test should detect regressions.