The following range Foo is trying to be helpful by adding as many attributes as it can ('const' is missing because ranges cannot be 'const' because at least popFront() needs to be mutable):

import std.algorithm;

struct Foo(R) {
    R r;
    int i;

    bool empty() @nogc nothrow pure @safe scope {
        return r.empty;
    }

    auto front() @nogc nothrow pure @safe scope {
        return r.front;
    }

    auto popFront() @nogc nothrow pure @safe scope {
        r.popFront();
    }
}

auto foo(R)(R r) {
    return Foo!R(r);
}

int count;

void main() {
    [ 1, 2 ]
        .map!((i) {
                ++count;    // <-- Impure
                return i;
            })
        .foo;
}

Of course there are compilation errors inside the member functions because e.g. r.front that it dispatches to is not pure (it touches the module variable 'count'):

  Error: `pure` function `deneme.Foo!(MapResult!(__lambda1, int[])).Foo.front` cannot call impure function `deneme.main.MapResult!(__lambda1, int[]).MapResult.front`

(Other attributes would cause similar issues if e.g. the lambda were @nogc.)

What are best practices here?

Is this accurate: Because Foo is a template, it should not put any attribute on member functions? Or only member functions that use a member that depends on a template parameter? And non-members that are templates?

It is scary because Foo works just fine until it is used with impure code.

Is putting function attributes on unittest blocks for catching such issues?

@nogc nothrow pure @safe
unittest
{
    // ...
}

No, it isn't because unless my unittest code is impure, I can't catch my incorrect 'pure' etc. on my member functions.

Help! :)

Ali

On Monday, 12 September 2022 at 16:14:42 UTC, Ali Çehreli wrote:
> Is this accurate: Because Foo is a template, it should not put any attribute on member functions? Or only member functions that use a member that depends on a template parameter? And non-members that are templates?

Yes. Except for `@trusted`, explicit attributes on template code are a smell.

> Is putting function attributes on unittest blocks for catching such issues?
>
> @nogc nothrow pure @safe
> unittest
> {
>     // ...
> }
>
> No, it isn't because unless my unittest code is impure, I can't catch my incorrect 'pure' etc. on my member functions.

To test that a particular piece of code *isn't* pure, you can use the following idiom:

    static assert(!__traits(compiles, () pure {
        // code to test goes here
    });

On Mon, Sep 12, 2022 at 09:14:42AM -0700, Ali Çehreli via Digitalmars-d-learn wrote: [...]
> struct Foo(R) {
>     R r;
>     int i;
> 
>     bool empty() @nogc nothrow pure @safe scope {
>         return r.empty;
>     }
> 
>     auto front() @nogc nothrow pure @safe scope {
>         return r.front;
>     }
> 
>     auto popFront() @nogc nothrow pure @safe scope {
>         r.popFront();
>     }
> }
[...]
> What are best practices here?
> 
> Is this accurate: Because Foo is a template, it should not put any attribute on member functions? Or only member functions that use a member that depends on a template parameter? And non-members that are templates?

IMO, attributes on members of template aggregates should be omitted, *except* where you want to enforce that attribute on all instantiations. E.g., if there is some kind of semantic requirement that a method should not mutate the state no matter what, then you could put `const` on it.

Otherwise, I'd say let the compiler infer the actual attributes, and use appropriately-crafted unittests to catch attribute violations in the template code. The reason for this is to maximize generality:

1) If some user wants to use your code with their own data type, but they needed it to be, e.g., impure for some reason, or throwing, then your template should "gracefully degrade" rather than refuse to compile (because instantiating Foo with a type whose .front is throwing, for example, would be a compile error).  To do this, we must let the compiler infer attributes as much as possible -- so for an instantiation with a nothrow .front it would infer nothrow, for example. But for an instantiation involving a throwing user type, the compiler would infer .front as throwing.

2) If some user uses your code in nothrow code, then your template should not introduce throwing behaviour which would fail to compile. For this, you should use appropriately-attributed unittests to ensure that, eg., when Foo is instantiated with a non-throwing type it does not introduce something that throws.


> It is scary because Foo works just fine until it is used with impure code.

Exactly, this is issue (1) above.


> Is putting function attributes on unittest blocks for catching such issues?
> 
> @nogc nothrow pure @safe
> unittest
> {
>     // ...
> }
> 
> No, it isn't because unless my unittest code is impure, I can't catch my incorrect 'pure' etc. on my member functions.
[...]

