Skip to content

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

Closed
Nicholas-Baron wants to merge 11 commits into
rust-lang:masterfrom
Nicholas-Baron:master
Closed

Marking remapping with an enum, instead of a bool#75616
Nicholas-Baron wants to merge 11 commits into
rust-lang:masterfrom
Nicholas-Baron:master

Conversation

@Nicholas-Baron

@Nicholas-BaronNicholas-Baron commented Aug 17, 2020

Copy link
Copy Markdown
Contributor

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
Copy Markdown
Contributor

r? @lcnr

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

@rust-highfiverust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 17, 2020
Comment threadsrc/librustc_span/lib.rs Outdated
Comment threadsrc/librustc_span/lib.rs Outdated

Copy link
Copy Markdown
ContributorAuthor

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

bors commented Aug 20, 2020

Copy link
Copy Markdown
Collaborator

☔ 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 freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-reviewStatus: Awaiting review from the assignee but also interested parties.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@Nicholas-Baron@rust-highfive@bors@lcnr