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

incorrect visitor use #91441

Closed
wants to merge 2 commits into from
Closed

Conversation

estebank
Copy link
Contributor

@estebank estebank commented Dec 1, 2021

No description provided.

@rust-highfive
Copy link
Collaborator

r? @jackh726

(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 Dec 1, 2021
@rust-log-analyzer

This comment has been minimized.

@petrochenkov petrochenkov assigned petrochenkov and unassigned jackh726 Dec 2, 2021
@petrochenkov petrochenkov added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Dec 2, 2021
// println!("{:?} {:?} {:?}", self.ctxt, sp.ctxt(), sp);
// tracing::info!("marked");
// *sp.with_ctxt();
*sp = sp.apply_mark(self.container_id.to_expn_id(), rustc_span::hygiene::Transparency::SemiTransparent);
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not what I was talking about in #90519, all the marks are already added in the TraitDef::span span (self.span in fn create_derived_impl).

This visitor only needs to replace the visited span's context with the TraitDef::span context while keeping the location.

fn visit_span(&mut self, span: &mut Span) {
    *span = span.with_ctxt(self.self_span_from_TraitDef);
}

Copy link
Contributor Author

@estebank estebank Dec 2, 2021

Choose a reason for hiding this comment

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

When doing that I get failures during bootstrap when computing derives, so I assumed I was doing something wrong:

error[E0412]: cannot find type `Range` in this scope
   --> library/core/src/ops/range.rs:78:37
    |
78  |   #[derive(Clone, Default, PartialEq, Eq, Hash)] // not Copy -- see #27186
    |                                       ^^
    |                                       |
    |                                       not found in this scope
    |                                       in this derive macro expansion
    |
   ::: library/core/src/cmp.rs:293:1
    |
293 | / pub macro Eq($item:item) {
294 | |     /* compiler built-in */
295 | | }
    | |_- in this expansion of `#[derive(Eq)]`
    |
help: consider importing this struct
    |
1   | use crate::ops::Range;

With the apply_mark approach on the other hand I could get through bootstrap consistently.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, of course, some parts of the output should not be visited, after all.

The Foo parts, specifically, i.e. references to the type for which we are deriving the impl.

#[derive(Default)]
struct Foo;
#[automatically_derived]
#[allow(unused_qualifications)]
impl ::core::default::Default for Foo {
    #[inline]
    fn default() -> Foo { Foo{} }
}

@petrochenkov petrochenkov 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 Dec 2, 2021
@petrochenkov petrochenkov 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 Dec 2, 2021
@petrochenkov
Copy link
Contributor

Let's just land #90519 with all the pre-existing issues left in place, and no MutVisitor.
I'll look how to fix this in a more centralized way.

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. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants