Fix some of the more egregious aspects of 19961

[thewilsonator]

cc @TurkeyMan

[dlang-bot]

Thanks for your pull request and interest in making D better, @thewilsonator! 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 annotated coverage diff directly on GitHub with CodeCov's browser extension
• 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 fetch digger
dub run digger -- build "master + dmd#10035"

[TurkeyMan]

This doesn't look right...

[TurkeyMan]

This should be an error, i is immutable in this scope.

[thewilsonator]

It is, this is a fail_compilation test.

[TurkeyMan]

i is const in this scope.

[TurkeyMan]

Ditto

[TurkeyMan]

The error is misleading then; "cannot access mutable data i" ... i is immutable in that scope, so the error message is wrong?
The error would be that you can't increment an immutable variable...

What does pragma(msg, typeof(i)); in that scope print?

[thewilsonator]

pragma(msg, typeof(i));

int, I haven't fixed that problem yet.

[PetarKirov]

Can you rename test/fail_compilation/test19961.d back to test/fail_compilation/test15306.d? It's difficult to trace the changes that you made in the GitHub diff.

[thewilsonator]

I could, but everything in that test case is wrong.

[PetarKirov]

This needs to be fixed before the PR can be merged. I believe that if you are able to fix the root cause and change the type of the p inside the delegate it would automatically fix this as well ;)

[thewilsonator]

The problem is that the context pointer's qualifiers are not applied at all to access from within the nested function. I'm probably going to end up deleting the entire loop and else branch, the comment of the cascaded nested function is a complete lie, i can modify fx even if it is pure.

[TurkeyMan]

Isn't it simple the case that the context pointer is not qualified? It mustn't be, or the type checker would be freaking out everywhere. Wherever the function receives the Context* argument, if it swaps for qualified(Context)*, then everything should wring out naturally?

[PetarKirov]

but everything in that test case is wrong.

In that case it's even more important for us to be able to track the changes that you made.

[WalterBright]

When referring to an issue, please link to it, like this: https://issues.dlang.org/show_bug.cgi?id=19961

Also, insert into bugzilla a link to this page.

[PetarKirov]

You should also add a compilable test along the lines of:

// const should be able to access mutable and immutable alike, but not modify it:
int i = 42;
int* p = &i;
auto dg = delegate int() const
{
    static assert(!__traits(compiles, { i++; })); // correctly disallowed
    
    // For all outer variables V of type Q1(T) accessed through context
    // with qualifier Q2 do implicit qualifier conversion to Q1(Q2(T)),
    // where Q1(Q2) is the resulting qualifier according to the table in:
    // https://dlang.org/spec/const3.html#implicit_qualifier_conversions
    static assert(is(typeof(p) == const(int*)));

    return *p; // OK: implicit conversion from `const(int)` to `int`
};
assert(dg2() == i);

[PetarKirov]

Also, insert into bugzilla a link to this page.

@WalterBright there's no need to do this manually anymore as the @dlang-bot will do this automatically if the commit message follows the right format (https://github.com/dlang/dlang-bot#nerdy-details).

[thewilsonator]

Yes, but this PR doesn't fix this, so the dlang-bot didn't pick it up.

[PetarKirov]

@thewilsonator check the regex in the link I posted above ;)

In other words, you can use "Issue 19961 - Fix some of the more egregious aspects".

[WalterBright]

@ZombineDev it didn't, so I added the link myself.

[PetarKirov]

@WalterBright it didn't because the commit message was not in the right format. If it was, it would.

[TurkeyMan]

I'm surprised by the approach here; it looks like the capture code is hard-coding qualifier conversions, and all sorts of magic.
I don't think the capture needs to do anything special; assuming the capture produces a Context*, the function just needs to have its signature tweaked to receive a qualified(Context)* (same as if it was a regular this pointer), and then within the function the Context object and everything it refers to would correctly be const...?