Auto merge of #53711 - arielb1:macro-table, r=michaelwoerister

create a valid DefIdTable for proc macro crates

At least the incremental compilation code, and a few other places in the
compiler, require the CrateMetadata for a loaded target crate to contain a
valid DefIdTable for the DefIds in the target.

Previously, the CrateMetadata for a proc macro contained the crate's
"host" DefIdTable, which is of course incompatible with the "target"
DefIdTable, causing ICEs. This creates a DefIdTable that properly refers
to the "proc macro" DefIds.

Fixes #49482.

r? @michaelwoerister

Should we beta-nominate this?

Author: bors

src/librustc/hir/def_id.rs

@@ -9,7 +9,7 @@
 // except according to those terms.
 
 use ty;
-
+use hir::map::definitions::FIRST_FREE_HIGH_DEF_INDEX;
 use rustc_data_structures::indexed_vec::Idx;
 use serialize;
 use std::fmt;
@@ -125,15 +125,25 @@ impl DefIndex {
     // index of the macro in the CrateMetadata::proc_macros array) to the
     // corresponding DefIndex.
     pub fn from_proc_macro_index(proc_macro_index: usize) -> DefIndex {
-        let def_index = DefIndex::from_array_index(proc_macro_index,
-                                                   DefIndexAddressSpace::High);
+        // DefIndex for proc macros start from FIRST_FREE_HIGH_DEF_INDEX,
+        // because the first FIRST_FREE_HIGH_DEF_INDEX indexes are reserved
+        // for internal use.
+        let def_index = DefIndex::from_array_index(
+            proc_macro_index.checked_add(FIRST_FREE_HIGH_DEF_INDEX)
+                .expect("integer overflow adding `proc_macro_index`"),
+            DefIndexAddressSpace::High);
         assert!(def_index != CRATE_DEF_INDEX);
         def_index
     }
 
     // This function is the reverse of from_proc_macro_index() above.
     pub fn to_proc_macro_index(self: DefIndex) -> usize {
-        self.as_array_index()
+        assert_eq!(self.address_space(), DefIndexAddressSpace::High);
+
+        self.as_array_index().checked_sub(FIRST_FREE_HIGH_DEF_INDEX)
+            .unwrap_or_else(|| {
+                bug!("using local index {:?} as proc-macro index", self)
+            })
     }
 
     // Don't use this if you don't know about the DefIndex encoding.
@@ -150,7 +160,7 @@ impl DefIndex {
 impl serialize::UseSpecializedEncodable for DefIndex {}
 impl serialize::UseSpecializedDecodable for DefIndex {}
 
-#[derive(Copy, Clone, Hash)]
+#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)]
 pub enum DefIndexAddressSpace {
     Low = 0,
     High = 1,

src/librustc/hir/map/definitions.rs

@@ -380,6 +380,17 @@ impl Borrow<Fingerprint> for DefPathHash {
 
 impl Definitions {
     /// Create new empty definition map.
+    ///
+    /// The DefIndex returned from a new Definitions are as follows:
+    /// 1. At DefIndexAddressSpace::Low,
+    ///     CRATE_ROOT has index 0:0, and then new indexes are allocated in
+    ///     ascending order.
+    /// 2. At DefIndexAddressSpace::High,
+    ///     the first FIRST_FREE_HIGH_DEF_INDEX indexes are reserved for
+    ///     internal use, then 1:FIRST_FREE_HIGH_DEF_INDEX are allocated in
+    ///     ascending order.
+    ///
+    /// FIXME: there is probably a better place to put this comment.
     pub fn new() -> Definitions {
         Definitions {
             table: DefPathTable {
@@ -665,6 +676,11 @@ impl DefPathData {
     }
 }
 
+macro_rules! count {
+    () => (0usize);
+    ( $x:tt $($xs:tt)* ) => (1usize + count!($($xs)*));
+}
+
 // We define the GlobalMetaDataKind enum with this macro because we want to
 // make sure that we exhaustively iterate over all variants when registering
 // the corresponding DefIndices in the DefTable.
@@ -678,6 +694,7 @@ macro_rules! define_global_metadata_kind {
         }
 
         const GLOBAL_MD_ADDRESS_SPACE: DefIndexAddressSpace = DefIndexAddressSpace::High;
+        pub const FIRST_FREE_HIGH_DEF_INDEX: usize = count!($($variant)*);
 
         impl GlobalMetaDataKind {
             fn allocate_def_indices(definitions: &mut Definitions) {

src/librustc_metadata/creader.rs

@@ -12,6 +12,7 @@
 
 use cstore::{self, CStore, CrateSource, MetadataBlob};
 use locator::{self, CratePaths};
+use decoder::proc_macro_def_path_table;
 use schema::CrateRoot;
 use rustc_data_structures::sync::{Lrc, RwLock, Lock};
 
@@ -219,8 +220,16 @@ impl<'a> CrateLoader<'a> {
 
         let dependencies: Vec<CrateNum> = cnum_map.iter().cloned().collect();
 
+        let proc_macros = crate_root.macro_derive_registrar.map(|_| {
+            self.load_derive_macros(&crate_root, dylib.clone().map(|p| p.0), span)
+        });
+
         let def_path_table = record_time(&self.sess.perf_stats.decode_def_path_tables_time, || {
-            crate_root.def_path_table.decode((&metadata, self.sess))
+            if let Some(proc_macros) = &proc_macros {
+                proc_macro_def_path_table(&crate_root, proc_macros)
+            } else {
+                crate_root.def_path_table.decode((&metadata, self.sess))
+            }
         });
 
         let interpret_alloc_index: Vec<u32> = crate_root.interpret_alloc_index
@@ -237,9 +246,7 @@ impl<'a> CrateLoader<'a> {
             extern_crate: Lock::new(None),
             def_path_table: Lrc::new(def_path_table),
             trait_impls,
-            proc_macros: crate_root.macro_derive_registrar.map(|_| {
-                self.load_derive_macros(&crate_root, dylib.clone().map(|p| p.0), span)
-            }),
+            proc_macros,
             root: crate_root,
             blob: metadata,
             cnum_map,

src/librustc_metadata/decoder.rs

@@ -14,14 +14,14 @@ use cstore::{self, CrateMetadata, MetadataBlob, NativeLibrary, ForeignModule};
 use schema::*;
 
 use rustc_data_structures::sync::{Lrc, ReadGuard};
-use rustc::hir::map::{DefKey, DefPath, DefPathData, DefPathHash,
-                      DisambiguatedDefPathData};
+use rustc::hir::map::{DefKey, DefPath, DefPathData, DefPathHash, Definitions};
 use rustc::hir;
 use rustc::middle::cstore::LinkagePreference;
 use rustc::middle::exported_symbols::{ExportedSymbol, SymbolExportLevel};
 use rustc::hir::def::{self, Def, CtorKind};
-use rustc::hir::def_id::{CrateNum, DefId, DefIndex,
+use rustc::hir::def_id::{CrateNum, DefId, DefIndex, DefIndexAddressSpace,
                          CRATE_DEF_INDEX, LOCAL_CRATE, LocalDefId};
+use rustc::hir::map::definitions::DefPathTable;
 use rustc_data_structures::fingerprint::Fingerprint;
 use rustc::middle::lang_items;
 use rustc::mir::{self, interpret};
@@ -41,7 +41,8 @@ use syntax::attr;
 use syntax::ast::{self, Ident};
 use syntax::source_map;
 use syntax::symbol::InternedString;
-use syntax::ext::base::MacroKind;
+use syntax::ext::base::{MacroKind, SyntaxExtension};
+use syntax::ext::hygiene::Mark;
 use syntax_pos::{self, Span, BytePos, Pos, DUMMY_SP, NO_EXPANSION};
 
 pub struct DecodeContext<'a, 'tcx: 'a> {
@@ -441,6 +442,40 @@ impl<'tcx> EntryKind<'tcx> {
     }
 }
 
+/// Create the "fake" DefPathTable for a given proc macro crate.
+///
+/// The DefPathTable is as follows:
+///
+/// CRATE_ROOT (DefIndex 0:0)
+///  |- GlobalMetaDataKind data (DefIndex 1:0 .. DefIndex 1:N)
+///  |- proc macro #0 (DefIndex 1:N)
+///  |- proc macro #1 (DefIndex 1:N+1)
+///  \- ...
+crate fn proc_macro_def_path_table(crate_root: &CrateRoot,
+                                   proc_macros: &[(ast::Name, Lrc<SyntaxExtension>)])
+                                   -> DefPathTable
+{
+    let mut definitions = Definitions::new();
+
+    let name = crate_root.name.as_str();
+    let disambiguator = crate_root.disambiguator;
+    debug!("creating proc macro def path table for {:?}/{:?}", name, disambiguator);
+    let crate_root = definitions.create_root_def(&name, disambiguator);
+    for (index, (name, _)) in proc_macros.iter().enumerate() {
+        let def_index = definitions.create_def_with_parent(
+            crate_root,
+            ast::DUMMY_NODE_ID,
+            DefPathData::MacroDef(name.as_interned_str()),
+            DefIndexAddressSpace::High,
+            Mark::root(),
+            DUMMY_SP);
+        debug!("definition for {:?} is {:?}", name, def_index);
+        assert_eq!(def_index, DefIndex::from_proc_macro_index(index));
+    }
+
+    definitions.def_path_table().clone()
+}
+
 impl<'a, 'tcx> CrateMetadata {
     fn is_proc_macro(&self, id: DefIndex) -> bool {
         self.proc_macros.is_some() && id != CRATE_DEF_INDEX
@@ -669,6 +704,10 @@ impl<'a, 'tcx> CrateMetadata {
         where F: FnMut(def::Export)
     {
         if let Some(ref proc_macros) = self.proc_macros {
+            /* If we are loading as a proc macro, we want to return the view of this crate
+             * as a proc macro crate, not as a Rust crate. See `proc_macro_def_path_table`
+             * for the DefPathTable we are corresponding to.
+             */
             if id == CRATE_DEF_INDEX {
                 for (id, &(name, ref ext)) in proc_macros.iter().enumerate() {
                     let def = Def::Macro(
@@ -1066,28 +1105,12 @@ impl<'a, 'tcx> CrateMetadata {
 
     #[inline]
     pub fn def_key(&self, index: DefIndex) -> DefKey {
-        if !self.is_proc_macro(index) {
-            self.def_path_table.def_key(index)
-        } else {
-            // FIXME(#49271) - It would be better if the DefIds were consistent
-            //                 with the DefPathTable, but for proc-macro crates
-            //                 they aren't.
-            let name = self.proc_macros
-                           .as_ref()
-                           .unwrap()[index.to_proc_macro_index()].0;
-            DefKey {
-                parent: Some(CRATE_DEF_INDEX),
-                disambiguated_data: DisambiguatedDefPathData {
-                    data: DefPathData::MacroDef(name.as_interned_str()),
-                    disambiguator: 0,
-                }
-            }
-        }
+        self.def_path_table.def_key(index)
     }
 
     // Returns the path leading to the thing with this `id`.
     pub fn def_path(&self, id: DefIndex) -> DefPath {
-        debug!("def_path(id={:?})", id);
+        debug!("def_path(cnum={:?}, id={:?})", self.cnum, id);
         DefPath::make(self.cnum, id, |parent| self.def_path_table.def_key(parent))
     }
 

src/librustc_metadata/lib.rs

@@ -22,6 +22,7 @@
 #![feature(quote)]
 #![feature(rustc_diagnostic_macros)]
 #![feature(slice_sort_by_cached_key)]
+#![feature(crate_visibility_modifier)]
 #![feature(specialization)]
 #![feature(rustc_private)]
 

src/test/incremental-fulldeps/auxiliary/issue_49482_macro_def.rs

@@ -0,0 +1,49 @@
+// Copyright 2018 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.
+
+// no-prefer-dynamic
+
+#![crate_type="proc-macro"]
+#![allow(non_snake_case)]
+
+extern crate proc_macro;
+
+macro_rules! proc_macro_expr_impl {
+    ($(
+        $( #[$attr:meta] )*
+        pub fn $func:ident($input:ident: &str) -> String;
+    )+) => {
+        $(
+            $( #[$attr] )*
+            #[proc_macro_derive($func)]
+            pub fn $func(_input: ::proc_macro::TokenStream) -> ::proc_macro::TokenStream {
+                panic!()
+            }
+        )+
+    };
+}
+
+proc_macro_expr_impl! {
+    pub fn f1(input: &str) -> String;
+    pub fn f2(input: &str) -> String;
+    pub fn f3(input: &str) -> String;
+    pub fn f4(input: &str) -> String;
+    pub fn f5(input: &str) -> String;
+    pub fn f6(input: &str) -> String;
+    pub fn f7(input: &str) -> String;
+    pub fn f8(input: &str) -> String;
+    pub fn f9(input: &str) -> String;
+    pub fn fA(input: &str) -> String;
+    pub fn fB(input: &str) -> String;
+    pub fn fC(input: &str) -> String;
+    pub fn fD(input: &str) -> String;
+    pub fn fE(input: &str) -> String;
+    pub fn fF(input: &str) -> String;
+}

src/test/incremental-fulldeps/auxiliary/issue_49482_reexport.rs

@@ -0,0 +1,16 @@
+// Copyright 2018 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.
+
+#[macro_use]
+extern crate issue_49482_macro_def;
+
+pub use issue_49482_macro_def::*;
+
+pub fn foo() {}

src/test/incremental-fulldeps/issue-49482.rs

@@ -0,0 +1,41 @@
+// Copyright 2018 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.
+
+// aux-build:issue_49482_macro_def.rs
+// aux-build:issue_49482_reexport.rs
+// ignore-stage1
+// revisions: rpass1
+
+extern crate issue_49482_reexport;
+
+pub trait KvStorage
+{
+    fn get(&self);
+}
+
+impl<K> KvStorage for Box<K>
+where
+    K: KvStorage + ?Sized,
+{
+    fn get(&self) {
+        (**self).get()
+    }
+}
+
+impl KvStorage for u32 {
+    fn get(&self) {}
+}
+
+fn main() {
+    /* force issue_49482_reexport to be loaded */
+    issue_49482_reexport::foo();
+
+    Box::new(2).get();
+}