Specify that the struct invariant may be violated when the destructor is called

[FeepingCreature]

Specify that the struct invariant may be violated when the destructor is called, because the struct may be in its T.init state.
See also Issue #19065, PR #19037.
Note that moveEmplace already implicitly assumes that this is the case! Otherwise, there'd be little sense in it resetting its source to T.init if it has an elaborate destructor.

The underlying problem is that struct destructors are the one struct lifetime method that is always, unavoidably called by the compiler and cannot be bypassed. (this() can be bypassed with T.init, this(this) can be bypassed with moveEmplace.)

[dlang-bot]

Thanks for your pull request and interest in making D better, @FeepingCreature! 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 annotated coverage diff directly on GitHub with CodeCov's browser extension
• 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.

[FeepingCreature]

The deeper reasoning behind this is that since we allow t = T.init in @safe code, this is equivalent to me to admitting that struct variables may at any time either be an initialized struct value or T.init. Since struct destructors are called on variables regardless of whether they're properly initialized or not, struct destructors must be able to handle both T.init and T(...).

[dukc]

I think this should be done EVEN if there wasn't the case with .init values: The last public call checks the same value anyway. thus a check on destructor is waste of computational power.

[FeepingCreature]

Are you sure this commit is still correct! The diff looks weird.

edit: Please at least make it a 72h merge so people can look it over again? It's been ages, and Nullable - the reason I pushed this - doesn't even trigger the destructor anymore.

edit: Note that the "disable struct destructor" commit that I pushed above did not get merged!

edit: Thanks.

[thewilsonator]

It looks fine to me. Done. Ideally the compiler should insert a unittest to make sure that the destructor hanles the case in (T.init).~this().

[FeepingCreature]

To copypaste the reasoning from the Nullable debate: the problem is that this code becomes impossible:

struct S {
  Object o;
  invariant (o !is null);
   // also just pretend there's another variable here that has an elaborate destructor.
  @disable this();
  this(Object o_) in (o_ !is null) { this.o = o_; }
}

In case it was unclear, this is a really common pattern in our codebase. And with existing semantics, S.init is very much not something that you can use - becauseS.init.~this() will always crash.

My plan for this was to disable invariant checking on destructor, but that code did not get merged - see the referenced PR for the debate. So this PR is left somewhat dangling.

I ended up closing that PR in the end because the horrible union hack provided a better solution for Nullable - see the Turducken Technique thread on the newsgroup.

edit: Resurrected the debate thread.

[thewilsonator]

... so ah, 3 days later ...

[thewilsonator]

Should this be merged or closed?

[FeepingCreature]

Yeah, this can be removed. The change in question was never implemented, and the union hack has proven a viable - if exceedingly dirty - workaround.

[thewilsonator]

Thanks.