[REG2.067] Issue 14508 - compiling with -unittest instantiates templates in non-root modules

[9rnsr]

https://issues.dlang.org/show_bug.cgi?id=14508

All template instances outside of version(unittest){} or debug(num){} need to use the normal instantiation strategy.

When -unittest or -debug(=num) switches specified, the instantiation strategy will be tweaked - a conditionally instantiated template code by version(unittest){} or debug(num){} will be generated conservatively, even if it's done in non root module.

It can be formalized as follows:

R = templates instantiated in root module
N = templates instantiated in non root module
S = templates instantiated speculatively

• Default instantiation strategy:
  codegen = R - (R ∩ N)
• The new instantiation strategy for -unittest or -debug=num:
  codegen = Rd - (Rd ∩ N)
  Rd = R ∪ Nd
  Nd = version(unittest) and debug template instances in non-root modules
• The old instantiation strategy for -allInst:
  codegen = R ∪ N

[9rnsr]

@WalterBright This is also an answer for the instantiation strategy workaround that you implemented in #2661, to fix issue 11239.
Please review this.

[MartinNowak]

Not sure that this or #2661 are good ideas. You can't generally expect that linking code compiled with different debug/version settings works.

struct Foo
{
    debug size_t cnt;
}
void bar(Foo* foo) { debug ++foo.cnt; }

[9rnsr]

@MartinNowak Yes, it generally doesn't work. But for the instantiated code we can relax the restriction.
Normally we don't provide unittest version library, as such debug/release version libraries. Therefore the relaxation would be useful.

[9rnsr]

Auto-tester now shows all green.

The previous Phobos failure in Win64 was another link-failure issue 14541.
Now this PR also fix it.

[MartinNowak]

Shouldn't the codegen be like this instead?

codegen = R - (R ∩ N \ Nd)

Nd = version(unittest) and debug template instances in non-root modules

[9rnsr]

Pushed additional fix commit. I'll squash it after the review finished.

[9rnsr]

Shouldn't the codegen be like this instead?

By using Nd, it would be formalized like:

  codegen = Rd - (Rd ∩ N)
  Rd = R ∪ Nd
  Nd = version(unittest) and debug template instances in non-root modules

[9rnsr]

Sit, Making instantiation strategy more strict will directly hit the root of issue 12661. We need to fix it radically, at first.

[9rnsr]

OK, I added one more feature "force instantiation mechanism".

---

More feature for root of issue 12661 - Add forceInst mechanism

• If an instantiated function contains debug{} or version(unittest){}, the instance code will be forcibly generated.
• If an instantiated function calls other forcibly instantiated function and its attribute is influenced by inference, that code is also generated forcibly.

[WalterBright]

I'm not in favor of reordering code as in f0080e8 because it makes it very difficult to compare code that crosses such a diff. Can you please back that out?

[MartinNowak]

I'm a bit worried, that doing it transitively will generate an awful amount of code.
Using version (unittest) or debug (PRINTF) in a template is not that uncommon.
Might be worth to think about alternatives, like shipping a debug phobos library.

If an instantiated function contains debug{} or version(unittest){}, the instance code will be forcibly generated.

How can you know that function contains such conditions before instantiating it?
Is this just looking at the AST?

[MartinNowak]

I also have a suggestion to fix part of the instantiation bloat.
https://github.com/D-Programming-Language/dmd/pull/4384/files#r29910422.

[WalterBright]

Does this resolve the compilation slowdown caused by #4384 ?

[WalterBright]

I suspect the rule we should be using is that all templates in non-root modules should be instantiated lazily, meaning only if a particular instantiation is actually needed. No eager instantiations. Thus certainly would resolve the particular case in https://issues.dlang.org/show_bug.cgi?id=14508

I did this for enum members a while back, and the results have been very satisfactory. It also resolves order-of-evaluation issues.

[9rnsr]

I'm a bit worried, that doing it transitively will generate an awful amount of code.
Using version (unittest) or debug (PRINTF) in a template is not that uncommon.

An alternate suggestion would be to change the behavior of -unittest and -debug switches. For example:
"If a template which defined in non-root module is instantiated, unittest {} blocks and version(unittest){} conditions in it are just ignored even if -unittest switch is specified. Same strategy will be applied to -debug."

But, it would prohibit to define a module just for unit-testing/debug utilities. I think it would need a design decision to the D module system.

Might be worth to think about alternatives, like shipping a debug phobos library.

It would be horrible idea. How many combinations would be necessary for the user code conditional compilations?
phobos.lib phobos_unittest.lib phobos_debug1.lib phobos_debug2.lib phobos_debug_PRINTF.lib ...

[9rnsr]

Does this resolve the compilation slowdown caused by #4384 ?

No. It makes more slow down compilation speed to make linking more stable.

[9rnsr]

If an instantiated function contains debug{} or version(unittest){}, the instance code will be forcibly generated.

How can you know that function contains such conditions before instantiating it?

There's no general way, so a condition can depend on the semantic result. See the following example code:

// this function template may or may not depend on -unittest
void foo(string code)() {
    mixin(code);
}
foo!"version(unittest) unsafeCheckFunc();"();

[9rnsr]

I removed commits for code cleanup in order to concentrate on bug fix.

[MartinNowak]

I suspect the rule we should be using is that all templates in non-root modules should be instantiated lazily, meaning only if a particular instantiation is actually needed. No eager instantiations. Thus certainly would resolve the particular case in https://issues.dlang.org/show_bug.cgi?id=14508

See my comment here, #4384 (comment).
We should establish a rule which module is responsible for instantiating a template if that's ambiguous (e.g. mutual import). A simple rule would be to use the module with the lexicographically smaller module name.

[MartinNowak]

I suspect the rule we should be using is that all templates in non-root modules should be instantiated lazily

Sounds good, but is that feasible for the next release? It wouldn't solve linkage issues when the template is used though.

An alternate suggestion would be to change the behavior of -unittest and -debug switches. For example:
"If a template which defined in non-root module is instantiated, unittest {} blocks and version(unittest){} conditions in it are just ignored even if -unittest switch is specified. Same strategy will be applied to -debug."

That would actually solve the problem we had with MonoTimeImpl and vibe.d.

But, it would prohibit to define a module just for unit-testing/debug utilities. I think it would need a design decision to the D module system.

How is that? You mean for separate compilation?
That build mode is mostly used to mitigate the CTFE mem consumption.

[WalterBright]

Does this resolve the compilation slowdown caused by #4384 ?
No. It makes more slow down compilation speed to make linking more stable.

There's got to be a better answer than a 30% slowdown. What about the idea of lazy instantiation?

[9rnsr]

Superseded by: #4784