Skip to content

Commit

Permalink
ParamSpec::new_parts
Browse files Browse the repository at this point in the history 
Summary: Safer constructor which takes different parameter modes as separate arguments.

Reviewed By: JakobDegen

Differential Revision: D63012408

fbshipit-source-id: b9431d3511f0d71be68744498b50066f09b28fac
  • Loading branch information
@stepancheg @facebook-github-bot
stepancheg authored and facebook-github-bot committed Sep 20, 2024
1 parent 723a272 commit fbc94d9
Show file tree
Hide file tree
Showing 3 changed files with 80 additions and 36 deletions.
4 changes: 2 additions & 2 deletions starlark/src/eval/bc/instr_impl.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1421,8 +1421,8 @@ impl InstrNoFlowImpl for InstrDefImpl {
}

match &x.node {
ParameterCompiled::Normal(n, _, _, None) => parameters.required(&n.name),
ParameterCompiled::Normal(n, _, ty, Some(v)) => {
ParameterCompiled::Normal(n, _, None) => parameters.required(&n.name),
ParameterCompiled::Normal(n, ty, Some(v)) => {
assert!(*v == pop_index);
let value = pop[pop_index as usize];
pop_index += 1;
Expand Down
78 changes: 44 additions & 34 deletions starlark/src/eval/compiler/def.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ use starlark_syntax::syntax::def::DefParam;
use starlark_syntax::syntax::def::DefParamIndices;
use starlark_syntax::syntax::def::DefParamKind;
use starlark_syntax::syntax::def::DefParams;
use starlark_syntax::syntax::def::DefRegularParamMode;

use crate as starlark;
use crate::any::ProvidesStaticType;
Expand Down Expand Up @@ -83,11 +82,12 @@ use crate::eval::runtime::slots::LocalSlotId;
use crate::eval::runtime::slots::LocalSlotIdCapturedOrNot;
use crate::eval::Arguments;
use crate::starlark_complex_values;
use crate::typing::Param;
use crate::typing::callable_param::ParamIsRequired;
use crate::typing::ParamSpec;
use crate::typing::Ty;
use crate::values::frozen_ref::AtomicFrozenRefOption;
use crate::values::function::FUNCTION_TYPE;
use crate::values::layout::heap::profile::arc_str::ArcStr;
use crate::values::typing::type_compiled::compiled::TypeCompiled;
use crate::values::Freeze;
use crate::values::Freezer;
Expand Down Expand Up @@ -152,8 +152,6 @@ pub(crate) enum ParameterCompiled<T> {
Normal(
/// Name.
ParameterName,
/// Pos, named or both.
DefRegularParamMode,
/// Type.
Option<TypeCompiled<FrozenValue>>,
/// Default value.
Expand All @@ -166,8 +164,8 @@ pub(crate) enum ParameterCompiled<T> {
impl<T> ParameterCompiled<T> {
pub(crate) fn map_expr<U>(&self, f: impl FnMut(&T) -> U) -> ParameterCompiled<U> {
match self {
ParameterCompiled::Normal(n, m, o, t) => {
ParameterCompiled::Normal(n.clone(), *m, *o, t.as_ref().map(f))
ParameterCompiled::Normal(n, o, t) => {
ParameterCompiled::Normal(n.clone(), *o, t.as_ref().map(f))
}
ParameterCompiled::Args(n, o) => ParameterCompiled::Args(n.clone(), *o),
ParameterCompiled::KwArgs(n, o) => ParameterCompiled::KwArgs(n.clone(), *o),
Expand All @@ -187,7 +185,7 @@ impl<T> ParameterCompiled<T> {

pub(crate) fn name_ty(&self) -> (&ParameterName, Option<TypeCompiled<FrozenValue>>) {
match self {
Self::Normal(n, _, t, _) => (n, *t),
Self::Normal(n, t, _) => (n, *t),
Self::Args(n, t) => (n, *t),
Self::KwArgs(n, t) => (n, *t),
}
Expand All @@ -207,6 +205,15 @@ impl<T> ParameterCompiled<T> {
}
}

pub(crate) fn required(&self) -> ParamIsRequired {
match self {
ParameterCompiled::Normal(_, _, None) => ParamIsRequired::Yes,
ParameterCompiled::Normal(_, _, Some(_)) => ParamIsRequired::No,
ParameterCompiled::Args(..) => ParamIsRequired::No,
ParameterCompiled::KwArgs(..) => ParamIsRequired::No,
}
}

pub(crate) fn is_star_or_star_star(&self) -> bool {
matches!(
self,
Expand Down Expand Up @@ -266,31 +273,35 @@ impl<T> ParametersCompiled<T> {
}

pub(crate) fn to_ty_params(&self) -> ParamSpec {
ParamSpec::new(
self.params
.iter()
.map(|p| {
let ty = p.ty();
match &p.node {
ParameterCompiled::Normal(name, m, _ty, None) => match m {
DefRegularParamMode::PosOnly => Param::pos_only(ty),
DefRegularParamMode::PosOrName => Param::pos_or_name(&name.name, ty),
DefRegularParamMode::NameOnly => Param::name_only(&name.name, ty),
},
ParameterCompiled::Normal(name, m, _ty, Some(_)) => match m {
DefRegularParamMode::PosOnly => Param::pos_only(ty).optional(),
DefRegularParamMode::PosOrName => {
Param::pos_or_name(&name.name, ty).optional()
}
DefRegularParamMode::NameOnly => {
Param::name_only(&name.name, ty).optional()
}
},
ParameterCompiled::Args(..) => Param::args(ty),
ParameterCompiled::KwArgs(..) => Param::kwargs(ty),
}
})
.collect(),
ParamSpec::new_parts(
self.indices.pos_only().map(|i| {
let p = &self.params[i as usize].node;
(p.required(), p.ty())
}),
self.indices.pos_or_named().map(|i| {
let p = &self.params[i as usize].node;
(
ArcStr::from(p.name_ty().0.name.as_str()),
p.required(),
p.ty(),
)
}),
self.indices.args.map(|i| {
let p = &self.params[i as usize].node;
p.ty()
}),
self.indices.named_only(self.params.len() as u32).map(|i| {
let p = &self.params[i as usize].node;
(
ArcStr::from(p.name_ty().0.name.as_str()),
p.required(),
p.ty(),
)
}),
self.indices.kwargs.map(|i| {
let p = &self.params[i as usize].node;
p.ty()
}),
)
// TODO(nga): do not unwrap.
.unwrap()
Expand Down Expand Up @@ -415,9 +426,8 @@ impl Compiler<'_, '_, '_, '_> {
Ok(IrSpanned {
span,
node: match &x.node.kind {
DefParamKind::Regular(mode, default_value) => ParameterCompiled::Normal(
DefParamKind::Regular(_mode, default_value) => ParameterCompiled::Normal(
parameter_name,
*mode,
self.expr_for_type(x.ty).map(|t| t.node),
default_value.as_ref().map(|d| self.expr(d)).transpose()?,
),
Expand Down
34 changes: 34 additions & 0 deletions starlark/src/typing/callable_param.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
use std::fmt;
use std::fmt::Display;
use std::fmt::Formatter;
use std::iter;

use allocative::Allocative;
use dupe::Dupe;
Expand Down Expand Up @@ -321,6 +322,39 @@ impl ParamSpec {
}
}

pub(crate) fn new_parts(
pos_only: impl IntoIterator<Item = (ParamIsRequired, Ty)>,
pos_or_name: impl IntoIterator<Item = (ArcStr, ParamIsRequired, Ty)>,
args: Option<Ty>,
named_only: impl IntoIterator<Item = (ArcStr, ParamIsRequired, Ty)>,
kwargs: Option<Ty>,
) -> crate::Result<ParamSpec> {
Self::new(
iter::empty()
.chain(pos_only.into_iter().map(|(req, ty)| Param {
mode: ParamMode::PosOnly(req),
ty,
}))
.chain(pos_or_name.into_iter().map(|(name, req, ty)| Param {
mode: ParamMode::PosOrName(name, req),
ty,
}))
.chain(args.map(|ty| Param {
mode: ParamMode::Args,
ty,
}))
.chain(named_only.into_iter().map(|(name, req, ty)| Param {
mode: ParamMode::NameOnly(name, req),
ty,
}))
.chain(kwargs.map(|ty| Param {
mode: ParamMode::Kwargs,
ty,
}))
.collect(),
)
}

/// `*args`.
pub(crate) fn args(ty: Ty) -> ParamSpec {
ParamSpec::new(vec![Param::args(ty)]).unwrap()
Expand Down

0 comments on commit fbc94d9

Please sign in to comment.