[Issue 9571] Proof-of-concept fix for emitting only required template symbols

[mihails-strasuns]

Don't have easy access to different OS and linking is quite platform-dependent, so I'll abuse pull tester a bit ;)

This pull attempts to clean up template symbol emitting to object files. Currently all of them are generated to object file associated with module supplied to the command line. For separate compilation scenario that means that ALL template symbols from ALL imported modules are emitted into compiled single module.

Most time it is just extra job for linker, but sometimes this results in nasty bugs when resulting symbols are not weak ones (issue 9581 is exactly one such case).

After this pull request is merged dmd will try to find root module where template instantiation chain has started via call from non-templated scope. Implementation improvementn suggestions are welcome, of course, this is quick and dirty attempt.

[mihails-strasuns]

(Working on tester failures, expected, do not pull)

[eskimor]

I get linker errors:
$DMD main.d b.d -Ja -Jb -J. -deps=file -L-lcurl
main.o: In function _Dmain': b.d:(.text._Dmain+0x1a): undefined reference to_D3std5stdio16__T7writelnTAyaZ7writelnFAyaZv'
b.d:(.text._Dmain+0x5c): undefined reference to `_D3std5stdio20__T8writeflnTAyaTAaZ8writeflnFAyaAaZv'
collect2: error: ld returned 1 exit status
--- errorlevel 1

Also the following code in Import::semantic, still delivers the wrong module:

        TemplateInstance* ti=sc->tinst;
        while(ti) {
            ti=ti->tinst;
        }
        Module* md= ti ? ti->getRootModule() : sc->module;
        if(ti) {
            ob->writestring("file : ");
            ob->writestring(ti->loc.filename);
        }
        ob->writestring(md->toPrettyChars());

(ti->loc.filename prints the right one)

I have the following test code:

module b:

char[] test(A)(A i) {
    import std.file;
    return cast(char[])read(i);
}

main.d:

import std.stdio;
import b;
void main() {
  writefln("Contents: %s", test("mystringdeps")); // Replace with some file
}

The code in Import::semantic() should then print the instantiating module, but it does not.

[mihails-strasuns]

@eskimor Thanks, I am already on it, you saved me some time to find extra place where tweaks are needed ;)

[MartinNowak]

Please use isTemplateInstance instead of dynamic_cast.

[MartinNowak]

if (ti->enclosing && ti->enclosing->isTemplateInstance())

What if the enclosing context is a function within a template instance?

[MartinNowak]

Did you have a look at Module *Dsymbol::getModule() and Module *Dsymbol::getAccessModule()?
Maybe that could help here.

[eskimor]

Both seem to retrieve the module where the template is defined, but not the instantiating one. Thanks for your help!

[mihails-strasuns]

@dawgfoto I have checked almost all existing functions that give you some Module, they are not suitable here because of reason @eskimor has mentioned :)

What if the enclosing context is a function within a template instance?

That is one of the issues found by test suite, I am working on better handling of various tricky cases right now, new commits can be expected pretty soon.

[mihails-strasuns]

I may need help with one specific case:

// b.d
import std.stdio;

void foo(T)(T arg)
{
    import std.stdio;
    writeln(arg, 42);
}

// a.d
import b;

void main()
{
    foo(1);
}

a.d gets compiled. "foo" symbol is hit in TemplateInstance::Semantic. tinst is NULL. enclosing is NULL. sc->module is b. Wtf? "foo" is template function and thus all used symbols need to be emitted in the instantiation scope. Any ideas how to get "a" in this case?

[mihails-strasuns]

And sc->parent is "foo" in a "FunctionDeclaration" form (not TemplateInstance). Weird.

[MartinNowak]

sc->module is b. Wtf? "foo" is template function and thus all used symbols need to be emitted in the instantiation scope.

That's correct, see "Instantiation Scope" at http://dlang.org/template.html.
It's also the reason we're using ->importedFrom.

[mihails-strasuns]

@dawgfoto I don't see how that implies that scope for TemplateInstance should be b. It essentially means that all members declared within TemplateDeclaration are fully qualified with TemplateDeclaration module (which is fine, nothing is declared in foo in my example), but scope of TemplateInstance itself must match instantiation scope. Template simply don't work other way, not with separate compilation at least.

importedFrom is just a hack as compiled module has nothing to do with actual module template symbol belongs to. It happens to work only because weak symbols rock and linker does its job.

[mihails-strasuns]

@dawgfoto To be most specific, I'll highlight an example omitted in docs:

// b.d
template Templ(T)
{
    void bar() { foo!T(); }
}

void foo(T)()
{
}

// a.d
import b;
void main()
{
    Templ!int.bar();
}

b can't possibly know what types to make instances of foo with, so it will never have a single instance of foo. a, however, is prohibited to have foo in its scope by this weird rule. So, in case of separate compilation, foo instance has no place to go.

Sounds broken to me.

[mihails-strasuns]

As it is about template specification, I should probably call for @andralex

[eskimor]

The "scope" as far as I understand is correct, as a template can only refer to declarations at the definition site, as opposed to a template mixin.
Where the template is instantiated almost only matters for reporting errors and for deciding into which module the code should be put.

So in summary if you were able to retrieve the instantiating module via scope it would be weird, but there should definitely be some way and I am getting more and more confident that this way has to be created, because currently there does not seem to be any code (apart from error reporting) that would make use of this information.

[mihails-strasuns]

@eskimor well, that essentially means having symbol "b.foo" stored in "a" object file. Probably will work, but may be hard to explain for someone checking object files with nm. I think that can be an acceptable compromise. I am afraid you are right about necessity to add this to context. Such data is available up the call stack but lost in nested semantic processing.

[eskimor]

@eskimor well, that essentially means having symbol "b.foo" stored in "a" object file. Probably will work, but may be hard to explain for someone checking object files with nm.

Huh? Where else would you put it? foo gets instantiated (indirectly but still) in a so it has to be put there.

[mihails-strasuns]

@eskimor Ye, agreed, still can't sidestep from thinking in C translation unit terms instead of modules. This part is a bit tricky, I am trying to find some relatively easy way that does not involve updating construction of all Dsymbols.

[eskimor]

Enriching TemplateInstance with a variable instantiatedIn or something should be enough? I'll check this out in the evening.

[mihails-strasuns]

@eskimor I am trying this approach, but have issues with selecting right symbol to put there. I have tried sc->scopesym and sc->parent but those are not guaranteed to have a Module symbol up the chain.

[eskimor]

Check it out: #1887
:-) It seems to work!

[jpf91]

Where the template is instantiated almost only matters for reporting errors and for deciding into which module the code should be put.

-vgc could also use this. (so yes, error reporting)

Enriching TemplateInstance with a variable instantiatedIn or something should be enough?

For -vgc I also need to know if a template instance is the "toplevel" instance, i.e if it's nested directly or indirectly in another template. But this information might be already available?

[mihails-strasuns]

@jpf91 It is a bit uncertain. It is kind of available (you can check for "tinst" and if "enclosing" is also TemplateInstance), but does not work for all template cases. This is exactly what me and @eskimor are trying to get from dmd data model. It feels like initially it wasn't created with such necessity in mind and so far it is more trial-and-error than determined progress.

Btw I'll close this one until proper solution is found, discussion moves to #1887

[eskimor]

tinst in my experience does the right thing (at least printInstantiationTrace relies on tinst). For error reporting you can look at printInstantiationTrace -> the loc is right available.

[mihails-strasuns]

And for my experience it is not ;) It will work if you have template symbol a1 calling template symbol a2 but not when template a1 calls non-template a2 which calls template a3. printInstantiationTrace will show you a2 as root, not a1. If it perfectly worked, your pull wouldn't be needed ;)

[eskimor]

which is correct, if a2 is not a template. Maybe I don't understand correctly, could you provide some example code?

[mihails-strasuns]

@eskimor Everything is kind of mixed in my memory now but it is something like this:

// b.d
void foo(T)(T arg)
{
    void bar()
    {
     // some other template call
    }

    bar();
}

// a.d
import b;
void main() { foo!int(); }

While bar actually is templated function, it is not represented as such in dmd symbol tree. In this case "enclosing" probably does the trick but there were also cases where both didn't. If you enable printInstantiationTrace for every symbol added from TemplateInstance::semantic and study output for something template-intensive (most phobos modules will do ;)), those cases should bring attention naturally.

[eskimor]

But the following works correctly:

// module b
import std.stdio;
void fooOuter(T)(T val) {
    void fooInner(int arg) {
        writeln(arg, 43);
    }
    fooInner(val);
}

// a.d
import b;

void main()
{
    fooOuter(8);
}

If you compile it with the patched version, you'll find the fooInner symbol in a.o and not in b.o. This would not be the case if walking up tinst was not working.

[mihails-strasuns]

I have meant non-patched version, if you simply go up the tinsts with no instantiatedIn tracking. That was my initial naive approach.

[eskimor]

But the patched version relies on tinst being correct. The only thing changed is that the top level template has an instantiatedIn. This would be useless if it wasn't possible to reliable retrieve the toplevel package by traversing tinst.

[eskimor]

The only problem was that previously although you could retrieve the top level template, it had no information on where it got instantiated.