August 21, 2010
Hello, guys!

I've noticed that DMD casts TemplateInstance to Identifier (and vice
versa) in a few places and I doubt its correctness (since they are in
no way related).
It works in DMD (since pointer casts are unchecked in C++) but fails
to do so in DDMD (resulting in a null pointer).

Here is a simple snippet:

Type *Parser::parseBasicType() {
...
tid = new TypeIdentifier(loc, id);
...
TemplateInstance *tempinst = new TemplateInstance(loc, id);
tid->addIdent((Identifier *)tempinst);
...
}

here is another good one:

void TypeQualified::syntaxCopyHelper(TypeQualified *t)
{
    //printf("TypeQualified::syntaxCopyHelper(%s) %s\n", t->toChars(),
toChars());
    idents.setDim(t->idents.dim);
    for (int i = 0; i < idents.dim; i++)
    {
	Identifier *id = (Identifier *)t->idents.data[i];
	if (id->dyncast() == DYNCAST_DSYMBOL)
	{
	    TemplateInstance *ti = (TemplateInstance *)id;

	    ti = (TemplateInstance *)ti->syntaxCopy(NULL);
	    id = (Identifier *)ti;
	}
	idents.data[i] = id;
    }
}

I wonder how correct that is. It looks like TypeQualified.idents can store both Identifiers and TemplateInstances, but why put them into same bag? I believe there should either be 2 distinct methods (TypeQualified::addIdent/TypeQualified::addTemplateInstance) or just one that accepts generic Object. The former also helps getting rid of dyncast in syntaxCopyHelper, but the latter one is plain easier to implement.

Any tip would be very appreciated, thanks!
August 21, 2010
Yes, it's correct, although it's a hack.

Denis wrote:
> Hello, guys!
>
> I've noticed that DMD casts TemplateInstance to Identifier (and vice
> versa) in a few places and I doubt its correctness (since they are in
> no way related).
> It works in DMD (since pointer casts are unchecked in C++) but fails
> to do so in DDMD (resulting in a null pointer).
>
> Here is a simple snippet:
>
> Type *Parser::parseBasicType() {
> ...
> tid = new TypeIdentifier(loc, id);
> ...
> TemplateInstance *tempinst = new TemplateInstance(loc, id);
> tid->addIdent((Identifier *)tempinst);
> ...
> }
>
> here is another good one:
>
> void TypeQualified::syntaxCopyHelper(TypeQualified *t)
> {
>     //printf("TypeQualified::syntaxCopyHelper(%s) %s\n", t->toChars(),
> toChars());
>     idents.setDim(t->idents.dim);
>     for (int i = 0; i < idents.dim; i++)
>     {
> 	Identifier *id = (Identifier *)t->idents.data[i];
> 	if (id->dyncast() == DYNCAST_DSYMBOL)
> 	{
> 	    TemplateInstance *ti = (TemplateInstance *)id;
>
> 	    ti = (TemplateInstance *)ti->syntaxCopy(NULL);
> 	    id = (Identifier *)ti;
> 	}
> 	idents.data[i] = id;
>     }
> }
>
> I wonder how correct that is. It looks like TypeQualified.idents can store both Identifiers and TemplateInstances, but why put them into same bag? I believe there should either be 2 distinct methods (TypeQualified::addIdent/TypeQualified::addTemplateInstance) or just one that accepts generic Object. The former also helps getting rid of dyncast in syntaxCopyHelper, but the latter one is plain easier to implement.
>
> Any tip would be very appreciated, thanks!
> _______________________________________________
> dmd-internals mailing list
> dmd-internals at puremagic.com
> http://lists.puremagic.com/mailman/listinfo/dmd-internals
>
>
>