I reduced another verify_cgraph issue (again, only shown on 4.7 and
with --enable-checking=yes).

This is the example:
---------------
void parseJSON()(char val)
{
    void error()
    {
    }

    void getChar()() //Scope not set to parseJSON
    {
        error();
    }

    void parseString()
    {
        getChar();
    }
}

void main()
{
    parseJSON('n');
}
---------------

If getChar is a template, it's scope is not set to parseJSON. If it's a function the scope is set and everything works.

A short printf log for the failing case looks like this:
---------------
function  json.parseJSON!().parseJSON
1716 Calling error->toObjFile
function  json.parseJSON!().parseJSON.error
function end  json.parseJSON!().parseJSON.error
1716 Calling getChar()->toObjFile
1716 Calling parseString->toObjFile
function  json.parseJSON!().parseJSON.parseString
function end  json.parseJSON!().parseJSON.parseString
function end  json.parseJSON!().parseJSON
function  json.parseJSON!().parseJSON.getChar!().getChar
function end  json.parseJSON!().parseJSON.getChar!().getChar
---------------

So getChar's toObjfile isn't called at the correct time. If getChar is
a function, it's toObjfile is called after this line:
1716 Calling getChar()->toObjFile

This call is happening in d-glue.cc DeclarationExp::toElem,
declaration->toObjFile (false);

A template's toObjFile does nothing, but all my naive attempts to call
e.g.
declaration->isTemplateDeclaration()>onemember->isDeclaration()->toObjFile(false)
fail with a strange error. So do you have any idea how to fix this?

BTW: Other combinations of templates in functions are affected as well:
----------
void parseJSON()(char val)
{
    void error()
    {
    }

    struct S() //Scope not set to parseJSON
    {
        void getChar()
        {
            error();
        }
    }


    void parseString()
    {
        S!() s;
        s.getChar();
    }
}

void main()
{
    parseJSON('n');
}
-------

I'll have to recheck the test case from last week, maybe it's also scope related.

Am Sat, 12 Jan 2013 11:57:10 +0100
schrieb Johannes Pfau <nospam@example.com>:

> A template's toObjFile does nothing, but all my naive attempts to call
> e.g.
> declaration->isTemplateDeclaration()>onemember->isDeclaration()->toObjFile(false)
> fail with a strange error.

OK, of course it can't work. toObjFile has to be called on the TemplateInstance, not on the declaration...

All TemplateInstances are currently emitted by Module::genobjfile which loops through all Dsymbols in the module. Therefore the scope for functions in template instances is always the module scope which is incorrect.

A simple approach to fix this is delaying emitting those templates and
emitting functions first, then change function output to emit template
instances.
Here's this simple approach: https://gist.github.com/4524721

The problem is that this can still miss cases where we have a template nested in a template*. In this case the order is important and the top level template instance should be output first, so I'm not sure how to fix this.

I wonder whether the frontend should (or even already does) sort
template instances into a tree?

> 
> I'll have to recheck the test case from last week, maybe it's also scope related.

Update: no, that test case was not related to this scoping issue.


*
------------------------------------------------------------------
struct TopLevel(T)
{
    void topFunc()()
    {
        void level2Helper(){} //scope topFunc

        void level2Func()() //scope topFunc
        {
            void level3Helper(){}  //scope level2Func
            void level3()()  //scope level2Func
            {
                level3Helper();
            }

            level2Helper();
        }
    }
}

TopLevel!int abcd;

On 13 January 2013 16:02, Johannes Pfau <nospam@example.com> wrote:

> Am Sat, 12 Jan 2013 11:57:10 +0100
> schrieb Johannes Pfau <nospam@example.com>:
>
> > A template's toObjFile does nothing, but all my naive attempts to call e.g.
> >
> declaration->isTemplateDeclaration()>onemember->isDeclaration()->toObjFile(false)
> > fail with a strange error.
>
> OK, of course it can't work. toObjFile has to be called on the TemplateInstance, not on the declaration...
>
> All TemplateInstances are currently emitted by Module::genobjfile which loops through all Dsymbols in the module. Therefore the scope for functions in template instances is always the module scope which is incorrect.
>
> A simple approach to fix this is delaying emitting those templates and
> emitting functions first, then change function output to emit template
> instances.
> Here's this simple approach: https://gist.github.com/4524721


 //What should multiobj be?



multiobj is ignored by gdc, but just for consistency reasons, just go with false.


Also, do you find that templates must go after *all* functions have been processed, or is it fine if you just add them to toSymbol()->deferredNestedFuncs for the function they are associated with to be processed immediately afterwards.

Regards,
Iain

Am Sun, 13 Jan 2013 22:54:12 +0000
schrieb Iain Buclaw <ibuclaw@ubuntu.com>:

> 
> multiobj is ignored by gdc, but just for consistency reasons, just go with false.

OK, thanks
> 
> 
> Also, do you find that templates must go after *all* functions have been processed, or is it fine if you just add them to toSymbol()->deferredNestedFuncs for the function they are associated with to be processed immediately afterwards.

That would be fine in theory, as far as I understand this issue. But we also have to emit class/struct/interface template instances at that point, not only function template instances, so adding them to deferredNestedFuncs is not practical.

(They must be emitted from the associated function as they have to
get the scope of that function)


OK, the frontend is more intelligent than I first though. If a template instance is directly inside a struct, class or interface, the instance is not added to the module-level members, it's added to the class/struct/interface members instead. This means in those cases the emission order / scope is already correct. It also means that templates nested in templates etc need no additional work.

So it's only template instances nested in functions which needs some special handling and the simple approach I showed should be mostly working.

Then there's this case:

struct S(T)
{
    void abcd()
    {
        void getChar()()  //goes to module members array
        {

        }
        getChar();
    }
}

S!int instance; //Goes to module members array

Although emitting S!int now also emits getChar with my changes (so
correct scope), there's an issue if getChar is first emitted from the
module level members array and then S!int (so wrong scope).

I think the solution is to only emit templates that have .parent == module. It's important that we still emit all templates, but I think emitting class/interface/struct/function nested + those nested directly in module should cover all templates?


I'll see if those changes pass the test suite and file a pull request if it works as expected.