Skip to content

Makefiles: Enable -checkaction-context for unittest builds#7252

Closed
MoonlightSentinel wants to merge 1 commit intodlang:masterfrom
MoonlightSentinel:enable-checkaction
Closed

Makefiles: Enable -checkaction-context for unittest builds#7252
MoonlightSentinel wants to merge 1 commit intodlang:masterfrom
MoonlightSentinel:enable-checkaction

Conversation

@MoonlightSentinel
Copy link
Copy Markdown
Contributor Author

MoonlightSentinel commented Feb 14, 2020

Some more DIP1000 issues in std.range:

void main() @safe
{
    int[5] a = [ 1, 2, 3, 4, 5 ];
    assert(retro(a[]).source is a[]);
}

auto retro(return scope int[] s) @safe
{
    static struct R
    {
        int[] source;
    }
    return R(s);
}
safe.d(6): Error: address of variable a assigned to __assertOp4 with longer lifetime

https://issues.dlang.org/show_bug.cgi?id=20581

@MoonlightSentinel MoonlightSentinel changed the base branch from master to stable March 6, 2021 15:07
@MoonlightSentinel MoonlightSentinel force-pushed the enable-checkaction branch 2 times, most recently from d539c78 to 69bc4d1 Compare March 7, 2021 14:25
@MoonlightSentinel MoonlightSentinel changed the base branch from stable to master March 22, 2021 06:49
@MoonlightSentinel MoonlightSentinel force-pushed the enable-checkaction branch 2 times, most recently from 58f3969 to dba9a2b Compare March 22, 2021 07:18
@RazvanN7
Copy link
Copy Markdown
Collaborator

@MoonlightSentinel This seems to pass all the tests now. Does this still depend on dlang/dmd#10677 ?

@RazvanN7 RazvanN7 added the Review:WIP Work In Progress - not ready for review or pulling label Apr 21, 2021
@MoonlightSentinel
Copy link
Copy Markdown
Contributor Author

Not ATM, but there could be new problems if another deprecated type is used in an assert.

Also there is still a potential for linker errors due to missing template instances (because Druntime and Phobos are compiled without -checkaction=context). That's why -checkaction=D is currently required for the publictest target.

@RazvanN7
Copy link
Copy Markdown
Collaborator

How should we move forward? Should we merge this or should we first merge dlang/dmd#10677 and compile druntime and dmd with checkaction=context ?

@MoonlightSentinel
Copy link
Copy Markdown
Contributor Author

I'll close this PR for now, it has caught all remaining issues with -checkaction=context in Phobos.

We can gradually enable -checkaction=context for druntime/phobos once it get around to finish dlang/dmd#10677.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Review:WIP Work In Progress - not ready for review or pulling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants