Skip to content

Suggest fn if fun, func, function or def is used to define a function#100547

Merged
bindsdev merged 0 commit into
rust-lang:masterfrom
bindsdev:akabinds/improved-invalid-function-qual-error
Aug 18, 2022
Merged

Suggest fn if fun, func, function or def is used to define a function#100547
bindsdev merged 0 commit into
rust-lang:masterfrom
bindsdev:akabinds/improved-invalid-function-qual-error

Conversation

@bindsdev

Copy link
Copy Markdown
Contributor

Closes #99751

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Aug 14, 2022
@rust-highfive

Copy link
Copy Markdown
Contributor

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @lcnr (or someone else) soon.

Please see the contribution instructions for more information.

@rustbot

rustbot commented Aug 14, 2022

Copy link
Copy Markdown
Collaborator

The Miri submodule was changed

cc @rust-lang/miri

@rust-highfive

Copy link
Copy Markdown
Contributor

⚠️ Warning ⚠️

  • These commits modify submodules.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 14, 2022
@rust-log-analyzer

This comment has been minimized.

@bindsdev

Copy link
Copy Markdown
Contributor Author

@lcnr: According to the CI failure message, and a couple bot messages above, it seems like I somehow modified miri. To make the CI pass and hopefully unblock the merge, what would I do to prevent the CI from failing?

Comment thread compiler/rustc_parse/src/parser/diagnostics.rs Outdated
@lcnr

lcnr commented Aug 15, 2022

Copy link
Copy Markdown
Contributor

you've committed a submodule change. I don't know enough about git to give you the best way to do it, but it should be easiest to use git reset HEAD~1 to undo your commit and then add all files while being careful to not readd the change to the miri submodule.

If you're still stuck I suggest asking for help with this in #compiler/help on zulip

@rust-log-analyzer

This comment has been minimized.

@lcnr

lcnr commented Aug 17, 2022

Copy link
Copy Markdown
Contributor

please merge your commits, this can be done using

git rebase HEAD~4 -i

and then changing pick for all but the first commit to fixup or just f.

I don't know this code enough to tell whether this would also emit this suggestion at places where it doesn't make sense, so

r? rust-lang/compiler

@rust-highfive rust-highfive assigned davidtwco and unassigned lcnr Aug 17, 2022

@davidtwco davidtwco left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This looks good, left one comment, but you'll need to remove the merge commits and reverts and things like that, for this change, you should have only one commit with the changes you're making (if you end up with conflicts, rebase instead of merging).

Comment thread compiler/rustc_parse/src/parser/diagnostics.rs Outdated
@davidtwco davidtwco 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 Aug 17, 2022
@bindsdev bindsdev requested a review from davidtwco August 18, 2022 00:46
@davidtwco

Copy link
Copy Markdown
Member

This still isn't right. I'd suggest just deleting your branch locally, creating a new identically-named branch from master, applying your change again, commiting and force-pushing. That's probably going to be easier.

@bindsdev bindsdev merged commit b8c0a01 into rust-lang:master Aug 18, 2022
@bindsdev bindsdev force-pushed the akabinds/improved-invalid-function-qual-error branch from 371cb5a to 801821d Compare August 18, 2022 11:08
@rustbot rustbot added this to the 1.65.0 milestone Aug 18, 2022
@bindsdev

Copy link
Copy Markdown
Contributor Author

@davidtwco was that supposed to happen? I have never done a force push before and something feels wrong.

@davidtwco

Copy link
Copy Markdown
Member

@davidtwco was that supposed to happen? I have never done a force push before and something feels wrong.

I think it's just that you force pushed master to your branch, rather than master plus additional changes, and that made GitHub think it was merged.

@bindsdev

Copy link
Copy Markdown
Contributor Author

@davidtwco was that supposed to happen? I have never done a force push before and something feels wrong.

I think it's just that you force pushed master to your branch, rather than master plus additional changes, and that made GitHub think it was merged.

I see. It seems like no changes were recorded also. So what shall I do now?

@davidtwco

Copy link
Copy Markdown
Member

I see. It seems like no changes were recorded also. So what shall I do now?

I think you should be able to make the changes for this fix and then commit and push it.

@bindsdev

Copy link
Copy Markdown
Contributor Author

I see. It seems like no changes were recorded also. So what shall I do now?

I think you should be able to make the changes for this fix and then commit and push it.

Will I have to make another PR?

@davidtwco

Copy link
Copy Markdown
Member

I don't think so, if you add changes this should re-open, but there's no harm in another PR if that's easier too.

@bindsdev

Copy link
Copy Markdown
Contributor Author

I don't think so, if you add changes this should re-open, but there's no harm in another PR if that's easier too.

I pushed the changes about 14 hours ago. Seems like it wasn't reopened.

@davidtwco

Copy link
Copy Markdown
Member

I don't think so, if you add changes this should re-open, but there's no harm in another PR if that's easier too.

I pushed the changes about 14 hours ago. Seems like it wasn't reopened.

Feel free to open a new PR then, and include r? @davidtwco (outside of a code block) in the description so that I'm assigned.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. 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.

Suggest fn if fun, func, function or def is used instead

7 participants