Skip to content

Commit

Permalink
Auto merge of #36016 - petrochenkov:union, r=nikomatsakis
Browse files Browse the repository at this point in the history
Implement untagged unions (RFC 1444)

cc #32836

Notes:
- The RFC doesn't talk about `#[packed]` unions, this implementation supports them, packing changes union's alignment to 1 and removes trailing padding.
- The RFC doesn't talk about dynamically sized unions, this implementation doesn't support them and rejects them during wf-checking (similarly, dynamically sized enums are not supported as well).
- The lint for drop fields in unions can't work precisely before monomorphization, so it works pessimistically - non-`Copy` generic fields are reported, types not implementing `Drop` directly, but having non-trivial drop code are reported.

    ```
    struct S(String); // Doesn't implement `Drop`
    union U<T> {
        a: S, // Reported
        b: T, // Reported
    }
    ```

- #35764 was indeed helpful and landed timely, I didn't have to implement internal drop flags for unions.
- Unions are not permitted in constant patterns, because matching on union fields is unsafe, I didn't want unsafety checker to dig into all constants to uncover this possible unsafety.
- The RFC doesn't talk about `#[derive]`, generally trait impls cannot be derived for unions, but some of them can. I implemented only `#[derive(Copy)]` so far. In theory shallow `#[derive(Clone)]` can be derived as well if all union fields are `Copy`, I left it for later though, it requires changing how `Clone` impls are generated.
- Moving union fields is implemented as per #32836 (comment).
- Testing strategy: union specific behavior is tested, sometimes very basically (e.g. debuginfo), behavior common for all ADTs (e.g. something like coherence
checks) is not generally tested.

r? @eddyb
  • Loading branch information
