Skip to content

Conversation

@dkorpel
Copy link
Contributor

@dkorpel dkorpel commented Jun 1, 2021

Part of #8113

Turns foreach over an AliasSeq into static foreach because of issue 21209 - scope attribute inference with does not work well with foreach. Without this, text and chain are not scope compatible.

@dlang-bot
Copy link
Contributor

Thanks for your pull request and interest in making D better, @dkorpel! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

  • My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
  • My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
  • I have provided a detailed rationale explaining my changes
  • New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.


If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

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 "master + phobos#8123"

@thewilsonator
Copy link
Contributor

can you add a test for this?

@dkorpel dkorpel force-pushed the static-foreach-21209 branch 2 times, most recently from d76cce2 to d2c08b1 Compare June 1, 2021 14:44
@dkorpel
Copy link
Contributor Author

dkorpel commented Jun 1, 2021

I added a test for text. I tried to add one for chain, but I was unable with the current compiler behavior. Because of issue 20150 (-dip1000 defeated by pure), I have to force chain to be impure somehow, which I can only do by using an impure copy constructor. But then scope inference fails even after this patch, and I don't know why.

// Test `scope` inference on `chain` parameters
version(unittest) private struct ImpureCopyCtor
{
    // not pure, for https://issues.dlang.org/show_bug.cgi?id=20150
    this(ref ImpureCopyCtor r) scope @nogc nothrow {}
    bool empty = true;
    void popFront() {}
    int* front;
}

@safe unittest
{
    auto chain1(ref ImpureCopyCtor r)
    {
        return chain(r, r);
    }
    scope ImpureCopyCtor r;
    chain1(r);
}

@thewilsonator
Copy link
Contributor

CI seems stuck, can you please rebase or force push?

@RazvanN7
Copy link
Collaborator

RazvanN7 commented Jun 7, 2021

The autotester seems to be stuck. Rebase this to restart it.

@thewilsonator
Copy link
Contributor

Still stuck.

@RazvanN7 RazvanN7 force-pushed the static-foreach-21209 branch from 537fb89 to 11b9834 Compare June 8, 2021 08:41
@dlang-bot dlang-bot merged commit 010f6b7 into dlang:master Jun 8, 2021
@dkorpel dkorpel deleted the static-foreach-21209 branch June 8, 2021 11:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants