Fix Issue 17307 - [REG2.072.0][ICE] TypeBasic::implicitConvTo

[JinShil]

This fixes the ICE, but there may still be an underlying problem with the semantics.

Output for...

struct { enum bitsPerWord = size_t; }

void main()
{ }

... is now...

main.d(1): Error: anonymous struct can only be a part of an aggregate, not module main
main.d(1): Error: cannot interpret ulong at compile time

I'd be happy to add a test case, but I first need confirmation that that is the expected output.

[dlang-bot]

Thanks for your pull request, @JinShil! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.

Some tips to help speed things up:

• smaller, focused PRs are easier to review than big ones

• try not to mix up refactoring or style changes with bug fixes or feature enhancements

• provide helpful commit messages explaining the rationale behind each change

Bear in mind that large or tricky changes may require multiple rounds of review and revision.

Please see CONTRIBUTING.md for more information.

Bugzilla references

Auto-close	Bugzilla	Description
✓	17307	[REG2.072.0][ICE] TypeBasic::implicitConvTo: After error "anonymous struct can only be a part of an aggregate"

[PetarKirov]

The error message sounds correct to me, as D's grammar doesn't support neither inline typedefs, nor inline struct variables (declaring a structure and variables of that type in a single declaration). So really there's another way to use anonymous structs or unions in D, except as members of other aggregates.

[rainers]

I think this is fighting the symptoms, but a type pointer null will cause a lot of trouble elsewhere, too. I think a better fix might be to not stop semantic analysis if "anonymous struct can only be a part of an aggregate" has been detected, i.e. remove the return here: https://github.com/dlang/dmd/blob/stable/src/ddmd/attrib.d#L805

[rainers]

You should also add the test somwhere to the fail_compilation test suite.

[JinShil]

I think a better fix might be to not stop semantic analysis if "anonymous struct can only be a part of an aggregate" has been detected, i.e. remove the return here: https://github.com/dlang/dmd/blob/stable/src/ddmd/attrib.d#L805

Done

You should also add the test somwhere to the fail_compilation test suite.

Done

[rainers]

Thanks.

It's a bit annoying that code coverage marks this PR as bad as it just removes a line. The percentage drops indeed, but the number of uncovered lines stays the same.

[WalterBright]

The theory of dealing with compilation errors is not to attempt to recover from them, but to 'poison' anything that depends on an AST node that has an error in it. This is the rationale of TypeError and similar in the compiler. Recovery attempts (i.e. guess what the user intended and attempt to repair the error) rarely work out well, and result in cascaded misleading error messages.

This PR is trying to recover from the error by putting the erroneous AnonDeclaration back into a known state, which later passes depend on.

A fix more in keeping with the error design in the compiler is to 'poison' the AnonDeclaration node by setting AnonDeclaration.errors. Then, when the node is used, errors is checked and the error is propagated instead. Making this work in the most effective manner is to put the errors check inside the various include() member functions, having them return null if errors is set.

[WalterBright]

Set errors instead, per my other comment.

[WalterBright]

@JinShil when submitting a bug fix PR, please note in bugzilla the PR url. This helps prevent someone else from wasting their time preparing a fix for an issue if there already is an outstanding PR. Thanks!

(I already took care of it for this one.)

[JinShil]

A fix more in keeping with the error design in the compiler is to 'poison' the AnonDeclaration node by setting AnonDeclaration.errors. Then, when the node is used, errors is checked and the error is propagated instead. Making this work in the most effective manner is to put the errors check inside the various include() member functions, having them return null if errors is set.

Done. I hope my changes are what you intended.

[WalterBright]

Also need to do the overrides of include().

[JinShil]

Done

[WalterBright]

Good! Well done.