Skip to content

Commit

Permalink
KCL: Unlabeled first param defaults to %
Browse files Browse the repository at this point in the history
Part of #4600

KCL functions can declare one special argument that doesn't require a label on its parameter when called.

This PR will default that arg to % (the current pipeline) if not given.
  • Loading branch information
adamchalmers committed Dec 16, 2024
1 parent 9de16a6 commit 692088e
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 33 deletions.
8 changes: 7 additions & 1 deletion src/wasm-lib/kcl/src/execution/exec_ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,7 @@ impl Node<CallExpressionKw> {
},
self.into(),
ctx.clone(),
exec_state.pipe_value.clone().map(Arg::synthetic),
);
match ctx.stdlib.get_either(fn_name) {
FunctionKind::Core(func) => {
Expand Down Expand Up @@ -450,7 +451,12 @@ impl Node<CallExpression> {
match ctx.stdlib.get_either(fn_name) {
FunctionKind::Core(func) => {
// Attempt to call the function.
let args = crate::std::Args::new(fn_args, self.into(), ctx.clone());
let args = crate::std::Args::new(
fn_args,
self.into(),
ctx.clone(),
exec_state.pipe_value.clone().map(Arg::synthetic),
);
let mut result = func.std_lib_fn()(exec_state, args).await?;
update_memory_for_tags_of_geometry(&mut result, exec_state)?;
Ok(result)
Expand Down
29 changes: 0 additions & 29 deletions src/wasm-lib/kcl/src/parsing/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -351,22 +351,6 @@ fn pipe_expression(i: &mut TokenSlice) -> PResult<Node<PipeExpression>> {
))
.parse_next(i)?;

// All child parsers have been run.
// First, ensure they all have a % in their args.
let calls_without_substitution = tail.iter().find_map(|(_nc, call_expr, _nc2)| {
if !call_expr.has_substitution_arg() {
Some(call_expr.into())
} else {
None
}
});
if let Some(source_range) = calls_without_substitution {
let err = CompilationError::fatal(
source_range,
"All expressions in a pipeline must use the % (substitution operator)",
);
return Err(ErrMode::Cut(err.into()));
}
// Time to structure the return value.
let mut code_count = 0;
let mut max_noncode_end = 0;
Expand Down Expand Up @@ -4039,19 +4023,6 @@ let myBox = box([0,0], -3, -16, -10)
crate::parsing::top_level_parse(some_program_string).unwrap();
}

#[test]
fn must_use_percent_in_pipeline_fn() {
let some_program_string = r#"
foo()
|> bar(2)
"#;
assert_err(
some_program_string,
"All expressions in a pipeline must use the % (substitution operator)",
[30, 36],
);
}

#[test]
fn arg_labels() {
let input = r#"length: 3"#;
Expand Down
12 changes: 10 additions & 2 deletions src/wasm-lib/kcl/src/std/args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,28 +61,34 @@ impl KwArgs {
pub struct Args {
/// Positional args.
pub args: Vec<Arg>,
/// Keyword arguments
pub kw_args: KwArgs,
pub source_range: SourceRange,
pub ctx: ExecutorContext,
/// If this call happens inside a pipe (|>) expression, this holds the LHS of that |>.
/// Otherwise it's None.
pipe_value: Option<Arg>,
}

impl Args {
pub fn new(args: Vec<Arg>, source_range: SourceRange, ctx: ExecutorContext) -> Self {
pub fn new(args: Vec<Arg>, source_range: SourceRange, ctx: ExecutorContext, pipe_value: Option<Arg>) -> Self {
Self {
args,
kw_args: Default::default(),
source_range,
ctx,
pipe_value,
}
}

/// Collect the given keyword arguments.
pub fn new_kw(kw_args: KwArgs, source_range: SourceRange, ctx: ExecutorContext) -> Self {
pub fn new_kw(kw_args: KwArgs, source_range: SourceRange, ctx: ExecutorContext, pipe_value: Option<Arg>) -> Self {
Self {
args: Default::default(),
kw_args,
source_range,
ctx,
pipe_value,
}
}

Expand All @@ -101,6 +107,7 @@ impl Args {
settings: Default::default(),
context_type: crate::execution::ContextType::Mock,
},
pipe_value: None,
})
}

Expand Down Expand Up @@ -138,6 +145,7 @@ impl Args {
.unlabeled
.as_ref()
.or(self.args.first())
.or(self.pipe_value.as_ref())
.ok_or(KclError::Semantic(KclErrorDetails {
source_ranges: vec![self.source_range],
message: format!("This function requires a value for the special unlabeled first parameter, '{label}'"),
Expand Down
2 changes: 1 addition & 1 deletion src/wasm-lib/kcl/src/std/loft.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ pub async fn loft(exec_state: &mut ExecState, args: Args) -> Result<KclValue, Kc
/// circleSketch1 = startSketchOn(offsetPlane('XY', 150))
/// |> circle({ center = [0, 100], radius = 20 }, %)
///
/// loft([squareSketch, circleSketch0, circleSketch1])
/// [squareSketch, circleSketch0, circleSketch1] |> loft()
/// ```
///
/// ```no_run
Expand Down

0 comments on commit 692088e

Please sign in to comment.