Follow-up to old http://forum.dlang.org/thread/m9lhc3$1r1v$1@digitalmars.com thread by Benjamin

Short reminder of the issue:

Currently unsolved issue with finishing `export` implementation is lack of convenient semantics for its interaction with templates. If template function is marked as `export`, its non-template dependencies (called functions) will also need to be marked as effectively export (even if private) to be put in object file and avoid linker errors.

Which is impossible to do automatically because dependencies can't be figured out without instantiaton. And to do it manually you'd need to mark all dependencies with `export` too which is impossible without making them also public because currently `export` is defined as protection attribute. One of Benjamin proposals was to split it as a separate attribute kind but doing all manual annotation would still be hardly convenient / maintainable.

Proposal essentials:

Define `unittest export { /* tests here */ }` which will verify that all directly used symbols from same module/package are marked as export and automatically mark dependencies for placement into object files while doing semantic phase for tested instances.

Rationale:

- fits existing "documented unittest" feature by providing verified example of using the API
- easier change to grammar than re-defining export itself
- reasonably simple maintenance (no need to hunt each small dependency function after internal changes, risking linker errors if sloppy)
- if https://issues.dlang.org/show_bug.cgi?id=14825 is ever fixed, combining this with -cov will ensure reasonable confidence in proper API annotation

Cons:

