Skip to content

Commit

Permalink
Rollup merge of rust-lang#61545 - flip1995:internal_lints, r=oli-obk
Browse files Browse the repository at this point in the history
Implement another internal lints

cc rust-lang#49509

This adds ~~two~~ one internal lint~~s~~:
1. LINT_PASS_IMPL_WITHOUT_MACRO: Make sure, that the `{declare,impl}_lint_pass` macro is used to implement lint passes. cc rust-lang#59669
2. ~~USAGE_OF_TYCTXT_AND_SPAN_ARGS: item 2 on the list in rust-lang#49509~~

~~With 2. I wasn't sure, if this lint should be applied everywhere. That means a careful review of 0955835 would be great. Also 73fb9b4 allows this lint on some functions. Should I also apply this lint there?~~

TODO (not directly relevant for review):
- [ ] rust-lang#59316 (comment) (not sure yet, if this works or how to query for `rustc_private`, since it's not in [`Features`](https://doc.rust-lang.org/nightly/nightly-rustc/syntax/feature_gate/struct.Features.html) 🤔 cc @eddyb)
- [x] rust-lang#61735 (comment)
- [x] Check explicitly for the `{declare,impl}_lint_pass!` macros

r? @oli-obk
  • Loading branch information
Centril authored Jul 5, 2019
2 parents c57637e + d0625a3 commit cf5e9e5
Show file tree
Hide file tree
Showing 49 changed files with 209 additions and 89 deletions.
15 changes: 14 additions & 1 deletion src/bootstrap/bin/rustc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,20 @@ fn main() {
}

// This is required for internal lints.
cmd.arg("-Zunstable-options");
if let Some(crate_name) = args.windows(2).find(|a| &*a[0] == "--crate-name") {
let crate_name = crate_name[1].to_string_lossy();
if crate_name != "rustc_version"
&& (crate_name.starts_with("rustc")
|| crate_name.starts_with("syntax")
|| crate_name == "arena"
|| crate_name == "fmt_macros")
{
cmd.arg("-Zunstable-options");
if stage != "0" {
cmd.arg("-Wrustc::internal");
}
}
}

// Force all crates compiled by this compiler to (a) be unstable and (b)
// allow the `rustc_private` feature to link to other unstable crates
Expand Down
1 change: 0 additions & 1 deletion src/libarena/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
test(no_crate_inject, attr(deny(warnings))))]

#![deny(rust_2018_idioms)]
#![deny(internal)]
#![deny(unused_lifetimes)]

#![feature(core_intrinsics)]
Expand Down
1 change: 0 additions & 1 deletion src/libfmt_macros/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
test(attr(deny(warnings))))]

#![deny(rust_2018_idioms)]
#![deny(internal)]
#![deny(unused_lifetimes)]

#![feature(nll)]
Expand Down
1 change: 0 additions & 1 deletion src/librustc/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
#![doc(html_root_url = "https://doc.rust-lang.org/nightly/")]

#![deny(rust_2018_idioms)]
#![deny(internal)]
#![deny(unused_lifetimes)]

#![feature(arbitrary_self_types)]
Expand Down
2 changes: 2 additions & 0 deletions src/librustc/lint/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1341,6 +1341,7 @@ struct LateLintPassObjects<'a> {
lints: &'a mut [LateLintPassObject],
}

#[cfg_attr(not(bootstrap), allow(rustc::lint_pass_impl_without_macro))]
impl LintPass for LateLintPassObjects<'_> {
fn name(&self) -> &'static str {
panic!()
Expand Down Expand Up @@ -1510,6 +1511,7 @@ struct EarlyLintPassObjects<'a> {
lints: &'a mut [EarlyLintPassObject],
}

#[cfg_attr(not(bootstrap), allow(rustc::lint_pass_impl_without_macro))]
impl LintPass for EarlyLintPassObjects<'_> {
fn name(&self) -> &'static str {
panic!()
Expand Down
82 changes: 56 additions & 26 deletions src/librustc/lint/internal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,12 @@ use crate::lint::{
};
use errors::Applicability;
use rustc_data_structures::fx::FxHashMap;
use syntax::ast::Ident;
use syntax::ast::{Ident, Item, ItemKind};
use syntax::symbol::{sym, Symbol};
use syntax_pos::ExpnInfo;

