sociomantic-tsunami / dlsproto

Distributed Log Store protocol definition, client, fake node, and tests
Boost Software License 1.0
3 stars 18 forks source link

D2: Make sure the correct 'RequestContext' type is picked #67

Closed mathias-lang-sociomantic closed 6 years ago

mathias-lang-sociomantic commented 6 years ago

Otherwise this result in a static assert being triggered, mentioning that RequestContext is not handled.

mathias-lang-sociomantic commented 6 years ago

FYI @mihails-strasuns-sociomantic

nemanja-boric-sociomantic commented 6 years ago

What's the static assert that fails?

mathias-lang-sociomantic commented 6 years ago
./submodules/swarm/src/swarm/neo/client/request_options/RequestOptions.d-mixin-87(87): Error: static assert  "Unhandled argument types: RequestContext"
./submodules/dlsproto/src/dlsproto/client/mixins/NeoSupport.d(441):        instantiated from here: setupOptionalArgs!(2LU, void delegate(SmartUnion!(NotificationUnion), Args), RequestContext, void delegate(SmartUnion!(ContextUnion_) context) pure nothrow @nogc @safe, void delegate(void delegate(SmartUnion!(NotificationUnion), Args) notifier) pure nothrow @nogc @safe)
./submodules/dlsproto/src/dlsproto/client/mixins/NeoSupport.d(775):        instantiated from here: put!(char[], void delegate(SmartUnion!(NotificationUnion) info, Args args), RequestContext)
nemanja-boric-sociomantic commented 6 years ago

Do you want to target v13.2.x instead?

mathias-lang-sociomantic commented 6 years ago

Oops, indeed that was the plan

nemanja-boric-sociomantic commented 6 years ago

Please just force-push to trigger travis ;-)

mathias-lang-sociomantic commented 6 years ago

I restarted the build (it was already based on v13.2.x)

gavin-norman-sociomantic commented 6 years ago

What does it get mixed up with, without this change?

nemanja-boric-sociomantic commented 6 years ago

I assumed that RequestContext parameter in setupOptionalArgs without this change actually refers to the module, and not the struct inside the module.

nemanja-boric-sociomantic commented 6 years ago

(here: https://github.com/sociomantic-tsunami/dlsproto/pull/67/files#diff-a891d44e2336e2d0098d538a7d0b7c61R445)

gavin-norman-sociomantic commented 6 years ago

I assumed that RequestContext parameter in setupOptionalArgs without this change actually refers to the module, and not the struct inside the module.

Bloody D2. Really??

mathias-lang-sociomantic commented 6 years ago

I don't think so, no. It probably refers to the legacy RequestContext. The same fix had to be applied here

nemanja-boric-sociomantic commented 6 years ago

That's a relief, thanks for the explanation!

mathias-lang-sociomantic commented 6 years ago

actually refers to the module

I think you might remember what happened with MakD here. We had a Version symbol inside of a Version module, and it did conflict. But it's only because the module was not in a package. If the module was makd.Version, the conflict wouldn't have existed. IMO that's not a strong limitation, and is actually more consistent.

nemanja-boric-sociomantic commented 6 years ago

Yeah, I totally missed the trick with the package playing a role here.

This is interesting though:

// MyModule.d
module MyModule;

struct MyModuleS
{
    int x;
}

alias MyModuleS MyModule;
// main.d
import MyModule;

void foo (MyModule m)
{
}

void main()
{
    MyModule x;
    foo(x);
}

This fails with

MyModule.d(8): Error: size of type MyModuleS is not known
main.d(9): Error: variable main.main.x size of type MyModuleS is invalid
main.d(10): Error: function main.foo (MyModuleS m) is not callable using argument types (MyModuleS)
Failed: ["/usr/bin/dmd", "-v", "-o-", "main.d", "-I."]

As expected, import MyModule: MyModule or moving it into the package fixes the things. But, as not expected, this doesn't fail if you remove the alias and just name the struct MyModule:

module MyModule;

struct MyModule
{
    int x;
}

I'm not sure what's so special about the alias