Skip to content

Commit

Permalink
Auto merge of rust-lang#44111 - zackmdavis:feature_attr_error_span, r…
Browse files Browse the repository at this point in the history
…=nikomatsakis

feature error span on attribute for fn_must_use, SIMD/align reprs, macro reëxport

There were several feature-gated attributes for which the feature-not-available
error spans would point to the item annotated with the gated attribute, when it
would make more sense for the span to point to the attribute itself: if the
attribute is removed, the function/struct/_&c._ likely still makes sense and the
program will compile. (Note that we decline to make the analogous change for
the `main`, `start`, and `plugin_registrar` features, for in those cases it
makes sense for the span to implicate the entire function, of which there is
little hope of using without the gated attribute.)

![feature_attr_error_span](https://user-images.githubusercontent.com/1076988/29746531-fd700bfe-8a91-11e7-9c5b-6f5324083887.png)
  • Loading branch information
bors committed Aug 29, 2017
2 parents 6f82dea + 8bb2946 commit faf477a
Show file tree
Hide file tree
Showing 10 changed files with 107 additions and 36 deletions.
4 changes: 4 additions & 0 deletions src/libsyntax/attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -490,6 +490,10 @@ pub fn contains_name(attrs: &[Attribute], name: &str) -> bool {
})
}

pub fn find_by_name<'a>(attrs: &'a [Attribute], name: &str) -> Option<&'a Attribute> {
attrs.iter().find(|attr| attr.check_name(name))
}

