Skip to content

Commit

Permalink
Use DefParamIndices in ParamsSpec
Browse files Browse the repository at this point in the history 
Summary: This is less code. But also needed later to convert `ParamsSpec` to `ParametersSpec`.

Reviewed By: JakobDegen

Differential Revision: D62984431

fbshipit-source-id: 10228862e4a67ed134a966bed93836571d73c2ba
  • Loading branch information
@stepancheg @facebook-github-bot
stepancheg authored and facebook-github-bot committed Sep 19, 2024
1 parent dc365c4 commit 4ecf8d8
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 55 deletions.
100 changes: 46 additions & 54 deletions starlark/src/typing/callable_param.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@ use std::fmt::Formatter;
use allocative::Allocative;
use dupe::Dupe;
use starlark_map::small_set::SmallSet;
use starlark_syntax::internal_error;
use starlark_syntax::other_error;
use starlark_syntax::syntax::def::DefParamIndices;

use crate::eval::runtime::params::display::fmt_param_spec;
use crate::eval::runtime::params::display::ParamFmt;
Expand Down Expand Up @@ -156,50 +158,13 @@ impl Param {
#[derive(Debug, Eq, PartialEq, Clone, Dupe, Hash, PartialOrd, Ord, Allocative)]
pub struct ParamSpec {
params: SmallArcVec1OrStatic<Param>,
indices: DefParamIndices,
}

impl Display for ParamSpec {
fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
let num_pos_only = self
.params
.iter()
.take_while(|p| matches!(p.mode, ParamMode::PosOnly(_)))
.count();

let (pos_only, rem) = self.params.split_at(num_pos_only);

let num_pos_or_named = rem
.iter()
.take_while(|p| matches!(p.mode, ParamMode::PosOrName(..)))
.count();

let (pos_or_named, rem) = rem.split_at(num_pos_or_named);

let (args, rem) = match rem.split_first() {
Some((args, rem)) if matches!(args.mode, ParamMode::Args) => (Some(args), rem),
_ => (None, rem),
};

let num_named_only = rem
.iter()
.take_while(|p| matches!(p.mode, ParamMode::NameOnly(..)))
.count();

let (named_only, rem) = rem.split_at(num_named_only);

let kwargs = match rem {
[] => None,
[x] if matches!(x.mode, ParamMode::Kwargs) => Some(x),
_ => {
if cfg!(test) {
panic!("inconsistent parameter spec: {:?}", self);
}
write!(f, "<inconsistent parameter spec: {:?}>", self)?;
return Ok(());
}
};

fn pf(p: &Param) -> ParamFmt<'_, &'_ Ty, &'static str> {
let pf = |i: u32| -> ParamFmt<'_, &'_ Ty, &'static str> {
let p = &self.params()[i as usize];
ParamFmt {
name: match &p.mode {
ParamMode::PosOrName(name, _) | ParamMode::NameOnly(name, _) => name,
Expand All @@ -218,15 +183,15 @@ impl Display for ParamSpec {
ParamMode::Args | ParamMode::Kwargs => None,
},
}
}
};

fmt_param_spec(
f,
pos_only.iter().map(pf),
pos_or_named.iter().map(pf),
args.map(pf),
named_only.iter().map(pf),
kwargs.map(pf),
self.indices.pos_only().map(pf),
self.indices.pos_or_named().map(pf),
self.indices.args.map(pf),
self.indices.named_only(self.params().len() as u32).map(pf),
self.indices.kwargs.map(pf),
)
}
}
Expand Down Expand Up @@ -258,7 +223,12 @@ impl ParamSpec {

let mut seen_optional = false;

for param in &params {
let mut num_positional_only = 0;
let mut num_positional = 0;
let mut args = None;
let mut kwargs = None;

for (i, param) in params.iter().enumerate() {
if let Some(name) = param.name() {
if !names.insert(name) {
return Err(other_error!("duplicate parameter name: `{}`", name));
Expand Down Expand Up @@ -302,6 +272,10 @@ impl ParamSpec {
if state >= State::Args {
return Err(other_error!("*args must not come after *-args"));
}
if args.is_some() {
return Err(internal_error!("args must not be already set"));
}
args = Some(i.try_into().unwrap());
state = State::Args;
}
ParamMode::NameOnly(_, req) => {
Expand All @@ -318,23 +292,34 @@ impl ParamSpec {
if state >= State::Kwargs {
return Err(other_error!("**kwargs must be the last parameter"));
}
if kwargs.is_some() {
return Err(internal_error!("kwargs must not be already set"));
}
kwargs = Some(i.try_into().unwrap());
state = State::Kwargs;
}
}

if state <= State::PosOnly {
num_positional_only += 1;
}
if state <= State::PosOrName {
num_positional += 1;
}
}

Ok(ParamSpec {
params: SmallArcVec1OrStatic::clone_from_slice(&params),
indices: DefParamIndices {
num_positional_only,
num_positional,
args,
kwargs,
},
})
}
}

/// `*args`, `**kwargs` parameters.
fn any_params() -> &'static [Param] {
static ANY_PARAMS: [Param; 2] = [Param::args(Ty::any()), Param::kwargs(Ty::any())];
&ANY_PARAMS
}

/// `*args`.
pub(crate) fn args(ty: Ty) -> ParamSpec {
ParamSpec::new(vec![Param::args(ty)]).unwrap()
Expand Down Expand Up @@ -370,8 +355,15 @@ impl ParamSpec {
}

pub(crate) fn any() -> ParamSpec {
static ANY_PARAMS: [Param; 2] = [Param::args(Ty::any()), Param::kwargs(Ty::any())];
ParamSpec {
params: SmallArcVec1OrStatic::new_static(Self::any_params()),
params: SmallArcVec1OrStatic::new_static(&ANY_PARAMS),
indices: DefParamIndices {
num_positional: 0,
num_positional_only: 0,
args: Some(0),
kwargs: Some(1),
},
}
}

Expand Down
4 changes: 3 additions & 1 deletion starlark_syntax/src/syntax/def.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,9 @@ pub struct DefParam<'a, P: AstPayload> {
/// Parameters internally in starlark-rust are commonly represented as a flat list of parameters,
/// with markers `/` and `*` omitted.
/// This struct contains sizes and indices to split the list into parts.
#[derive(Copy, Clone, Dupe, Debug, Allocative)]
#[derive(
Copy, Clone, Dupe, Debug, Eq, PartialEq, Hash, Ord, PartialOrd, Allocative
)]
pub struct DefParamIndices {
/// Number of parameters which can be filled positionally.
/// That is, number of parameters before first `*`, `*args` or `**kwargs`.
Expand Down

0 comments on commit 4ecf8d8

Please sign in to comment.