www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.learn - Function attribute best practices

reply =?UTF-8?Q?Ali_=c3=87ehreli?= <acehreli yahoo.com> writes:
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
Sep 12 2022
next sibling parent reply Paul Backus <snarwin gmail.com> writes:
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
     });
Sep 12 2022
next sibling parent jmh530 <john.michael.hall gmail.com> writes:
On Monday, 12 September 2022 at 16:39:14 UTC, Paul Backus wrote:

 [snip]

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

 [snip]


If I can be 100% sure that something will always be 
 safe/nothrow/pure/ nogc, then I might consider marking them as 
such. For instance, a function that takes any floating point 
type, does some calculation, and then returns it. I figure it is 
documented for the user and at least this will save the compiler 
the effort of figuring it. If I can't, then I don't.
Sep 13 2022
prev sibling parent reply =?UTF-8?Q?Ali_=c3=87ehreli?= <acehreli yahoo.com> writes:
On 9/12/22 09:39, Paul Backus wrote:


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


Except for 'const' as well because some templates are member functions. 
And 'const' on a member function cannot be left to inference because it 
happens to be a part of the type of the function, which can be overloaded.

Somebody needs to create a two dimensional table that shows what it 
means for each function attribute on a regular function, member 
function, and templates of those, and hopefully come up with some 
guidelines.

I started thinking about it but will not have time these coming days. :/

Ali
Sep 13 2022
parent reply Paul Backus <snarwin gmail.com> writes:
On Tuesday, 13 September 2022 at 14:16:39 UTC, Ali Çehreli wrote:

 On 9/12/22 09:39, Paul Backus wrote:


 Yes. Except for ` trusted`, explicit attributes on template


 code are a

 smell.


 Except for 'const' as well because some templates are member 
 functions. And 'const' on a member function cannot be left to 
 inference because it happens to be a part of the type of the 
 function, which can be overloaded.


Yes, good point. In my head, I think of attributes that apply to 
the `this` parameter like `const`, `inout`, `shared`, and so as 
being in a separate category from attributes that apply to the 
function itself, like ` safe` and ` trusted`.


 Somebody needs to create a two dimensional table that shows 
 what it means for each function attribute on a regular 
 function, member function, and templates of those, and 
 hopefully come up with some guidelines.


Here's my attempt, covering all the attributes found under 
[`MemberFunctionAttribute`][1] in the language spec:

|Attribute|Affects |Inferred?|
|---------|--------|---------|
|nothrow  |Function|Yes      |
|pure     |Function|Yes      |
| nogc    |Function|Yes      |
| safe    |Function|Yes      |
| system  |Function|Yes      |
| trusted |Function|No       |
| property|Function|No       |
| disable |Function|No       |
|const    |this    |No       |
|immutable|this    |No       |
|inout    |this    |No       |
|shared   |this    |No       |
|return   |this    |Yes      |
|scope    |this    |Yes      |

In general, attributes with a 'Yes' in the 'Inferred?' column 
should not be applied explicitly to functions that are subject to 
[attribute inference][2]. This includes functions defined inside 
templates, as well as nested functions and functions with an 
inferred return type (i.e., `auto` functions).

[1]: https://dlang.org/spec/function.html#MemberFunctionAttributes
[2]: 
https://dlang.org/spec/function.html#function-attribute-inference
Sep 13 2022
parent =?UTF-8?Q?Ali_=c3=87ehreli?= <acehreli yahoo.com> writes:
On 9/13/22 10:08, Paul Backus wrote:


 Here's my attempt, covering all the attributes found under
 [`MemberFunctionAttribute`][1] in the language spec:

 |Attribute|Affects |Inferred?|
 |---------|--------|---------|
 |nothrow  |Function|Yes      |
 |pure     |Function|Yes      |
 | nogc    |Function|Yes      |
 | safe    |Function|Yes      |
 | system  |Function|Yes      |
 | trusted |Function|No       |
 | property|Function|No       |
 | disable |Function|No       |
 |const    |this    |No       |
 |immutable|this    |No       |
 |inout    |this    |No       |
 |shared   |this    |No       |
 |return   |this    |Yes      |
 |scope    |this    |Yes      |

 In general, attributes with a 'Yes' in the 'Inferred?' column should not
 be applied explicitly to functions that are subject to [attribute
 inference][2]. This includes functions defined inside templates, as well
 as nested functions and functions with an inferred return type (i.e.,
 `auto` functions).

 [1]: https://dlang.org/spec/function.html#MemberFunctionAttributes
 [2]: https://dlang.org/spec/function.html#function-attribute-inference


