PoC DFA engine for nullability and truthiness

[rikkimax]

Apart from one CI that was timed out, it was green in previous PR: #21494

[dlang-bot]

Thanks for your pull request and interest in making D better, @rikkimax! 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

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

[rikkimax]

Will retrigger after: #21966

[thewilsonator]

dependant PR merged

[thewilsonator]

Calling it fast DFA implies there is or will be a slow one. Is there going to be a slow one or can we move all of the .d files in dfa/fast into dfa and remove the fast folder?

[rikkimax]

Calling it fast DFA implies there is or will be a slow one. Is there going to be a slow one or can we move all of the .d files in dfa/fast into dfa and remove the fast folder?

If it succeeds, then yes, I intend to also implement a slow DFA.

The slow DFA is meant to be more false positive heavy, but also more accurate. Requiring the use of attributes and changing of code to satisfy the DFA. This would be opt-in.

The fast DFA is meant to be free of false positives, but not necessarily able to model all code. No attributes or changing of code is meant to happen unless it is in some way bad. This would be on by default.

Different target audiences. So they are quite opposite of each other.

[rikkimax]

Why hasn't this been merged?

[thewilsonator]

Well it still has PoC in the title of the PR for one, I take it that it is good to go now?

[rikkimax]

Yes.

It is PoC since no attributes, and missing features rather than PR being incomplete for its scope.

[rikkimax]

Okay something changed since the testsuite ran.

src\dmd\dfa\fast\expression.d(1612): Error: no property `toBool` for `ie` of type `dmd.expression.IntegerExp`

                    auto b = ie.toBool;

[thewilsonator]

Oh, you want toBool(ie) there

[rikkimax]

It was moved out of being a method, and put as a free-function in expressionsem, fixing.

[thewilsonator]

Of you way need to import dmd.expressionsem;

[nordlow]

This is awesome! How come this was approved so quickly? Are we suddenly more liberal in adding new -preview features in dmd or has this always been possible?

[rikkimax]

This is awesome! How come this was approved so quickly? Are we suddenly more liberal in adding new -preview features in dmd or has this always been possible?

Sort of.

I proposed adding it under a preview switch a few months ago, but I placed some rules upon myself to minimize risk:

1. No AST modifications, however I do intend to set flags like this will be non-null so can elide on null dereference throw an Error (upcoming work).
2. No attributes, there will be attributes in the future if this works out, and I intend to have a @__borrows defined in core.attribute just to test that the borrow checker works, however thats a fair bit out from where I am today.

This approach has been applied to Walter's work with the ARM backend. I also got permission for Stefan, to do the same for newCTFE but he hasn't done a PR yet. Better to merge something that is low risk under a preview switch than to see 6+ months worth of work wasted, times X.

And thanks :)

[nordlow]

This is awesome! How come this was approved so quickly? Are we suddenly more liberal in adding new -preview features in dmd or has this always been possible?

Sort of.

I proposed adding it under a preview switch a few months ago, but I placed some rules upon myself to minimize risk:

1. No AST modifications, however I do intend to set flags like this will be non-null so can elide on null dereference throw an Error (upcoming work).

2. No attributes, there will be attributes in the future if this works out, and I intend to have a `@__borrows` defined in core.attribute just to test that the borrow checker works, however thats a fair bit out from where I am today.

This approach has been applied to Walter's work with the ARM backend. I also got permission for Stefan, to do the same for newCTFE but he hasn't done a PR yet. Better to merge something that is low risk under a preview switch than to see 6+ months worth of work wasted, times X.

And thanks :)

This is fantastic news. Thanks for doing this. I too did experiment with dmd detection of unused variables years ago. What about Stefans' work on type functions?

[nordlow]

What about extending this DFA to also cover referencing of non-intialized variable values or pointes in cases such as

int *x = void;
auto y = *x; // should error

?

[rikkimax]

This is fantastic news. Thanks for doing this. I too did experiment with dmd detection of unused variables years ago. What about Stefans' work on type functions?

If I remember right he gave up on it, and it is full of risk to try to do something like this with that.

What about extending this DFA to also cover referencing of non-intialized variable values or pointes in cases such as

int *x = void;
auto y = *x; // should error

?

Originally I wanted to model type state as a whole, with nullability being a later value after uninitialized.

I may yet decide to implement it.

For now though it's very incremental in development, and I need to see it succeeding before using more of my time on it.

[nordlow]

Do you have a list of upcoming diagnostics that make use of the infrastructure added here? Just curious which of these that overlap with my pre-existing work and feature requests.

[rikkimax]

I do not no, I have goals but the exact order isn't known to me currently.

But the end goal is to try to get a borrow checker akin to what people expect. Anything else is a nice (but intended) bonus.

[ibuclaw]

This PR introduced a regression

Segmentation fault
...
0x11ed471 Semantic3Visitor::visit(FuncDeclaration*)
	dmd/semantic3.d:1165
0x11e9dcf _D3dmd9semantic3QkFCQs7dsymbol7DsymbolPSQBm6dscope5ScopeZv
	dmd/semantic3.d:90
0x11ea1f6 Semantic3Visitor::visit(Module*)
	dmd/semantic3.d:210
0x11ea1f6 Semantic3Visitor::visit(Module*)
	dmd/semantic3.d:192
0x11e9dcf _D3dmd9semantic3QkFCQs7dsymbol7DsymbolPSQBm6dscope5ScopeZv
	dmd/semantic3.d:90
...

dmd/compiler/src/dmd/semantic3.d

Lines 1164 to 1166 in 24a4107

	auto ls = new LabelStatement(Loc.initial, Id.returnLabel, fens);
	funcdecl.returnLabel.statement = ls;
	a.push(funcdecl.returnLabel.statement);

[ibuclaw]

... it would seem this is the regressing line.