From 4e16e30b07c24b6ae5db1269c894d618eb55b1c0 Mon Sep 17 00:00:00 2001 From: Virgil Palanciuc Date: Wed, 25 Oct 2017 16:56:13 -0700 Subject: [PATCH 1/4] =?UTF-8?q?fix=20#44953=20-=20The=20=E2=80=9Cuse=20of?= =?UTF-8?q?=20unstable=20library=20feature=20'rustc=5Fprivate'=E2=80=9D=20?= =?UTF-8?q?error=20is=20very=20repetitive?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/librustc/middle/stability.rs | 30 +++++++++++++++++++++++++++--- src/librustc/session/mod.rs | 4 ++-- 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/src/librustc/middle/stability.rs b/src/librustc/middle/stability.rs index 4e4fc8b3118ff..d83560963f762 100644 --- a/src/librustc/middle/stability.rs +++ b/src/librustc/middle/stability.rs @@ -19,7 +19,7 @@ use hir::def_id::{CrateNum, CRATE_DEF_INDEX, DefId, LOCAL_CRATE}; use ty::{self, TyCtxt}; use middle::privacy::AccessLevels; use syntax::symbol::Symbol; -use syntax_pos::{Span, DUMMY_SP}; +use syntax_pos::{Span, MultiSpan, DUMMY_SP}; use syntax::ast; use syntax::ast::{NodeId, Attribute}; use syntax::feature_gate::{GateIssue, emit_feature_err, find_lang_feature_accepted_version}; @@ -597,8 +597,32 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { feature.as_str(), &r), None => format!("use of unstable library feature '{}'", &feature) }; - emit_feature_err(&self.sess.parse_sess, &feature.as_str(), span, - GateIssue::Library(Some(issue)), &msg); + + let msp: MultiSpan = span.into(); + let cm = &self.sess.parse_sess.codemap(); + let real_file_location = + msp.primary_span().and_then(|sp:Span| + if sp != DUMMY_SP { + let fname = cm.lookup_char_pos(sp.lo()).file.as_ref().name.clone(); + if fname.starts_with("<") && fname.ends_with(" macros>") { + None + } else { + Some(fname) + } + } else { + None + } + ); + + if let Some(_) = real_file_location { + let tuple = (None, Some(span), msg.clone()); + let fresh = self.sess.one_time_diagnostics.borrow_mut().insert(tuple); + if fresh { + emit_feature_err(&self.sess.parse_sess, &feature.as_str(), span, + GateIssue::Library(Some(issue)), &msg); + } + } + } Some(_) => { // Stable APIs are always ok to call and deprecated APIs are diff --git a/src/librustc/session/mod.rs b/src/librustc/session/mod.rs index 42c633dc83fe5..177e595273b8e 100644 --- a/src/librustc/session/mod.rs +++ b/src/librustc/session/mod.rs @@ -78,7 +78,7 @@ pub struct Session { /// Set of (LintId, Option, message) tuples tracking lint /// (sub)diagnostics that have been set once, but should not be set again, /// in order to avoid redundantly verbose output (Issue #24690). - pub one_time_diagnostics: RefCell, String)>>, + pub one_time_diagnostics: RefCell, Option, String)>>, pub plugin_llvm_passes: RefCell>, pub plugin_attributes: RefCell>, pub crate_types: RefCell>, @@ -361,7 +361,7 @@ impl Session { }, _ => { let lint_id = lint::LintId::of(lint); - let id_span_message = (lint_id, span, message.to_owned()); + let id_span_message = (Some(lint_id), span, message.to_owned()); let fresh = self.one_time_diagnostics.borrow_mut().insert(id_span_message); if fresh { do_method() From 52d0e514731f185026d0fdf7807cb97ab9f60218 Mon Sep 17 00:00:00 2001 From: Virgil Palanciuc Date: Thu, 26 Oct 2017 09:34:39 -0700 Subject: [PATCH 2/4] relax restriction - allow errors to be emmitted within macro spans, as long as the same error message has not been used before (i.e. use the None as the span key, for errors that occur within macros) --- src/librustc/middle/stability.rs | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/librustc/middle/stability.rs b/src/librustc/middle/stability.rs index d83560963f762..0d946c3bbcd68 100644 --- a/src/librustc/middle/stability.rs +++ b/src/librustc/middle/stability.rs @@ -598,29 +598,28 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { None => format!("use of unstable library feature '{}'", &feature) }; + let msp: MultiSpan = span.into(); let cm = &self.sess.parse_sess.codemap(); - let real_file_location = + let span_key = msp.primary_span().and_then(|sp:Span| if sp != DUMMY_SP { let fname = cm.lookup_char_pos(sp.lo()).file.as_ref().name.clone(); if fname.starts_with("<") && fname.ends_with(" macros>") { None } else { - Some(fname) + Some(span) } } else { None } ); - if let Some(_) = real_file_location { - let tuple = (None, Some(span), msg.clone()); - let fresh = self.sess.one_time_diagnostics.borrow_mut().insert(tuple); - if fresh { - emit_feature_err(&self.sess.parse_sess, &feature.as_str(), span, - GateIssue::Library(Some(issue)), &msg); - } + let tuple = (None, span_key, msg.clone()); + let fresh = self.sess.one_time_diagnostics.borrow_mut().insert(tuple); + if fresh { + emit_feature_err(&self.sess.parse_sess, &feature.as_str(), span, + GateIssue::Library(Some(issue)), &msg); } } From 304c8b1edabcd6758600a525b7aaa3505800511a Mon Sep 17 00:00:00 2001 From: Virgil Palanciuc Date: Sat, 28 Oct 2017 20:39:00 +0300 Subject: [PATCH 3/4] implemented code review --- src/librustc/middle/stability.rs | 27 +++++++++++++-------------- src/librustc/session/mod.rs | 15 +++++++++++---- 2 files changed, 24 insertions(+), 18 deletions(-) diff --git a/src/librustc/middle/stability.rs b/src/librustc/middle/stability.rs index 0d946c3bbcd68..b30d5e384884e 100644 --- a/src/librustc/middle/stability.rs +++ b/src/librustc/middle/stability.rs @@ -18,6 +18,7 @@ use hir::def::Def; use hir::def_id::{CrateNum, CRATE_DEF_INDEX, DefId, LOCAL_CRATE}; use ty::{self, TyCtxt}; use middle::privacy::AccessLevels; +use session::DiagnosticMessageId; use syntax::symbol::Symbol; use syntax_pos::{Span, MultiSpan, DUMMY_SP}; use syntax::ast; @@ -601,27 +602,25 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { let msp: MultiSpan = span.into(); let cm = &self.sess.parse_sess.codemap(); - let span_key = - msp.primary_span().and_then(|sp:Span| - if sp != DUMMY_SP { - let fname = cm.lookup_char_pos(sp.lo()).file.as_ref().name.clone(); - if fname.starts_with("<") && fname.ends_with(" macros>") { - None - } else { - Some(span) - } - } else { + let span_key = msp.primary_span().and_then(|sp: Span| + if sp != DUMMY_SP { + let file = cm.lookup_char_pos(sp.lo()).file; + if file.name.starts_with("<") && file.name.ends_with(" macros>") { None + } else { + Some(span) } - ); + } else { + None + } + ); - let tuple = (None, span_key, msg.clone()); - let fresh = self.sess.one_time_diagnostics.borrow_mut().insert(tuple); + let error_id = (DiagnosticMessageId::StabilityId(issue), span_key, msg.clone()); + let fresh = self.sess.one_time_diagnostics.borrow_mut().insert(error_id); if fresh { emit_feature_err(&self.sess.parse_sess, &feature.as_str(), span, GateIssue::Library(Some(issue)), &msg); } - } Some(_) => { // Stable APIs are always ok to call and deprecated APIs are diff --git a/src/librustc/session/mod.rs b/src/librustc/session/mod.rs index 177e595273b8e..9254b2e3364b1 100644 --- a/src/librustc/session/mod.rs +++ b/src/librustc/session/mod.rs @@ -75,10 +75,10 @@ pub struct Session { pub working_dir: (String, bool), pub lint_store: RefCell, pub buffered_lints: RefCell>, - /// Set of (LintId, Option, message) tuples tracking lint + /// Set of (DiagnosticId, Option, message) tuples tracking /// (sub)diagnostics that have been set once, but should not be set again, - /// in order to avoid redundantly verbose output (Issue #24690). - pub one_time_diagnostics: RefCell, Option, String)>>, + /// in order to avoid redundantly verbose output (Issue #24690, #44953). + pub one_time_diagnostics: RefCell, String)>>, pub plugin_llvm_passes: RefCell>, pub plugin_attributes: RefCell>, pub crate_types: RefCell>, @@ -164,6 +164,13 @@ enum DiagnosticBuilderMethod { // add more variants as needed to support one-time diagnostics } +/// Diagnostic message id - used in order to avoid emitting the same message more than once +#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] +pub enum DiagnosticMessageId { + LintId(lint::LintId), + StabilityId(u32) +} + impl Session { pub fn local_crate_disambiguator(&self) -> CrateDisambiguator { match *self.crate_disambiguator.borrow() { @@ -361,7 +368,7 @@ impl Session { }, _ => { let lint_id = lint::LintId::of(lint); - let id_span_message = (Some(lint_id), span, message.to_owned()); + let id_span_message = (DiagnosticMessageId::LintId(lint_id), span, message.to_owned()); let fresh = self.one_time_diagnostics.borrow_mut().insert(id_span_message); if fresh { do_method() From bb0049bcd21ca8c9707207de4669fadfd7362367 Mon Sep 17 00:00:00 2001 From: Virgil Palanciuc Date: Sat, 28 Oct 2017 22:38:15 +0300 Subject: [PATCH 4/4] fixed tidy error --- src/librustc/session/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/librustc/session/mod.rs b/src/librustc/session/mod.rs index 9254b2e3364b1..58e83faef90b6 100644 --- a/src/librustc/session/mod.rs +++ b/src/librustc/session/mod.rs @@ -367,8 +367,8 @@ impl Session { do_method() }, _ => { - let lint_id = lint::LintId::of(lint); - let id_span_message = (DiagnosticMessageId::LintId(lint_id), span, message.to_owned()); + let lint_id = DiagnosticMessageId::LintId(lint::LintId::of(lint)); + let id_span_message = (lint_id, span, message.to_owned()); let fresh = self.one_time_diagnostics.borrow_mut().insert(id_span_message); if fresh { do_method()