-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Extended save-analysis to support generated code #31097
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
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @brson (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
@nrc Heads up |
@@ -909,6 +914,20 @@ impl CodeMap { | |||
output | |||
} | |||
|
|||
pub fn root_callsite(&self, sp: Span) -> Span { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rename to source_callsite and add a comment
Could you add some test stuff to the DXR test please? |
mod_data.scope, | ||
&mod_data.filename); | ||
if let Some(mod_data) = self.save_ctxt.get_item_data(item) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: remove blank line
Looks good, needs a few style nits addressing and some minor refactoring, then r+ |
and a tidy error to fix |
c32fd53
to
a52e8af
Compare
Applied nits, it should be good to go now, if the linter allows it. Only added a few tests - deriving everything, various forms of printing, and some macros inside a macro_use module. |
@DanielJCampbell you can quickly check the formatting with |
if enum_data.is_none() { | ||
return; | ||
} | ||
let enum_data = enum_data.unwrap(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer let enum_data = match enum_data ...
instead of if
then unwrap
Nice, r+ with the last couple of nits tidied up. |
… in format_args! and derive to maintain compatability
a52e8af
to
616bfb6
Compare
Seems good now |
@bors: r+ |
📌 Commit 616bfb6 has been approved by |
Also altered the format_args! syntax extension, and \#[derive(debug)], to maintain compatability. r? @ nrc
Also altered the format_args! syntax extension, and #[derive(debug)], to maintain compatability.
r? @ nrc