From 74e538dbd5cfd5e5304c686f5b3579c29d7216e2 Mon Sep 17 00:00:00 2001 From: zhiayang Date: Sat, 29 Jun 2019 20:49:10 +0800 Subject: [PATCH] fix the thing where optionals would steal varargs --- build/ultratiny.flx | 6 +-- source/codegen/call.cpp | 16 +++++- source/typecheck/function.cpp | 4 -- source/typecheck/polymorph/solver.cpp | 74 +++++++++++++++++++++++++-- 4 files changed, 86 insertions(+), 14 deletions(-) diff --git a/build/ultratiny.flx b/build/ultratiny.flx index 54626646..d0770099 100644 --- a/build/ultratiny.flx +++ b/build/ultratiny.flx @@ -7,7 +7,7 @@ import libc as _ -fn qux(x: str, y: int = 3, args: [str: ...]) +fn qux(x: str, y: int = 4, args: [str: ...]) { printf("x = %s, y = %d\n", x, y) for a in args @@ -24,8 +24,8 @@ fn foo(a: int, b: int, c: int, x: int = 9, y: int = 8, z: int = 7) @entry fn main() { - foo(x: 4, 1, 2, y: 5, z: 6, 3) - qux(y: 17, "hello, world!", "hi", "my", "name", "is", "bob", "ross") + foo(x: 4, 1, y: 2, 5, z: 6, 3, 2) + qux("hello, world!", 33, "hi", "my", "name", "is", "bob", "ross") } diff --git a/source/codegen/call.cpp b/source/codegen/call.cpp index eee2f6e9..b271c355 100644 --- a/source/codegen/call.cpp +++ b/source/codegen/call.cpp @@ -33,9 +33,11 @@ static std::vector _codegenAndArrangeFunctionCallArguments(cgn::Cod util::hash_map argExprs; + // this thing below operates similarly to the list solver in typecheck/polymorph/solver.cpp size_t last_arg = std::min(numNormalArgs, arguments.size()); size_t positionalCounter = 0; + size_t varArgStart = numNormalArgs; for(size_t i = 0; i < last_arg; i++) { const auto& arg = arguments[i]; @@ -52,6 +54,17 @@ static std::vector _codegenAndArrangeFunctionCallArguments(cgn::Cod } else { + // so, `positionalCounter` counts the paramters on the declaration-side. thus, once we encounter a default value, + // it must mean that the rest of the parameters will be optional as well. + //* ie. we've passed all the positional arguments already, leaving the optional ones, which means every argument from + //* here onwards (including this one) must be named. since this is *not* named, we just skip straight to the varargs if + //* it was present. + if(fvararg && defaultArgumentValues.find(positionalCounter) != defaultArgumentValues.end()) + { + varArgStart = i; + break; + } + argExprs[positionalCounter] = arg.value; positionalCounter++; } @@ -130,8 +143,7 @@ static std::vector _codegenAndArrangeFunctionCallArguments(cgn::Cod // check the variadic arguments. note that IRBuilder will handle actually wrapping the values up into a slice // and/or creating an empty slice and/or forwarding an existing slice. we just need to supply the values. - // TODO: i'm still a bit suspicious if we are counting things correctly here, wrt. varargs and optional args. - for(size_t i = numNormalArgs; i < arguments.size(); i++) + for(size_t i = varArgStart; i < arguments.size(); i++) { auto arg = arguments[i].value; fir::Type* infer = 0; diff --git a/source/typecheck/function.cpp b/source/typecheck/function.cpp index a517758e..1e2d5857 100644 --- a/source/typecheck/function.cpp +++ b/source/typecheck/function.cpp @@ -47,10 +47,6 @@ TCResult ast::FuncDefn::generateDeclaration(sst::TypecheckState* fs, fir::Type* p.name, p.type, p.defaultVal->type); } } - else if(p.type->isVariadicArrayType()) - { - p.defaultVal = util::pool(p.loc, std::vector())->typecheck(fs, p.type).expr(); - } ps.push_back(p); ptys.push_back(p.type); diff --git a/source/typecheck/polymorph/solver.cpp b/source/typecheck/polymorph/solver.cpp index 83bac153..a8531200 100644 --- a/source/typecheck/polymorph/solver.cpp +++ b/source/typecheck/polymorph/solver.cpp @@ -167,15 +167,18 @@ namespace sst unsolvedtargets.insert(i); }); + // record which optionals we passed, for a better error message. + std::set providedOptionals; - size_t last_arg = std::min(target.size() + (fvararg ? -1 : 0), given.size()); + size_t last_arg = std::min(target.size() + (fvararg ? -1 : 0), given.size()); // we used to do this check in the parser, but to support more edge cases (like passing varargs) // we moved it here so we can actually check stuff. bool didNames = false; size_t positionalCounter = 0; + size_t varArgStart = last_arg; for(size_t i = 0; i < last_arg; i++) { const ArgType* targ = 0; @@ -210,24 +213,85 @@ namespace sst didNames = true; positionalCounter++; } + else + { + providedOptionals.insert(given[i].name); + } } else { + /* + we didn't pass a name. if the function is variadic, we might have wanted to pass the following argument(s) + variadically. so, instead of assuming we made a mistake (like not passing the optional by name), assume we + wanted to pass it to the vararg. + + so, `positionalCounter` counts the paramters on the declaration-side. thus, once we encounter a default value, + it must mean that the rest of the parameters will be optional as well. + + * ie. we've passed all the positional arguments already, leaving the optional ones, which means every argument from + * here onwards (including this one) must be named. since this is *not* named, we just skip straight to the varargs if + * it was present. + */ + targ = &target[positionalCounter]; - unsolvedtargets.erase(positionalCounter); + if(fvararg && targ->optional) + { + varArgStart = i; + break; + } + + unsolvedtargets.erase(positionalCounter); positionalCounter++; } + /* + TODO: not sure if there's a way to get around this, but if we have a function like this: + + fn foo(a: int, b: int, c: int, x: int = 9, y: int = 8, z: int = 7) { ... } + + then calling it wrongly like this: foo(x: 4, 1, 2, 5, z: 6, 3) + + results in an error at the last argument ('3') saying taht optional argument 'x' must be passed by name. + the problem is that we can't really tell what argument you wanted to pass; after seeing '1', '2', and '5', + the positionalCounter now points to the 4th argument, 'x'. + + even though you already passed x prior, we don't really know that? and we assume you wanted to pass x (again) + */ + if(given[i].name.empty()) { if(didNames) return SimpleError::make(given[i].loc, "positional arguments cannot appear after named arguments"); else if(targ->optional) - return SimpleError::make(given[i].loc, "optional argument '%s' must be passed by name", targ->name); - } + { + std::string probablyIntendedArgumentName; + for(const auto& a : target) + { + if(!a.optional) + continue; + + if(auto it = providedOptionals.find(a.name); it == providedOptionals.end()) + { + probablyIntendedArgumentName = a.name; + break; + } + }; + if(probablyIntendedArgumentName.empty()) + { + //* this shouldn't happen, because we only get here if we're not variadic, but if we weren't + //* variadic, then we would've errored out if the argument count was wrong to begin with. + return SimpleError::make(given[i].loc, "extraneous argument without corresponding parameter"); + } + else + { + return SimpleError::make(given[i].loc, "optional argument '%s' must be passed by name", + probablyIntendedArgumentName); + } + } + } iceAssert(targ); auto err = solveSingleType(soln, targ->toFLT(), given[i].toFLT()); @@ -284,7 +348,7 @@ namespace sst auto varty = target.back()->toArraySliceType()->getArrayElementType(); auto ltvarty = fir::LocatedType(varty, target.back().loc); - for(size_t i = last_arg; i < given.size(); i++) + for(size_t i = varArgStart; i < given.size(); i++) { auto err = solveSingleType(soln, ltvarty, given[i].toFLT()); if(err) return err->append(SimpleError::make(MsgType::Note, target.back().loc, "in argument of variadic parameter"));