-
Notifications
You must be signed in to change notification settings - Fork 13.2k
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
For E0277 suggest adding Result
return type for function when using QuestionMark ?
in the body.
#126187
Conversation
Result
return type for function which using QuestionMark ?
in the body.Result
return type for function when using QuestionMark ?
in the body.
compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs
Outdated
Show resolved
Hide resolved
compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs
Show resolved
Hide resolved
compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs
Outdated
Show resolved
Hide resolved
compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs
Outdated
Show resolved
Hide resolved
compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs
Outdated
Show resolved
Hide resolved
if let hir::ExprKind::Block(b, _) = body.value.kind | ||
&& b.expr.is_none() | ||
{ | ||
// The span will point to the closing curly brace `}` of the block. |
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.
Please add this information as a doc comment to the span
field.
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.
Please add this information as a doc comment to the
span
field.
Thank you.
Sorry, I didn't understand, what should I do?
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.
Add documentation to https://doc.rust-lang.org/nightly/nightly-rustc/rustc_hir/hir/struct.Block.html#structfield.span stating that this span includes the squirly brackets around the block
{ | ||
// The span will point to the closing curly brace `}` of the block. | ||
sugg_spans.push(( | ||
b.span.shrink_to_hi().with_lo(b.span.hi() - BytePos(1)), |
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.
since we know there must be at least one statement, you can instead do b.stmts.last().unwrap().span.shrink_to_hi()
, this way you'll get a span right after the last statement.
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.
since we know there must be at least one statement, you can instead do
b.stmts.last().unwrap().span.shrink_to_hi()
, this way you'll get a span right after the last statement.
Thank you. I tried this way. But when the last stmt is a macro call like println!();
, it's span will point to std
like: D:\source\rust\rust\library\std\src\macros.rs:85:6: 85:6 (#8)
.
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.
ah indeed. hmm... Maybe we should change the value of Block::span
to not include the brackets around it. We can already obtain the "with brackets" span by taking the span of the outer expression. That's a bit more involved though, as you'd probably need to touch the parser.
I don't think the - BytePos(1)
will work in case the block itself is created by a macro or other expansion. Please add some tests for async functions, which should show us any problematic behaviour. And try to generate an entire function with a macro and see how your suggestion behaves on that.
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.
ah indeed. hmm... Maybe we should change the value of
Block::span
to not include the brackets around it. We can already obtain the "with brackets" span by taking the span of the outer expression. That's a bit more involved though, as you'd probably need to touch the parser.I don't think the
- BytePos(1)
will work in case the block itself is created by a macro or other expansion. Please add some tests for async functions, which should show us any problematic behaviour. And try to generate an entire function with a macro and see how your suggestion behaves on that.
Hello, thank you very much.
I tried async and declaring macros and it handles the case of declaring macros correctly.
For async functions, now the current logic will skip it. I added the corresponding conditions and check a async function like:
async fn test2() {
let mut file = File::create("foo.txt")?;
println!();
}
The fix result will be :
async fn test2() -> Result<(), Box<dyn std::error::Error>> {
Ok(())
}
The span seems point to the body correct. But after fixed the expr let mut file = File::create("foo.txt")?;
and println!();
in the body will be overwriten.
Because the current logic does not handle the closure situation, and I have not thought of an effective way to repair the closure, I think we can skip it in this PR.
…g QuesionMark `?` in the body.
Thanks for looking into the details! @bors r+ rollup |
Adding suggestions for following function in E0277.
to
According to the issue #125997, only the code examples in the issue are targeted, but the issue covers a wider range of situations.