declare_lint! {
pub DEFAULT_HASH_TYPES,
declare_tool_lint! {
pub rustc::DEFAULT_HASH_TYPES,
Allow,
"forbid HashMap and HashSet and suggest the FxHash* variants"
}
Expand All @@ -22,7 +23,7 @@ pub struct DefaultHashTypes {

impl DefaultHashTypes {
// we are allowed to use `HashMap` and `HashSet` as identifiers for implementing the lint itself
#[allow(internal)]
#[cfg_attr(not(bootstrap), allow(rustc::default_hash_types))]
pub fn new() -> Self {
let mut map = FxHashMap::default();
map.insert(sym::HashMap, sym::FxHashMap);
Expand All @@ -36,40 +37,34 @@ impl_lint_pass!(DefaultHashTypes => [DEFAULT_HASH_TYPES]);
impl EarlyLintPass for DefaultHashTypes {
fn check_ident(&mut self, cx: &EarlyContext<'_>, ident: Ident) {
if let Some(replace) = self.map.get(&ident.name) {
let msg = format!(
"Prefer {} over {}, it has better performance",
replace, ident
);
let msg = format!("Prefer {} over {}, it has better performance", replace, ident);
let mut db = cx.struct_span_lint(DEFAULT_HASH_TYPES, ident.span, &msg);
db.span_suggestion(
ident.span,
"use",
replace.to_string(),
Applicability::MaybeIncorrect, // FxHashMap, ... needs another import
);
db.note(&format!(
"a `use rustc_data_structures::fx::{}` may be necessary",
replace
))
.emit();
db.note(&format!("a `use rustc_data_structures::fx::{}` may be necessary", replace))
.emit();
}
}
}

declare_lint! {
pub USAGE_OF_TY_TYKIND,
declare_tool_lint! {
pub rustc::USAGE_OF_TY_TYKIND,
Allow,
"usage of `ty::TyKind` outside of the `ty::sty` module"
}

declare_lint! {
pub TY_PASS_BY_REFERENCE,
declare_tool_lint! {
pub rustc::TY_PASS_BY_REFERENCE,
Allow,
"passing `Ty` or `TyCtxt` by reference"
}

declare_lint! {
pub USAGE_OF_QUALIFIED_TY,
declare_tool_lint! {
pub rustc::USAGE_OF_QUALIFIED_TY,
Allow,
"using `ty::{Ty,TyCtxt}` instead of importing it"
}
Expand Down Expand Up @@ -137,13 +132,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for TyTyKind {
}
}
}
TyKind::Rptr(
_,
MutTy {
ty: inner_ty,
mutbl: Mutability::MutImmutable,
},
) => {
TyKind::Rptr(_, MutTy { ty: inner_ty, mutbl: Mutability::MutImmutable }) => {
if let Some(impl_did) = cx.tcx.impl_of_method(ty.hir_id.owner_def_id()) {
if cx.tcx.impl_trait_ref(impl_did).is_some() {
return;
Expand Down Expand Up @@ -225,3 +214,44 @@ fn gen_args(segment: &PathSegment) -> String {

String::new()
}

declare_tool_lint! {
pub rustc::LINT_PASS_IMPL_WITHOUT_MACRO,
Allow,
"`impl LintPass` without the `declare_lint_pass!` or `impl_lint_pass!` macros"
}

declare_lint_pass!(LintPassImpl => [LINT_PASS_IMPL_WITHOUT_MACRO]);

impl EarlyLintPass for LintPassImpl {
fn check_item(&mut self, cx: &EarlyContext<'_>, item: &Item) {
if let ItemKind::Impl(_, _, _, _, Some(lint_pass), _, _) = &item.node {
if let Some(last) = lint_pass.path.segments.last() {
if last.ident.name == sym::LintPass {
match &lint_pass.path.span.ctxt().outer_expn_info() {
Some(info) if is_lint_pass_expansion(info) => {}
_ => {
cx.struct_span_lint(
LINT_PASS_IMPL_WITHOUT_MACRO,
lint_pass.path.span,
"implementing `LintPass` by hand",
)
.help("try using `declare_lint_pass!` or `impl_lint_pass!` instead")
.emit();
}
}
}
}
}
}
}

fn is_lint_pass_expansion(expn_info: &ExpnInfo) -> bool {
if expn_info.format.name() == sym::impl_lint_pass {
true
} else if let Some(info) = expn_info.call_site.ctxt().outer_expn_info() {
info.format.name() == sym::declare_lint_pass
} else {
false
}
}
2 changes: 2 additions & 0 deletions src/librustc/ty/codec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ pub trait EncodableWithShorthand: Clone + Eq + Hash {
fn variant(&self) -> &Self::Variant;
}

#[cfg_attr(not(bootstrap), allow(rustc::usage_of_ty_tykind))]
impl<'tcx> EncodableWithShorthand for Ty<'tcx> {
type Variant = ty::TyKind<'tcx>;
fn variant(&self) -> &Self::Variant {
Expand Down Expand Up @@ -159,6 +160,7 @@ where
Ok(decoder.map_encoded_cnum_to_current(cnum))
}

#[cfg_attr(not(bootstrap), allow(rustc::usage_of_ty_tykind))]
#[inline]
pub fn decode_ty<D>(decoder: &mut D) -> Result<Ty<'tcx>, D::Error>
where
Expand Down
3 changes: 3 additions & 0 deletions src/librustc/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ impl<'tcx> CtxtInterners<'tcx> {
}

/// Intern a type
#[cfg_attr(not(bootstrap), allow(rustc::usage_of_ty_tykind))]
#[inline(never)]
fn intern_ty(&self,
st: TyKind<'tcx>
Expand Down Expand Up @@ -2107,6 +2108,7 @@ impl<'tcx> Hash for Interned<'tcx, TyS<'tcx>> {
}
}

#[cfg_attr(not(bootstrap), allow(rustc::usage_of_ty_tykind))]
impl<'tcx> Borrow<TyKind<'tcx>> for Interned<'tcx, TyS<'tcx>> {
fn borrow<'a>(&'a self) -> &'a TyKind<'tcx> {
&self.0.sty
Expand Down Expand Up @@ -2321,6 +2323,7 @@ impl<'tcx> TyCtxt<'tcx> {
self.mk_fn_ptr(converted_sig)
}

#[cfg_attr(not(bootstrap), allow(rustc::usage_of_ty_tykind))]
#[inline]
pub fn mk_ty(&self, st: TyKind<'tcx>) -> Ty<'tcx> {
self.interners.intern_ty(st)
Expand Down
2 changes: 2 additions & 0 deletions src/librustc/ty/flags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ impl FlagComputation {
}
}

#[cfg_attr(not(bootstrap), allow(rustc::usage_of_ty_tykind))]
pub fn for_sty(st: &ty::TyKind<'_>) -> FlagComputation {
let mut result = FlagComputation::new();
result.add_sty(st);
Expand Down Expand Up @@ -61,6 +62,7 @@ impl FlagComputation {
} // otherwise, this binder captures nothing
}

#[cfg_attr(not(bootstrap), allow(rustc::usage_of_ty_tykind))]
fn add_sty(&mut self, st: &ty::TyKind<'_>) {
match st {
&ty::Bool |
Expand Down
37 changes: 18 additions & 19 deletions src/librustc/ty/mod.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
// ignore-tidy-filelength

#![allow(usage_of_ty_tykind)]

pub use self::Variance::*;
pub use self::AssocItemContainer::*;
pub use self::BorrowKind::*;
Expand Down Expand Up @@ -484,6 +482,7 @@ bitflags! {
}
}

#[cfg_attr(not(bootstrap), allow(rustc::usage_of_ty_tykind))]
pub struct TyS<'tcx> {
pub sty: TyKind<'tcx>,
pub flags: TypeFlags,
Expand Down Expand Up @@ -541,29 +540,29 @@ impl<'tcx> Hash for TyS<'tcx> {
impl<'tcx> TyS<'tcx> {
pub fn is_primitive_ty(&self) -> bool {
match self.sty {
TyKind::Bool |
TyKind::Char |
TyKind::Int(_) |
TyKind::Uint(_) |
TyKind::Float(_) |
TyKind::Infer(InferTy::IntVar(_)) |
TyKind::Infer(InferTy::FloatVar(_)) |
TyKind::Infer(InferTy::FreshIntTy(_)) |
TyKind::Infer(InferTy::FreshFloatTy(_)) => true,
TyKind::Ref(_, x, _) => x.is_primitive_ty(),
Bool |
Char |
Int(_) |
Uint(_) |
Float(_) |
Infer(InferTy::IntVar(_)) |
Infer(InferTy::FloatVar(_)) |
Infer(InferTy::FreshIntTy(_)) |
Infer(InferTy::FreshFloatTy(_)) => true,
Ref(_, x, _) => x.is_primitive_ty(),
_ => false,
}
}

pub fn is_suggestable(&self) -> bool {
match self.sty {
TyKind::Opaque(..) |
TyKind::FnDef(..) |
TyKind::FnPtr(..) |
TyKind::Dynamic(..) |
TyKind::Closure(..) |
TyKind::Infer(..) |
TyKind::Projection(..) => false,
Opaque(..) |
FnDef(..) |
FnPtr(..) |
Dynamic(..) |
Closure(..) |
Infer(..) |
Projection(..) => false,
_ => true,
}
}
Expand Down
2 changes: 2 additions & 0 deletions src/librustc/ty/sty.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
//! This module contains `TyKind` and its major components.
#![cfg_attr(not(bootstrap), allow(rustc::usage_of_ty_tykind))]

use crate::hir;
use crate::hir::def_id::DefId;
use crate::infer::canonical::Canonical;
Expand Down
1 change: 0 additions & 1 deletion src/librustc_allocator/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
#![feature(rustc_private)]

#![deny(rust_2018_idioms)]
#![deny(internal)]
#![deny(unused_lifetimes)]

pub mod expand;
Expand Down
1 change: 0 additions & 1 deletion src/librustc_borrowck/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

#![allow(non_camel_case_types)]
#![deny(rust_2018_idioms)]
#![deny(internal)]
#![deny(unused_lifetimes)]

#![feature(in_band_lifetimes)]
Expand Down
1 change: 0 additions & 1 deletion src/librustc_codegen_llvm/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
#![feature(trusted_len)]
#![feature(mem_take)]
#![deny(rust_2018_idioms)]
#![deny(internal)]
#![deny(unused_lifetimes)]

use back::write::{create_target_machine, create_informational_target_machine};
Expand Down
1 change: 0 additions & 1 deletion src/librustc_codegen_ssa/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
#![allow(unused_attributes)]
#![allow(dead_code)]
#![deny(rust_2018_idioms)]
#![deny(internal)]
#![deny(unused_lifetimes)]

#![recursion_limit="256"]
Expand Down
1 change: 0 additions & 1 deletion src/librustc_codegen_utils/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
#![recursion_limit="256"]

#![deny(rust_2018_idioms)]
#![deny(internal)]
#![deny(unused_lifetimes)]

#[macro_use]
Expand Down
1 change: 1 addition & 0 deletions src/librustc_data_structures/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#![cfg_attr(test, feature(test))]

#![deny(rust_2018_idioms)]
#![cfg_attr(not(bootstrap), allow(rustc::default_hash_types))]

#[macro_use]
extern crate log;
Expand Down
1 change: 0 additions & 1 deletion src/librustc_driver/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
#![recursion_limit="256"]

#![deny(rust_2018_idioms)]
#![deny(internal)]
#![deny(unused_lifetimes)]

pub extern crate getopts;
Expand Down
1 change: 0 additions & 1 deletion src/librustc_errors/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
#![feature(nll)]
#![feature(optin_builtin_traits)]
#![deny(rust_2018_idioms)]
#![deny(internal)]
#![deny(unused_lifetimes)]

#[allow(unused_extern_crates)]
Expand Down
1 change: 0 additions & 1 deletion src/librustc_incremental/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
#![recursion_limit="256"]

#![deny(rust_2018_idioms)]
#![deny(internal)]
#![deny(unused_lifetimes)]

#[macro_use] extern crate rustc;
Expand Down
Loading

0 comments on commit cf5e9e5

Please sign in to comment.