Fixes ICE when using reexported unit-like structs

Michael Kainer · Michael Kainer · commit 065a5b042422 · 2014-09-30T16:22:55.000+02:00
Fixes that unit-like structs cannot be used if they are reexported and used in another crate. The compiler fails with an ICE, because unit-like structs are exported as DefFn and the expression `UnitStruct` is interpreted as function pointer instead of a call to the constructor. To resolve this ambiguity tuple-like struct constructors are now exported as CtorFn. When `rustc::metadata::decoder` finds a CtorFn it sets a new flag `is_ctor` in DefFn to true. Relevant changes are in `rustc::metadata::{encoder, decoder}` and in `rustc::middle::ty`. Closes #12660 and #16973.
diff --git a/src/librustc/metadata/decoder.rs b/src/librustc/metadata/decoder.rs
@@ -112,6 +112,7 @@ enum Family {
     MutStatic,             // b
     Fn,                    // f
     UnsafeFn,              // u
+    CtorFn,                // o
     StaticMethod,          // F
     UnsafeStaticMethod,    // U
     Type,                  // y
@@ -135,6 +136,7 @@ fn item_family(item: rbml::Doc) -> Family {
       'b' => MutStatic,
       'f' => Fn,
       'u' => UnsafeFn,
+      'o' => CtorFn,
       'F' => StaticMethod,
       'U' => UnsafeStaticMethod,
       'y' => Type,
@@ -304,8 +306,9 @@ fn item_to_def_like(item: rbml::Doc, did: ast::DefId, cnum: ast::CrateNum)
         ImmStatic => DlDef(def::DefStatic(did, false)),
         MutStatic => DlDef(def::DefStatic(did, true)),
         Struct    => DlDef(def::DefStruct(did)),
-        UnsafeFn  => DlDef(def::DefFn(did, ast::UnsafeFn)),
-        Fn        => DlDef(def::DefFn(did, ast::NormalFn)),
+        UnsafeFn  => DlDef(def::DefFn(did, ast::UnsafeFn, false)),
+        Fn        => DlDef(def::DefFn(did, ast::NormalFn, false)),
+        CtorFn    => DlDef(def::DefFn(did, ast::NormalFn, true)),
         StaticMethod | UnsafeStaticMethod => {
             let fn_style = if fam == UnsafeStaticMethod {
                 ast::UnsafeFn
diff --git a/src/librustc/metadata/encoder.rs b/src/librustc/metadata/encoder.rs
@@ -760,7 +760,7 @@ fn encode_info_for_struct_ctor(ecx: &EncodeContext,
 
     rbml_w.start_tag(tag_items_data_item);
     encode_def_id(rbml_w, local_def(ctor_id));
-    encode_family(rbml_w, 'f');
+    encode_family(rbml_w, 'o');
     encode_bounds_and_type(rbml_w, ecx,
                            &lookup_item_type(ecx.tcx, local_def(ctor_id)));
     encode_name(rbml_w, name.name);
diff --git a/src/librustc/middle/astencode.rs b/src/librustc/middle/astencode.rs
@@ -440,7 +440,7 @@ fn decode_def(dcx: &DecodeContext, doc: rbml::Doc) -> def::Def {
 impl tr for def::Def {
     fn tr(&self, dcx: &DecodeContext) -> def::Def {
         match *self {
-          def::DefFn(did, p) => def::DefFn(did.tr(dcx), p),
+          def::DefFn(did, p, is_ctor) => def::DefFn(did.tr(dcx), p, is_ctor),
           def::DefStaticMethod(did, wrapped_did2, p) => {
             def::DefStaticMethod(did.tr(dcx),
                                    match wrapped_did2 {
diff --git a/src/librustc/middle/check_const.rs b/src/librustc/middle/check_const.rs
@@ -133,7 +133,7 @@ fn check_expr(v: &mut CheckCrateVisitor, e: &Expr) {
             }
             match v.tcx.def_map.borrow().find(&e.id) {
               Some(&DefStatic(..)) |
-              Some(&DefFn(_, _)) |
+              Some(&DefFn(..)) |
               Some(&DefVariant(_, _, _)) |
               Some(&DefStruct(_)) => { }
 
diff --git a/src/librustc/middle/def.rs b/src/librustc/middle/def.rs
@@ -14,7 +14,7 @@ use syntax::ast_util::local_def;
 
 #[deriving(Clone, PartialEq, Eq, Encodable, Decodable, Hash, Show)]
 pub enum Def {
-    DefFn(ast::DefId, ast::FnStyle),
+    DefFn(ast::DefId, ast::FnStyle, bool /* is_ctor */),
     DefStaticMethod(/* method */ ast::DefId, MethodProvenance, ast::FnStyle),
     DefSelfTy(/* trait id */ ast::NodeId),
     DefMod(ast::DefId),
@@ -57,7 +57,7 @@ pub enum MethodProvenance {
 impl Def {
     pub fn def_id(&self) -> ast::DefId {
         match *self {
-            DefFn(id, _) | DefStaticMethod(id, _, _) | DefMod(id) |
+            DefFn(id, _, _) | DefStaticMethod(id, _, _) | DefMod(id) |
             DefForeignMod(id) | DefStatic(id, _) |
             DefVariant(_, id, _) | DefTy(id, _) | DefAssociatedTy(id) |
             DefTyParam(_, id, _) | DefUse(id) | DefStruct(id) | DefTrait(id) |
diff --git a/src/librustc/middle/intrinsicck.rs b/src/librustc/middle/intrinsicck.rs
@@ -121,7 +121,7 @@ impl<'a, 'tcx, 'v> Visitor<'v> for IntrinsicCheckingVisitor<'a, 'tcx> {
         match expr.node {
             ast::ExprPath(..) => {
                 match ty::resolve_expr(self.tcx, expr) {
-                    DefFn(did, _) if self.def_id_is_transmute(did) => {
+                    DefFn(did, _, _) if self.def_id_is_transmute(did) => {
                         let typ = ty::node_id_to_type(self.tcx, expr.id);
                         match ty::get(typ).sty {
                             ty_bare_fn(ref bare_fn_ty)
diff --git a/src/librustc/middle/privacy.rs b/src/librustc/middle/privacy.rs
@@ -934,7 +934,7 @@ impl<'a, 'tcx, 'v> Visitor<'v> for PrivacyVisitor<'a, 'tcx> {
                     }
                     // Tuple struct constructors across crates are identified as
                     // DefFn types, so we explicitly handle that case here.
-                    Some(&def::DefFn(did, _)) if !is_local(did) => {
+                    Some(&def::DefFn(did, _, _)) if !is_local(did) => {
                         match csearch::get_tuple_struct_definition_if_ctor(
                                     &self.tcx.sess.cstore, did) {
                             Some(did) => guard(did),
diff --git a/src/librustc/middle/resolve.rs b/src/librustc/middle/resolve.rs
@@ -1247,7 +1247,7 @@ impl<'a> Resolver<'a> {
                 let name_bindings =
                     self.add_child(ident, parent.clone(), ForbidDuplicateValues, sp);
 
-                let def = DefFn(local_def(item.id), fn_style);
+                let def = DefFn(local_def(item.id), fn_style, false);
                 name_bindings.define_value(def, sp, is_public);
                 parent
             }
@@ -1705,7 +1705,7 @@ impl<'a> Resolver<'a> {
 
         match foreign_item.node {
             ForeignItemFn(_, ref generics) => {
-                let def = DefFn(local_def(foreign_item.id), UnsafeFn);
+                let def = DefFn(local_def(foreign_item.id), UnsafeFn, false);
                 name_bindings.define_value(def, foreign_item.span, is_public);
 
                 self.with_type_parameter_rib(
@@ -2022,7 +2022,8 @@ impl<'a> Resolver<'a> {
                                                        DUMMY_SP);
                                     let def = DefFn(
                                         static_method_info.def_id,
-                                        static_method_info.fn_style);
+                                        static_method_info.fn_style,
+                                        false);
 
                                     method_name_bindings.define_value(
                                         def, DUMMY_SP,
@@ -2591,7 +2592,8 @@ impl<'a> Resolver<'a> {
 
         match value_result {
             BoundResult(ref target_module, ref name_bindings) => {
-                debug!("(resolving single import) found value target");
+                debug!("(resolving single import) found value target: {:?}",
+                       { name_bindings.value_def.borrow().clone().unwrap().def });
                 self.check_for_conflicting_import(
                     &import_resolution.value_target,
                     directive.span,
diff --git a/src/librustc/middle/save/mod.rs b/src/librustc/middle/save/mod.rs
@@ -234,7 +234,7 @@ impl <'l, 'tcx> DxrVisitor<'l, 'tcx> {
             def::DefVariant(_, _, _) |
             def::DefUpvar(..) => Some(recorder::VarRef),
 
-            def::DefFn(_, _) => Some(recorder::FnRef),
+            def::DefFn(..) => Some(recorder::FnRef),
 
             def::DefSelfTy(_) |
             def::DefRegion(_) |
@@ -792,10 +792,10 @@ impl <'l, 'tcx> DxrVisitor<'l, 'tcx> {
                                        Some(declid),
                                        self.cur_scope);
             },
-            def::DefFn(def_id, _) => self.fmt.fn_call_str(ex.span,
-                                                          sub_span,
-                                                          def_id,
-                                                          self.cur_scope),
+            def::DefFn(def_id, _, _) => self.fmt.fn_call_str(ex.span,
+                                                             sub_span,
+                                                             def_id,
+                                                             self.cur_scope),
             _ => self.sess.span_bug(ex.span,
                                     format!("Unexpected def kind while looking up path in '{}'",
                                             self.span.snippet(ex.span)).as_slice()),
@@ -808,7 +808,7 @@ impl <'l, 'tcx> DxrVisitor<'l, 'tcx> {
             def::DefLocal(_) |
             def::DefStatic(_,_) |
             def::DefStruct(_) |
-            def::DefFn(_, _) => self.write_sub_paths_truncated(path),
+            def::DefFn(..) => self.write_sub_paths_truncated(path),
             _ => {},
         }
 
diff --git a/src/librustc/middle/trans/callee.rs b/src/librustc/middle/trans/callee.rs
@@ -142,7 +142,7 @@ fn trans<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, expr: &ast::Expr)
         debug!("trans_def(def={}, ref_expr={})", def.repr(bcx.tcx()), ref_expr.repr(bcx.tcx()));
         let expr_ty = node_id_type(bcx, ref_expr.id);
         match def {
-            def::DefFn(did, _) if {
+            def::DefFn(did, _, _) if {
                 let maybe_def_id = inline::get_local_instance(bcx.ccx(), did);
                 let maybe_ast_node = maybe_def_id.and_then(|def_id| bcx.tcx().map
                                                                              .find(def_id.node));
@@ -157,15 +157,15 @@ fn trans<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, expr: &ast::Expr)
                     data: NamedTupleConstructor(substs, 0)
                 }
             }
-            def::DefFn(did, _) if match ty::get(expr_ty).sty {
+            def::DefFn(did, _, _) if match ty::get(expr_ty).sty {
                 ty::ty_bare_fn(ref f) => f.abi == synabi::RustIntrinsic,
                 _ => false
             } => {
                 let substs = node_id_substs(bcx, ExprId(ref_expr.id));
                 let def_id = inline::maybe_instantiate_inline(bcx.ccx(), did);
                 Callee { bcx: bcx, data: Intrinsic(def_id.node, substs) }
             }
-            def::DefFn(did, _) |
+            def::DefFn(did, _, _) |
             def::DefStaticMethod(did, def::FromImpl(_), _) => {
                 fn_callee(bcx, trans_fn_ref(bcx, did, ExprId(ref_expr.id)))
             }
diff --git a/src/librustc/middle/trans/closure.rs b/src/librustc/middle/trans/closure.rs
@@ -532,7 +532,7 @@ pub fn get_wrapper_for_bare_fn(ccx: &CrateContext,
                                is_local: bool) -> ValueRef {
 
     let def_id = match def {
-        def::DefFn(did, _) | def::DefStaticMethod(did, _, _) |
+        def::DefFn(did, _, _) | def::DefStaticMethod(did, _, _) |
         def::DefVariant(_, did, _) | def::DefStruct(did) => did,
         _ => {
             ccx.sess().bug(format!("get_wrapper_for_bare_fn: \
diff --git a/src/librustc/middle/trans/consts.rs b/src/librustc/middle/trans/consts.rs
@@ -623,7 +623,7 @@ fn const_expr_unadjusted(cx: &CrateContext, e: &ast::Expr,
 
             let opt_def = cx.tcx().def_map.borrow().find_copy(&e.id);
             match opt_def {
-                Some(def::DefFn(def_id, _fn_style)) => {
+                Some(def::DefFn(def_id, _fn_style, _)) => {
                     if !ast_util::is_local(def_id) {
                         let ty = csearch::get_type(cx.tcx(), def_id).ty;
                         (base::trans_external_path(cx, def_id, ty), true)
diff --git a/src/librustc/middle/trans/expr.rs b/src/librustc/middle/trans/expr.rs
@@ -1194,7 +1194,7 @@ fn trans_def_fn_unadjusted<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
     let _icx = push_ctxt("trans_def_datum_unadjusted");
 
     let llfn = match def {
-        def::DefFn(did, _) |
+        def::DefFn(did, _, _) |
         def::DefStruct(did) | def::DefVariant(_, did, _) |
         def::DefStaticMethod(did, def::FromImpl(_), _) => {
             callee::trans_fn_ref(bcx, did, ExprId(ref_expr.id))
diff --git a/src/librustc/middle/ty.rs b/src/librustc/middle/ty.rs
@@ -3588,6 +3588,12 @@ pub fn expr_kind(tcx: &ctxt, expr: &ast::Expr) -> ExprKind {
                     }
                 }
 
+                // Special case: A unit like struct's constructor must be called without () at the
+                // end (like `UnitStruct`) which means this is an ExprPath to a DefFn. But in case
+                // of unit structs this is should not be interpretet as function pointer but as
+                // call to the constructor.
+                def::DefFn(_, _, true) => RvalueDpsExpr,
+
                 // Fn pointers are just scalar values.
                 def::DefFn(..) | def::DefStaticMethod(..) => RvalueDatumExpr,
 
diff --git a/src/librustc/middle/typeck/check/mod.rs b/src/librustc/middle/typeck/check/mod.rs
@@ -5082,7 +5082,7 @@ pub fn polytype_for_def(fcx: &FnCtxt,
           let typ = fcx.local_ty(sp, nid);
           return no_params(typ);
       }
-      def::DefFn(id, _) | def::DefStaticMethod(id, _, _) |
+      def::DefFn(id, _, _) | def::DefStaticMethod(id, _, _) |
       def::DefStatic(id, _) | def::DefVariant(_, id, _) |
       def::DefStruct(id) => {
         return ty::lookup_item_type(fcx.ccx.tcx, id);
diff --git a/src/librustdoc/clean/inline.rs b/src/librustdoc/clean/inline.rs
@@ -73,12 +73,8 @@ fn try_inline_def(cx: &DocContext, tcx: &ty::ctxt,
             record_extern_fqn(cx, did, clean::TypeTrait);
             clean::TraitItem(build_external_trait(cx, tcx, did))
         }
-        def::DefFn(did, style) => {
+        def::DefFn(did, style, false) => {
             // If this function is a tuple struct constructor, we just skip it
-            if csearch::get_tuple_struct_definition_if_ctor(&tcx.sess.cstore,
-                                                            did).is_some() {
-                return None
-            }
             record_extern_fqn(cx, did, clean::TypeFunction);
             clean::FunctionItem(build_external_function(cx, tcx, did, style))
         }
diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs
@@ -2078,7 +2078,7 @@ fn resolve_type(cx: &DocContext, path: Path,
 
 fn register_def(cx: &DocContext, def: def::Def) -> ast::DefId {
     let (did, kind) = match def {
-        def::DefFn(i, _) => (i, TypeFunction),
+        def::DefFn(i, _, _) => (i, TypeFunction),
         def::DefTy(i, false) => (i, TypeTypedef),
         def::DefTy(i, true) => (i, TypeEnum),
         def::DefTrait(i) => (i, TypeTrait),
diff --git a/src/test/auxiliary/issue-12660-aux.rs b/src/test/auxiliary/issue-12660-aux.rs
@@ -0,0 +1,21 @@
+// Copyright 2014 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.
+
+#![crate_type="lib"]
+#![crate_name="issue12660aux"]
+
+pub use my_mod::{MyStruct, my_fn};
+
+mod my_mod {
+    pub struct MyStruct;
+
+    pub fn my_fn(my_struct: MyStruct) {
+    }
+}
diff --git a/src/test/run-pass/issue-12660.rs b/src/test/run-pass/issue-12660.rs
@@ -0,0 +1,21 @@
+// Copyright 2014 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-12660-aux.rs
+
+extern crate issue12660aux;
+
+use issue12660aux::{my_fn, MyStruct};
+
+#[allow(path_statement)]
+fn main() {
+    my_fn(MyStruct);
+    MyStruct;
+}

Original file line number	Diff line number	Diff line change
`@@ -133,7 +133,7 @@ fn check_expr(v: &mut CheckCrateVisitor, e: &Expr) {`
`133`	`133`	`}`
`134`	`134`	`match v.tcx.def_map.borrow().find(&e.id) {`
`135`	`135`	`Some(&DefStatic(..)) \|`
`136`		`- Some(&DefFn(_, _)) \|`
	`136`	`+ Some(&DefFn(..)) \|`
`137`	`137`	`Some(&DefVariant(_, _, _)) \|`
`138`	`138`	`Some(&DefStruct(_)) => { }`
`139`	`139`
Original file line number	Diff line number	Diff line change
`@@ -934,7 +934,7 @@ impl<'a, 'tcx, 'v> Visitor<'v> for PrivacyVisitor<'a, 'tcx> {`
`934`	`934`	`}`
`935`	`935`	`// Tuple struct constructors across crates are identified as`
`936`	`936`	`// DefFn types, so we explicitly handle that case here.`
`937`		`- Some(&def::DefFn(did, _)) if !is_local(did) => {`
	`937`	`+ Some(&def::DefFn(did, _, _)) if !is_local(did) => {`
`938`	`938`	`match csearch::get_tuple_struct_definition_if_ctor(`
`939`	`939`	`&self.tcx.sess.cstore, did) {`
`940`	`940`	`Some(did) => guard(did),`