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.

Sign up for GitHub

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

Do not provide suggestions when the spans come from expanded code that doesn't point at user code #109082

Closed
wants to merge 14 commits into from
Closed

Do not provide suggestions when the spans come from expanded code that doesn't point at user code #109082

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
ba7d52a
Handle proc-macro spans pointing at attribute in suggestions
estebank Mar 10, 2023
fbb70c4
Fix range_minus_one and range_plus_one clippy lints
estebank Mar 14, 2023
c07af66
Tweak E0412 label for proc-macros
estebank Mar 14, 2023
b534f3b
Do not suggest binding from outside of a macro in macro
estebank Mar 14, 2023
88bbe80
Avoid incorrect argument suggestions in macros
estebank Mar 15, 2023
d1606b1
Tweak `alloc_error_handler` desugaring
estebank Mar 15, 2023
6421705
Rename and document span marking method
estebank Mar 15, 2023
d3eb0f8
Fix #90557
estebank Mar 15, 2023
0986978
Small cleanup
estebank Mar 15, 2023
0a0b470
make mark_span_for_resize inert for perf comparison
estebank Mar 21, 2023
a220652
remove DesugaringKind::Resize
estebank Mar 21, 2023
91d1a3f
only keep peel_ctxt removed
estebank Mar 23, 2023
d42105b
wip
estebank Mar 24, 2023
3003f72
fix test
estebank Apr 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions compiler/rustc_ast_lowering/src/path.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -288,10 +288,15 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
segment_ident_span.find_ancestor_inside(path_span).unwrap_or(path_span)
} else if generic_args.is_empty() {
// If there are brackets, but not generic arguments, then use the opening bracket
generic_args.span.with_hi(generic_args.span.lo() + BytePos(1))
self.tcx
.mark_span_for_resize(generic_args.span)
.with_hi(generic_args.span.lo() + BytePos(1))
} else {
// Else use an empty span right after the opening bracket.
generic_args.span.with_lo(generic_args.span.lo() + BytePos(1)).shrink_to_lo()
self.tcx
.mark_span_for_resize(generic_args.span)
.with_lo(generic_args.span.lo() + BytePos(1))
.shrink_to_lo()
};

