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.

Sign up for GitHub

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

Jump to bottom

Add TyCtxtAt::{ty_error, ty_error_with_message} #73176

Merged
merged 1 commit into from
Aug 16, 2020
Merged

Add TyCtxtAt::{ty_error, ty_error_with_message} #73176

merged 1 commit into from
Aug 16, 2020

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: Marked as 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
self.sess.delay_span_bug(self.span, msg);
self.mk_ty(Error(super::sty::DelaySpanBugEmitted(())))
Copy link
Member

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

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

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
Member

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
Member

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
Contributor

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
Member

This is unblocked now

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

I’ll rebase as soon as I can.

@LeSeulArtichaut
Copy link
Contributor Author

Rebased.

@mark-i-m
Copy link
Member

cc @varkor (who reviewed my PR too)

@bors
Copy link
Contributor

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
#[track_caller]
pub fn ty_error_with_message(self, msg: &str) -> Ty<'tcx> {
self.sess.delay_span_bug(self.span, msg);
self.mk_ty(Error(super::sty::DelaySpanBugEmitted(())))
Copy link
Member

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
Member

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

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
Member

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

@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
Contributor

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
Contributor

bors commented Aug 16, 2020

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

@bors
Copy link
Contributor

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

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

@eddyb eddyb eddyb approved these changes

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