Fix debug assertion ICE in suggest_remove_deref with empty span#151508
Fix debug assertion ICE in suggest_remove_deref with empty span#151508zzjas wants to merge 1 commit intorust-lang:mainfrom
Conversation
|
Can you add a test for it? |
Yeah for sure, on it. |
Add is_empty() guard before span_suggestion_verbose to handle proc macros that create deref expressions with overlapping spans.
|
@mati865 Added a test case. Without the fix, running |
|
Thanks, I can't tell whether it's better to use dummy span or skip the error, so I'll reroll. @rustbot reroll |
| ); | ||
| // Empty suggestions with empty spans ICE with debug assertions | ||
| let span = expr.span.until(inner.span); | ||
| if !span.is_empty() { |
There was a problem hiding this comment.
This seems like a very ad-hoc fix, would using !span.from_expansion() do the trick as well?
There was a problem hiding this comment.
Thanks for the suggestion! I tested locally and !span.from_expansion() alone wouldn't work here. The proc macro in the test uses set_span() to copy the input token's span onto the generated * token. Since that span comes from the original source, from_expansion() returns false, and the ICE still triggers. Do you think checking both (!expr.span.from_expansion() && !span.is_empty()) would be better here?
There was a problem hiding this comment.
i feel like i have/know a better solution here, but would able to demonstrate this only tomorrow since i don't remember a name of method and don't have an access to my pc :c
|
Reminder, once the PR becomes ready for a review, use |
|
@fee1-dead, here is what i think, do you have opinion on that? it adds a let suggest_remove_deref = |err: &mut Diag<'_, G>, expr: &hir::Expr<'_>| {
if let Some(pred) = predicate.as_trait_clause()
&& tcx.is_lang_item(pred.def_id(), LangItem::Sized)
&& let hir::ExprKind::Unary(hir::UnOp::Deref, inner) = expr.kind
&& let Some(span) = expr.span.find_ancestor_in_same_ctxt(inner.span)
{
err.span_suggestion_verbose(
span.until(inner.span),
"references are always `Sized`, even if they point to unsized data; consider \
not dereferencing the expression",
String::new(),
Applicability::MaybeIncorrect,
);
}
}; |
This fixes a debug assertion ICE in
suggest_remove_derefwhen a proc macro creates a dereference expression with overlapping spans.When the
*token shares the same span as the inner expression,expr.span.until(inner.span)produces an empty span, which combined with an empty suggestion string triggers the debug assertion indiagnostic.rs.This can be triggered with a proc macro like:
And the following code:
The fix adds an
is_empty()guard following several existing guards in the same file:rust/compiler/rustc_trait_selection/src/error_reporting/traits/suggestions.rs
Lines 686 to 689 in 0f14563
I can provide a full runnable PoC if needed.
Thanks for looking into this!