generic_args.args.insert_many(
Expand Down
12 changes: 9 additions & 3 deletions compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2250,12 +2250,18 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
Ok(string) => {
if string.starts_with("async ") {
let pos = args_span.lo() + BytePos(6);
(args_span.with_lo(pos).with_hi(pos), "move ")
(
self.infcx.tcx.mark_span_for_resize(args_span).with_lo(pos).with_hi(pos),
"move ",
)
} else if string.starts_with("async|") {
let pos = args_span.lo() + BytePos(5);
(args_span.with_lo(pos).with_hi(pos), " move")
(
self.infcx.tcx.mark_span_for_resize(args_span).with_lo(pos).with_hi(pos),
" move",
)
} else {
(args_span.shrink_to_lo(), "move ")
(self.infcx.tcx.mark_span_for_resize(args_span).shrink_to_lo(), "move ")
}
}
Err(_) => (args_span, "move |<args>| <body>"),
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_borrowck/src/diagnostics/move_errors.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -470,15 +470,15 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
match self.infcx.tcx.sess.source_map().span_to_snippet(span) {
Ok(snippet) if snippet.starts_with('*') => {
err.span_suggestion_verbose(
span.with_hi(span.lo() + BytePos(1)),
self.infcx.tcx.mark_span_for_resize(span).with_hi(span.lo() + BytePos(1)),
"consider removing the dereference here",
String::new(),
Applicability::MaybeIncorrect,
);
}
_ => {
err.span_suggestion_verbose(
span.shrink_to_lo(),
self.infcx.tcx.mark_span_for_resize(span).shrink_to_lo(),
"consider borrowing here",
'&',
Applicability::MaybeIncorrect,
Expand Down
24 changes: 14 additions & 10 deletions compiler/rustc_builtin_macros/src/alloc_error_handler.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,16 @@ pub fn expand(

// Allow using `#[alloc_error_handler]` on an item statement
// FIXME - if we get deref patterns, use them to reduce duplication here
let (item, is_stmt, sig_span) =
let (item, is_stmt, sig) =
if let Annotatable::Item(item) = &item
&& let ItemKind::Fn(fn_kind) = &item.kind
{
(item, false, ecx.with_def_site_ctxt(fn_kind.sig.span))
(item, false, &fn_kind.sig)
} else if let Annotatable::Stmt(stmt) = &item
&& let StmtKind::Item(item) = &stmt.kind
&& let ItemKind::Fn(fn_kind) = &item.kind
{
(item, true, ecx.with_def_site_ctxt(fn_kind.sig.span))
(item, true, &fn_kind.sig)
} else {
ecx.sess.parse_sess.span_diagnostic.emit_err(errors::AllocErrorMustBeFn {span: item.span() });
return vec![orig_item];
Expand All @@ -40,10 +40,10 @@ pub fn expand(
let span = ecx.with_def_site_ctxt(item.span);

// Generate item statements for the allocator methods.
let stmts = thin_vec![generate_handler(ecx, item.ident, span, sig_span)];
let stmts = thin_vec![generate_handler(ecx, item.ident, span, sig)];

// Generate anonymous constant serving as container for the allocator methods.
let const_ty = ecx.ty(sig_span, TyKind::Tup(ThinVec::new()));
let const_ty = ecx.ty(ecx.with_def_site_ctxt(sig.span), TyKind::Tup(ThinVec::new()));
let const_body = ecx.expr_block(ecx.block(span, stmts));
let const_item = ecx.item_const(span, Ident::new(kw::Underscore, span), const_ty, const_body);
let const_item = if is_stmt {
Expand All @@ -60,27 +60,31 @@ pub fn expand(
// unsafe fn __rg_oom(size: usize, align: usize) -> ! {
// handler(core::alloc::Layout::from_size_align_unchecked(size, align))
// }
fn generate_handler(cx: &ExtCtxt<'_>, handler: Ident, span: Span, sig_span: Span) -> Stmt {
fn generate_handler(cx: &ExtCtxt<'_>, handler: Ident, span: Span, sig: &FnSig) -> Stmt {
let usize = cx.path_ident(span, Ident::new(sym::usize, span));
let ty_usize = cx.ty_path(usize);
let size = Ident::from_str_and_span("size", span);
let align = Ident::from_str_and_span("align", span);

let sig_span = cx.with_def_site_ctxt(sig.span);
let ret_sp = cx.with_def_site_ctxt(sig.decl.output.span());

// core::alloc::Layout::from_size_align_unchecked(size, align)
let layout_new = cx.std_path(&[sym::alloc, sym::Layout, sym::from_size_align_unchecked]);
let layout_new = cx.expr_path(cx.path(span, layout_new));
let layout = cx.expr_call(
span,
cx.with_def_site_ctxt(sig.decl.inputs.get(0).map_or(span, |p| p.span)),
layout_new,
thin_vec![cx.expr_ident(span, size), cx.expr_ident(span, align)],
);

let call = cx.expr_call_ident(sig_span, handler, thin_vec![layout]);
let call =
cx.expr(ret_sp, ast::ExprKind::Call(cx.expr_ident(span, handler), thin_vec![layout]));

let never = ast::FnRetTy::Ty(cx.ty(span, TyKind::Never));
let params = thin_vec![cx.param(span, size, ty_usize.clone()), cx.param(span, align, ty_usize)];
let decl = cx.fn_decl(params, never);
let header = FnHeader { unsafety: Unsafe::Yes(span), ..FnHeader::default() };
let sig = FnSig { decl, header, span: span };
let sig = FnSig { decl, header, span };

let body = Some(cx.block_expr(call));
let kind = ItemKind::Fn(Box::new(Fn {
Expand Down
10 changes: 8 additions & 2 deletions compiler/rustc_const_eval/src/transform/check_consts/ops.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,11 +256,17 @@ impl<'tcx> NonConstOp<'tcx> for FnCallNonConst<'tcx> {
{
let rhs_pos =
span.lo() + BytePos::from_usize(eq_idx + 2 + rhs_idx);
let rhs_span = span.with_lo(rhs_pos).with_hi(rhs_pos);
let rhs_span = tcx
.mark_span_for_resize(span)
.with_lo(rhs_pos)
.with_hi(rhs_pos);
err.multipart_suggestion(
"consider dereferencing here",
vec![
(span.shrink_to_lo(), deref.clone()),
(
tcx.mark_span_for_resize(span).shrink_to_lo(),
deref.clone(),
),
(rhs_span, deref),
],
Applicability::MachineApplicable,
Expand Down
18 changes: 16 additions & 2 deletions compiler/rustc_errors/src/diagnostic.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -621,6 +621,10 @@ impl Diagnostic {
.map(|(span, snippet)| SubstitutionPart { snippet, span })
.collect::<Vec<_>>();

if !parts.iter().all(|sub| sub.span.can_be_used_for_suggestions()) {
return self;
}

parts.sort_unstable_by_key(|part| part.span);

assert!(!parts.is_empty());
Expand Down Expand Up @@ -711,6 +715,9 @@ impl Diagnostic {
!(sp.is_empty() && suggestion.to_string().is_empty()),
"Span must not be empty and have no suggestion"
);
if !sp.can_be_used_for_suggestions() {
return self;
}
self.push_suggestion(CodeSuggestion {
substitutions: vec![Substitution {
parts: vec![SubstitutionPart { snippet: suggestion.to_string(), span: sp }],
Expand Down Expand Up @@ -774,6 +781,9 @@ impl Diagnostic {
!(sp.is_empty() && suggestions.iter().any(|suggestion| suggestion.is_empty())),
"Span must not be empty and have no suggestion"
);
if !sp.can_be_used_for_suggestions() {
return self;
}

let substitutions = suggestions
.into_iter()
Expand All @@ -799,12 +809,16 @@ impl Diagnostic {
) -> &mut Self {
let substitutions = suggestions
.into_iter()
.map(|sugg| {
.filter_map(|sugg| {
let mut parts = sugg
.into_iter()
.map(|(span, snippet)| SubstitutionPart { snippet, span })
.collect::<Vec<_>>();

if !parts.iter().all(|sub| sub.span.can_be_used_for_suggestions()) {
return None;
}

parts.sort_unstable_by_key(|part| part.span);

assert!(!parts.is_empty());
Expand All @@ -819,7 +833,7 @@ impl Diagnostic {
"suggestion must not have overlapping parts",
);

Substitution { parts }
Some(Substitution { parts })
})
.collect();

Expand Down
16 changes: 9 additions & 7 deletions compiler/rustc_errors/src/emitter.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@
use Destination::*;

use rustc_span::source_map::SourceMap;
use rustc_span::{FileLines, SourceFile, Span};
use rustc_span::{DesugaringKind, FileLines, SourceFile, Span};
// use rustc_span::{FileLines, SourceFile, Span};

use crate::snippet::{
Annotation, AnnotationColumn, AnnotationType, Line, MultilineAnnotation, Style, StyledString,
Expand Down Expand Up @@ -402,12 +403,13 @@ pub trait Emitter: Translate {
// entries we don't want to print, to make sure the indices being
// printed are contiguous (or omitted if there's only one entry).
let macro_backtrace: Vec<_> = sp.macro_backtrace().collect();
for (i, trace) in macro_backtrace.iter().rev().enumerate() {
if trace.def_site.is_dummy() {
continue;
}

if always_backtrace && !matches!(trace.kind, ExpnKind::Inlined) {
for (i, trace) in macro_backtrace.iter().rev().enumerate().filter(|(_, trace)| {
!matches!(
trace.kind,
ExpnKind::Inlined | ExpnKind::Desugaring(DesugaringKind::Resize)
) && !trace.def_site.is_dummy()
}) {
if always_backtrace {
new_labels.push((
trace.def_site,
format!(
Expand Down
9 changes: 6 additions & 3 deletions compiler/rustc_hir_analysis/src/check/compare_impl_item.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1355,7 +1355,8 @@ fn compare_number_of_method_arguments<'tcx>(
if pos == 0 {
arg.span
} else {
arg.span.with_lo(trait_m_sig.decl.inputs[0].span.lo())
tcx.mark_span_for_resize(arg.span)
.with_lo(trait_m_sig.decl.inputs[0].span.lo())
}
})
})
Expand All @@ -1371,7 +1372,7 @@ fn compare_number_of_method_arguments<'tcx>(
if pos == 0 {
arg.span
} else {
arg.span.with_lo(impl_m_sig.decl.inputs[0].span.lo())
tcx.mark_span_for_resize(arg.span).with_lo(impl_m_sig.decl.inputs[0].span.lo())
}
})
.unwrap_or_else(|| tcx.def_span(impl_m.def_id));
Expand Down Expand Up @@ -1468,7 +1469,9 @@ fn compare_synthetic_generics<'tcx>(

// in case there are no generics, take the spot between the function name
// and the opening paren of the argument list
let new_generics_span = tcx.def_ident_span(impl_def_id)?.shrink_to_hi();
let new_generics_span = tcx
.mark_span_for_resize(tcx.def_ident_span(impl_def_id)?)
.shrink_to_hi();
// in case there are generics, just replace them
let generics_span =
impl_m.generics.span.substitute_dummy(new_generics_span);
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_analysis/src/check/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ fn missing_items_err(
let hi = full_impl_span.hi() - BytePos(1);
// Point at the place right before the closing brace of the relevant `impl` to suggest
// adding the associated item at the end of its body.
let sugg_sp = full_impl_span.with_lo(hi).with_hi(hi);
let sugg_sp = tcx.mark_span_for_resize(full_impl_span).with_lo(hi).with_hi(hi);
// Obtain the level of indentation ending in `sugg_sp`.
let padding =
tcx.sess.source_map().indentation_before(sugg_sp).unwrap_or_else(|| String::new());
Expand Down
45 changes: 32 additions & 13 deletions compiler/rustc_hir_analysis/src/structured_errors/wrong_number_of_generic_args.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -594,7 +594,7 @@ impl<'a, 'tcx> WrongNumberOfGenericArgs<'a, 'tcx> {

match self.angle_brackets {
AngleBrackets::Missing => {
let span = self.path_segment.ident.span;
let span = self.tcx.mark_span_for_resize(self.path_segment.ident.span);

// insert a suggestion of the form "Y<'a, 'b>"
let sugg = format!("<{}>", suggested_args);
Expand All @@ -610,11 +610,23 @@ impl<'a, 'tcx> WrongNumberOfGenericArgs<'a, 'tcx> {

AngleBrackets::Available => {
let (sugg_span, is_first) = if self.num_provided_lifetime_args() == 0 {
(self.gen_args.span().unwrap().shrink_to_lo(), true)
(
self.tcx.mark_span_for_resize(self.gen_args.span().unwrap()).shrink_to_lo(),
true,
)
} else {
let last_lt = &self.gen_args.args[self.num_provided_lifetime_args() - 1];
(last_lt.span().shrink_to_hi(), false)
(self.tcx.mark_span_for_resize(last_lt.span()).shrink_to_hi(), false)
};
let path_sp = self.path_segment.ident.span;
if !self.gen_args.args.iter().all(|arg| {
arg.span().can_be_used_for_suggestions()
&& arg.span().peel_ctxt() == path_sp.peel_ctxt()
}) || !path_sp.can_be_used_for_suggestions()
{
// Do not suggest syntax when macros are involved. (#90557)
return;
}
let has_non_lt_args = self.num_provided_type_or_const_args() != 0;
let has_bindings = !self.gen_args.bindings.is_empty();

Expand Down Expand Up @@ -644,7 +656,7 @@ impl<'a, 'tcx> WrongNumberOfGenericArgs<'a, 'tcx> {

match self.angle_brackets {
AngleBrackets::Missing | AngleBrackets::Implied => {
let span = self.path_segment.ident.span;
let span = self.tcx.mark_span_for_resize(self.path_segment.ident.span);

// insert a suggestion of the form "Y<T, U>"
let sugg = format!("<{}>", suggested_args);
Expand All @@ -658,14 +670,24 @@ impl<'a, 'tcx> WrongNumberOfGenericArgs<'a, 'tcx> {
);
}
AngleBrackets::Available => {
let gen_args_span = self.gen_args.span().unwrap();
let path_sp = self.path_segment.ident.span;
if !self.gen_args.args.iter().all(|arg| {
arg.span().can_be_used_for_suggestions()
&& arg.span().peel_ctxt() == path_sp.peel_ctxt()
}) || !path_sp.can_be_used_for_suggestions()
{
// Do not suggest syntax when macros are involved. (#90557)
return;
}
let gen_args_span = self.tcx.mark_span_for_resize(self.gen_args.span().unwrap());
let sugg_offset =
self.get_lifetime_args_offset() + self.num_provided_type_or_const_args();

let (sugg_span, is_first) = if sugg_offset == 0 {
(gen_args_span.shrink_to_lo(), true)
(self.tcx.mark_span_for_resize(gen_args_span).shrink_to_lo(), true)
} else {
let arg_span = self.gen_args.args[sugg_offset - 1].span();
let arg_span =
self.tcx.mark_span_for_resize(self.gen_args.args[sugg_offset - 1].span());
// If we came here then inferred lifetime's spans can only point
// to either the opening bracket or to the space right after.
// Both of these spans have an `hi` lower than or equal to the span
Expand Down Expand Up @@ -770,7 +792,7 @@ impl<'a, 'tcx> WrongNumberOfGenericArgs<'a, 'tcx> {
&& let Ok(snippet) = self.tcx.sess.source_map().span_to_snippet(span) {
let sugg = vec![
(self.path_segment.ident.span, format!("{}::{}", snippet, self.path_segment.ident)),
(span.with_lo(self.path_segment.ident.span.hi()), "".to_owned())
(self.tcx.mark_span_for_resize(span).with_lo(self.path_segment.ident.span.hi()), "".to_owned())
];

err.multipart_suggestion(
Expand Down Expand Up @@ -953,11 +975,8 @@ impl<'a, 'tcx> WrongNumberOfGenericArgs<'a, 'tcx> {
}
} else if remove_entire_generics {
let span = self
.path_segment
.args
.unwrap()
.span_ext()
.unwrap()
.tcx
.mark_span_for_resize(self.path_segment.args.unwrap().span_ext().unwrap())
.with_lo(self.path_segment.ident.span.hi());

let msg = format!(
Expand Down
Loading