Implement fast DFA's escape analysis

[rikkimax]

Escape analysis for the fast dfa engine based upon my previous DIP design work.

Two relationship strengths are implemented as well as an unknown state.
In outer cell of parameter @escape(var&) + via indirection @escape(var=)

No new attributes for users to use are implemented as that requires DIP.

There is a UDA recognized for specifically named test files, this does not affect the rest of the frontend. It is required for verifying inference.

Currently the engine is forced on for all code, except some specific test suite files, both of this will be disabled prior to merging. Have to verify what the CI as a whole does and that requires this behaviour. Same as previous PRs.

[rikkimax]

Ooo that is a fun little error, because in theory it should be true. But it requires assumptions in the AST.

src/dmd/glue/package.d(914): Error: Assert can be proven to be false
        assert(sthis);
               ^

[rikkimax]

I don't think this should be erroring: EDIT: it shouldn't be, it wasn't gated by unmodellable.

src\core\internal\dassert.d(176): Error: Parameter is required to be scope but escapes
private string miniFormat(V)(const scope ref V v)
                                               ^
src\core\internal\dassert.d(521): Error: template instance `core.internal.dassert.miniFormat!(MonoTimeImpl!(ClockType.coarse))` error instantiating
    alias miniT = miniFormat!T;
                  ^

Ooo another fun one:

  compilable/test14781.d(13): Error: cannot rebind scope variables
              x = 1;      // accessing pure function context is just ok
                ^

I think that is because it's marking x as on stack.

[rikkimax]

Looks like the following code is not seeing the cell like it should.

                *(cast(void[]*)&ret) = _d_newarrayUTrace(__FILE__, __LINE__,
                __FUNCTION__, typeid(E[]), size);

It thinks that its a null deref on ret, which isn't correct.

[rikkimax]

For gcc iasm:

    scope tint32 = new TypeBasic(ASTCodegen.Tint32);
    tint32.merge();
    ASTCodegen.Type.tint32 = tint32;

Doesn't look right:

src/dmd/iasm/gcc.d(580): Error: Escape of unknown lifetime via global
    ASTCodegen.Type.tint32 = tint32;
                           ^
src/dmd/iasm/gcc.d(578):        Pointer stored in variable `tint32` has potentially escaped
    scope tint32 = new TypeBasic(ASTCodegen.Tint32);
          ^

Will need fixing.

That should be allowed due to it being @system but for some reason its not gated.

[rikkimax]

Aw after fixing bugs I've lost Ocean's null deref report that I was quite proud of.

Oh well, at least it was 0 + 0.

I don't think I can bring it back there is a rather large comment at this point showing why it can't be.

[rikkimax]

It looks like I may need to rebase. EDIT: didn't fix it.

compilable\dtoh_ignored.d ~~ is erroring out:~~

..\test\compilable\dtoh_ignored.d(103): Error: internal compiler error: type `ifloat` cannot be mapped to C++

    ifloat onReturn()
           ^

But only after I fix a segfault in dmd.mangle.cppwin:

    extern (D) this(VisualCPPMangler rvl) scope @safe
    {
        this.saved_idents[] = rvl.saved_idents[];
        this.saved_types[] = rvl.saved_types[];
        this.loc = rvl.loc;
        this.eSink = rvl.eSink;
    }

Missing eSink assignment.

I haven't figured out what commit I'm missing because clearly something is different between the two branches allowing this new untested pathway.

EDIT: it is due to mangling happening by the fast dfa engine's entry point for CI purposes, and it's hitting this untested path for the c++ header generator.

[rikkimax]

Oh this gets even more interesting.

If I add -o- the errors go away for dtoh_ignored.d
Due to glue code not calling the mangler.

This is no good at all.

EDIT: #23405

[rikkimax]

Hit a null deref in higgs: https://forum.dlang.org/post/cokarawrrezphlcftdtx@forum.dlang.org filed bug report.

[rikkimax]

dmd's and druntime's testsuites seem to be passing now.

  std/utf.d(2278): Error: Argument is expected to be non-null but was null
          static assert(isSafe!({ S str; size_t i = 0; decode(str, i);      }));
                                                             ^
  std/utf.d(1172):        For parameter `str` in argument 0
  auto ref scope S str, ref size_t index) @trusted pure
                   ^

Just phobos with std.utf that appears to be failing. EDIT: correctly, the test is a CT one that may be resolvable with @__ctfe.

[rikkimax]

Ooo a true positive in sdc:

src/format/rulevalues.d(44,4): Error: Expression reads from an uninitialized variable, it must be written to at least once before reading
            ret.indirect = indirect.dup;
            ^
src/format/rulevalues.d(40,14):        For variable `ret`
        RuleValues ret = void;
                   ^

Same problem as std.json, indirect is a method that will initialize.

[rikkimax]

Another in vibe.d, this time it's a test that won't run.

web/vibe/web/internal/rest/jsclient.d(181,30): Error: Argument is expected to be non-null but was null
        app.generateInterface!DUMMY(null, null, true);
                                   ^
web/vibe/web/internal/rest/jsclient.d(37,24):        For parameter `jsgenset` in argument 2
        JSRestClientSettings jsgenset, bool parent)
                             ^

[github-actions]

DMD perf check

Metric	Base	PR	delta
compile hello.d (instr)	216.0 M	216.0 M	0.00%
compile hello.d -O (instr)	234.5 M	234.5 M	0.00%
compile Phobos (instr)	5,077.5 M	5,078.3 M	+0.01%
compile Phobos codegen (instr)	1,453.3 M	1,453.3 M	0.00%
compile vibe.d (instr)	15,138.5 M	15,139.9 M	+0.01%
dmd binary size (stripped)	6.90 MB	6.94 MB	+0.68%
hello binary size	0.72 MB	0.72 MB	0.00%
peak RSS (compile hello.d)	44 MB	44 MB	+0.11%
peak RSS (compile Phobos)	631 MB	631 MB	+0.01%
peak RSS (compile vibe.d)	1981 MB	1981 MB	+0.04%

[rikkimax]

Okay this is about as good as it's going to get.

[thewilsonator]

why? is the name magic?

[rikkimax]

Yes, hence the comment.

[thewilsonator]

Why? this sounds like a bad idea to have magic for this.

[rikkimax]

It verifies that inference worked correctly.

There is no way to do this without an attribute of some kind and right now, I don't think it should be user-facing.

[rikkimax]

To be specific, this function should never run outside of the CI: https://github.com/dlang/dmd/pull/23314/changes#diff-21bdd9bfc8b25235bf3818d6be16364822ced9b72ed4ff3c894a9e8b8f175e84R327

[thewilsonator]

ditto

[rikkimax]

As far as I'm concerned this is complete at this point, there is nothing else to really do in this PR.