View mode: basic / threaded / horizontal-split · Log in · Help
January 12, 2013
Another verify_cgraph problem
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.
January 13, 2013
Re: Another verify_cgraph problem
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;
January 13, 2013
Re: Another verify_cgraph problem
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
January 14, 2013
Re: Another verify_cgraph problem
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.
Top | Discussion index | About this forum | D home