- implies test author to be smart enough to do all necessary instantiations (will become less of an issue with https://issues.dlang.org/show_bug.cgi?id=14825)
- may look like a hack for those coming from more restrictive languages

On Sunday, 30 August 2015 at 14:08:15 UTC, Dicebot wrote:
> Follow-up to old http://forum.dlang.org/thread/m9lhc3$1r1v$1@digitalmars.com thread by Benjamin
>
> Short reminder of the issue:
>
> Currently unsolved issue with finishing `export` implementation is lack of convenient semantics for its interaction with templates. If template function is marked as `export`, its non-template dependencies (called functions) will also need to be marked as effectively export (even if private) to be put in object file and avoid linker errors.
>
> Which is impossible to do automatically because dependencies can't be figured out without instantiaton. And to do it manually you'd need to mark all dependencies with `export` too which is impossible without making them also public because currently `export` is defined as protection attribute. One of Benjamin proposals was to split it as a separate attribute kind but doing all manual annotation would still be hardly convenient / maintainable.
>
> Proposal essentials:
>
> Define `unittest export { /* tests here */ }` which will verify that all directly used symbols from same module/package are marked as export and automatically mark dependencies for placement into object files while doing semantic phase for tested instances.
>
> Rationale:
>
> - fits existing "documented unittest" feature by providing verified example of using the API
> - easier change to grammar than re-defining export itself
> - reasonably simple maintenance (no need to hunt each small dependency function after internal changes, risking linker errors if sloppy)
> - if https://issues.dlang.org/show_bug.cgi?id=14825 is ever fixed, combining this with -cov will ensure reasonable confidence in proper API annotation
>
> Cons:
>
> - implies test author to be smart enough to do all necessary instantiations (will become less of an issue with https://issues.dlang.org/show_bug.cgi?id=14825)
> - may look like a hack for those coming from more restrictive languages

I don't really understand the proposal. Let me try to rephrase what I do get, and I'll let you fill in.

We want to have method that are not export to be, not exported. It allow the optimizer to do more, make executable slimmer and so on.

Problem arise with templates, for instance:

export void foo(T)(T t) { bar(); }

private void bar() { import std.stdio; writeln("bar was called"); }

Here bar is called. Problem is, foo can be instantiated in a different module than bar is compiled in. As bar is not exported, things will fail to link.

When compiling the module with bar, foo is not instantiated and the compiler has no way to know that this template may use bar (the template is simple here but it could be arbitrarily complex in practice).

Am I right so far ?

Good, then, what is the solution ?

Note that this is a reason why I like trait or whatever so that the compiler can figure out what is going on in the template before instantiating it. I tried this road with SDC at first, but the amount of infos that the compiler can infer from D template is way too small for it to be worth it.

On Tuesday, 1 September 2015 at 01:40:29 UTC, deadalnix wrote:
> Problem arise with templates, for instance:
>
> export void foo(T)(T t) { bar(); }
>
> private void bar() { import std.stdio; writeln("bar was called"); }
>
> Here bar is called. Problem is, foo can be instantiated in a different module than bar is compiled in. As bar is not exported, things will fail to link.
>
> When compiling the module with bar, foo is not instantiated and the compiler has no way to know that this template may use bar (the template is simple here but it could be arbitrarily complex in practice).
>
> Am I right so far ?

Yep, got it exactly right.


> Good, then, what is the solution ?

Something like this

Error : template foo is marked as export but not instantiated within any unittest export

To fix, add API test / example:

unittest export {
    foo(42);
}

Compiler sees that unittest export has created `foo!int` and checks what
non-template functions get called from that instance (I may be mistaken but
all necessary information for that is known once we have actual instance
for an argument set).

Of course, if some branches are missing in unittest export, it will still
cause linker errors - that is why fixing linked issue with coverage becomes
pretty important.

> Note that this is a reason why I like trait or whatever so that the compiler can figure out what is going on in the template before instantiating it. I tried this road with SDC at first, but the amount of infos that the compiler can infer from D template is way too small for it to be worth it.

Yep, this is why I feel that such a solution will feel dirty if you get
used to something like traits which allows for full analysis without
instantiating. But within existing semantics it does not seem possible,
it simply allows too much.

On Sunday, 30 August 2015 at 14:08:15 UTC, Dicebot wrote:
> Follow-up to old http://forum.dlang.org/thread/m9lhc3$1r1v$1@digitalmars.com thread by Benjamin
>
> Short reminder of the issue:
>
> Currently unsolved issue with finishing `export` implementation is lack of convenient semantics for its interaction with templates. If template function is marked as `export`, its non-template dependencies (called functions) will also need to be marked as effectively export (even if private) to be put in object file and avoid linker errors.
>
> Which is impossible to do automatically because dependencies can't be figured out without instantiaton. And to do it manually you'd need to mark all dependencies with `export` too which is impossible without making them also public because currently `export` is defined as protection attribute. One of Benjamin proposals was to split it as a separate attribute kind but doing all manual annotation would still be hardly convenient / maintainable.
>
> Proposal essentials:
>
> Define `unittest export { /* tests here */ }` which will verify that all directly used symbols from same module/package are marked as export and automatically mark dependencies for placement into object files while doing semantic phase for tested instances.
>
> Rationale:
>
> - fits existing "documented unittest" feature by providing verified example of using the API
> - easier change to grammar than re-defining export itself
> - reasonably simple maintenance (no need to hunt each small dependency function after internal changes, risking linker errors if sloppy)
> - if https://issues.dlang.org/show_bug.cgi?id=14825 is ever fixed, combining this with -cov will ensure reasonable confidence in proper API annotation
>
> Cons:
>
> - implies test author to be smart enough to do all necessary instantiations (will become less of an issue with https://issues.dlang.org/show_bug.cgi?id=14825)
> - may look like a hack for those coming from more restrictive languages

While your proposal sounds interresting to start with I don't like some of the implications:

1) You force people to write unittest. If people don't write a export unittest block their templates won't work across shared library boundaries.

2) A template in D might get __very__ complex. To make sure that each and every function needed is actually exported your unittests would need to have 100% coverage. Looking at some of the template code in phobos this won't be any more fun than manually putting export in front of every required function.

3) Its going to be hard to implement. You basically need to touch all the template instanciation code and make sure it tells you which functions have been used after its done. Thats going to be quite invasive.

4) Martins favorite argument. When doing C APIs you usually ensure that you public interface (e.g. whats exported form a shared library) stays the same between minor versions. This automatic export thing is going to make this a lot harder for D.