pub fn first_attr_value_str_by_name(attrs: &[Attribute], name: &str) -> Option<Symbol> {
attrs.iter()
.find(|at| at.check_name(name))
Expand Down
42 changes: 19 additions & 23 deletions src/libsyntax/feature_gate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1252,8 +1252,8 @@ impl<'a> Visitor<'a> for PostExpansionVisitor<'a> {
fn visit_item(&mut self, i: &'a ast::Item) {
match i.node {
ast::ItemKind::ExternCrate(_) => {
if attr::contains_name(&i.attrs[..], "macro_reexport") {
gate_feature_post!(&self, macro_reexport, i.span,
if let Some(attr) = attr::find_by_name(&i.attrs[..], "macro_reexport") {
gate_feature_post!(&self, macro_reexport, attr.span,
"macros reexports are experimental \
and possibly buggy");
}
Expand All @@ -1280,36 +1280,32 @@ impl<'a> Visitor<'a> for PostExpansionVisitor<'a> {
function may change over time, for now \
a top-level `fn main()` is required");
}
if attr::contains_name(&i.attrs[..], "must_use") {
gate_feature_post!(&self, fn_must_use, i.span,
if let Some(attr) = attr::find_by_name(&i.attrs[..], "must_use") {
gate_feature_post!(&self, fn_must_use, attr.span,
"`#[must_use]` on functions is experimental",
GateStrength::Soft);
}
}

ast::ItemKind::Struct(..) => {
if attr::contains_name(&i.attrs[..], "simd") {
gate_feature_post!(&self, simd, i.span,
if let Some(attr) = attr::find_by_name(&i.attrs[..], "simd") {
gate_feature_post!(&self, simd, attr.span,
"SIMD types are experimental and possibly buggy");
self.context.parse_sess.span_diagnostic.span_warn(i.span,
self.context.parse_sess.span_diagnostic.span_warn(attr.span,
"the `#[simd]` attribute \
is deprecated, use \
`#[repr(simd)]` instead");
}
for attr in &i.attrs {
if attr.path == "repr" {
for item in attr.meta_item_list().unwrap_or_else(Vec::new) {
if item.check_name("simd") {
gate_feature_post!(&self, repr_simd, i.span,
"SIMD types are experimental \
and possibly buggy");

}
if item.check_name("align") {
gate_feature_post!(&self, repr_align, i.span,
"the struct `#[repr(align(u16))]` attribute \
is experimental");
}
if let Some(attr) = attr::find_by_name(&i.attrs[..], "repr") {
for item in attr.meta_item_list().unwrap_or_else(Vec::new) {
if item.check_name("simd") {
gate_feature_post!(&self, repr_simd, attr.span,
"SIMD types are experimental and possibly buggy");
}
if item.check_name("align") {
gate_feature_post!(&self, repr_align, attr.span,
"the struct `#[repr(align(u16))]` attribute \
is experimental");
}
}
}
Expand Down Expand Up @@ -1338,8 +1334,8 @@ impl<'a> Visitor<'a> for PostExpansionVisitor<'a> {

for impl_item in impl_items {
if let ast::ImplItemKind::Method(..) = impl_item.node {
if attr::contains_name(&impl_item.attrs[..], "must_use") {
gate_feature_post!(&self, fn_must_use, impl_item.span,
if let Some(attr) = attr::find_by_name(&impl_item.attrs[..], "must_use") {
gate_feature_post!(&self, fn_must_use, attr.span,
"`#[must_use]` on methods is experimental",
GateStrength::Soft);
}
Expand Down
2 changes: 1 addition & 1 deletion src/test/compile-fail-fulldeps/gated-macro-reexports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,6 @@
#![crate_type = "dylib"]

#[macro_reexport(reexported)]
//~^ ERROR macros reexports are experimental and possibly buggy
#[macro_use] #[no_link]
extern crate macro_reexport_1;
//~^ ERROR macros reexports are experimental and possibly buggy
8 changes: 4 additions & 4 deletions src/test/compile-fail/feature-gate-fn_must_use.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,12 @@
struct MyStruct;

impl MyStruct {
#[must_use]
fn need_to_use_method() -> bool { true } //~ WARN `#[must_use]` on methods is experimental
#[must_use] //~ WARN `#[must_use]` on methods is experimental
fn need_to_use_method() -> bool { true }
}

#[must_use]
fn need_to_use_it() -> bool { true } //~ WARN `#[must_use]` on functions is experimental
#[must_use] //~ WARN `#[must_use]` on functions is experimental
fn need_to_use_it() -> bool { true }


// Feature gates are tidy-required to have a specially named (or
Expand Down
4 changes: 2 additions & 2 deletions src/test/compile-fail/feature-gate-repr-simd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

#[repr(simd)]
struct Foo(u64, u64); //~ error: SIMD types are experimental
#[repr(simd)] //~ error: SIMD types are experimental
struct Foo(u64, u64);

fn main() {}
4 changes: 2 additions & 2 deletions src/test/compile-fail/feature-gate-repr_align.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
// except according to those terms.
#![feature(attr_literals)]

#[repr(align(64))]
struct Foo(u64, u64); //~ error: the struct `#[repr(align(u16))]` attribute is experimental
#[repr(align(64))] //~ error: the struct `#[repr(align(u16))]` attribute is experimental
struct Foo(u64, u64);

fn main() {}
3 changes: 1 addition & 2 deletions src/test/compile-fail/feature-gate-simd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,12 @@

// pretty-expanded FIXME #23616

#[repr(simd)]
#[repr(simd)] //~ ERROR SIMD types are experimental
struct RGBA {
r: f32,
g: f32,
b: f32,
a: f32
}
//~^^^^^^ ERROR SIMD types are experimental and possibly buggy (see issue #27731)

pub fn main() {}
Original file line number Diff line number Diff line change
Expand Up @@ -354,8 +354,7 @@ mod repr {
#[repr = "3900"] fn f() { }
//~^ WARN unused attribute

#[repr = "3900"] struct S;
//~^ WARN unused attribute
struct S;

#[repr = "3900"] type T = S;
//~^ WARN unused attribute
Expand Down
39 changes: 39 additions & 0 deletions src/test/ui/span/gated-features-attr-spans.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

#![feature(attr_literals)]

#[repr(align(16))]
struct Gem {
mohs_hardness: u8,
poofed: bool,
weapon: Weapon,
}

#[repr(simd)]
struct Weapon {
name: String,
damage: u32
}

impl Gem {
#[must_use] fn summon_weapon(&self) -> Weapon { self.weapon }
}

#[must_use]
fn bubble(gem: Gem) -> Result<Gem, ()> {
if gem.poofed {
Ok(gem)
} else {
Err(())
}
}

fn main() {}
34 changes: 34 additions & 0 deletions src/test/ui/span/gated-features-attr-spans.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
error: the struct `#[repr(align(u16))]` attribute is experimental (see issue #33626)
--> $DIR/gated-features-attr-spans.rs:13:1
|
13 | #[repr(align(16))]
| ^^^^^^^^^^^^^^^^^^
|
= help: add #![feature(repr_align)] to the crate attributes to enable

error: SIMD types are experimental and possibly buggy (see issue #27731)
--> $DIR/gated-features-attr-spans.rs:20:1
|
20 | #[repr(simd)]
| ^^^^^^^^^^^^^
|
= help: add #![feature(repr_simd)] to the crate attributes to enable

warning: `#[must_use]` on methods is experimental (see issue #43302)
--> $DIR/gated-features-attr-spans.rs:27:5
|
27 | #[must_use] fn summon_weapon(&self) -> Weapon { self.weapon }
| ^^^^^^^^^^^
|
= help: add #![feature(fn_must_use)] to the crate attributes to enable

warning: `#[must_use]` on functions is experimental (see issue #43302)
--> $DIR/gated-features-attr-spans.rs:30:1
|
30 | #[must_use]
| ^^^^^^^^^^^
|
= help: add #![feature(fn_must_use)] to the crate attributes to enable

error: aborting due to 2 previous errors

0 comments on commit faf477a

Please sign in to comment.