Skip to content

Commit

Permalink
Auto merge of #60630 - nnethercote:use-Symbol-more, r=petrochenkov
Browse files Browse the repository at this point in the history
Use `Symbol` more

A `Symbol` can be equated with a string (e.g. `&str`). This involves a
TLS lookup to get the chars (and a Mutex lock in a parallel compiler)
and then a char-by-char comparison. This functionality is convenient but
avoids one of the main benefits of `Symbol`s, which is fast equality
comparisons.

This PR removes the `Symbol`/string equality operations, forcing a lot
of existing string occurrences to become `Symbol`s. Fortunately, these
are almost all static strings (many are attribute names) and we can add
static `Symbol`s as necessary, and very little extra interning occurs.
The benefits are (a) a slight speedup (possibly greater in a parallel
compiler), and (b) the code is a lot more principled about `Symbol` use.
The main downside is verbosity, particularly with more `use
syntax::symbol::symbols` items.

r? @Zoxc
  • Loading branch information
bors committed May 13, 2019
2 parents 4443957 + ea9fac5 commit fe5f42c
Show file tree
Hide file tree
Showing 135 changed files with 1,507 additions and 1,041 deletions.
39 changes: 20 additions & 19 deletions src/librustc/hir/check_attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use crate::hir;
use crate::hir::def_id::DefId;
use crate::hir::intravisit::{self, Visitor, NestedVisitorMap};
use std::fmt::{self, Display};
use syntax::symbol::sym;
use syntax_pos::Span;

#[derive(Copy, Clone, PartialEq)]
Expand Down Expand Up @@ -95,18 +96,18 @@ impl<'a, 'tcx> CheckAttrVisitor<'a, 'tcx> {
fn check_attributes(&self, item: &hir::Item, target: Target) {
if target == Target::Fn || target == Target::Const {
self.tcx.codegen_fn_attrs(self.tcx.hir().local_def_id_from_hir_id(item.hir_id));
} else if let Some(a) = item.attrs.iter().find(|a| a.check_name("target_feature")) {
} else if let Some(a) = item.attrs.iter().find(|a| a.check_name(sym::target_feature)) {
self.tcx.sess.struct_span_err(a.span, "attribute should be applied to a function")
.span_label(item.span, "not a function")
.emit();
}

for attr in &item.attrs {
if attr.check_name("inline") {
if attr.check_name(sym::inline) {
self.check_inline(attr, &item.span, target)
} else if attr.check_name("non_exhaustive") {
} else if attr.check_name(sym::non_exhaustive) {
self.check_non_exhaustive(attr, item, target)
} else if attr.check_name("marker") {
} else if attr.check_name(sym::marker) {
self.check_marker(attr, item, target)
}
}
Expand Down Expand Up @@ -166,7 +167,7 @@ impl<'a, 'tcx> CheckAttrVisitor<'a, 'tcx> {
// ```
let hints: Vec<_> = item.attrs
.iter()
.filter(|attr| attr.check_name("repr"))
.filter(|attr| attr.check_name(sym::repr))
.filter_map(|attr| attr.meta_item_list())
.flatten()
.collect();
Expand All @@ -177,9 +178,9 @@ impl<'a, 'tcx> CheckAttrVisitor<'a, 'tcx> {
let mut is_transparent = false;

for hint in &hints {
let (article, allowed_targets) = match hint.name_or_empty().get() {
name @ "C" | name @ "align" => {
is_c |= name == "C";
let (article, allowed_targets) = match hint.name_or_empty() {
name @ sym::C | name @ sym::align => {
is_c |= name == sym::C;
if target != Target::Struct &&
target != Target::Union &&
target != Target::Enum {
Expand All @@ -188,33 +189,33 @@ impl<'a, 'tcx> CheckAttrVisitor<'a, 'tcx> {
continue
}
}
"packed" => {
sym::packed => {
if target != Target::Struct &&
target != Target::Union {
("a", "struct or union")
} else {
continue
}
}
"simd" => {
sym::simd => {
is_simd = true;
if target != Target::Struct {
("a", "struct")
} else {
continue
}
}
"transparent" => {
sym::transparent => {
is_transparent = true;
if target != Target::Struct {
("a", "struct")
} else {
continue
}
}
"i8" | "u8" | "i16" | "u16" |
"i32" | "u32" | "i64" | "u64" |
"isize" | "usize" => {
sym::i8 | sym::u8 | sym::i16 | sym::u16 |
sym::i32 | sym::u32 | sym::i64 | sym::u64 |
sym::isize | sym::usize => {
int_reprs += 1;
if target != Target::Enum {
("an", "enum")
Expand Down Expand Up @@ -268,10 +269,10 @@ impl<'a, 'tcx> CheckAttrVisitor<'a, 'tcx> {
// When checking statements ignore expressions, they will be checked later
if let hir::StmtKind::Local(ref l) = stmt.node {
for attr in l.attrs.iter() {
if attr.check_name("inline") {
if attr.check_name(sym::inline) {
self.check_inline(attr, &stmt.span, Target::Statement);
}
if attr.check_name("repr") {
if attr.check_name(sym::repr) {
self.emit_repr_error(
attr.span,
stmt.span,
Expand All @@ -289,10 +290,10 @@ impl<'a, 'tcx> CheckAttrVisitor<'a, 'tcx> {
_ => Target::Expression,
};
for attr in expr.attrs.iter() {
if attr.check_name("inline") {
if attr.check_name(sym::inline) {
self.check_inline(attr, &expr.span, target);
}
if attr.check_name("repr") {
if attr.check_name(sym::repr) {
self.emit_repr_error(
attr.span,
expr.span,
Expand All @@ -305,7 +306,7 @@ impl<'a, 'tcx> CheckAttrVisitor<'a, 'tcx> {

fn check_used(&self, item: &hir::Item, target: Target) {
for attr in &item.attrs {
if attr.check_name("used") && target != Target::Static {
if attr.check_name(sym::used) && target != Target::Static {
self.tcx.sess
.span_err(attr.span, "attribute must be applied to a `static` variable");
}
Expand Down
Loading

0 comments on commit fe5f42c

Please sign in to comment.