On Sunday, 30 August 2015 at 14:08:15 UTC, Dicebot wrote:
> Follow-up to old http://forum.dlang.org/thread/m9lhc3$1r1v$1@digitalmars.com thread by Benjamin
>

Oh, don't get me wrong. Its great that you also think about this problem. Currently you seem to be the only other person here on the NG which is actually interested to solve the problems around export, other people seam to simply ignore them or want to abuse export for other language issues.

Keep up the good work ;-)

Kind Regards
Benjamin Thaut

On Tuesday, 1 September 2015 at 12:55:11 UTC, Benjamin Thaut wrote:
> While your proposal sounds interresting to start with I don't like some of the implications:
>
> 1) You force people to write unittest. If people don't write a export unittest block their templates won't work across shared library boundaries.

I think it is a nice encouragement but in context of the proposal it is
optional - you can remove error for not covering export template
with test if someone wants to do it manually.

> 2) A template in D might get __very__ complex. To make sure that each and every function needed is actually exported your unittests would need to have 100% coverage. Looking at some of the template code in phobos this won't be any more fun than manually putting export in front of every required function.

True, that is why I am stressing the point about https://issues.dlang.org/show_bug.cgi?id=14825 - if it gets fixed, putting exported API in separate module and checking for strict 100% cov in CI system allows for quite a reliable maintenance.

Phobos is a bit special though because it is dominated by independent
template utilities and vast majority of symbols is public anyway. For that just
slapping top-level module `export:` may be a better option. I am not
interested in Phobos much personally though :)

> 3) Its going to be hard to implement. You basically need to touch all the template instanciation code and make sure it tells you which functions have been used after its done. Thats going to be quite invasive.

Can't say much here, need someone more experienced with DMD to comment.
Tweaking all template semantic phase doesn't seem that invasive to me,
hardest part is transitivity - if export template `foo` uses non-export template `bar` which uses plain function `baz` you still want to put `baz` into the
binary. That may be hard to retro-fit into existing semantic step system in
DMD as I remember it.

> 4) Martins favorite argument. When doing C APIs you usually ensure that you public interface (e.g. whats exported form a shared library) stays the same between minor versions. This automatic export thing is going to make this a lot harder for D.

How so? Important thing about my proposal is that those "implicitly"
exported symbols are not considered public, they merely are available
for linkage. Anyone relying on presence of symbols that are not defined
by header / .di distributed by the library is asking for trouble anyway. It is like trying to link directly to functions in C which are marked as `static inline` but present in object file with obfuscated name because of address exposure.

On Tuesday, 1 September 2015 at 12:55:11 UTC, Benjamin Thaut wrote:
> While your proposal sounds interresting to start with I don't like some of the implications:
>
> 1) You force people to write unittest. If people don't write a export unittest block their templates won't work across shared library boundaries.
Not writing unittests is a bad idea anyway.
Much more so if you want to ship your code as library!

> 2) A template in D might get __very__ complex. To make sure that each and every function needed is actually exported your unittests would need to have 100% coverage. Looking at some of the template code in phobos this won't be any more fun than manually putting export in front of every required function.
Oh yes, it's much more fun, because you get something very valueable in return: 100% coverage!
The more complex the template gets, the more valueable such a unittest becomes.
Especially in phobos I pretty much expect such unittests to already exist. What a shame if not so!

On Tuesday, 1 September 2015 at 15:10:28 UTC, Dominikus Dittes Scherkl wrote:
> On Tuesday, 1 September 2015 at 12:55:11 UTC, Benjamin Thaut wrote:
>> While your proposal sounds interresting to start with I don't like some of the implications:
>>
>> 1) You force people to write unittest. If people don't write a export unittest block their templates won't work across shared library boundaries.
> Not writing unittests is a bad idea anyway.
> Much more so if you want to ship your code as library!
>
>> 2) A template in D might get __very__ complex. To make sure that each and every function needed is actually exported your unittests would need to have 100% coverage. Looking at some of the template code in phobos this won't be any more fun than manually putting export in front of every required function.
> Oh yes, it's much more fun, because you get something very valueable in return: 100% coverage!
> The more complex the template gets, the more valueable such a unittest becomes.
> Especially in phobos I pretty much expect such unittests to already exist. What a shame if not so!

