Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Marking remapping with an enum, instead of a bool #75616

Closed
wants to merge 11 commits into from

Conversation

Nicholas-Baron
Copy link
Contributor

@Nicholas-Baron Nicholas-Baron commented Aug 17, 2020

Solves #75550

This branch moves three fields relating to the name of a file of the SourceFile struct from rustc_span and moves them to a separate struct. The intent is to encapulate the state of the fields.

@rust-highfive
Copy link
Collaborator

r? @lcnr

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 17, 2020
pub name_was_remapped: bool,
name: FileName,
/// `true` if the name field was modified by `--remap-path-prefix`.
was_remapped: bool,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this field necessary? Could it instead be derived from the state of unmapped_name and name?

@bors
Copy link
Contributor

bors commented Aug 20, 2020

☔ The latest upstream changes (presumably #75563) made this pull request unmergeable. Please resolve the merge conflicts.

This commit is rather large, as simply moving from boolean to enum
marking results in many places in the crate changing.

This commit does not compile as the `Name` would have to be `pub`, which
may result in even more changes required and is not part of the
originally proposed cleanup.
Use the `new` function instead of an if
This commit has the rustc_span crate compiling, but its dependents need
to be updated.

It seems that the was_remapped bool is unused. It will probably be
removed before the branch lands.
`name.name()` seems a bit awkward, but it follows the semantics from the
old code.
@Nicholas-Baron Nicholas-Baron marked this pull request as ready for review August 20, 2020 20:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants