auto merge of #14977 : pcwalton/rust/address-insignificant-reform, r=brson

`#[inline(never)]` is used.

Closes #8958.

This can break some code that relied on the addresses of statics
being distinct; add `#[inline(never)]` to the affected statics.

[breaking-change]

r? @brson

Author: bors

src/doc/rust.md

@@ -1829,8 +1829,6 @@ type int8_t = i8;
 
 ### Static-only attributes
 
-- `address_insignificant` - references to this static may alias with
-  references to other statics, potentially of unrelated type.
 - `thread_local` - on a `static mut`, this signals that the value of this
   static may change depending on the current thread. The exact consequences of
   this are implementation-defined.
@@ -2141,13 +2139,22 @@ These types help drive the compiler's analysis
 ### Inline attributes
 
 The inline attribute is used to suggest to the compiler to perform an inline
-expansion and place a copy of the function in the caller rather than generating
-code to call the function where it is defined.
+expansion and place a copy of the function or static in the caller rather than
+generating code to call the function or access the static where it is defined.
 
 The compiler automatically inlines functions based on internal heuristics.
 Incorrectly inlining functions can actually making the program slower, so it
 should be used with care.
 
+Immutable statics are always considered inlineable
+unless marked with `#[inline(never)]`.
+It is undefined
+whether two different inlineable statics
+have the same memory address.
+In other words,
+the compiler is free
+to collapse duplicate inlineable statics together.
+
 `#[inline]` and `#[inline(always)]` always causes the function to be serialized
 into crate metadata to allow cross-crate inlining.
 

src/librustc/middle/lint.rs

