generate TypeInfo for structs only on demand

[WalterBright]

The original code always generates them when a struct declaration was found. But they were also generated again by other object modules whenever the typeinfo was needed. This relied on COMDATs to cull the duplicates. But with such, the typeinfo does not need to be generated just because the struct is declared - only when it is actually used.

[rainers]

The good thing about emitting it with the struct is that it is more likely to contain correct RTInfo. (See #2480 for an old attempt at fixing this). You might want to add a warning/error at https://github.com/dlang/dmd/blob/master/src/todt.d#L1374 checking for unevaluated RTInfo to see if the number increases.

[WalterBright]

@rainers I thought that was the point behind the various calls to semanticTypeInfo() in the front end. This shouldn't be an issue anymore.

[rainers]

I thought that was the point behind the various calls to semanticTypeInfo() in the front end. This shouldn't be an issue anymore.

Yes, but you have to perfectly (and sometimes pessimistically) predict what the glue layer will do. Just building phobos fails the suggested check for core.stdc.stdio._iobuf (I haven't analyzed where it is invoked from).

[WalterBright]

Please be exact about what you suggest as the check, and I'll add it to this PR.

[WalterBright]

Also, I see the rtinfo is calculated in semantic3() of structs. Is there any reason it cannot be done in semantic2() ?

[JinShil]

But with such, the typeinfo does not need to be generated just because the struct is declared - only when it is actually used.

When the struct is used, or when the typeinfo is used? I hope it's the latter.

[rainers]

Please be exact about what you suggest as the check, and I'll add it to this PR.

Sorry, the link above got invalid due to file moves. Here's a diff for the suggested check:

diff --git a/src/ddmd/todt.d b/src/ddmd/todt.d
index 65dc115..ee2921c 100644
--- a/src/ddmd/todt.d
+++ b/src/ddmd/todt.d
@@ -1379,6 +1379,8 @@ extern (C++) class TypeInfoDtVisitor : Visitor
         }
 
         // xgetRTInfo
+        if(!sd.getRTInfo)
+            error(sd.loc, "ICE: RTInfo not evaluated for %s", sd.toChars());
         if (sd.getRTInfo)
         {
             Expression_toDt(sd.getRTInfo, dtb);

Also, I see the rtinfo is calculated in semantic3() of structs. Is there any reason it cannot be done in semantic2() ?

I suspect it very much depends on what happens during RTInfo evaluation. Right now, it's very simplistic (always null), dlang/druntime#1603 will make it a little more complicated, but doesn't need a lot of more introspection than the trait getPointerBitmap. The original version of the precise GC created the pointer bitmap through CTFE and that was very brittle.

[rainers]

When the struct is used, or when the typeinfo is used? I hope it's the latter.

The latter, but with some pessimistic guesses regarding glue layer lowerings.

[rainers]

BTW: Kenji once proposed the opposite than this PR: only generate the type info with the struct. #3958. This requires that all files with struct declarations are actually compiled, not merely imported.

[WalterBright]

It turns out that TypeInfo for structs is rarely needed, but always generated and put in the executable. D is slowly evolving towards needing it even less. My goal here is to generate it only when actually required.

However, it could be computed eagerly, just not written to the object file.

@rainers I added the check you proposed. Thanks!

[WalterBright]

I moved the computation of getRTInfo into semantic2().

[rainers]

I moved the computation of getRTInfo into semantic2().

I tried this PR with dlang/druntime#1603 and it seemed to compile ok (though I've disabled the precise GC).

With RTInfo as in https://github.com/rainers/druntime/blob/gcx_precise/src/gc/gctemplates.d, the linker complains about undefined symbols on every test file (on symbols used during CTFE). That only happens for some with master (everything until inner.d compiles).

[WalterBright]

@rainers Can you please be more specific? Hopefully with a small test case?

[rainers]

Try this branch of druntime: https://github.com/rainers/druntime/tree/gctemplates with RTInfo from the precise GC presented at DConf2013.
Building the tests yields:

Running runnable tests
 ... runnable\A16.d                  -g ()
Test failed.  The logged output:
c:\s\d\rainers\windows\bin\dmd.exe -conf= -m32 -Irunnable  -g  -od..\..\gen\test_results_32\runnable -of..\..\gen\test_results_32\
runnable\A16_0.exe runnable\A16.d runnable\imports\A16a.d -map nul.map
OPTLINK (R) for Win32  Release 8.00.17
Copyright (C) Digital Mars 1989-2013  All rights reserved.
http://www.digitalmars.com/ctg/optlink.html
..\..\gen\test_results_32\runnable\A16_0.obj(A16_0)
 Error 42: Symbol Undefined _D2gc11gctemplates32__T17mkBitmapCompositeTC6ObjectZ17mkBitmapCompositeFNaNbNiNfPkkZv
Error: linker exited with status 1```

[rainers]

It seems the additional -g is necessary to trigger the error for A16.d and many other. -allinst helps until the test eh.d.

[WalterBright]

Can you please go a bit further and have a minimal RTInfo template that produces the error?

[rainers]

Sorry for the late reply, I've been busy with non-D stuff.

Most errors are again -dip1000 issues. Most tests build if druntime is built with -dip25 instead. Maybe related discussion: #5855 (comment)

inner.d still fails with RTInfo defined by

static import gc.gctemplates;
template RTInfo(T)
{
    enum RTInfo = gc.gctemplates.RTInfoImpl!T;
}

and gc/gctemplates.d as

module gc.gctemplates;

template RTInfoImpl(T)
{
    enum RTInfoImpl = RTInfoImpl2!T.ptr;
}

template RTInfoImpl2(T)
{
        immutable RTInfoImpl2 = bitmap!T;
}

size_t[1] bitmap(T)()
{
    size_t[1] A;
    return A;
}

inner.d is reduced to:

static if (is(typeof(new class {}))) {}

to produce

C:\tmp>dmd inner.d -main
OPTLINK (R) for Win32  Release 8.00.17
Copyright (C) Digital Mars 1989-2013  All rights reserved.
http://www.digitalmars.com/ctg/optlink.html
inner.obj(inner)
 Error 42: Symbol Undefined _D2gc11gctemplates39__T11RTInfoImpl2TC5inner12__anonclass1Z11RTInfoImpl2yG1k
Error: linker exited with status 1

I see tests link11931.d, link2500.d and a couple more understandable failures too.

[WalterBright]

Thanks! In investigating this, I've found some mass confusion in the front end about when semantic processing is done. Unfortunately, this PR needs that to be straightened out. In particular, attempting to run semantic3() when there's no scope available. I've been submitting PRs to gradually fix this problem, but it'll take a while.

[dlang-bot]

Thanks for your pull request, @WalterBright!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

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

[andralex]

@WalterBright this seems to be in good shape now, good to go?

[JinShil]

"ICE:"? Is this just a test?

[JinShil]

@WalterBright What's the state of this PR? Was it just an experiment?

[wilzbach]

(rebased to master to see what's failing now)

[andralex]

@WalterBright status?

[ghost]

with the current work on making TypeInfo templates hooks I think this can be closed. Do you agree ?