bors authored Sep 3, 2016
2 parents 01b35d8 + 436cfe5 commit d748fa6
Show file tree
Hide file tree
Showing 167 changed files with 2,899 additions and 362 deletions.
13 changes: 9 additions & 4 deletions src/librustc/hir/check_attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ use syntax::visit::Visitor;
enum Target {
Fn,
Struct,
Union,
Enum,
Other,
}
Expand All @@ -27,6 +28,7 @@ impl Target {
match item.node {
ast::ItemKind::Fn(..) => Target::Fn,
ast::ItemKind::Struct(..) => Target::Struct,
ast::ItemKind::Union(..) => Target::Union,
ast::ItemKind::Enum(..) => Target::Enum,
_ => Target::Other,
}
Expand Down Expand Up @@ -62,17 +64,20 @@ impl<'a> CheckAttrVisitor<'a> {
let message = match &*name {
"C" => {
conflicting_reprs += 1;
if target != Target::Struct && target != Target::Enum {
"attribute should be applied to struct or enum"
if target != Target::Struct &&
target != Target::Union &&
target != Target::Enum {
"attribute should be applied to struct, enum or union"
} else {
continue
}
}
"packed" => {
// Do not increment conflicting_reprs here, because "packed"
// can be used to modify another repr hint
if target != Target::Struct {
"attribute should be applied to struct"
if target != Target::Struct &&
target != Target::Union {
"attribute should be applied to struct or union"
} else {
continue
}
Expand Down
6 changes: 4 additions & 2 deletions src/librustc/hir/def.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ pub enum Def {
// If Def::Struct lives in value namespace (e.g. tuple struct, unit struct expressions)
// it denotes a constructor and its DefId refers to NodeId of the struct's constructor.
Struct(DefId),
Union(DefId),
Label(ast::NodeId),
Method(DefId),
Err,
Expand Down Expand Up @@ -109,7 +110,7 @@ impl Def {

Def::Fn(..) | Def::Mod(..) | Def::ForeignMod(..) | Def::Static(..) |
Def::Variant(..) | Def::Enum(..) | Def::TyAlias(..) | Def::AssociatedTy(..) |
Def::TyParam(..) | Def::Struct(..) | Def::Trait(..) |
Def::TyParam(..) | Def::Struct(..) | Def::Union(..) | Def::Trait(..) |
Def::Method(..) | Def::Const(..) | Def::AssociatedConst(..) |
Def::PrimTy(..) | Def::Label(..) | Def::SelfTy(..) | Def::Err => {
bug!("attempted .var_id() on invalid {:?}", self)
Expand All @@ -121,7 +122,7 @@ impl Def {
match *self {
Def::Fn(id) | Def::Mod(id) | Def::ForeignMod(id) | Def::Static(id, _) |
Def::Variant(_, id) | Def::Enum(id) | Def::TyAlias(id) | Def::AssociatedTy(_, id) |
Def::TyParam(id) | Def::Struct(id) | Def::Trait(id) |
Def::TyParam(id) | Def::Struct(id) | Def::Union(id) | Def::Trait(id) |
Def::Method(id) | Def::Const(id) | Def::AssociatedConst(id) |
Def::Local(id, _) | Def::Upvar(id, _, _, _) => {
id
Expand All @@ -147,6 +148,7 @@ impl Def {
Def::TyAlias(..) => "type",
Def::AssociatedTy(..) => "associated type",
Def::Struct(..) => "struct",
Def::Union(..) => "union",
Def::Trait(..) => "trait",
Def::Method(..) => "method",
Def::Const(..) => "constant",
Expand Down
4 changes: 4 additions & 0 deletions src/librustc/hir/fold.rs
Original file line number Diff line number Diff line change
Expand Up @@ -761,6 +761,10 @@ pub fn noop_fold_item_underscore<T: Folder>(i: Item_, folder: &mut T) -> Item_ {
let struct_def = folder.fold_variant_data(struct_def);
ItemStruct(struct_def, folder.fold_generics(generics))
}
ItemUnion(struct_def, generics) => {
let struct_def = folder.fold_variant_data(struct_def);
ItemUnion(struct_def, folder.fold_generics(generics))
}
ItemDefaultImpl(unsafety, ref trait_ref) => {
ItemDefaultImpl(unsafety, folder.fold_trait_ref((*trait_ref).clone()))
}
Expand Down
3 changes: 2 additions & 1 deletion src/librustc/hir/intravisit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,8 @@ pub fn walk_item<'v, V: Visitor<'v>>(visitor: &mut V, item: &'v Item) {
visitor.visit_ty(typ);
walk_list!(visitor, visit_impl_item, impl_items);
}
ItemStruct(ref struct_definition, ref generics) => {
ItemStruct(ref struct_definition, ref generics) |
ItemUnion(ref struct_definition, ref generics) => {
visitor.visit_generics(generics);
visitor.visit_id(item.id);
visitor.visit_variant_data(struct_definition, item.name, generics, item.id, item.span);
Expand Down
5 changes: 4 additions & 1 deletion src/librustc/hir/lowering.rs
Original file line number Diff line number Diff line change
Expand Up @@ -638,7 +638,10 @@ impl<'a> LoweringContext<'a> {
let struct_def = self.lower_variant_data(struct_def);
hir::ItemStruct(struct_def, self.lower_generics(generics))
}
ItemKind::Union(..) => panic!("`union` is not yet implemented"),
ItemKind::Union(ref vdata, ref generics) => {
let vdata = self.lower_variant_data(vdata);
hir::ItemUnion(vdata, self.lower_generics(generics))
}
ItemKind::DefaultImpl(unsafety, ref trait_ref) => {
hir::ItemDefaultImpl(self.lower_unsafety(unsafety),
self.lower_trait_ref(trait_ref))
Expand Down
9 changes: 5 additions & 4 deletions src/librustc/hir/map/def_collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -302,9 +302,9 @@ impl<'ast> intravisit::Visitor<'ast> for DefCollector<'ast> {
let def_data = match i.node {
hir::ItemDefaultImpl(..) | hir::ItemImpl(..) =>
DefPathData::Impl,
hir::ItemEnum(..) | hir::ItemStruct(..) | hir::ItemTrait(..) |
hir::ItemExternCrate(..) | hir::ItemMod(..) | hir::ItemForeignMod(..) |
hir::ItemTy(..) =>
hir::ItemEnum(..) | hir::ItemStruct(..) | hir::ItemUnion(..) |
hir::ItemTrait(..) | hir::ItemExternCrate(..) | hir::ItemMod(..) |
hir::ItemForeignMod(..) | hir::ItemTy(..) =>
DefPathData::TypeNs(i.name.as_str()),
hir::ItemStatic(..) | hir::ItemConst(..) | hir::ItemFn(..) =>
DefPathData::ValueNs(i.name.as_str()),
Expand All @@ -331,7 +331,8 @@ impl<'ast> intravisit::Visitor<'ast> for DefCollector<'ast> {
});
}
}
hir::ItemStruct(ref struct_def, _) => {
hir::ItemStruct(ref struct_def, _) |
hir::ItemUnion(ref struct_def, _) => {
// If this is a tuple-like struct, register the constructor.
if !struct_def.is_struct() {
this.create_def(struct_def.id(),
Expand Down
1 change: 1 addition & 0 deletions src/librustc/hir/map/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1030,6 +1030,7 @@ fn node_id_to_string(map: &Map, id: NodeId, include_id: bool) -> String {
ItemTy(..) => "ty",
ItemEnum(..) => "enum",
ItemStruct(..) => "struct",
ItemUnion(..) => "union",
ItemTrait(..) => "trait",
ItemImpl(..) => "impl",
ItemDefaultImpl(..) => "default impl",
Expand Down
3 changes: 3 additions & 0 deletions src/librustc/hir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1483,6 +1483,8 @@ pub enum Item_ {
ItemEnum(EnumDef, Generics),
/// A struct definition, e.g. `struct Foo<A> {x: A}`
ItemStruct(VariantData, Generics),
/// A union definition, e.g. `union Foo<A, B> {x: A, y: B}`
ItemUnion(VariantData, Generics),
/// Represents a Trait Declaration
ItemTrait(Unsafety, Generics, TyParamBounds, HirVec<TraitItem>),

Expand Down Expand Up @@ -1512,6 +1514,7 @@ impl Item_ {
ItemTy(..) => "type alias",
ItemEnum(..) => "enum",
ItemStruct(..) => "struct",
ItemUnion(..) => "union",
ItemTrait(..) => "trait",
ItemImpl(..) |
ItemDefaultImpl(..) => "item",
Expand Down
5 changes: 4 additions & 1 deletion src/librustc/hir/print.rs
Original file line number Diff line number Diff line change
Expand Up @@ -752,7 +752,10 @@ impl<'a> State<'a> {
self.head(&visibility_qualified(&item.vis, "struct"))?;
self.print_struct(struct_def, generics, item.name, item.span, true)?;
}

hir::ItemUnion(ref struct_def, ref generics) => {
self.head(&visibility_qualified(&item.vis, "union"))?;
self.print_struct(struct_def, generics, item.name, item.span, true)?;
}
hir::ItemDefaultImpl(unsafety, ref trait_ref) => {
self.head("")?;
self.print_visibility(&item.vis)?;
Expand Down
4 changes: 3 additions & 1 deletion src/librustc/infer/error_reporting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
match item.node {
hir::ItemImpl(..) => "impl",
hir::ItemStruct(..) => "struct",
hir::ItemUnion(..) => "union",
hir::ItemEnum(..) => "enum",
hir::ItemTrait(..) => "trait",
hir::ItemFn(..) => "function body",
Expand Down Expand Up @@ -1370,7 +1371,8 @@ impl<'a, 'gcx, 'tcx> Rebuilder<'a, 'gcx, 'tcx> {
}
hir::TyPath(ref maybe_qself, ref path) => {
match self.tcx.expect_def(cur_ty.id) {
Def::Enum(did) | Def::TyAlias(did) | Def::Struct(did) => {
Def::Enum(did) | Def::TyAlias(did) |
Def::Struct(did) | Def::Union(did) => {
let generics = self.tcx.lookup_generics(did);

let expected =
Expand Down
1 change: 1 addition & 0 deletions src/librustc/infer/freshen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@ impl<'a, 'gcx, 'tcx> TypeFolder<'gcx, 'tcx> for TypeFreshener<'a, 'gcx, 'tcx> {
ty::TyFnPtr(_) |
ty::TyTrait(..) |
ty::TyStruct(..) |
ty::TyUnion(..) |
ty::TyClosure(..) |
ty::TyNever |
ty::TyTuple(..) |
Expand Down
20 changes: 11 additions & 9 deletions src/librustc/middle/dead.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ impl<'a, 'tcx> MarkSymbolVisitor<'a, 'tcx> {
}

fn lookup_and_handle_definition(&mut self, id: ast::NodeId) {
use ty::TypeVariants::{TyEnum, TyStruct};
use ty::TypeVariants::{TyEnum, TyStruct, TyUnion};

let def = self.tcx.expect_def(id);

Expand All @@ -96,7 +96,7 @@ impl<'a, 'tcx> MarkSymbolVisitor<'a, 'tcx> {
if self.tcx.trait_of_item(def.def_id()).is_some() => {
if let Some(substs) = self.tcx.tables.borrow().item_substs.get(&id) {
match substs.substs.type_at(0).sty {
TyEnum(tyid, _) | TyStruct(tyid, _) => {
TyEnum(tyid, _) | TyStruct(tyid, _) | TyUnion(tyid, _) => {
self.check_def_id(tyid.did)
}
_ => {}
Expand Down Expand Up @@ -132,10 +132,11 @@ impl<'a, 'tcx> MarkSymbolVisitor<'a, 'tcx> {
}

fn handle_field_access(&mut self, lhs: &hir::Expr, name: ast::Name) {
if let ty::TyStruct(def, _) = self.tcx.expr_ty_adjusted(lhs).sty {
self.insert_def_id(def.struct_variant().field_named(name).did);
} else {
span_bug!(lhs.span, "named field access on non-struct")
match self.tcx.expr_ty_adjusted(lhs).sty {
ty::TyStruct(def, _) | ty::TyUnion(def, _) => {
self.insert_def_id(def.struct_variant().field_named(name).did);
}
_ => span_bug!(lhs.span, "named field access on non-struct/union"),
}
}

Expand All @@ -148,7 +149,7 @@ impl<'a, 'tcx> MarkSymbolVisitor<'a, 'tcx> {
fn handle_field_pattern_match(&mut self, lhs: &hir::Pat,
pats: &[codemap::Spanned<hir::FieldPat>]) {
let variant = match self.tcx.node_id_to_type(lhs.id).sty {
ty::TyStruct(adt, _) | ty::TyEnum(adt, _) => {
ty::TyStruct(adt, _) | ty::TyUnion(adt, _) | ty::TyEnum(adt, _) => {
adt.variant_of_def(self.tcx.expect_def(lhs.id))
}
_ => span_bug!(lhs.span, "non-ADT in struct pattern")
Expand Down Expand Up @@ -185,7 +186,7 @@ impl<'a, 'tcx> MarkSymbolVisitor<'a, 'tcx> {
match *node {
ast_map::NodeItem(item) => {
match item.node {
hir::ItemStruct(..) => {
hir::ItemStruct(..) | hir::ItemUnion(..) => {
self.struct_has_extern_repr = item.attrs.iter().any(|attr| {
attr::find_repr_attrs(self.tcx.sess.diagnostic(), attr)
.contains(&attr::ReprExtern)
Expand Down Expand Up @@ -423,7 +424,8 @@ impl<'a, 'tcx> DeadVisitor<'a, 'tcx> {
| hir::ItemConst(..)
| hir::ItemFn(..)
| hir::ItemEnum(..)
| hir::ItemStruct(..) => true,
| hir::ItemStruct(..)
| hir::ItemUnion(..) => true,
_ => false
};
let ctor_id = get_struct_ctor_id(item);
Expand Down
24 changes: 20 additions & 4 deletions src/librustc/middle/effect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,14 @@
use self::RootUnsafeContext::*;

use dep_graph::DepNode;
use hir::def::Def;
use ty::{self, Ty, TyCtxt};
use ty::MethodCall;

use syntax::ast;
use syntax_pos::Span;
use hir;
use hir::intravisit;
use hir::intravisit::{FnKind, Visitor};
use hir::{self, PatKind};
use hir::def::Def;
use hir::intravisit::{self, FnKind, Visitor};

#[derive(Copy, Clone)]
struct UnsafeContext {
Expand Down Expand Up @@ -178,11 +177,28 @@ impl<'a, 'tcx, 'v> Visitor<'v> for EffectCheckVisitor<'a, 'tcx> {
self.require_unsafe(expr.span, "use of mutable static");
}
}
hir::ExprField(ref base_expr, field) => {
if let ty::TyUnion(..) = self.tcx.expr_ty_adjusted(base_expr).sty {
self.require_unsafe(field.span, "access to union field");
}
}
_ => {}
}

intravisit::walk_expr(self, expr);
}

fn visit_pat(&mut self, pat: &hir::Pat) {
if let PatKind::Struct(_, ref fields, _) = pat.node {
if let ty::TyUnion(..) = self.tcx.pat_ty(pat).sty {
for field in fields {
self.require_unsafe(field.span, "matching on union field");
}
}
}

intravisit::walk_pat(self, pat);
}
}

pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) {
Expand Down
8 changes: 4 additions & 4 deletions src/librustc/middle/expr_use_visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,7 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> {
}

hir::ExprStruct(_, ref fields, ref opt_with) => {
self.walk_struct_expr(expr, fields, opt_with);
self.walk_struct_expr(fields, opt_with);
}

hir::ExprTup(ref exprs) => {
Expand Down Expand Up @@ -655,7 +655,6 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> {
}

fn walk_struct_expr(&mut self,
_expr: &hir::Expr,
fields: &[hir::Field],
opt_with: &Option<P<hir::Expr>>) {
// Consume the expressions supplying values for each field.
Expand Down Expand Up @@ -695,7 +694,7 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> {
with_expr.span,
"with expression doesn't evaluate to a struct");
}
};
}

// walk the with expression so that complex expressions
// are properly handled.
Expand Down Expand Up @@ -1012,7 +1011,8 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> {
debug!("variant downcast_cmt={:?} pat={:?}", downcast_cmt, pat);
delegate.matched_pat(pat, downcast_cmt, match_mode);
}
Some(Def::Struct(..)) | Some(Def::TyAlias(..)) | Some(Def::AssociatedTy(..)) => {
Some(Def::Struct(..)) | Some(Def::Union(..)) |
Some(Def::TyAlias(..)) | Some(Def::AssociatedTy(..)) => {
debug!("struct cmt_pat={:?} pat={:?}", cmt_pat, pat);
delegate.matched_pat(pat, cmt_pat, match_mode);
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/middle/mem_categorization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -572,7 +572,7 @@ impl<'a, 'gcx, 'tcx> MemCategorizationContext<'a, 'gcx, 'tcx> {
id, expr_ty, def);

match def {
Def::Struct(..) | Def::Variant(..) | Def::Const(..) |
Def::Struct(..) | Def::Union(..) | Def::Variant(..) | Def::Const(..) |
Def::AssociatedConst(..) | Def::Fn(..) | Def::Method(..) => {
Ok(self.cat_rvalue_node(id, span, expr_ty))
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/middle/reachable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ impl<'a, 'tcx> ReachableContext<'a, 'tcx> {
hir::ItemMod(..) | hir::ItemForeignMod(..) |
hir::ItemImpl(..) | hir::ItemTrait(..) |
hir::ItemStruct(..) | hir::ItemEnum(..) |
hir::ItemDefaultImpl(..) => {}
hir::ItemUnion(..) | hir::ItemDefaultImpl(..) => {}
}
}
ast_map::NodeTraitItem(trait_method) => {
Expand Down
1 change: 1 addition & 0 deletions src/librustc/middle/resolve_lifetime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ impl<'a, 'tcx, 'v> Visitor<'v> for LifetimeContext<'a, 'tcx> {
hir::ItemTy(_, ref generics) |
hir::ItemEnum(_, ref generics) |
hir::ItemStruct(_, ref generics) |
hir::ItemUnion(_, ref generics) |
hir::ItemTrait(_, ref generics, _, _) |
hir::ItemImpl(_, _, ref generics, _, _, _) => {
// These kinds of items have only early bound lifetime parameters.
Expand Down
Loading

0 comments on commit d748fa6

Please sign in to comment.