Bug 12287 – infinite loop on std.traits.moduleName on templated struct member
Status
RESOLVED
Resolution
FIXED
Severity
major
Priority
P2
Component
dmd
Product
D
Version
D2
Platform
All
OS
All
Creation time
2014-03-02T12:55:00Z
Last change time
2014-06-15T01:00:42Z
Keywords
pull
Assigned to
nobody
Creator
oivind.loe
Comments
Comment #0 by oivind.loe — 2014-03-02T12:55:35Z
Compile of the following code never completes. If the struct is not a template, the behavior is as expected. Don't know if this is a DMD or Phobos issue. Running on 2.065.0
import std.stdio;
import std.traits;
struct X(T) {
T i;
}
void main(){
writeln(moduleName!((X!int).i));
}
Comment #1 by dlang-bugzilla — 2014-03-02T13:20:31Z
It looks like the underlying problem is that the parent of a template instance is the template instance itself.
Reduced:
struct X(T) { }
alias Xi = X!int;
static assert(!__traits(isSame, Xi, __traits(parent, Xi)));
Comment #2 by dlang-bugzilla — 2014-03-02T13:42:03Z
I think this only applies to eponymous templates. The parent of the member is the struct declaration, the parent of which is the template instance. But when referring to the template instance of an eponymous template, it is "converted" to the symbol it wraps (the struct declaration), so it never exits the cycle.
Comment #3 by dlang-bugzilla — 2014-03-02T13:58:21Z
(In reply to comment #2)
> I think this only applies to eponymous templates. The parent of the member is
> the struct declaration, the parent of which is the template instance. But when
> referring to the template instance of an eponymous template, it is "converted"
> to the symbol it wraps (the struct declaration), so it never exits the cycle.
No, this is a Phobos bug. I think it's not good to add a special case in __traits(parent) for eponymous templates.
Comment #5 by dlang-bugzilla — 2014-03-02T18:21:23Z
(In reply to comment #4)
> No, this is a Phobos bug. I think it's not good to add a special case in
> __traits(parent) for eponymous templates.
How can you fix it in Phobos, if it is not possible to get the parent of an eponymous template?
Comment #6 by k.hara.pg — 2014-03-02T18:33:03Z
(In reply to comment #5)
> (In reply to comment #4)
> > No, this is a Phobos bug. I think it's not good to add a special case in
> > __traits(parent) for eponymous templates.
>
> How can you fix it in Phobos, if it is not possible to get the parent of an
> eponymous template?
https://github.com/D-Programming-Language/phobos/pull/1978
Comment #7 by dlang-bugzilla — 2014-03-02T18:34:43Z
OK, so you pattern-match the template.
But I don't think this is a good solution, because any recursive use of __traits(parent) will need to take care of this special case. It goes against the principle of least surprise.
What are your arguments against a compiler change?
Comment #8 by k.hara.pg — 2014-03-02T18:46:53Z
(In reply to comment #7)
> OK, so you pattern-match the template.
>
> But I don't think this is a good solution, because any recursive use of
> __traits(parent) will need to take care of this special case. It goes against
> the principle of least surprise.
>
> What are your arguments against a compiler change?
Because your compiler change will make some part of the internal structure of the template instantiation invisible.
Any compiler-side special handlings are unavoidable behavior for the all language users.
So I'm afraid that your change might become harmful blocker for some users.
Comment #9 by dlang-bugzilla — 2014-03-02T18:49:01Z
(In reply to comment #8)
> Because your compiler change will make some part of the internal structure of
> the template instantiation invisible.
I don't understand. Why?
Before my patch: __traits(parent, X!foo) is the same symbol as X!foo. It did not reveal any internal structure at all, the result was completely useless.
After my patch, it produces the result that a casual user may expect from an eponymous template, especially when declared like struct S(Args) {...}
Comment #10 by k.hara.pg — 2014-03-02T19:00:17Z
(In reply to comment #9)
> (In reply to comment #8)
> > Because your compiler change will make some part of the internal structure of
> > the template instantiation invisible.
>
> I don't understand. Why?
>
> Before my patch: __traits(parent, X!foo) is the same symbol as X!foo. It did
> not reveal any internal structure at all, the result was completely useless.
Internally X!foo is exactly same as X!foo.X. So __traits(parent, X!foo) will return the parent of the instantiated type X - that is the TemplateInstance 'X!foo'. They are completely different objects.
But, latter semantic analysis will translate X!foo to X!foo.X again. The behavior is not directly related to the __traits(parent) behavior.
> After my patch, it produces the result that a casual user may expect from an
> eponymous template, especially when declared like struct S(Args) {...}
I think it's not consistent behavior in generic case. For example:
template S(T)
{
struct S {}
int x;
static assert(__traits(isSame, __traits(parent, S),
__traits(parent, x)));
}
alias s = S!int;
Your change will break this case.
Comment #11 by dlang-bugzilla — 2014-03-02T19:03:05Z
(In reply to comment #10)
> But, latter semantic analysis will translate X!foo to X!foo.X again. The
> behavior is not directly related to the __traits(parent) behavior.
The __traits code runs semantic on its result immediately after calculating the parent. So "later" here means "immediately after".
> I think it's not consistent behavior in generic case. For example:
>
> template S(T)
> {
> struct S {}
>
> int x;
> static assert(__traits(isSame, __traits(parent, S),
> __traits(parent, x)));
> }
> alias s = S!int;
>
> Your change will break this case.
I think that makes no sense. In the same spirit, you could argue that since the parent of x is struct S, then x is a member of struct S.
Comment #12 by dlang-bugzilla — 2014-03-02T19:14:12Z
(In reply to comment #10)
> template S(T)
> {
> struct S {}
>
> int x;
> static assert(__traits(isSame, __traits(parent, S),
> __traits(parent, x)));
> }
> alias s = S!int;
>
> Your change will break this case.
I've amended my pull so that it does not break this case. Now the parent of any member of an eponymous template is the template instantiation's parent, not just the eponymous member.
BTW, why is the parent of a template instantiation not the template declaration? Right now it goes right to the module. I think going through the declaration would be more logical.
Comment #13 by k.hara.pg — 2014-03-02T19:41:13Z
(In reply to comment #11)
> The __traits code runs semantic on its result immediately after calculating the
> parent. So "later" here means "immediately after".
The semantic will return ScopeExp, and it still contain the TemplateInstance object. So you are misunderstanding.
> I think that makes no sense. In the same spirit, you could argue that since the
> parent of x is struct S, then x is a member of struct S.
I don't understand what you saying. The parent of struct S and variable x is the template instance S!int. There's nothing else.
Comment #14 by dlang-bugzilla — 2014-03-02T19:53:28Z
(In reply to comment #13)
> The semantic will return ScopeExp, and it still contain the TemplateInstance
> object. So you are misunderstanding.
I just checked again with a debugger and this is not what I am seeing. The returned expression is a TypeExp, with the type being the TypeStruct pointing at the struct of course.
> I don't understand what you saying. The parent of struct S and variable x is
> the template instance S!int. There's nothing else.
Maybe I am wrong but from looking at what is going on with a debugger I don't see that happening.
But even if you are right, isn't the correct fix to move the replacing of the template instance with the eponymous member so that it doesn't happen with chained __traits(parent)?
Your library solution is a crutch; I don't think there should ever be a case where __traits(parent) enters an infinite loop.
Comment #15 by k.hara.pg — 2014-03-02T20:35:34Z
(In reply to comment #14)
> (In reply to comment #13)
> > The semantic will return ScopeExp, and it still contain the TemplateInstance
> > object. So you are misunderstanding.
>
> I just checked again with a debugger and this is not what I am seeing. The
> returned expression is a TypeExp, with the type being the TypeStruct pointing
> at the struct of course.
>
> > I don't understand what you saying. The parent of struct S and variable x is
> > the template instance S!int. There's nothing else.
>
> Maybe I am wrong but from looking at what is going on with a debugger I don't
> see that happening.
Ah, I was wrong. Indeed __traits(parent) will return TypeExp for the instantiated type of the eponymous template.
> But even if you are right, isn't the correct fix to move the replacing of the
> template instance with the eponymous member so that it doesn't happen with
> chained __traits(parent)?
>
> Your library solution is a crutch; I don't think there should ever be a case
> where __traits(parent) enters an infinite loop.
I think the library solution is far better than the hack for __traits(parent).
In other word, the infinite loop is the expected behavior with 2.065 Phobos code.
Comment #16 by dlang-bugzilla — 2014-03-02T20:50:26Z
I use __traits(parent) in my code as well. Just because you added the crutch to one of the uses of __traits(parent) in the standard library doesn't mean that the problem is fixed for everyone. Every user who uses __traits(parent) recursively directly will need to implement this crutch as well, and they will find out about this by having their compiler CRASH or HANG during certain parameters to their templates! I don't understand how you can think that this is acceptable. Unless I am missing something, the advantages of the solution you presented clearly do not outweigh the disadvantages. If you still think your solution is better, please clearly list its advantages and why you think it stands up to my criticism.
Comment #17 by dlang-bugzilla — 2014-03-02T21:00:55Z
If we are to go with a library solution, then parentOf needs to be made public, documented, and advocated in favor of the __traits primitive. And the documentation of the __traits primitive needs to be updated to say that it is unreliable as in certain cases it returns the symbol itself rather than its parent. (What a mess.)
Comment #18 by github-bugzilla — 2014-03-09T23:01:19Z