Replace @SubCommands SumType!(TYPES...) with SubCommand!(TYPES...)

[andrey-zherikov]

Closes #80

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (3d1e8b2) 99.22% compared to head (e5bb105) 99.23%.

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #143   +/-   ##
=======================================
  Coverage   99.22%   99.23%           
=======================================
  Files          27       28    +1     
  Lines        1943     1966   +23     
=======================================
+ Hits         1928     1951   +23     
  Misses         15       15           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[SirNickolas]

Suggested change

	package(argparse) enum bool isSubCommand(T) = is(T : SubCommand!Args, Args...);
	public template match(handlers...)
	{
	auto ref match(Args...)(auto ref SubCommand!Args sc)
	{
	alias RETURN_TYPE = typeof(SumType!(SubCommand!Args.Types).init.sumtype_match!handlers);
	package(argparse) enum bool isSubCommand(T) = is(T : const SubCommand!Args, Args...);
	public template match(handlers...)
	{
	auto ref match(Sub : const SubCommand!Args, Args...)(auto ref Sub sc)
	{
	alias RETURN_TYPE = typeof(SumType!(Sub.Types).init.sumtype_match!handlers);

immutable sub = SubCommand!(A, B)(A(1));
static assert(isSubCommand!(typeof(sub)));
assert(sub.match!(_ => _.i) == 1);

(Sorry for focusing on such tiny bits. Just can’t see anything more substantial.)

[andrey-zherikov]

I fixed match but compiler didn't complain about isSubCommand.

[SirNickolas]

Interesting. A reduced example:

struct A(T)
{
    T* p;
}

static if(is(immutable A!int : A!Args, Args...)) // Surprisingly, this is true.
{
    // Let's see what it deduced:
    static assert(Args.length == 1);
    static assert(is(Args[0] == int));
    // Let's check it again, just in case:
    static assert(!is(immutable A!int : A!Args)); // Now it's false!
}
else
    static assert(false);

Congratulations, I suppose we’ve discovered a bug in the compiler. I’ll need to mock this a bit more; will then report to D’s Bugzilla.

A unit test that passes now but IMO should fail with constless isSubCommand:

struct A
{
    int[] a;
}

static assert(isSubCommand!(immutable SubCommand!A));

I think it is sensible to put const in isSubCommand to be safe against future fixes in the compiler in case it is really a bug.

[andrey-zherikov]

Before submitting issue to compiler, I'd suggest checking SumType/isSumType

[andrey-zherikov]

Slightly modified examples from dlang reference:
Fails for short

onlineapp.d(2): Error: static assert: `is(const(short) == short)` is false

Works for SumType:

assert(isSumType!(const SumType!int));

[SirNickolas]

With indirections:

pragma(msg, is(const(int)* : int*));                           // => false
pragma(msg, is(const SumType!(int*) : SumType!Args, Args...)); // => true

[andrey-zherikov]

But:

struct A(T) {}
writeln(is(const(A!int) : A!T1,T1));    // => true
writeln(is(const(A!int) == A!T2,T2));   // => true
writeln(is(const(A!int) : A!int));      // => true
writeln(is(const(A!int) == A!int));     // => false

[SirNickolas]

Yeah, your last snippet shows it clearly. Intuitively, const(A!int) cannot be == A!T2 for any T2, but DMD insists it is.

[SirNickolas]

Asked on the forum about this, too.

[andrey-zherikov]

tbh I don't expect any answers from forum

[SirNickolas]

struct A
{
    int[] arr;
}

immutable one = SubCommand!A(A([1]));
SubCommand!A mut;
mut = one;
mut.match!(_ => _.arr[0] = 2);
assert(one.match!(_ => _.arr[0]) == 2); // Successfully modified an immutable object!

Still have to investigate if it’s a SubCommand’s fault or SumType’s.

[andrey-zherikov]

Still have to investigate if it’s a SubCommand’s fault or SumType’s

It seems also related to int[] array - this fails as expected:

struct A
{
    int arr;
}

immutable one = SubCommand!A(A(1));

SubCommand!A mut = one;
mut = one;
mut.match!(_ => _.arr = 2);

assert(one.match!(_ => _.arr) == 2);

[andrey-zherikov]

struct None {}

struct S(T)
{
    SumType!(None,T) a;
}

struct A
{
    int[] arr;
}

immutable one = S!A.init;
immutable two = SumType!(None,A)(A([1]));
immutable three = SumType!(A,None)(A([1]));
immutable four = SumType!(A)(A([1]));

one and two do compile and three and four do not (cannot implicitly convert expression)

[andrey-zherikov]

Asked question on forum.

[andrey-zherikov]

May be adding None to SumType was not good solution so I'd like to re-think my proposal a bit:

• There are two use cases: with default subcommand and without.
• Public API for both cases should be the same although internal implementation might not.
• I think the following public API is useful:
  • Check whether any subcommand is chosen. This case always be true for a case with default subcommand.
  • Check whether specific subcommand is chosen.
  • Ctor and assignment operator that take subcommand object as parameter (without Default!).
• In case of default subcommand, init value should be set to that default subcommand's init.

[andrey-zherikov]

Pushed changes and confirmed that this does not compile:

    struct A
    {
        int[] i;
    }

    immutable sub = SubCommand!(A)(A([1]));

Error:

Error: cannot implicitly convert expression SubCommand(Nullable(DontCallDestructorT(SumType(Storage(A(null)), cast(ubyte)0u)), true)).this(A([1])) of type SubCommand!(A) to immutable(SubCommand!(A))

[andrey-zherikov]

@SirNickolas Is this now good from your point of view?