Sure you can.  The `pure unittest` code obviously must itself be pure (otherwise it wouldn't compile). If Foo introduces impure behaviour, then the unittest, being pure, wouldn't be allowed to call Foo's impure methods, which is what we want.  What's the problem?


T

-- 
Fact is stranger than fiction.

On 9/12/22 09:48, H. S. Teoh wrote:

>> @nogc nothrow pure @safe
>> unittest
>> {
>>      // ...
>> }
>>
>> No, it isn't because unless my unittest code is impure, I can't catch
>> my incorrect 'pure' etc. on my member functions.
> [...]
>
> Sure you can.  The `pure unittest` code obviously must itself be pure
> (otherwise it wouldn't compile). If Foo introduces impure behaviour,
> then the unittest, being pure, wouldn't be allowed to call Foo's impure
> methods, which is what we want.  What's the problem?

There was a problem until you and others put me straigth. :)

What I meant was

- if I put 'pure' etc. on my templatized code,

- and then tested with a 'pure' unittest,

I wouldn't know that the gratuitous use of my 'pure' on the member function was wrong. I would be fooling myself thinking that I smartly wrote a 'pure' member function and a 'pure' unittest and all worked. Wrong idea! :)

Now I know I must leave attributes as much to inference as possible.

Ali

On Mon, Sep 12, 2022 at 10:08:29AM -0700, Ali Çehreli via Digitalmars-d-learn wrote: [...]
> What I meant was
> 
> - if I put 'pure' etc. on my templatized code,
> 
> - and then tested with a 'pure' unittest,
> 
> I wouldn't know that the gratuitous use of my 'pure' on the member function was wrong. I would be fooling myself thinking that I smartly wrote a 'pure' member function and a 'pure' unittest and all worked. Wrong idea! :)

That's an easy one: write a unittest where you instantiate Foo with a deliberately-impure type (e.g., .front references some local variable in the unittest outside the aggregate).  If there was a gratuitous `pure` in Foo, this will fail to compile.


T

-- 
"I'm running Windows '98." "Yes." "My computer isn't working now." "Yes, you already said that." -- User-Friendly

On 9/12/22 10:29, H. S. Teoh wrote:

> write a unittest where you instantiate Foo with a
> deliberately-impure type

Yes. A current on-topic thread on the difficulties of covering all corner cases:

  https://forum.dlang.org/thread/dmnfdqiplbldxkecpned@forum.dlang.org

Ali

On 9/12/22 11:29, Steven Schveighoffer wrote:

> So you are thinking about this the wrong way I believe.

Clearly.

> When you put `pure` on a template function, you are saying "only
> instantiations where this function can be pure are allowed".

Makes sense. I was trying to put as many attributes as possible, being proud like, "look: my function is pure, @nogc, etc."

Ali

On Mon, Sep 12, 2022 at 02:29:58PM -0400, Steven Schveighoffer via Digitalmars-d-learn wrote: [...]
> If your intent is to *enforce* pure functions only, then that's what you do.  If your intent instead is to ensure that given proper parameters, the function will be pure, then the answer is to unittest.
> 
> I will say, sometimes this gets really annoying. Like if the unittest fails, you get very little information about *why* it's not working.
> 
> i.e. you expect the inference to be pure, but it's not. All you get is "impure unittest can't call impure function foo(...)". Figuring out the misinference cause is a chore today. I wish it would be easier.
[...]

+1, we need better diagnostics around this.

I wonder if it could be done this way: whenever the compiler infers attributes for some function F, along with the inferred attributes it also attaches a list of locations where each attribute was excluded from the inference. For example, if F was inferred as impure, the compiler would also keep a reference to the first line where an impure operation was performed in F.  Whenever pure code tries to call F, the compiler would print out this reference (file + line + column) along with the error message.  I.e., "pure function G cannot call impure function F because impure was inferred from line 1234 in F."

Obviously, this applies only to functions with inferred attributes; if the attribute was explicitly in the code, then there's nothing else to say.  Since functions with inferred attributes always must have their bodies accessible (otherwise inference cannot be done), it's guaranteed that the aforementioned reference can always be found.

//

In the meantime, my usual approach to debugging this sort of problem is to explicitly (temporarily) mark the target function pure, and recompile to find out what in the function made it impure. If it's a call to another inferred function, then mark *that* function pure too, and recompile, etc., until I arrive at the statement that first made it impure.  This doesn't always work, though.  Sometimes forcefully marking something pure will break the compile in other places, making it a pain to debug (usually involving making a local copy of the function and redirecting calls to/from it).

The compiler really should be giving us this information rather than making us figure it out ourselves.


T

-- 
Ignorance is bliss... until you suffer the consequences!