Fix Issue 15459 - [REG2.065.0] stdin.byLine.each!(map!somefunc) compiles, fails to link with ld

[RazvanN7]

enum e = is(typeof(map()));

struct MapResult()
{
    this(char[] a) {}
    
    char front()
    {
        dchar d;
        return d; /* should fail compilation */
    }
}

void map()()
{
    auto mr = MapResult!()([]);
}

void main()
{
    map();
}

When the is(typeof(map()) expression is compiled, map instantiated, however semantic3 is not done on the MapResult template instance. This leads typeof(map()) to return void and is(void) evaluates to true. Later on, when map is used in main, the template instance is not re-analyzed because we already have a "valid" instantiation. When semantic3 is performed this, this time it is also gagged for some reason.

My patch builds on the fact that full semantic needs to be performed in the situations where the root instantiating expression is inside a typeof context. To do that, I am adding a field to the TemplateInstance AST node that keeps track whether the instantiation subtree's root inside a typeof context.

Targeting master as this is a very old regression and the fix might be controversial.

[dlang-bot]

Thanks for your pull request and interest in making D better, @RazvanN7! 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 coverage diff by visiting the details link of the codecov check)
• 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
✓	15459	regression	[REG2.065.0] stdin.byLine.each!(map!somefunc) compiles, fails to link with ld

⚠️⚠️⚠️ Warnings ⚠️⚠️⚠️

• Regression or critical bug fixes should always target the stable branch. Learn more about rebasing to stable or the D release process.

To target stable perform these two steps:

1. Rebase your branch to upstream/stable:

git rebase --onto upstream/stable upstream/master

2. Change the base branch of your PR to stable

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#13359"

[RazvanN7]

Since semantic3 is made eagerly on template instances coming from typeof expressions, this is now known earlier, however, I don't really get the changes.

[BorisCarvajal]

Note: __traits(compiles) has the same problem.

I don't remember from what issue I ended up trying to solve the same bug but the solution that I came up with (*) has the same failure as yours (in checkedint.d), a speculative instantiation is not marked as such.
It seems it happens when some lazy semantic is run via the deferred field in TemplateInstance.

(*) reduced fix:

diff --git a/src/dmd/dscope.d b/src/dmd/dscope.d
index 42c0d18b2..064acd7f5 100644
--- a/src/dmd/dscope.d
+++ b/src/dmd/dscope.d
@@ -75,7 +75,7 @@ enum SCOPE
 private enum PersistentFlags =
     SCOPE.contract | SCOPE.debug_ | SCOPE.ctfe | SCOPE.compile | SCOPE.constraint |
     SCOPE.noaccesscheck | SCOPE.onlysafeaccess | SCOPE.ignoresymbolvisibility |
-    SCOPE.printf | SCOPE.scanf | SCOPE.Cfile;
+    SCOPE.printf | SCOPE.scanf | SCOPE.fullinst | SCOPE.Cfile;
 
 struct Scope
 {

[maxhaton]

I think there are a few duplicates of the bug being fixed here. If I can find them I will see if they really are dupes.

The thing I observed debugging a similar issue with a gagged is(typeof(...)) issue is that the compiler bails out of the semantic analysis way too early. Initially it seems like the compiler is gagging too aggressively (-verrors=spec makes it seem that way), but it turns out that as you mention above the semantic analysis is never run. Instead it's deferred to the end of compilation, as a gagged template instantiation, where it silently fails - at very least I suppose we could warn if this template instantiation fails.

Short of actually making dmd handle this stuff properly, is there/can we come up with a way of elegantly forcing (i.e. it runs all the analysis or it's an error) semantic analysis? Some kind of fence-like abstraction if you will. Hopefully patches like this can get most of the pain, but the fact that this can actually happen is not encouraging.

[RazvanN7]

Maybe the recent template emission fixes will make this work.