Issue 14875 & 14876 - improve deprecation message diagnostic

[9rnsr]

Issue 14875 - A template instance with deprecated symbol/type needlessly repeats "Deprecation:" messages
Issue 14876 - Deprecation message is sometimes duplicated

And, 14875 is the root problem of issue 13738 (its fix is added by #4147). After the fix, we can remove the RTInfo specific code.

[WalterBright]

This is a bit hackish. It relies on the presumption that TypeSArray::resolve() is only called with next as the this pointer, making the logic very hard to follow.

[9rnsr]

I'm not sure what's the hackish.

If a part of a type (eg. TypeSArray::next) is determined to be valid (== semantic analysis is succeeded),
to store back it again in the part of whole expression (eg. TypeSArray) is completely valid.

In Type(SArray|Darray|AArray|Pointer|Delegate)::semantic(), same logic is actually used.

Type *TypeSArray::semantic(Loc loc, Scope *sc)
{
    ...
    Type *tn = next->semantic(loc, sc);
    if (tn->ty == Terror)
        return terror;

    ...
    this->next = tn;
    ...
    t = this->addMod(tn->mod);
    return t->merge();
}

On the other hand, if next->deco is alredy set, next->resolve(.., &t, ..) must return the next itself to t.
It's guaranteed by the Type::merge()logic, so it's definitely reliable.

[9rnsr]

Perhaps you're worried the case that the resolve() call happens with error gagging?
That's not a problem. As you see, if *pt is Terror, it won't be stored back to next, then the error object won't leak out of the gagged scope.

[WalterBright]

I'll look at it again and see if I can figure out a better way.

[WalterBright]

What I'm concerned about is much more meta. There doesn't seem to be a clear separation and encapsulation of processing in the compiler. It's become a mass of ad-hoc patches with no discernible organization. The symptom of this is the parade of regressions introduced by bug fixes. I'm afraid of it becoming completely intractable. We need to adopt a more principled approach. (The switch to ddmd will help a lot with this.)

[9rnsr]

Indeed this change looks hacky - but this change follows current compiler design.

Currently (Expression|Type|Dsymbol)::semantic() step will cause two side effects at least - the deprecation and information warning message. They don't affect the code meaning. Then, if semantic() step is invoked twice on same AST entity, they would make duplicated console outputs. To avoid the issue, we should cache the valid semantic result in each steps. After that, the repeated semantic analysis on the cached AST entity will be stopped by the previous flagging, that is (setting Expression::type to non-null | merge Type objects and set Type::deco to non-null | the increment of Dsymbol::semanticRun value).

In here, if the semantic result in *pt is valid type, it is cached into this->next to prevent rerunning semantic and messaging side effects. From that reason, this change is clean fix to me.

And, it's free that you think the better dmd internal design. Most of my bugfix work in several years was to fill holes in the well designed but rough implemented compiler internals. Honestly, I cannot say I'm genious architect programmer to design enough clean & efficient compiler process. Better design is welcome.

[WalterBright]

Unfortunately, I'm going to have to concede the point here.

[WalterBright]

The fix for 14876 seems to be completely independent of the rest.

[9rnsr]

Not directly related with, but necessary to test the diagnostic message in the rest fix.

[WalterBright]

Auto-merge toggled on

[9rnsr]

Thanks.