fix Issue 19463 - DIP1008 - _d_newclass is called instead of _d_newTh…

[WalterBright]

…rowable

Well, this is embarrassing. I must have never checked that code in. The trouble is, I'm not sure how to test it, other than manually checking the output which I've done.

[dlang-bot]

Thanks for your pull request, @WalterBright!

Bugzilla references

Auto-close	Bugzilla	Severity	Description
✓	19463	major	DIP1008 - _d_newclass is called instead of _d_newThrowable

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub fetch digger
dub run digger -- build "master + dmd#9481"

[thewilsonator]

A simple shell test of ! ( dmd file.d -o file.o && nm file.o | grep _d_new_class) should do the trick.

[Geod24]

Well, this is embarrassing. I must have never checked that code in. The trouble is, I'm not sure how to test it, other than manually checking the output which I've done.

Two options I can think of:

• In a test, define _d_newClass to be assert(0) so calling it would crash the binary
• Check the resulting binary's symbol table

The first option has the advantage of being rather simple and platform-independent

[WalterBright]

Unfortunately, even in a trivial program _d_newclass is being called from elsewhere, and that idea fails. So it's the script plan.

[thewilsonator]

program

Thats why I suggested an object file.

[WalterBright]

@thewilsonator I added a script, can you please have a look at it? Not very experienced with scripts.

[thewilsonator]

I hate script tests too, but that looks fine to me. We'll have to see what the autotester thinks.

[WalterBright]

@atilaneves can you please look into the buildkite/dmd failure, as it is kaliedoscope

[thewilsonator]

Seems to be a problem with exception.toString

[thewilsonator]

e.g.

tests/ut/wrap/traits.d:61 - Expected: "Unsupported function type double(int) for foo"
tests/ut/wrap/traits.d:61 -      Got: ""

[WalterBright]

This PR also appears to resolve https://issues.dlang.org/show_bug.cgi?id=19317

[jacob-carlborg]

Isn't it better to check that _d_newThrowable is called? Implement that function, in the implementation change a module level variable and assert that is changed correctly.

Unfortunately, even in a trivial program _d_newclass is being called from elsewhere, and that idea fails.

Perhaps check which ClassInfo is passed and assert only when it matches the ClassInfo of the thrown class.

[atilaneves]

I pushed and tagged a new excel-d, should work now.

[WalterBright]

Added test case from https://issues.dlang.org/show_bug.cgi?id=19317

[WalterBright]

Thanks, @atilaneves !

[WalterBright]

@atilaneves sadly, it's still failing kaleidoscope

[atilaneves]

@WalterBright It's failing on the old tag (0.4.1), I pushed the new tag (0.4.2) to the wrong remote by mistake. Should work now.

[WalterBright]

Thank you, @atilaneves ! Now, if only @thewilsonator or @wilzbach would approve this!

[thewilsonator]

This PR also appears to resolve https://issues.dlang.org/show_bug.cgi?id=19317

Please adjust the commit message to reflect that so the dlang bot picks up on it.

[WalterBright]

I resolved 19317 as a duplicate of 19463, and the test case for it was added to this PR.

[thewilsonator]

OK.

[jacob-carlborg]

@WalterBright As far as I understand this verifies that _d_newclass is not called, not that _d_newThrowable is called. I recommend implementing _d_newThrowable instead and verify it's getting called. This way a shell script is not required.

[rainers]

Well, this is embarrassing.

Still embarrassing that this now always calls _d_newThrowable, irrespective of whether -dip1008 is specified or not.