-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Optional arguments and C# #5
Comments
HOOOLY MOLY, I didn't even know this was possible. Hmmm, I'd need to investigate this more, but it's definitely something I'd like to implement within Reflaxe so it can be fixed for everything, not just this target (could make it an option to auto-generate the functions before passing to the compiler, and configure whether the new functions use the same name, or generate new ones). Give me a couple days to try that! |
Alterantively, we should consider: removing the default argument from the C# output (make non of the arguments default), then whenever the function is called, we just inject the default argument ( |
I once had a similar idea when we were working on a C target. I still have the test case for that: https://gist.github.com/Simn/f2718bc8037af3c146bc Basically, when you can be sure about the nullability of your arguments, you can call the Note that you can only do call-site replacement for methods which are definitely known at compile-time, so mostly class Parent {
public function new() {}
public function test(a = 0) {
trace(a);
}
}
class Child extends Parent {
override function test(a = 1) {
trace(a);
}
}
function main() {
var c:Parent = new Child();
c.test(); // should trace 1
} |
@RobertBorghese careful with moving that to Reflaxe. It could be an interesting option but I’m afraid it would also make it more difficult to get clear info on optional args when needed (like for Reflection). Maybe it would be better to provide a helper provided by reflaxe that you can explicitly call to get a list of functions from one given as argument, as it would be more flexible than having reflaxe do that beforehand. @Simn yes, changing on-site calls seems tricky for default values, that is why I’m interested in taking advantage of c# overload in cases where it’s possible, as it should work well with non-final and non-static methods too. Will take a look at your C example |
I just remembered another reason I wanted to do it like that: function test(a = 0, ?b = 0) {
$type(a); // Int
$type(b); // Null<Int>
}
function main() {
test();
} This means that |
Maybe a naive approach, but what if we set default values for all variables after the first optional argument? public virtual void foo(int optInt = 4, string reqString = null ) {
// ...
} |
Yes I meant to imply that because it works well with my approach. That way you have one wrapper function which deals with all the default value stuff (which is also the one that is called from reflection and such) and one "clean" function. |
I've been working on overhauling hxcpp's functions to be strongly typed and optional arguments makes it all a bit of a nightmare. Haxe functions allow for implicit conversions of functions when swapping out arguments with Dynamic, switching nullability on arguments, and signatures changing when round tripping throught Dynamic. What I'm updating hxcpp to have is a callable class which represents a function you can call. template<class TReturn, class... TArgs>
struct Callable<TReturn(TArgs...)> : public hx::Object
{
virtual TReturn _hx_run(TArgs... args) = 0;
}; All local functions and closures are created as sub classes of this callable, each member and static class function also has a internal function which returns a sub class of this callable for invoking that function. Because default values aren't part of the function type and are not inserted at call site TFun doesn't contain enough information to be able to tell if a given This means we have to be pessimistic for this Callable and you don't really have any other option but to require boxing even if the user types out a strongly typed closure. e.g.
becomes struct _hx_closure0 : ::hx::Callable<void(::Dynamic)> {
// actual implementation
}; In situations where the user swaps and arguments with Dynamic, I generate a wrapper functions to handle that. final c0 : Int->Void = x -> trace(x);
final c1 : Dynamic->Void = c0; The C++ callable object has an implicit conversion to different callable templated types, this generates a callable which simply holds another callable and forwards the arguments. This works as hxcpp's Dynamic has implicit conversions of ints and other types so it works with little effort required. Callable(const Callable<TOtherReturn(TOtherArgs...)>& inCallable)
{
struct AdapterCallable final : public Callable<TReturn(TArgs...)>
{
Callable<TOtherReturn(TOtherArgs...)> wrapped;
AdapterCallable(Callable<TOtherReturn(TOtherArgs...)> _wrapped) : wrapped(_wrapped) {}
TReturn _hx_run(TArgs... args) override
{
return wrapped(args...);
}
};
super::mPtr = new AdapterCallable(inCallable);
} You can make an optimisation on class member and static functions when calling them directly if they have optional arguments. class MyClass {
static function MultiplyBy(arg0 : Int, arg1 : Int = 2) { return arg0 * arg1 }
} int MultiplyBy(int arg0, ::hx::Null<int> __o_arg1) {
int arg1 = __o_arg1.Default(2);
return arg0 * arg1;
} Despite MyClass.MultiplyBy(7); // No Boxing
final f = MyClass.MultiplyBy;
f(7); // boxing Hopefully all this rambling is useful and isn't too irrelevant being mostly about C++. C# generics are not templates so there might be a better approach and already having a GC might allow for a better C# implementation. You may also want better interop with Action / Func as you don't have to worry about the GC causing issues. But, a direct 1:1 C# version of my described approach might be something like. readonly ref struct Arg<T>
{
private readonly bool hasValue;
private readonly T value;
public Arg()
{
hasValue = false;
value = default;
}
public Arg(T value)
{
this.value = value;
this.hasValue = true;
}
public T ValueOr(T defaultValue)
{
return hasValue ? value : defaultValue;
}
}
class __MyClassMultiplyBy : Function<int, int, int?>
{
public override int Run(int arg0, int? arg1)
{
return MyClass.MultiplyBy(arg0, arg1.HasValue ? new Arg<int>(arg1.Value) : new Arg<int>());
}
}
class MyClass
{
public static int MultiplyBy(int arg0, Arg<int> __o_arg1)
{
var arg1 = __o_arg1.ValueOr(12);
return arg0 * arg1;
}
public static Function<int, int, int?> MultiplyBy_dyn()
{
return new __MyClassMyFunc();
}
}
void Main()
{
MyClass.MultiplyBy(7, new Arg<int>());
MyClass.MultiplyBy(12, new Arg<int>(24));
Function<int, int, int?> f = MyClass.MultiplyBy_dyn();
f.Run(12, 7);
} You may need some custom types for implicit conversions or have the generator call explicit conversions. For the generic function approach with adapters to work in C# you'll also need to make sure the type constraints are setup properly for implicit conversions. |
Thanks for all the feedback, will experiment while keeping in mind all the things that have been said here! |
Added a couple functions in Reflaxe to help. This provides all the possible variations of arguments. Checked to make sure it works, but not tested thoroughly. // Find all argument variations for optional params
// frontOptionalsOnly - only gen variations for optional arguments before required arguments
// preventRepeats - remove repeated type combos
ClassFuncData.findAllArgumentVariations(frontOptionalsOnly: Bool = false, preventRepeats: Bool = false): Array<{ args: Array<ClassFuncArg>, padExprs: Array<TypedExpr> }>; --- In Reflaxe/C++, I'm trying my "inject defaults at call expression" idea. To prevent the previously discussed issue, it manually checks if there is a "conflicting default" and reverts to the "passing ClassFuncData.replacePadNullsWithDefaults(passedArgs: Array<TypedExpr>): Array<TypedExpr>;
ClassFuncArg.isFrontOptional(): Bool;
ClassFuncArg.hasConflicingDefaultValue(): Bool; |
Did implement a first iteration of optional args via C# overloads: From this Haxe Code: function foo(optInt:Int = 4, reqString:String) {
}
function foo2(optInt:Int = 4, reqString:String, optBool:Bool = false) {
return false;
} We get this: public void foo(int optInt, String reqString) {
}
public void foo(String reqString) {
foo(4, reqString);
}
public bool foo2(int optInt, String reqString, bool optBool = false) {
return false;
}
public bool foo2(String reqString, bool optBool = false) {
return foo2(4, reqString, optBool);
} This definitely doesn't cover all uses cases, but that still could be complementary to the other solutions provided by Aidan and Simon. We'll probably need to have a signature that boxes nullables anyway, to cover the situations where the function type is unknown at compile time. |
(Thanks to @RobertBorghese for the helpers added in reflaxe for that) |
want to point out somth - if this is implemented, overloading should be opted for - much more comfortable for haxe interop with other c# code, and much more understandable |
Let's assume we have this Haxe method that has one optional argument followed with a required one:
In Haxe this is valid. (I believe @Simn was thinking about maybe removing that feature in favor of overload at some point).
Anyway, the naive C# output would be:
But that is not valid in C# because optional args need to be after the required ones.
In current C# target, it would handle that situation by making every argument non-optional in the C# method, but wrap every haxe optional arg into
haxe.lang.Null
. The above example gives that in current C# target:Then when you call with haxe
foo("hello");
, it would becomefoo(default(global::haxe.lang.Null<int>), "hello")
in output to match the C# method.This isn't ideal because wrapping with
haxe.lang.Null
primitive types means creating garbage memory even if it is definitely not needed: an int with a default value should never be null anyway.In the new Reflaxe/C# target I'd like to explore another approach: solving that case with C# overload.
The above method with optional arg could generate this C# code instead:
Then no need to do anything special when calling as one or the other signature should be resolved depending on the arguments provided.
Will need to confirm that later, but it might make it easier to deal with reflection as well (I know there are also issues with dynamic calls and optional args in current C# target).
The text was updated successfully, but these errors were encountered: