Skip to content

Add TyCtxtAt::{ty_error, ty_error_with_message}#73176

Merged
bors merged 1 commit intorust-lang:masterfrom
LeSeulArtichaut:tyctxtat-err
Aug 16, 2020
Merged

Add TyCtxtAt::{ty_error, ty_error_with_message}#73176
bors merged 1 commit intorust-lang:masterfrom
LeSeulArtichaut:tyctxtat-err

Conversation

@LeSeulArtichaut
Copy link
Contributor

@LeSeulArtichaut LeSeulArtichaut commented Jun 9, 2020 •
edited
Loading

Only e2d957d was added, the rest comes from #70551.
I was unsure where to put the implementation for those methods, please tell me if there is a better place for it.

Closes #72619, blocked on #70551.

r? @eddyb cc @mark-i-m, maybe this should be part of #70551? If so feel free to cherry-pick or ask me to file a PR against your fork.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 9, 2020
@LeSeulArtichaut LeSeulArtichaut added the S-blocked Status: Blocked on something else such as an RFC or other implementation work. label Jun 9, 2020
mark-i-m
mark-i-m reviewed Jun 9, 2020
View reviewed changes
src/librustc_middle/ty/context.rs Outdated
Comment on lines 2627 to 2605
Copy link
Contributor

@mark-i-m mark-i-m Jun 9, 2020

Choose a reason for hiding this comment

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

Suggested change
self.sess.delay_span_bug(self.span, msg);
self.mk_ty(Error(super::sty::DelaySpanBugEmitted(())))
self.tcx.ty_error_with_message(self.span, msg)

Perhaps I'm missing something?

Copy link
Contributor Author

@LeSeulArtichaut LeSeulArtichaut Jun 9, 2020

Choose a reason for hiding this comment

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

No I'm just dumb :P

Copy link
Member

@eddyb eddyb Jun 25, 2020

Choose a reason for hiding this comment

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

This change doesn't seem to have been made?

Copy link
Contributor

@mark-i-m mark-i-m Jul 16, 2020

Choose a reason for hiding this comment

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

This change still hasn't been made?

@mark-i-m
Copy link
Contributor

mark-i-m commented Jun 9, 2020

I'm personally ok with merging this into my PR, but I don't know whether @eddyb or the compiler team would want it separate.

@bors
Copy link
Collaborator

bors commented Jun 15, 2020

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

@mark-i-m
Copy link
Contributor

mark-i-m commented Jun 19, 2020

This is unblocked now

@LeSeulArtichaut LeSeulArtichaut removed the S-blocked Status: Blocked on something else such as an RFC or other implementation work. label Jun 19, 2020
@LeSeulArtichaut
Copy link
Contributor Author

LeSeulArtichaut commented Jun 19, 2020

I’ll rebase as soon as I can.

@LeSeulArtichaut
Copy link
Contributor Author

LeSeulArtichaut commented Jun 19, 2020

Rebased.

@mark-i-m
Copy link
Contributor

mark-i-m commented Jun 19, 2020

cc @varkor (who reviewed my PR too)

@bors
Copy link
Collaborator

bors commented Jun 22, 2020

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

eddyb
eddyb reviewed Jun 25, 2020
View reviewed changes
src/librustc_middle/ty/context.rs Outdated
Copy link
Member

@eddyb eddyb Jun 25, 2020

Choose a reason for hiding this comment

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

I'm worried about DelaySpanBugEmitted being defined in ty::sty, maybe it should be defined in ty::context so that only ty::context can create it.

Copy link
Contributor

@mark-i-m mark-i-m Jun 25, 2020

Choose a reason for hiding this comment

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

I think the reason I put it where I did is that it needs to be in the type definition of TyKind...

Copy link
Member

@eddyb eddyb Jul 15, 2020

Choose a reason for hiding this comment

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

@mark-i-m that's where it needs to be used, but it's a public type, you can use it from anywhere.

Copy link
Contributor

@mark-i-m mark-i-m Jul 16, 2020

Choose a reason for hiding this comment

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

@eddyb Ah, I think I see what you mean. Opened #74399

@JohnCSimon JohnCSimon added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 14, 2020
@Muirrum Muirrum added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 30, 2020
@LeSeulArtichaut
Copy link
Contributor Author

LeSeulArtichaut commented Aug 4, 2020

@eddyb Just pushed to resolve the previous review. Sorry for being so slow!

@LeSeulArtichaut LeSeulArtichaut added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 5, 2020
@eddyb
Copy link
Member

eddyb commented Aug 16, 2020

@bors r+

@bors
Copy link
Collaborator

bors commented Aug 16, 2020

📌 Commit 30fa84e has been approved by eddyb

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 16, 2020
@bors
Copy link
Collaborator

bors commented Aug 16, 2020

⌛ Testing commit 30fa84e with merge 9b4db69...

@bors
Copy link
Collaborator

bors commented Aug 16, 2020

☀️ Test successful - checks-actions, checks-azure
Approved by: eddyb
Pushing 9b4db69 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 16, 2020
@bors bors merged commit 9b4db69 into rust-lang:master Aug 16, 2020
@LeSeulArtichaut LeSeulArtichaut deleted the tyctxtat-err branch August 16, 2020 21:20
@cuviper cuviper added this to the 1.47.0 milestone May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@mark-i-m mark-i-m mark-i-m left review comments

@eddyb eddyb eddyb approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@eddyb eddyb

Labels

merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

Projects

None yet

Milestone

1.47.0

Development

Successfully merging this pull request may close these issues.

Augment TyCtxt::err to be available via TyCxAt

8 participants

@LeSeulArtichaut @mark-i-m @bors @eddyb @cuviper @JohnCSimon @rust-highfive @Muirrum