Skip to content

Commit

Permalink
Auto merge of rust-lang#99151 - Dylan-DPC:rollup-40aqkxy, r=Dylan-DPC
Browse files Browse the repository at this point in the history
Rollup of 5 pull requests

Successful merges:

 - rust-lang#98882 (explain doc comments in macros a bit)
 - rust-lang#98907 (Deny float const params even when `adt_const_params` is enabled)
 - rust-lang#99091 (Do not mention private types from other crates as impl candidates)
 - rust-lang#99140 (Implement `SourceMap::is_span_accessible`)
 - rust-lang#99147 (Mention similarly named associated type even if it's not clearly in supertrait)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
  • Loading branch information
bors committed Jul 11, 2022
2 parents 7d1f57a + 21d6b1f commit 9fb32dc
Show file tree
Hide file tree
Showing 31 changed files with 348 additions and 96 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
));

// Check first whether the source is accessible (issue #87060)
if self.infcx.tcx.sess.source_map().span_to_snippet(deref_target).is_ok() {
if self.infcx.tcx.sess.source_map().is_span_accessible(deref_target) {
err.span_note(deref_target, "deref defined here");
}
}
Expand Down
9 changes: 1 addition & 8 deletions compiler/rustc_borrowck/src/diagnostics/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -975,14 +975,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
if self.fn_self_span_reported.insert(fn_span) {
err.span_note(
// Check whether the source is accessible
if self
.infcx
.tcx
.sess
.source_map()
.span_to_snippet(self_arg.span)
.is_ok()
{
if self.infcx.tcx.sess.source_map().is_span_accessible(self_arg.span) {
self_arg.span
} else {
fn_call_span
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@ impl<'tcx> NonConstOp<'tcx> for FnCallNonConst<'tcx> {
err.note(&format!("attempting to deref into `{}`", deref_target_ty));

// Check first whether the source is accessible (issue #87060)
if tcx.sess.source_map().span_to_snippet(deref_target).is_ok() {
if tcx.sess.source_map().is_span_accessible(deref_target) {
err.span_note(deref_target, "deref defined here");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ impl Qualif for CustomEq {
// because that component may be part of an enum variant (e.g.,
// `Option::<NonStructuralMatchTy>::Some`), in which case some values of this type may be
// structural-match (`Option::None`).
traits::search_for_structural_match_violation(cx.body.span, cx.tcx, ty).is_some()
traits::search_for_structural_match_violation(cx.body.span, cx.tcx, ty, true).is_some()
}

fn in_adt_inherently<'tcx>(
Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_error_messages/locales/en-US/expand.ftl
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
expand-explain-doc-comment-outer =
outer doc comments expand to `#[doc = "..."]`, which is what this macro attempted to match
expand-explain-doc-comment-inner =
inner doc comments expand to `#![doc = "..."]`, which is what this macro attempted to match
3 changes: 2 additions & 1 deletion compiler/rustc_error_messages/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,12 @@ pub use unic_langid::{langid, LanguageIdentifier};
fluent_messages! {
borrowck => "../locales/en-US/borrowck.ftl",
builtin_macros => "../locales/en-US/builtin_macros.ftl",
const_eval => "../locales/en-US/const_eval.ftl",
expand => "../locales/en-US/expand.ftl",
lint => "../locales/en-US/lint.ftl",
parser => "../locales/en-US/parser.ftl",
privacy => "../locales/en-US/privacy.ftl",
typeck => "../locales/en-US/typeck.ftl",
const_eval => "../locales/en-US/const_eval.ftl",
}

pub use fluent_generated::{self as fluent, DEFAULT_LOCALE_RESOURCES};
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_errors/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1558,7 +1558,7 @@ pub fn add_elided_lifetime_in_path_suggestion(
insertion_span: Span,
) {
diag.span_label(path_span, format!("expected lifetime parameter{}", pluralize!(n)));
if source_map.span_to_snippet(insertion_span).is_err() {
if !source_map.is_span_accessible(insertion_span) {
// Do not try to suggest anything if generated by a proc-macro.
return;
}
Expand Down
38 changes: 35 additions & 3 deletions compiler/rustc_expand/src/mbe/macro_rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use rustc_ast::{NodeId, DUMMY_NODE_ID};
use rustc_ast_pretty::pprust;
use rustc_attr::{self as attr, TransparencyError};
use rustc_data_structures::fx::FxHashMap;
use rustc_errors::{Applicability, Diagnostic, DiagnosticBuilder};
use rustc_errors::{Applicability, Diagnostic, DiagnosticBuilder, ErrorGuaranteed};
use rustc_feature::Features;
use rustc_lint_defs::builtin::{
RUST_2021_INCOMPATIBLE_OR_PATTERNS, SEMICOLON_IN_EXPRESSIONS_FROM_MACROS,
Expand All @@ -25,6 +25,7 @@ use rustc_session::parse::ParseSess;
use rustc_session::Session;
use rustc_span::edition::Edition;
use rustc_span::hygiene::Transparency;
use rustc_span::source_map::SourceMap;
use rustc_span::symbol::{kw, sym, Ident, MacroRulesNormalizedIdent};
use rustc_span::Span;

Expand Down Expand Up @@ -345,7 +346,7 @@ fn expand_macro<'cx>(
if !def_span.is_dummy() && !cx.source_map().is_imported(def_span) {
err.span_label(cx.source_map().guess_head_span(def_span), "when calling this macro");
}

annotate_doc_comment(&mut err, sess.source_map(), span);
// Check whether there's a missing comma in this macro call, like `println!("{}" a);`
if let Some((arg, comma_span)) = arg.add_comma() {
for lhs in lhses {
Expand Down Expand Up @@ -453,7 +454,10 @@ pub fn compile_declarative_macro(
Failure(token, msg) => {
let s = parse_failure_msg(&token);
let sp = token.span.substitute_dummy(def.span);
sess.parse_sess.span_diagnostic.struct_span_err(sp, &s).span_label(sp, msg).emit();
let mut err = sess.parse_sess.span_diagnostic.struct_span_err(sp, &s);
err.span_label(sp, msg);
annotate_doc_comment(&mut err, sess.source_map(), sp);
err.emit();
return dummy_syn_ext();
}
Error(sp, msg) => {
Expand Down Expand Up @@ -590,6 +594,34 @@ pub fn compile_declarative_macro(
(mk_syn_ext(expander), rule_spans)
}

#[derive(SessionSubdiagnostic)]
enum ExplainDocComment {
#[label(expand::explain_doc_comment_inner)]
Inner {
#[primary_span]
span: Span,
},
#[label(expand::explain_doc_comment_outer)]
Outer {
#[primary_span]
span: Span,
},
}

fn annotate_doc_comment(
err: &mut DiagnosticBuilder<'_, ErrorGuaranteed>,
sm: &SourceMap,
span: Span,
) {
if let Ok(src) = sm.span_to_snippet(span) {
if src.starts_with("///") || src.starts_with("/**") {
err.subdiagnostic(ExplainDocComment::Outer { span });
} else if src.starts_with("//!") || src.starts_with("/*!") {
err.subdiagnostic(ExplainDocComment::Inner { span });
}
}
}

fn check_lhs_nt_follows(sess: &ParseSess, def: &ast::Item, lhs: &mbe::TokenTree) -> bool {
// lhs is going to be like TokenTree::Delimited(...), where the
// entire lhs is those tts. Or, it can be a "bare sequence", not wrapped in parens.
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir_build/src/thir/pattern/check_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -432,7 +432,7 @@ impl<'p, 'tcx> MatchVisitor<'_, 'p, 'tcx> {
"`let` bindings require an \"irrefutable pattern\", like a `struct` or \
an `enum` with only one variant",
);
if self.tcx.sess.source_map().span_to_snippet(span).is_ok() {
if self.tcx.sess.source_map().is_span_accessible(span) {
let semi_span = span.shrink_to_hi().with_lo(span.hi() - BytePos(1));
let start_span = span.shrink_to_lo();
let end_span = semi_span.shrink_to_lo();
Expand Down
57 changes: 31 additions & 26 deletions compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,32 +120,37 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> {
}

fn search_for_structural_match_violation(&self, ty: Ty<'tcx>) -> Option<String> {
traits::search_for_structural_match_violation(self.span, self.tcx(), ty).map(|non_sm_ty| {
with_no_trimmed_paths!(match non_sm_ty.kind {
traits::NonStructuralMatchTyKind::Adt(adt) => self.adt_derive_msg(adt),
traits::NonStructuralMatchTyKind::Dynamic => {
"trait objects cannot be used in patterns".to_string()
}
traits::NonStructuralMatchTyKind::Opaque => {
"opaque types cannot be used in patterns".to_string()
}
traits::NonStructuralMatchTyKind::Closure => {
"closures cannot be used in patterns".to_string()
}
traits::NonStructuralMatchTyKind::Generator => {
"generators cannot be used in patterns".to_string()
}
traits::NonStructuralMatchTyKind::Param => {
bug!("use of a constant whose type is a parameter inside a pattern")
}
traits::NonStructuralMatchTyKind::Projection => {
bug!("use of a constant whose type is a projection inside a pattern")
}
traits::NonStructuralMatchTyKind::Foreign => {
bug!("use of a value of a foreign type inside a pattern")
}
})
})
traits::search_for_structural_match_violation(self.span, self.tcx(), ty, true).map(
|non_sm_ty| {
with_no_trimmed_paths!(match non_sm_ty.kind {
traits::NonStructuralMatchTyKind::Adt(adt) => self.adt_derive_msg(adt),
traits::NonStructuralMatchTyKind::Dynamic => {
"trait objects cannot be used in patterns".to_string()
}
traits::NonStructuralMatchTyKind::Opaque => {
"opaque types cannot be used in patterns".to_string()
}
traits::NonStructuralMatchTyKind::Closure => {
"closures cannot be used in patterns".to_string()
}
traits::NonStructuralMatchTyKind::Generator => {
"generators cannot be used in patterns".to_string()
}
traits::NonStructuralMatchTyKind::Float => {
"floating-point numbers cannot be used in patterns".to_string()
}
traits::NonStructuralMatchTyKind::Param => {
bug!("use of a constant whose type is a parameter inside a pattern")
}
traits::NonStructuralMatchTyKind::Projection => {
bug!("use of a constant whose type is a projection inside a pattern")
}
traits::NonStructuralMatchTyKind::Foreign => {
bug!("use of a value of a foreign type inside a pattern")
}
})
},
)
}

fn type_marked_structural(&self, ty: Ty<'tcx>) -> bool {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_resolve/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1647,7 +1647,7 @@ impl<'a> Resolver<'a> {

fn binding_description(&self, b: &NameBinding<'_>, ident: Ident, from_prelude: bool) -> String {
let res = b.res();
if b.span.is_dummy() || self.session.source_map().span_to_snippet(b.span).is_err() {
if b.span.is_dummy() || !self.session.source_map().is_span_accessible(b.span) {
// These already contain the "built-in" prefix or look bad with it.
let add_built_in =
!matches!(b.res(), Res::NonMacroAttr(..) | Res::PrimTy(..) | Res::ToolMod);
Expand Down
7 changes: 7 additions & 0 deletions compiler/rustc_span/src/source_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -597,6 +597,13 @@ impl SourceMap {
local_begin.sf.src.is_some() && local_end.sf.src.is_some()
}

pub fn is_span_accessible(&self, sp: Span) -> bool {
self.span_to_source(sp, |src, start_index, end_index| {
Ok(src.get(start_index..end_index).is_some())
})
.map_or(false, |is_accessible| is_accessible)
}

/// Returns the source snippet as `String` corresponding to the given `Span`.
pub fn span_to_snippet(&self, sp: Span) -> Result<String, SpanSnippetError> {
self.span_to_source(sp, |src, start_index, end_index| {
Expand Down
24 changes: 22 additions & 2 deletions compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -673,6 +673,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
if !self.report_similar_impl_candidates(
impl_candidates,
trait_ref,
obligation.cause.body_id,
&mut err,
) {
// This is *almost* equivalent to
Expand Down Expand Up @@ -707,6 +708,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
self.report_similar_impl_candidates(
impl_candidates,
trait_ref,
obligation.cause.body_id,
&mut err,
);
}
Expand Down Expand Up @@ -1353,6 +1355,7 @@ trait InferCtxtPrivExt<'hir, 'tcx> {
&self,
impl_candidates: Vec<ImplCandidate<'tcx>>,
trait_ref: ty::PolyTraitRef<'tcx>,
body_id: hir::HirId,
err: &mut Diagnostic,
) -> bool;

Expand Down Expand Up @@ -1735,6 +1738,7 @@ impl<'a, 'tcx> InferCtxtPrivExt<'a, 'tcx> for InferCtxt<'a, 'tcx> {
&self,
impl_candidates: Vec<ImplCandidate<'tcx>>,
trait_ref: ty::PolyTraitRef<'tcx>,
body_id: hir::HirId,
err: &mut Diagnostic,
) -> bool {
let report = |mut candidates: Vec<TraitRef<'tcx>>, err: &mut Diagnostic| {
Expand Down Expand Up @@ -1805,8 +1809,24 @@ impl<'a, 'tcx> InferCtxtPrivExt<'a, 'tcx> for InferCtxt<'a, 'tcx> {
|| self.tcx.is_builtin_derive(def_id)
})
.filter_map(|def_id| self.tcx.impl_trait_ref(def_id))
// Avoid mentioning type parameters.
.filter(|trait_ref| !matches!(trait_ref.self_ty().kind(), ty::Param(_)))
.filter(|trait_ref| {
let self_ty = trait_ref.self_ty();
// Avoid mentioning type parameters.
if let ty::Param(_) = self_ty.kind() {
false
}
// Avoid mentioning types that are private to another crate
else if let ty::Adt(def, _) = self_ty.peel_refs().kind() {
// FIXME(compiler-errors): This could be generalized, both to
// be more granular, and probably look past other `#[fundamental]`
// types, too.
self.tcx
.visibility(def.did())
.is_accessible_from(body_id.owner.to_def_id(), self.tcx)
} else {
true
}
})
.collect();
return report(normalized_impl_candidates, err);
}
Expand Down
22 changes: 20 additions & 2 deletions compiler/rustc_trait_selection/src/traits/structural_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ pub enum NonStructuralMatchTyKind<'tcx> {
Closure,
Generator,
Projection,
Float,
}

/// This method traverses the structure of `ty`, trying to find an
Expand Down Expand Up @@ -53,12 +54,16 @@ pub enum NonStructuralMatchTyKind<'tcx> {
/// For more background on why Rust has this requirement, and issues
/// that arose when the requirement was not enforced completely, see
/// Rust RFC 1445, rust-lang/rust#61188, and rust-lang/rust#62307.
///
/// The floats_allowed flag is used to deny constants in floating point
pub fn search_for_structural_match_violation<'tcx>(
span: Span,
tcx: TyCtxt<'tcx>,
ty: Ty<'tcx>,
floats_allowed: bool,
) -> Option<NonStructuralMatchTy<'tcx>> {
ty.visit_with(&mut Search { tcx, span, seen: FxHashSet::default() }).break_value()
ty.visit_with(&mut Search { tcx, span, seen: FxHashSet::default(), floats_allowed })
.break_value()
}

/// This method returns true if and only if `adt_ty` itself has been marked as
Expand Down Expand Up @@ -119,6 +124,8 @@ struct Search<'tcx> {
/// Tracks ADTs previously encountered during search, so that
/// we will not recur on them again.
seen: FxHashSet<hir::def_id::DefId>,

floats_allowed: bool,
}

impl<'tcx> Search<'tcx> {
Expand Down Expand Up @@ -192,13 +199,24 @@ impl<'tcx> TypeVisitor<'tcx> for Search<'tcx> {
// for empty array.
return ControlFlow::CONTINUE;
}
ty::Bool | ty::Char | ty::Int(_) | ty::Uint(_) | ty::Float(_) | ty::Str | ty::Never => {
ty::Bool | ty::Char | ty::Int(_) | ty::Uint(_) | ty::Str | ty::Never => {
// These primitive types are always structural match.
//
// `Never` is kind of special here, but as it is not inhabitable, this should be fine.
return ControlFlow::CONTINUE;
}

ty::Float(_) => {
if self.floats_allowed {
return ControlFlow::CONTINUE;
} else {
return ControlFlow::Break(NonStructuralMatchTy {
ty,
kind: NonStructuralMatchTyKind::Float,
});
}
}

ty::Array(..) | ty::Slice(_) | ty::Ref(..) | ty::Tuple(..) => {
// First check all contained types and then tell the caller to continue searching.
return ty.super_visit_with(self);
Expand Down
Loading

0 comments on commit 9fb32dc

Please sign in to comment.