Fix issue 21340 - extern(C++,(emptyTuple)) should result in no namespace not an error

[thewilsonator]

…)) should result in no namespace not an error

[dlang-bot]

Thanks for your pull request and interest in making D better, @thewilsonator! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

• My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
• My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
• I have provided a detailed rationale explaining my changes
• New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.

---

If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Auto-close	Bugzilla	Severity	Description
✓	21340	enhancement	extern(C++,(emptyTuple)) should result in no namespace not an error

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + dmd#11905"

[thewilsonator]

This is principally to allow

template ScopeClass(C)
if (is(C == class) && __traits(getLinkage, C) == "C++")
{
    static if (__traits(getCppNamespaces,C).length)
    {
        enum _namespaces_ =  "\""~ __traits(getCppNamespaces,C)[0] ~ "\"";
    }
    else 
        enum _namespaces_ = "\"\"';
    extern(C++, class)
    extern(C++,mixin(_namespaces_))
    struct ScopeClass { ... }
}

without having to duplicate the definition.

[thewilsonator]

I'm not sure why toString() doesn't work here

[WalterBright]

What does the spec have to say about this? We were talking in the beerconf yesterday about the divergence of the compiler from the spec.

[ibuclaw]

Not sure if this makes much sense to me. When I first looked at extern(C++, "") I presumed that it must be an anonymous namespace, which can't be binded to from D.

[thewilsonator]

@WalterBright Not much it is thoroughly under-specced : there is the grammar and https://dlang.org/spec/cpp_interface.html#cpp-namespaces

[Geod24]

Having the ability to write extern(C++, "A", "", "C") seems a bit error-prone. I understand why empty tuples are wanted, but empty strings don't make much sense to me.

[thewilsonator]

This is primarily about generated code I suppose that can be added later if need be.

[thewilsonator]

By the time we get to semantic there is no difference between the tuple and nested namespaces anyway.

[thewilsonator]

@ibuclaw anonymous namespaces are private symbols anyway, I don't see a point in exposing the ability to interact with them . OTOH this solves a real issue.

[thewilsonator]

Having the ability to write extern(C++, "A", "", "C") seems a bit error-prone. I understand why empty tuples are wanted, but empty strings don't make much sense to me.
By the time we get to semantic there is no difference between the tuple and nested namespaces anyway.

That is to say extern(C++, "A", "", "C") is indistinguishable from extern(C++, "A") extern(C++, "") extern(C++,"C") which is the whole point of this PR.

[Geod24]

But why do we need to support extern(C++, "") to begin with ?

[thewilsonator]

#11905 (comment)

and because extern(C++,mixin(_namespaces_)) will fail with _namespaces_ == ""

[Geod24]

template ScopeClass(C)
if (is(C == class) && __traits(getLinkage, C) == "C++")
{
    static if (__traits(getCppNamespaces,C).length)
        alias _namespaces_ =  __traits(getCppNamespaces,C)[0];
    else 
        alias _namespaces_ = AliasSeq!();
    extern(C++, class)
    extern(C++, _namespaces_)
    struct ScopeClass { ... }
}

What's wrong with this ?

[ibuclaw]

@ibuclaw anonymous namespaces are private symbols anyway, I don't see a point in exposing the ability to interact with them .

Yes, and that is precisely the reason why allowing extern(C++, "") makes no sense to me.

[thewilsonator]

extern(C++, _namespaces_) is a non-string namespace.

test.d(7): Error: alias issue21340.ScopeClass!(Foo)._namespaces_ conflicts with namespace test.ScopeClass!(Foo)._namespaces_ at test/compilable/issue21340.d(9)
Error: template instance test.ScopeClass!(Foo) error instantiating

I mean I'm all for deprecating them, but that requires convincing Walter and waiting out the deprecation period.

[Geod24]

extern(C++, (namespaces))

Use an extra set of parenthesis for disambiguation.

[thewilsonator]

test.d(7): Error: compile time string constant (or tuple) expected, not ()
test.d(13): Error: template instance test.ScopeClass!(Foo) error instantiating

back to square one

[Geod24]

It just means you need to ensure it gets parsed as an empty tuple

[thewilsonator]

Done.

[Geod24]

Where did the parser changes go ? This doesn't seem to fix the issue. The trailing comma change was good, and the test for empty tuple with parentheses should be there too.

[thewilsonator]

Where did the parser changes go ? ... The trailing comma change was good.

Sorry I misinterpreted #11905 (comment) to mean just the empty tuple.

the test for empty tuple with parentheses should be there too.

Done.

[ibuclaw]

Is it possible for this to return null?

[thewilsonator]

its has gone through semantic at this point so I don't think so.

[WalterBright]

Need a spec PR explaining just what the semantics of this are. Need a rationale as to why, too. "should result in no namespace and not an error" isn't sufficient. What actual programming problem does this solve?

[thewilsonator]

What actual programming problem does this solve?

#11905 (comment)

[thewilsonator]

dlang/dlang.org#2872

[thewilsonator]

@Geod24 ping

[Geod24]

Yep, will give another round of review sound. TBH implementation seems a bit too tailored, but feature is 👍

[thewilsonator]

Why does circle keep failing (randomly?)?

 ... fail_compilation/fail9.d       -verrors=0  -fPIC ()
FAILED targets:
- .

Exited with code exit status 1

spec PR done, rationale given

[Geod24]

LGTM. Sorry for the delay, the fresh eyes did help :D

[thewilsonator]

Thanks!