fix Issue 18480 - dmd 2.079 hangs

[timotheecour]

No description provided.

[dlang-bot]

Thanks for your pull request and interest in making D better, @timotheecour! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

• My PR is fully covered with tests (you can see the annotated coverage diff directly on GitHub with CodeCov's browser extension
• My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
• I have provided a detailed rationale explaining my changes
• New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.

---

If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Auto-close	Bugzilla	Severity	Description
✓	18480	regression	dmd 2.079 hangs

[wilzbach]

Thanks, but mind the CI failures + please add a test for it to fail_compilation.

[JinShil]

Why is it not allowed? What's the fundamental issue? Also, isn't there a way to print the actual syntax the user wrote in their source code (s.toChars() maybe), rather than giving them a syntax pattern?

[timotheecour]

should be fixed with latest commit

the issue is to fix the infinite loop.

Also, isn't there a way to print the actual syntax the user wrote

alias X=X is more searchable in bug reports

[wilzbach]

Why is it not allowed? What's the fundamental issue?

See also: #6676 (comment)

[timotheecour]

Thanks, but mind the CI failures + please add a test for it to fail_compilation.

seems to (at least so far) pass now;
not sure how to reduce (haven't tried dustmite on this though)
help welcome if anyone wants to provide a reduced test case

[RazvanN7]

This solves this issue : https://issues.dlang.org/show_bug.cgi?id=18432

[MartinNowak]

This fixes the issue more on the symptom side (by detecting the infinite loop in the weird overload "data structure"), I'd like to spent a bit more time to better understand what's happening (and have a good test case) and to address the issue around AliasDeclaration::overloadInsert where the loop is created.
An interesting aspect, we're checking protection/visibility on the unresolved alias to support public aliases to private symbols.

The current overload structure is unfortunately a horrible implementation that needs a design change and some refactoring. Would be great if we just collected all overloads in a plain array and checked for duplicates during semantic, rather than using those recursive, polymorphic calls to overloadInsert.
Also there are 2 helper classes of little use (FuncAliasDeclaration and OverDeclaration) that are only needed to deal with the fact that overloading aliases depends on whether the aliasee is overloadable.

[timotheecour]

@MartinNowak how about merging this in the meantime to avoid having this bug in git master? one can always revert this once a better solution comes along. This PR doesn't make anything worse but it fixes the above (serious) bug

[JinShil]

Reduced test case

--- moduleA.d
module moduleA;

template TestTemplate() { }

--- moduleB.d
module moduleB;

import moduleA : TestTemplate;
alias TestTemplate = TestTemplate;

--- main.d
import moduleB;

alias TestTemplate = moduleB.TestTemplate;

void main() { }

compile with: dmd main.d moduleA.d moduleB.d

[jacob-carlborg]

@timotheecour 65 commits? I think you made a mistake with Git.

[timotheecour]

@timotheecour 65 commits? I think you made a mistake with Git.

fixed... is targetting dlang:master ok in this PR?

[timotheecour]

@JinShil thanks, added reduced test

[timotheecour]

PTAL, rebased against stable;
GDC fails but seems unrelated to this PR : https://semaphoreci.com/dlang/dmd-2/branches/pull-request-7930/builds/8 (Unknown compiler 'gdc-' ??)

[MartinNowak]

import moduleB; alias TestTemplate = moduleB.TestTemplate; void main() { }

This or any other usage of the alias

import moduleB : TestTemplate;

is sufficient for main.d and just running dmd -c main.d moduleA.d moduleB.d. We just need sth. to check visibility.

[MartinNowak]

Let's go for now.

[timotheecour]

@MartinNowak changed to import imports.test18480a : TestTemplate; and added // REQUIRED_ARGS: -i

[timotheecour]

PTAL, failure in https://ci.dlang.io/blue/organizations/jenkins/dlang-org%2Fdmd/detail/PR-7930/14/pipeline seems irrelevant

[JinShil]

I think the correct place to address this problem is in AliasDeclaration.overloadInsert and check if the overload being inserted will recursively refer to this.

dmd/src/dmd/declaration.d

Line 712 in 75b460e

override bool overloadInsert(Dsymbol s)

I tried to create a fix, but failed. I don't see anyway to compare two symbol instances to see if they are actually pointing to the same thing.

[timotheecour]

how about merging this in meantime and if you find a better fix we can always revert? this bug is serious and we shd fix before upcoming release

[MartinNowak]

The FBSD error is unrelated, seems like Brad is trying to migrate or update the boxes as Github rightfully dropped SSLv23 support.

[wilzbach]

The FBSD error is unrelated, seems like Brad is trying to migrate or update the boxes as Github rightfully dropped SSLv23 support.

Yes, for those interested in following / subscribing / learning more about this -> braddr/d-tester#70

[MartinNowak]

The order of what happens is as follows.

alias sym => template mod.sym from import mod : sym; is added to symbol table.
alias sym => type sym (unresolved) is overloaded with above alias (cause it's aliasee is a template which are overloadable).
type sym gets resolved and finds the overload sym in symbol table, now the second entry in the overload points to the overload itself and mostVisibleOverload happily traverses this loop ad infinitum.

This bug was there since, it has just been additionally exposed now when that loop is selectively imported. Calls to mostVisibibleOverload are inserted into basically every symbol usage though, so it was just dormant.

[MartinNowak]

It's unclear why the alias sym = sym isn't resolved before inserting it in the overload, as only overloadable aliasees should be added AFAIR.
Needs a bit more work, but here is a self-alias detection https://github.com/MartinNowak/dmd/tree/fix18480.