That is great! I think we can improve it with guidelines for when to 
write an attribute or not.

For example, carried over from C++, I have this guideline: "put const to 
as many member function as you can." That guideline makes the function 
more useful because I can call it on mutable, const, and immutable 
objects. Great... Programmers can understand that.

Now let's compare it to what the const attribute on a member function 
says (I did not find it in the spec; so making it up): "Makes the 'this' 
reference const." Although correct, it does not help the programmer.

Or... What does pure mean? Does it tell the outside world that the 
function is pure or does it require to be called from pure code? I put 
that here because e.g. 'immutable' on a member function kind of does 
that: It requires to be called on an immutable object. Ok, not a fair 
comparison because there is no "pure object" but still, these are 
thoughts that I think programmers have in mind. (I do. :) )

Ali
Sep 13 2022
prev sibling next sibling parent Steven Schveighoffer <schveiguy gmail.com> writes:
On 9/12/22 12:14 PM, Ali Çehreli wrote:

 What are best practices here?


attributes such as `pure`, ` nogc`, `nothrow`, ` safe` should all be 
left to inference. Either the function can do those attributes, or it 
cannot.

attributes such as `const` or `inout` are different -- these are *not* 
inferred, and you need to use introspection to determine these. Which is 
really unfortunate, because there's no easy way to say "const if this is 
allowed" -- you have to repeat the implementation.


 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.


It's not that scary, because I'm not sure what one would expect passing 
in an impure function to the template.


 
 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.


Yes, this is exactly what you should do. You don't need to unittest 
compiler inference -- just expect this to work. What you want to test is 
if there's any code you wrote for Foo can make impure something that 
should be pure. Things that Foo calls on its parameter should not count 
towards your test, that's on the caller.

So for instance, create a dummy range that is pure, nogc, nothrow, safe, 
and test Foo as a wrapper on that range, attributing the unittest with 
that. And if that works, you should be good. You shouldn't have to test 
that if you pass in an impure function, the thing becomes impure.

Caveat: if you have code that is compiled differently based on those 
attributes, you should test those cases too to ensure coverage of the 
code in question.

-Steve
Sep 12 2022
prev sibling parent reply "H. S. Teoh" <hsteoh qfbox.info> writes:
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.
Sep 12 2022
parent reply =?UTF-8?Q?Ali_=c3=87ehreli?= <acehreli yahoo.com> writes:
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
Sep 12 2022
next sibling parent reply "H. S. Teoh" <hsteoh qfbox.info> writes:
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
Sep 12 2022
parent =?UTF-8?Q?Ali_=c3=87ehreli?= <acehreli yahoo.com> writes:
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
Sep 12 2022
prev sibling parent reply Steven Schveighoffer <schveiguy gmail.com> writes:
On 9/12/22 1:08 PM, Ali Çehreli wrote:

 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! :)


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

When you put `pure` on a template function, you are saying "only 
instantiations where this function can be pure are allowed". 
Essentially, that's *you* telling your *user* "this must be pure!".

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.

-Steve
Sep 12 2022
next sibling parent =?UTF-8?Q?Ali_=c3=87ehreli?= <acehreli yahoo.com> writes:
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
Sep 12 2022
prev sibling parent "H. S. Teoh" <hsteoh qfbox.info> writes:
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!
Sep 12 2022