Fix Issue 21164 - segfault on incomplete static if

[UplinkCoder]

With this change, when evaluating a static if, we now check for the existence of an static if
condition and will only proceed if there is one.

[MoonlightSentinel]

Why is static if} not a parser error in this particular example?

[UplinkCoder]

@MoonlightSentinel it is.
extra-files/test21164d.d(3): Error: (expression) expected following static if
however since this is triggered within the semantic of function body within a template instance, it does not immediately abort.

[MoonlightSentinel]

[...] it does not immediately abort.

Put that one poorly, I'm not sure why it even attempts to do semantic on AB which caused parser errors. That doesn't sound like a good idea,...

EDIT: Maybe this needs some earlier error propagation?

[UplinkCoder]

Maybe this needs some earlier error propagation?

@MoonlightSentinel perhaps that would be the proper fix.
However I'd like to see if this one will at least compile ... seems like REQUIRED_ARGS doen't work as intended though?

[UplinkCoder]

Thanks @MoonlightSentinel

[Geod24]

As usual, please give a proper title to both PR and commits.
Remember the Contributing guidelines include an article about good commit messages.

[ghost]

ping @UplinkCoder, can you polish the formal aspect of this PR ? this should not take more than 5 minutes.

[UplinkCoder]

@Geod24 What exactly would you want to read, as far as I can see the information needed is in the commit message.

[ghost]

The message of the first commit must be fix issue 21164 - segfault on incomplete static if. Then the title of the PR should also be set to that, for now it's not descriptive. It's a bit like a click bait.

[UplinkCoder]

Aye I can do that

…

On Fri, Sep 4, 2020, 8:11 PM Nils Lankila ***@***.***> wrote: The message of the first commit must be fix issue 21164 - segfault on incomplete static if. Then the title of the PR should also be set to that, for now it's not descriptive. It's a bit like a click bait. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#11585 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAVWSCHN7WDBCQETMCFWHNTSEEUUXANCNFSM4QBYZAUA> .

[Geod24]

@UplinkCoder : Come on, I even linked an article. All one have to do is to read it.

[dlang-bot]

Thanks for your pull request, @UplinkCoder!

Bugzilla references

Auto-close	Bugzilla	Severity	Description
✓	21164	normal	segfault on incomplete static if

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 "stable + dmd#11585"

[UplinkCoder]

@Geod24 I assume you mean the line length, fixed now.

[UplinkCoder]

@MoonlightSentinel this fails on windows because of the different path separators, in the error message.
How do I fix this?

[MoonlightSentinel]

Seems like a bug with the new path handling. You could omit the additional import path and use imports.xxxx directly (it still segfaults).

EDIT:
It failed to change

fail_compilation/imports\test21164d.d(3): Error: (expression) expected following `static if`

into

fail_compilation\imports\test21164d.d(3): Error: (expression) expected following `static if`

instead.

I'll have a look.

[wilzbach]

Seems like a bug with the new path handling. You could omit the additional import path and use imports.xxxx directly (it still segfaults).

EDIT:
It failed to change

fail_compilation\imports\test21164d.d(3): Error: (expression) expected following `static if`

into

fail_compilation/imports\test21164d.d(3): Error: (expression) expected following `static if`

I'll have a look.

I had a look to and I couldn't reproduce it locally.

[UplinkCoder]

This is worrying.
The fix is rather important.

[MoonlightSentinel]

I would suggest using the workaround until we figure out what went wrong here.

Also, did you find anything regarding the missing error propagation?

[UplinkCoder]

I would suggest using the workaround until we figure out what went wrong here.

Done.

Also, did you find anything regarding the missing error propagation?

Unfortunately no.

[MoonlightSentinel]

Actually, DMD's completely random path handling on windows is to blame here.

d_do_test correctly translates the expected output:

fail_compilation/imports/test21164d.d(3): Error: (expression) expected following `static if`
fail_compilation/imports/test21164d.d(3): Error: found `}` instead of statement
fail_compilation/test21164.d(11): Error: template instance `test21164a.D!(R!(O(), 1))` error instantiating

into

fail_compilation\imports\test21164d.d(3): Error: (expression) expected following `static if`
fail_compilation\imports\test21164d.d(3): Error: found `}` instead of statement
fail_compilation\test21164.d(11): Error: template instance `test21164a.D!(R!(O(), 1))` error instantiating

But dmd seems to copy the import path, hence keeping the / on windows ...

[UplinkCoder]

@thewilsonator can you slap an auto-merge on?

[Geod24]

Rebased & squashed, retargeting stable so you get it in the next release.

[UplinkCoder]

Thanks @Geod24!

[UplinkCoder]

Auto-merge toggled on

[UplinkCoder]

The mac failure on the autotester is unrelated.

[thewilsonator]

autotester is required on stable.

[wilzbach]

Hold your horses. Auto-tester will restart the build automatically.

[UplinkCoder]

The build on osx just timed out a second time.
What's going on?