@@ -1087,7 +1087,6 @@ fn check_unused_attribute(cx: &Context, attr: &ast::Attribute) {
 
         // FIXME: #14406 these are processed in trans, which happens after the
         // lint pass
-        "address_insignificant",
         "cold",
         "inline",
         "link",

src/librustc/middle/reachable.rs

@@ -26,15 +26,20 @@ use std::collections::HashSet;
 use syntax::abi;
 use syntax::ast;
 use syntax::ast_map;
-use syntax::ast_util::{is_local};
+use syntax::ast_util::is_local;
+use syntax::ast_util;
+use syntax::attr::{InlineAlways, InlineHint, InlineNever, InlineNone};
 use syntax::attr;
 use syntax::visit::Visitor;
 use syntax::visit;
 
 // Returns true if the given set of attributes contains the `#[inline]`
 // attribute.
 fn attributes_specify_inlining(attrs: &[ast::Attribute]) -> bool {
-    attr::contains_name(attrs, "inline")
+    match attr::find_inline_attr(attrs) {
+        InlineNone | InlineNever => false,
+        InlineAlways | InlineHint => true,
+    }
 }
 
 // Returns true if the given set of generics implies that the item it's
@@ -118,8 +123,8 @@ impl<'a> Visitor<()> for ReachableContext<'a> {
                         match def {
                             // If this path leads to a static, then we may have
                             // to do some work to figure out whether the static
-                            // is indeed reachable (address_insignificant
-                            // statics are *never* reachable).
+                            // is indeed reachable. (Inlineable statics are
+                            // never reachable.)
                             def::DefStatic(..) => {
                                 self.worklist.push(def_id.node);
                             }
@@ -281,9 +286,10 @@ impl<'a> ReachableContext<'a> {
 
                     // Statics with insignificant addresses are not reachable
                     // because they're inlined specially into all other crates.
-                    ast::ItemStatic(_, _, ref init) => {
-                        if attr::contains_name(item.attrs.as_slice(),
-                                               "address_insignificant") {
+                    ast::ItemStatic(_, mutbl, ref init) => {
+                        if !ast_util::static_has_significant_address(
+                                mutbl,
+                                item.attrs.as_slice()) {
                             self.reachable_symbols.remove(&search_item);
                         }
                         visit::walk_expr(self, &**init, ());

src/librustc/middle/trans/base.rs

@@ -1994,7 +1994,7 @@ pub fn get_item_val(ccx: &CrateContext, id: ast::NodeId) -> ValueRef {
             let sym = exported_name(ccx, id, ty, i.attrs.as_slice());
 
             let v = match i.node {
-                ast::ItemStatic(_, _, ref expr) => {
+                ast::ItemStatic(_, mutbl, ref expr) => {
                     // If this static came from an external crate, then
                     // we need to get the symbol from csearch instead of
                     // using the current crate's name/version
@@ -2029,28 +2029,24 @@ pub fn get_item_val(ccx: &CrateContext, id: ast::NodeId) -> ValueRef {
 
                         // Apply the `unnamed_addr` attribute if
                         // requested
-                        if attr::contains_name(i.attrs.as_slice(),
-                                               "address_insignificant") {
-                            if ccx.reachable.contains(&id) {
-                                ccx.sess().span_bug(i.span,
-                                    "insignificant static is reachable");
-                            }
+                        if !ast_util::static_has_significant_address(
+                                mutbl,
+                                i.attrs.as_slice()) {
                             lib::llvm::SetUnnamedAddr(g, true);
 
                             // This is a curious case where we must make
                             // all of these statics inlineable. If a
-                            // global is tagged as
-                            // address_insignificant, then LLVM won't
-                            // coalesce globals unless they have an
-                            // internal linkage type. This means that
+                            // global is not tagged as `#[inline(never)]`,
+                            // then LLVM won't coalesce globals unless they
+                            // have an internal linkage type. This means that
                             // external crates cannot use this global.
                             // This is a problem for things like inner
                             // statics in generic functions, because the
                             // function will be inlined into another
                             // crate and then attempt to link to the
                             // static in the original crate, only to
                             // find that it's not there. On the other
-                            // side of inlininig, the crates knows to
+                            // side of inlining, the crates knows to
                             // not declare this static as
                             // available_externally (because it isn't)
                             inlineable = true;

src/librustc/middle/trans/inline.rs

@@ -17,7 +17,7 @@ use middle::ty;
 
 use syntax::ast;
 use syntax::ast_util::local_def;
-use syntax::attr;
+use syntax::ast_util;
 
 pub fn maybe_instantiate_inline(ccx: &CrateContext, fn_id: ast::DefId)
     -> ast::DefId {
@@ -62,12 +62,13 @@ pub fn maybe_instantiate_inline(ccx: &CrateContext, fn_id: ast::DefId)
             // however, so we use the available_externally linkage which llvm
             // provides
             match item.node {
-                ast::ItemStatic(..) => {
+                ast::ItemStatic(_, mutbl, _) => {
                     let g = get_item_val(ccx, item.id);
                     // see the comment in get_item_val() as to why this check is
                     // performed here.
-                    if !attr::contains_name(item.attrs.as_slice(),
-                                            "address_insignificant") {
+                    if ast_util::static_has_significant_address(
+                            mutbl,
+                            item.attrs.as_slice()) {
                         SetLinkage(g, AvailableExternallyLinkage);
                     }
                 }

src/libsyntax/ast_util.rs

@@ -11,6 +11,8 @@
 use ast::*;
 use ast;
 use ast_util;
+use attr::{InlineNever, InlineNone};
+use attr;
 use codemap;
 use codemap::Span;
 use owned_slice::OwnedSlice;
@@ -742,6 +744,17 @@ pub fn get_inner_tys(ty: P<Ty>) -> Vec<P<Ty>> {
     }
 }
 
+/// Returns true if the static with the given mutability and attributes
+/// has a significant address and false otherwise.
+pub fn static_has_significant_address(mutbl: ast::Mutability,
+                                              attrs: &[ast::Attribute])
+                                              -> bool {
+    if mutbl == ast::MutMutable {
+        return true
+    }
+    let inline = attr::find_inline_attr(attrs);
+    inline == InlineNever || inline == InlineNone
+}
 
 #[cfg(test)]
 mod test {

src/libsyntax/ext/format.rs

@@ -312,22 +312,14 @@ impl<'a, 'b> Context<'a, 'b> {
     /// These attributes are applied to all statics that this syntax extension
     /// will generate.
     fn static_attrs(&self) -> Vec<ast::Attribute> {
-        // Flag statics as `address_insignificant` so LLVM can merge duplicate
-        // globals as much as possible (which we're generating a whole lot of).
-        let unnamed = self.ecx
-                          .meta_word(self.fmtsp,
-                                     InternedString::new(
-                                         "address_insignificant"));
-        let unnamed = self.ecx.attribute(self.fmtsp, unnamed);
-
         // Do not warn format string as dead code
         let dead_code = self.ecx.meta_word(self.fmtsp,
                                            InternedString::new("dead_code"));
         let allow_dead_code = self.ecx.meta_list(self.fmtsp,
                                                  InternedString::new("allow"),
                                                  vec!(dead_code));
         let allow_dead_code = self.ecx.attribute(self.fmtsp, allow_dead_code);
-        return vec!(unnamed, allow_dead_code);
+        return vec!(allow_dead_code);
     }
 
     fn rtpath(&self, s: &str) -> Vec<ast::Ident> {

src/test/auxiliary/xcrate_address_insignificant.rs

@@ -9,7 +9,6 @@
 // except according to those terms.
 
 pub fn foo<T>() -> int {
-    #[address_insignificant]
     static a: int = 3;
     a
 }

src/test/auxiliary/xcrate_static_addresses.rs

@@ -8,9 +8,13 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
+#[inline(never)]
 pub static global: int = 3;
 
+#[inline(never)]
 static global0: int = 4;
+
+#[inline(never)]
 pub static global2: &'static int = &global0;
 
 pub fn verify_same(a: &'static int) {