Yeah, any arguments which are basically saying that you need a feature because you shouldn't have to have 100% code coverage is going to fall flat with Walter and Andrei. All code that is released should have 100% test coverage unless it has a really good reason why it can't, and it's egg on the face of the developer who releases the code without that - and no, we don't do a good enough job of that with Phobos, and that _is_ egg on our face. We have good code coverage, but we often don't have 100%, and when we don't, we miss bugs. That test coverage needs to be there, and without it, it's far too easy to release stuff that mostly works but falls apart in the corner cases.

So, if there's a feature that can be added which solves the export problems that dicebot is talking about here, but it requires that you unit test your code for it to catch everything, then so be it. I really don't see that as an issue. If it can be done in a simple way that doesn't require unit testing, then fine, but all of that code should be fully unit tested anyway, so arguments based on the premise that you shouldn't need to have full unit test coverage aren't going to convince anyone - especially not the folks who would approve the new feature.

- Jonathan M Davis

Ok I get it. I'm not sure that is the right way forward. The same problem arrise for devirtualization and the solution does not cover it. That is a problem.

First I'd like to have more compiler checks on template bodies. Andrei and Walter are not super happy with it, but deep down I do think their point is more the result of the clash with the C++ community over static if vs concept than based on any technical merit (the whole thing is a false dichotomy to start with so the clash and its results are nonsensical).

Second, it is most likely a good idea to issue some kind of error when export within template code calls non export code. That most likely needs to be an error. The error would show up with descent test coverage even without the special feature.

On Tuesday, 1 September 2015 at 17:35:23 UTC, Jonathan M Davis wrote:
> On Tuesday, 1 September 2015 at 15:10:28 UTC, Dominikus Dittes Scherkl wrote:
>> On Tuesday, 1 September 2015 at 12:55:11 UTC, Benjamin Thaut wrote:
>>> While your proposal sounds interresting to start with I don't like some of the implications:
>>>
>>> 1) You force people to write unittest. If people don't write a export unittest block their templates won't work across shared library boundaries.
>> Not writing unittests is a bad idea anyway.
>> Much more so if you want to ship your code as library!
>>
>>> 2) A template in D might get __very__ complex. To make sure that each and every function needed is actually exported your unittests would need to have 100% coverage. Looking at some of the template code in phobos this won't be any more fun than manually putting export in front of every required function.
>> Oh yes, it's much more fun, because you get something very valueable in return: 100% coverage!
>> The more complex the template gets, the more valueable such a unittest becomes.
>> Especially in phobos I pretty much expect such unittests to already exist. What a shame if not so!
>
> Yeah, any arguments which are basically saying that you need a feature because you shouldn't have to have 100% code coverage is going to fall flat with Walter and Andrei. All code that is released should have 100% test coverage unless it has a really good reason why it can't, and it's egg on the face of the developer who releases the code without that - and no, we don't do a good enough job of that with Phobos, and that _is_ egg on our face. We have good code coverage, but we often don't have 100%, and when we don't, we miss bugs. That test coverage needs to be there, and without it, it's far too easy to release stuff that mostly works but falls apart in the corner cases.
>
> So, if there's a feature that can be added which solves the export problems that dicebot is talking about here, but it requires that you unit test your code for it to catch everything, then so be it. I really don't see that as an issue. If it can be done in a simple way that doesn't require unit testing, then fine, but all of that code should be fully unit tested anyway, so arguments based on the premise that you shouldn't need to have full unit test coverage aren't going to convince anyone - especially not the folks who would approve the new feature.
>
> - Jonathan M Davis

No. That is very short sighted.

A ton of code is not unitestable. GUI as a starter. Random number generator. Very rare events handling outside the control of the program. Concurent code. Etc...