Fix Issue 18013 - DMD test suite assertion failure in test_cdvecfill.d #7454
Conversation
| ]), | ||
| ); | ||
| version(D_PIC) | ||
| { |
There was a problem hiding this comment.
As mentioned at #7427 (comment) running the update script is really useful and we shouldn't break that. Hence, I think it's best to make a copy of this file for version(D_PIC)?
There was a problem hiding this comment.
Specifically, which update script?
There was a problem hiding this comment.
The one which can be used to generate this output. Who did you get the -fPIC instructions?
Check the header of this file ;-)
There was a problem hiding this comment.
Ok, I changed the update code to generate either PIC or non-PIC depending on what kind of machine it's run on.
|
cc @MartinNowak |
|
btw, The diff in Github looks really screwy. It's probably easier to just review without the diff. |
test/runnable/test_cdvecfill.d
Outdated
| replaceFirstInto!((_, orng) => formattedWrite(orng, "// dfmt off\n%s// dfmt on", sink.data))(orng, content, ctRegex!(`^// dfmt off[^$]*// dfmt on$`, "m")); | ||
| version(D_PIC) | ||
| { | ||
| replaceFirstInto!((_, orng) => formattedWrite(orng, "// dfmtpic off\n%s// dfmtpic on", sink.data))(orng, content, ctRegex!(`^// dfmtpic off[^$]*// dfmtpic on$`, "m")); |
There was a problem hiding this comment.
Ehm I think you need to leave // dfmt off as it's a known directive.
Maybe sth. like:
// PIC\n// dfmt off[^$]*// dfmt off
See also: https://github.com/dlang-community/dfmt#disabling-formatting
There was a problem hiding this comment.
😆 I thought that was a strange moniker to use. I've updated the code to keep the // dfmt directives and use different strings for the codegen.
wilzbach
left a comment
wilzbach
left a comment
There was a problem hiding this comment.
LGTM. So if @MartinNowak doesn't complain in a few days, I suggest we move forward with this. After all, we want to get the testsuite running with -fPIC ...
|
I would strongly prefer if we could include this in the 2.078.0 release, as not being able to run test suite is a regression. Can you change the target of your PR to |
PetarKirov
left a comment
PetarKirov
left a comment
There was a problem hiding this comment.
LGTM and thanks for doing the work @JinShil. I think we should get this in as soon as possible.
|
Would have been sufficient to put |
|
I was scratching my head like crazy trying to remember when I added this PIC code and more importantly why, see #7716. |
fixup for #7454 - always run asm tests with PIC merged-on-behalf-of: Mike Franklin <JinShil@users.noreply.github.com>
5 participants
No description provided.