Bug 15624 – opApply with @safe and @system variants can't be used with foreach syntax
Status
RESOLVED
Resolution
FIXED
Severity
enhancement
Priority
P1
Component
dmd
Product
D
Version
D2
Platform
x86_64
OS
Windows
Creation time
2016-01-29T20:16:05Z
Last change time
2021-11-06T01:02:11Z
Keywords
safe
Assigned to
No Owner
Creator
Neia Neutuladh
Comments
Comment #0 by dhasenan — 2016-01-29T20:16:05Z
Test case:
---
struct Foo {
int opApply(int delegate(int, string, string) @safe dg) @safe {
return 0;
}
int opApply(int delegate(int, string, string) @system dg) @system {
return 0;
}
}
void testSafe() @safe {
Foo foo;
foreach (i, k, v; foo) {
}
}
void testSystem() @system {
Foo foo;
foreach (i, k, v; foo) {
}
}
void main() {
testSafe;
testSystem;
}
---
The compiler correctly routes things if you explicitly pass a lambda. It even implicitly passes to the @safe variant if the lambda can be made @safe. So this is probably something special about lowering foreach to opApply.
Comment #1 by bugzilla — 2018-03-16T06:00:19Z
The foreach delegate is correctly constructed and its @safe-ty is correctly inferred. The overloading of opApply() is correct.
The trouble is that the opApply() is selected by inferApplyArgTypes() before the foreach delegate is constructed.
Comment #4 by qs.il.paperinik — 2018-10-08T21:01:20Z
*** Issue 15859 has been marked as a duplicate of this issue. ***
Comment #5 by dlang-bot — 2021-11-06T01:02:11Z
dlang/dmd pull request #13226 "Rework fix for attribute based opApply overloads (issue 15624)" was merged into master:
- a28680a845a06d0cac476442c25e7c7fe8e0d16d by MoonlightSentinel:
Rework fix for attribute based opApply overloads (issue 15624)
The old implementation uses `Type.covariant` to suppress overzealous
ambiguity errors for `opApply` specialisations. Deferring the ambiguity
checks after the attribute inference on the loop body allows for
`opApply` overloads with different attributes, e.g. `@safe / @system` for
`@safe / @system` delegates.
Example:
```d
struct Collection
{
/* A. */ int opApply(int delegate(int) @safe dg) @safe;
/* B. */ int opApply(int delegate(int) @system dg) @system;
}
void main() @safe
{
Collection col;
foreach (entry; col) {} // Resolves to A.
}
```
The previous code achieved this by checking whether the previously
selected `opApply` and the current overload are covariant in any
direction (i.e. `A.covariant(B)` or `B.covariant(A)`).
This implementation builds on the flawed assumption that the `opApply`
functions in this example are covariant - which they are not
(falsely reported due to a bug in `Type.covariant`).
Consider the types of the `opApply` overloads:
```d
A: int function(int delegate(int) @safe dg) @safe;
B: int function(int delegate(int) @system dg) @system;
```
Neither of those is an appropriate substitute for the other:
- A <= B is invalid due to the `opApply`'s attributes. A `@system`
function may not appear in place of a `@safe` function
- A => B is invalid due to the parameter types (`dg` attributes).
A function expecting a `@safe` delegate may not be treated as a
function accepting `@system` callbacks. Otherwise it could silently
execute a `@system` callback which is disallowed in `@safe` functions.
Note that `Type.covariant` already rejects covariance when using
`function` instead of `delegate` for the callbacks in the example.
---
The new implementation does not rely on the buggy behaviour of
`Type.covariant`. It checks two different aspects to defer the
`opApply` resolution:
1. different attributes on the `opApply` function
2. different attributes on the `opApply` function
3. covariance of the callbacks receiving the foreach body
(2) should handle different qualifiers on the foreach body
because missmatched parameter types are usually rejected by
`matchParamsToOpApply`.
---
This patch enables us to fix the aforementioned bug without breaking
existing code. It also enables the code added the test case wich
is valid despite being utterly useless.
https://github.com/dlang/dmd/pull/13226