Auto merge of rust-lang#13269 - Veykril:repr, r=Veykril

Properly set the enum variant body type from the repr attribute

Follow up for rust-lang/rust-analyzer#12966, fixes some type inference problems

Author: bors

crates/hir-def/src/adt.rs

@@ -1,6 +1,6 @@
 //! Defines hir-level representation of structs, enums and unions
 
-use std::sync::Arc;
+use std::{num::NonZeroU32, sync::Arc};
 
 use base_db::CrateId;
 use either::Either;
@@ -14,6 +14,7 @@ use tt::{Delimiter, DelimiterKind, Leaf, Subtree, TokenTree};
 
 use crate::{
     body::{CfgExpander, LowerCtx},
+    builtin_type::{BuiltinInt, BuiltinUint},
     db::DefDatabase,
     intern::Interned,
     item_tree::{AttrOwner, Field, Fields, ItemTree, ModItem, RawVisibilityId},
@@ -31,14 +32,15 @@ use cfg::CfgOptions;
 pub struct StructData {
     pub name: Name,
     pub variant_data: Arc<VariantData>,
-    pub repr: Option<ReprKind>,
+    pub repr: Option<ReprData>,
     pub visibility: RawVisibility,
 }
 
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct EnumData {
     pub name: Name,
     pub variants: Arena<EnumVariantData>,
+    pub repr: Option<ReprData>,
     pub visibility: RawVisibility,
 }
 
@@ -63,32 +65,80 @@ pub struct FieldData {
     pub visibility: RawVisibility,
 }
 
-#[derive(Debug, Clone, PartialEq, Eq)]
+#[derive(Copy, Debug, Clone, PartialEq, Eq)]
 pub enum ReprKind {
-    Packed,
-    Other,
+    C,
+    BuiltinInt { builtin: Either<BuiltinInt, BuiltinUint>, is_c: bool },
+    Transparent,
+    Default,
+}
+
+#[derive(Copy, Debug, Clone, PartialEq, Eq)]
+pub struct ReprData {
+    pub kind: ReprKind,
+    pub packed: bool,
+    pub align: Option<NonZeroU32>,
 }
 
 fn repr_from_value(
     db: &dyn DefDatabase,
     krate: CrateId,
     item_tree: &ItemTree,
     of: AttrOwner,
-) -> Option<ReprKind> {
+) -> Option<ReprData> {
     item_tree.attrs(db, krate, of).by_key("repr").tt_values().find_map(parse_repr_tt)
 }
 
-fn parse_repr_tt(tt: &Subtree) -> Option<ReprKind> {
+fn parse_repr_tt(tt: &Subtree) -> Option<ReprData> {
     match tt.delimiter {
         Some(Delimiter { kind: DelimiterKind::Parenthesis, .. }) => {}
         _ => return None,
     }
 
-    let mut it = tt.token_trees.iter();
-    match it.next()? {
-        TokenTree::Leaf(Leaf::Ident(ident)) if ident.text == "packed" => Some(ReprKind::Packed),
-        _ => Some(ReprKind::Other),
+    let mut data = ReprData { kind: ReprKind::Default, packed: false, align: None };
+
+    let mut tts = tt.token_trees.iter().peekable();
+    while let Some(tt) = tts.next() {
+        if let TokenTree::Leaf(Leaf::Ident(ident)) = tt {
+            match &*ident.text {
+                "packed" => {
+                    data.packed = true;
+                    if let Some(TokenTree::Subtree(_)) = tts.peek() {
+                        tts.next();
+                    }
+                }
+                "align" => {
+                    if let Some(TokenTree::Subtree(tt)) = tts.peek() {
+                        tts.next();
+                        if let Some(TokenTree::Leaf(Leaf::Literal(lit))) = tt.token_trees.first() {
+                            if let Ok(align) = lit.text.parse() {
+                                data.align = Some(align);
+                            }
+                        }
+                    }
+                }
+                "C" => {
+                    if let ReprKind::BuiltinInt { is_c, .. } = &mut data.kind {
+                        *is_c = true;
+                    } else {
+                        data.kind = ReprKind::C;
+                    }
+                }
+                "transparent" => data.kind = ReprKind::Transparent,
+                repr => {
+                    let is_c = matches!(data.kind, ReprKind::C);
+                    if let Some(builtin) = BuiltinInt::from_suffix(repr)
+                        .map(Either::Left)
+                        .or_else(|| BuiltinUint::from_suffix(repr).map(Either::Right))
+                    {
+                        data.kind = ReprKind::BuiltinInt { builtin, is_c };
+                    }
+                }
+            }
+        }
     }
+
+    Some(data)
 }
 
 impl StructData {
@@ -108,6 +158,7 @@ impl StructData {
             visibility: item_tree[strukt.visibility].clone(),
         })
     }
+
     pub(crate) fn union_data_query(db: &dyn DefDatabase, id: UnionId) -> Arc<StructData> {
         let loc = id.lookup(db);
         let krate = loc.container.krate;
@@ -133,6 +184,7 @@ impl EnumData {
         let krate = loc.container.krate;
         let item_tree = loc.id.item_tree(db);
         let cfg_options = db.crate_graph()[krate].cfg_options.clone();
+        let repr = repr_from_value(db, krate, &item_tree, ModItem::from(loc.id.value).into());
 
         let enum_ = &item_tree[loc.id.value];
         let mut variants = Arena::new();
@@ -158,6 +210,7 @@ impl EnumData {
         Arc::new(EnumData {
             name: enum_.name.clone(),
             variants,
+            repr,
             visibility: item_tree[enum_.visibility].clone(),
         })
     }
@@ -166,6 +219,13 @@ impl EnumData {
         let (id, _) = self.variants.iter().find(|(_id, data)| &data.name == name)?;
         Some(id)
     }
+
+    pub fn variant_body_type(&self) -> Either<BuiltinInt, BuiltinUint> {
+        match self.repr {
+            Some(ReprData { kind: ReprKind::BuiltinInt { builtin, .. }, .. }) => builtin,
+            _ => Either::Left(BuiltinInt::Isize),
+        }
+    }
 }
 
 impl HasChildSource<LocalEnumVariantId> for EnumId {

crates/hir-def/src/lib.rs

@@ -479,7 +479,6 @@ pub enum DefWithBodyId {
 
 impl_from!(FunctionId, ConstId, StaticId for DefWithBodyId);
 
-// FIXME: Rename EnumVariantId to VariantId so that the macro above can be used
 impl From<EnumVariantId> for DefWithBodyId {
     fn from(id: EnumVariantId) -> Self {
         DefWithBodyId::VariantId(id)

crates/hir-ty/src/consteval.rs

@@ -153,7 +153,7 @@ fn is_valid(scalar: &Scalar, value: i128) -> bool {
     }
 }
 
-fn get_name(variant: EnumVariantId, ctx: &mut ConstEvalCtx<'_>) -> String {
+fn get_name(ctx: &mut ConstEvalCtx<'_>, variant: EnumVariantId) -> String {
     let loc = variant.parent.lookup(ctx.db.upcast());
     let children = variant.parent.child_source(ctx.db.upcast());
     let item_tree = loc.id.item_tree(ctx.db.upcast());
@@ -167,20 +167,24 @@ pub fn eval_const(
     expr_id: ExprId,
     ctx: &mut ConstEvalCtx<'_>,
 ) -> Result<ComputedExpr, ConstEvalError> {
+    let u128_to_i128 = |it: u128| -> Result<i128, ConstEvalError> {
+        it.try_into().map_err(|_| ConstEvalError::NotSupported("u128 is too big"))
+    };
+
     let expr = &ctx.exprs[expr_id];
     match expr {
         Expr::Missing => match ctx.owner {
+            // evaluate the implicit variant index of an enum variant without expression
+            // FIXME: This should return the type of the enum representation
             DefWithBodyId::VariantId(variant) => {
                 let prev_idx: u32 = variant.local_id.into_raw().into();
-                let prev_idx = prev_idx.checked_sub(1).map(|idx| Idx::from_raw(RawIdx::from(idx)));
+                let prev_idx = prev_idx.checked_sub(1).map(RawIdx::from).map(Idx::from_raw);
                 let value = match prev_idx {
-                    Some(prev) => {
-                        let prev_variant = EnumVariantId { local_id: prev, ..variant };
+                    Some(local_id) => {
+                        let prev_variant = EnumVariantId { local_id, parent: variant.parent };
                         1 + match ctx.db.const_eval_variant(prev_variant)? {
                             ComputedExpr::Literal(Literal::Int(v, _)) => v,
-                            ComputedExpr::Literal(Literal::Uint(v, _)) => v
-                                .try_into()
-                                .map_err(|_| ConstEvalError::NotSupported("too big u128"))?,
+                            ComputedExpr::Literal(Literal::Uint(v, _)) => u128_to_i128(v)?,
                             _ => {
                                 return Err(ConstEvalError::NotSupported(
                                     "Enum can't contain this kind of value",
@@ -206,9 +210,7 @@ pub fn eval_const(
                             return Ok(ComputedExpr::Literal(Literal::Bool(!b)))
                         }
                         ComputedExpr::Literal(Literal::Int(v, _)) => v,
-                        ComputedExpr::Literal(Literal::Uint(v, _)) => v
-                            .try_into()
-                            .map_err(|_| ConstEvalError::NotSupported("too big u128"))?,
+                        ComputedExpr::Literal(Literal::Uint(v, _)) => u128_to_i128(v)?,
                         _ => return Err(ConstEvalError::NotSupported("this kind of operator")),
                     };
                     let r = match ty.kind(Interner) {
@@ -237,9 +239,7 @@ pub fn eval_const(
                 hir_def::expr::UnaryOp::Neg => {
                     let v = match ev {
                         ComputedExpr::Literal(Literal::Int(v, _)) => v,
-                        ComputedExpr::Literal(Literal::Uint(v, _)) => v
-                            .try_into()
-                            .map_err(|_| ConstEvalError::NotSupported("too big u128"))?,
+                        ComputedExpr::Literal(Literal::Uint(v, _)) => u128_to_i128(v)?,
                         _ => return Err(ConstEvalError::NotSupported("this kind of operator")),
                     };
                     Ok(ComputedExpr::Literal(Literal::Int(
@@ -258,16 +258,12 @@ pub fn eval_const(
             let op = op.ok_or(ConstEvalError::IncompleteExpr)?;
             let v1 = match lhs {
                 ComputedExpr::Literal(Literal::Int(v, _)) => v,
-                ComputedExpr::Literal(Literal::Uint(v, _)) => {
-                    v.try_into().map_err(|_| ConstEvalError::NotSupported("too big u128"))?
-                }
+                ComputedExpr::Literal(Literal::Uint(v, _)) => u128_to_i128(v)?,
                 _ => return Err(ConstEvalError::NotSupported("this kind of operator")),
             };
             let v2 = match rhs {
                 ComputedExpr::Literal(Literal::Int(v, _)) => v,
-                ComputedExpr::Literal(Literal::Uint(v, _)) => {
-                    v.try_into().map_err(|_| ConstEvalError::NotSupported("too big u128"))?
-                }
+                ComputedExpr::Literal(Literal::Uint(v, _)) => u128_to_i128(v)?,
                 _ => return Err(ConstEvalError::NotSupported("this kind of operator")),
             };
             match op {
@@ -380,7 +376,7 @@ pub fn eval_const(
                 }
                 ValueNs::EnumVariantId(id) => match ctx.db.const_eval_variant(id)? {
                     ComputedExpr::Literal(lit) => {
-                        Ok(ComputedExpr::Enum(get_name(id, ctx), id, lit))
+                        Ok(ComputedExpr::Enum(get_name(ctx, id), id, lit))
                     }
                     _ => Err(ConstEvalError::NotSupported(
                         "Enums can't evalute to anything but numbers",
@@ -389,6 +385,7 @@ pub fn eval_const(
                 _ => Err(ConstEvalError::NotSupported("path that are not const or local")),
             }
         }
+        // FIXME: Handle the cast target
         &Expr::Cast { expr, .. } => match eval_const(expr, ctx)? {
             ComputedExpr::Enum(_, _, lit) => Ok(ComputedExpr::Literal(lit)),
             _ => Err(ConstEvalError::NotSupported("Can't cast these types")),
@@ -463,15 +460,15 @@ pub(crate) fn const_eval_recover(
     Err(ConstEvalError::Loop)
 }
 
-pub(crate) fn const_eval_recover_variant(
+pub(crate) fn const_eval_variant_recover(
     _: &dyn HirDatabase,
     _: &[String],
     _: &EnumVariantId,
 ) -> Result<ComputedExpr, ConstEvalError> {
     Err(ConstEvalError::Loop)
 }
 
-pub(crate) fn const_eval_query(
+pub(crate) fn const_eval_variant_query(
     db: &dyn HirDatabase,
     const_id: ConstId,
 ) -> Result<ComputedExpr, ConstEvalError> {

crates/hir-ty/src/db.rs

@@ -7,8 +7,7 @@ use arrayvec::ArrayVec;
 use base_db::{impl_intern_key, salsa, CrateId, Upcast};
 use hir_def::{
     db::DefDatabase, expr::ExprId, BlockId, ConstId, ConstParamId, DefWithBodyId, EnumVariantId,
-    FunctionId, GenericDefId, ImplId, LifetimeParamId, LocalFieldId, Lookup, TypeOrConstParamId,
-    VariantId,
+    FunctionId, GenericDefId, ImplId, LifetimeParamId, LocalFieldId, TypeOrConstParamId, VariantId,
 };
 use la_arena::ArenaMap;
 
@@ -44,12 +43,12 @@ pub trait HirDatabase: DefDatabase + Upcast<dyn DefDatabase> {
     #[salsa::invoke(crate::lower::const_param_ty_query)]
     fn const_param_ty(&self, def: ConstParamId) -> Ty;
 
-    #[salsa::invoke(crate::consteval::const_eval_query)]
+    #[salsa::invoke(crate::consteval::const_eval_variant_query)]
     #[salsa::cycle(crate::consteval::const_eval_recover)]
     fn const_eval(&self, def: ConstId) -> Result<ComputedExpr, ConstEvalError>;
 
     #[salsa::invoke(crate::consteval::const_eval_query_variant)]
-    #[salsa::cycle(crate::consteval::const_eval_recover_variant)]
+    #[salsa::cycle(crate::consteval::const_eval_variant_recover)]
     fn const_eval_variant(&self, def: EnumVariantId) -> Result<ComputedExpr, ConstEvalError>;
 
     #[salsa::invoke(crate::lower::impl_trait_query)]
@@ -194,11 +193,7 @@ fn infer_wait(db: &dyn HirDatabase, def: DefWithBodyId) -> Arc<InferenceResult>
             db.const_data(it).name.clone().unwrap_or_else(Name::missing).to_string()
         }
         DefWithBodyId::VariantId(it) => {
-            let up_db: &dyn DefDatabase = db.upcast();
-            let loc = it.parent.lookup(up_db);
-            let item_tree = loc.id.item_tree(up_db);
-            let konst = &item_tree[loc.id.value];
-            konst.name.to_string()
+            db.enum_data(it.parent).variants[it.local_id].name.to_string()
         }
     });
     db.infer_query(def)

crates/hir-ty/src/infer.rs

@@ -19,6 +19,7 @@ use std::sync::Arc;
 use chalk_ir::{cast::Cast, ConstValue, DebruijnIndex, Mutability, Safety, Scalar, TypeFlags};
 use hir_def::{
     body::Body,
+    builtin_type::BuiltinType,
     data::{ConstData, StaticData},
     expr::{BindingAnnotation, ExprId, PatId},
     lang_item::LangItemTarget,
@@ -68,8 +69,10 @@ pub(crate) fn infer_query(db: &dyn HirDatabase, def: DefWithBodyId) -> Arc<Infer
         DefWithBodyId::FunctionId(f) => ctx.collect_fn(f),
         DefWithBodyId::StaticId(s) => ctx.collect_static(&db.static_data(s)),
         DefWithBodyId::VariantId(v) => {
-            // FIXME: This should return the `repr(...)` type of the enum
-            ctx.return_ty = TyBuilder::def_ty(db, v.parent.into()).fill_with_unknown().build()
+            ctx.return_ty = TyBuilder::builtin(match db.enum_data(v.parent).variant_body_type() {
+                Either::Left(builtin) => BuiltinType::Int(builtin),
+                Either::Right(builtin) => BuiltinType::Uint(builtin),
+            });
         }
     }
 

crates/hir-ty/src/tests.rs

@@ -461,6 +461,18 @@ fn visit_module(
                     let body = db.body(def);
                     visit_body(db, &body, cb);
                 }
+                ModuleDefId::AdtId(hir_def::AdtId::EnumId(it)) => {
+                    db.enum_data(it)
+                        .variants
+                        .iter()
+                        .map(|(id, _)| hir_def::EnumVariantId { parent: it, local_id: id })
+                        .for_each(|it| {
+                            let def = it.into();
+                            cb(def);
+                            let body = db.body(def);
+                            visit_body(db, &body, cb);
+                        });
+                }
                 ModuleDefId::TraitId(it) => {
                     let trait_data = db.trait_data(it);
                     for &(_, item) in trait_data.items.iter() {