Skip to content

Commit

Permalink
Assert all arguments consumed by ParameterSpec::parser callback
Browse files Browse the repository at this point in the history
Summary: Better alternative to D63165674: just check it unconditionally instead of checking at every call site. It is more expensive, but practically it is nothing, see the test plan.

Reviewed By: JakobDegen

Differential Revision: D63280260

fbshipit-source-id: 19197a228a4e7d1f18904bda6d4fd4dd88a96cd7
  • Loading branch information
stepancheg authored and facebook-github-bot committed Sep 24, 2024
1 parent db1c700 commit d454703
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 1 deletion.
5 changes: 5 additions & 0 deletions starlark/src/eval/runtime/params/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,11 @@ impl<'v, 'a> ParametersParser<'v, 'a> {
};
T::unpack_named_param(v, name)
}

#[inline]
pub(crate) fn is_eof(&self) -> bool {
self.0.len() == 0
}
}

#[cfg(test)]
Expand Down
10 changes: 9 additions & 1 deletion starlark/src/eval/runtime/params/spec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ use starlark_derive::Freeze;
use starlark_derive::Trace;
use starlark_map::small_map::SmallMap;
use starlark_map::Hashed;
use starlark_syntax::other_error;
use starlark_syntax::syntax::def::DefParamIndices;

use crate as starlark;
Expand Down Expand Up @@ -865,7 +866,14 @@ impl<'v> ParametersSpec<Value<'v>> {
|slots, eval| {
self.collect_inline(&args.0, slots, eval.heap())?;
let mut parser = ParametersParser::new(slots);
k(&mut parser, eval)
let r = k(&mut parser, eval)?;
if !parser.is_eof() {
return Err(other_error!(
"Parser for `{}` did not consume all arguments",
self.function_name
));
}
Ok(r)
},
)
}
Expand Down

0 comments on commit d454703

Please sign in to comment.