fix Issue 23598 - Circular reference bug with static if and eponymous…

[WalterBright]

… templates

[dlang-bot]

Thanks for your pull request, @WalterBright!

Bugzilla references

Auto-close	Bugzilla	Severity	Description
✓	23598	normal	Circular reference bug with static if and eponymous templates

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + dmd#14838"

[WalterBright]

It be really nice if the tester would run the compiler test suite before trying to build everything else first.

[RazvanN7]

The failure is not coming from any test. Druntime cannot be built with this patch.

[ibuclaw]

It be really nice if the tester would run the compiler test suite before trying to build everything else first.

Can't run the compiler test suite without druntime.

[RazvanN7]

"

src/core/lifetime.d(130): Error: static assert: "Don't know how to initialize an object of type AssertError with arguments (string, ulong)"

  | src/core/exception.d(873): instantiated from here: emplace!(AssertError, string, ulong)
  | src/core/exception.d(556): instantiated from here: staticError!(AssertError, string, ulong)

"

[WalterBright]

Can't run the compiler test suite without druntime.

Not always, but nearly always one can, by using the previous druntime.

[ibuclaw]

Agreeing with @BorisCarvajal

implemented

[WalterBright]

The buildkite error appears to be here:
https://github.com/jmdavis/dxml/blob/master/source/dxml/parser.d#L3631

The lines don't quite line up with line 3628.

[ghost]

This seems to have fixed the original issue, but it didn't take me long to hit into another one, which will take a while to reduce.

[WalterBright]

Please file separate issues with separate entries in Bugzilla.

[ghost]

Please file separate issues with separate entries in Bugzilla.

If it proves to be a different issue and not another manifestation of this same issue.

[WalterBright]

If the original is fixed, then another manifestation is a different issue. Please do not append more issues to the same issue. It makes things very hard to manage.

[WalterBright]

Related issues can be associated by adding to the [See Also] section.

[ghost]

If the original is fixed, then another manifestation is a different issue. Please do not append more issues to the same issue. It makes things very hard to manage.

The fact that the original test case compiles doesn't necessarily mean the issue has been fixed. That's why I wrote "it seems to have been fixed". If it proves to be a different issue, I will file a different issue. If it's an incomplete or incorrect fix, I will reopen this bug report.

[ibuclaw]

The buildkite error appears to be here:
https://github.com/jmdavis/dxml/blob/master/source/dxml/parser.d#L3631

The lines don't quite line up with line 3628.

You're looking at master, not the 0.4.3 release tag.

https://github.com/jmdavis/dxml/blob/d8eb8cff883d78a6258fc1e829484262c6f009fb/source/dxml/parser.d#L3628

[WalterBright]

If it's an incomplete or incorrect fix, I will reopen this bug report.

I have a lot of experience with this. Please open a new report. The problem with reopening with something else is we lose the correspondence between issues and PRs. Also, what may appear to be the same problem may not actually be the same one.

[WalterBright]

Anyone care to have a look at jmdavis' project to see what the actual problem is?

[ghost]

I have a lot of experience with this. Please open a new report. The problem with reopening with something else is we lose the correspondence between issues and PRs. Also, what may appear to be the same problem may not actually be the same one.

My experience is non-existent compared to yours. I can hardly walk yet. Anyway, this time you are right. It must be a duplicate of multiple other reports: https://issues.dlang.org/show_bug.cgi?id=23646

[WalterBright]

@maxsamukha Thanks for isolating it and filing the report. I'll look into it after this one is fixed.

[WalterBright]

Spent several hours boiling the problem down to this. Couldn't make it smaller.

template isCallable(alias callable)
{
    static if (is(typeof(&callable!())))
        enum bool isCallable = isCallable!(typeof(&callable!()));
    else
        enum bool isCallable = true;
}

string foo();

template FunctionTypeOf(alias func)
if (isCallable!func)
{
    alias FunctionTypeOf = typeof(foo);
}

template ReturnType(alias func)
{
    static if (is(FunctionTypeOf!func R == return))
        alias ReturnType = R;
}

template isAttrRange()
{
    alias NameType  = ReturnType!((string r) => r);
    pragma(msg, "isAttrRange ", NameType, " ", string);
    static assert(is(NameType == string));

    enum isAttrRange = is(NameType == string);
}

static assert(isAttrRange!());

Looks like I'll have to instrument the compiler next to see why string != string.

[WalterBright]

I spun off https://issues.dlang.org/show_bug.cgi?id=23651 which is blocking this PR. The problem is different from the one this PR is trying to solve.

[ghost]

See also https://issues.dlang.org/show_bug.cgi?id=23652

[RazvanN7]

What we are hitting here is a general problem: how are static if declarations supposed to be handled? The answer seems to be: before everything else. If that is the case, maybe we should strive to have a more general solution (a visitor that first looks for static if declarations, or maybe store static if declarations at parse time in a different tree) than just introducing special cases to fix particular bugs.

The more we add ad-hoc fixes the more complicated it's going to be to fix the general case.

[WalterBright]

We've struggled with that problem for a long time. There's no obvious solution that I can think of. In the meantime, we can't let perfection get in the way of progress. At the very least, we are building a test suite of cases that we need to work, to test against any new ideas.