D issues are now tracked on GitHub. This Bugzilla instance remains as a read-only archive.
Issue 8637 - Enforcement and purity
Summary: Enforcement and purity
Status: RESOLVED FIXED
Alias: None
Product: D
Classification: Unclassified
Component: phobos (show other issues)
Version: D2
Hardware: All All
: P2 normal
Assignee: monarchdodra
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-09-10 02:58 UTC by monarchdodra
Modified: 2012-09-14 19:12 UTC (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this issue.
Description monarchdodra 2012-09-10 02:58:42 UTC
The argument taken by enforce must be "castable to bool", so the the implementation can do the cast. However, enforce is declared pure, so if the cast operator is not pure, the compilation fails:

--------
import std.regex;
import std.exception;
void main()
{
    auto m = match("hello world", regex("world"));
    assert(m); //<- Fine
    enforce(m); //Here
}
--------
Error: pure function 'enforce' cannot call impure function '~this'
Error: pure function 'enforce' cannot call impure function 'opCast'
--------

The messages come from:

--------
T enforce(T)(T value, lazy const(char)[] msg = null, string file = __FILE__, size_t line = __LINE__) @safe pure
{
    if (!value) bailOut(file, line, msg);
    return value;
}
--------
"if(!value)": This makes an impure call to opCast.

"enforce(T)(T value..." This uses pass by value, and makes an impure call to the destructor



I have no idea what a good fix would be. Regarding pass by value, wouldn't this be a textbook example of using "auto ref" with "auto return"? I have no idea...
Comment 1 Kenji Hara 2012-09-10 08:14:02 UTC
(In reply to comment #0)
> The argument taken by enforce must be "castable to bool", so the the
> implementation can do the cast. However, enforce is declared pure, so if the
> cast operator is not pure, the compilation fails:
[snip]
> --------
> Error: pure function 'enforce' cannot call impure function '~this'
> Error: pure function 'enforce' cannot call impure function 'opCast'
> --------

The cause might be the explicit annotation with pure.

> The messages come from:
> 
> --------
> T enforce(T)(T value, lazy const(char)[] msg = null, string file = __FILE__,
> size_t line = __LINE__) @safe pure
> {
>     if (!value) bailOut(file, line, msg);
>     return value;
> }
> --------
> "if(!value)": This makes an impure call to opCast.
> 
> "enforce(T)(T value..." This uses pass by value, and makes an impure call to
> the destructor
>
> I have no idea what a good fix would be. Regarding pass by value, wouldn't this
> be a textbook example of using "auto ref" with "auto return"? I have no idea...

The strict pure annotation is introduced by the pull #263.
https://github.com/D-Programming-Language/phobos/pull/263

As you can see, the pure annotations were just for the documentation. At this point, impure destructor had not been considered at all in the discussion.

Then, now, the pure annotation causes this problem, so I think we should remove it and rely on the pure attribute inference.
Comment 2 monarchdodra 2012-09-10 08:43:02 UTC
If that is the fix, I can make the pull request. Do you want me to do it, or are you on it?
Comment 3 Kenji Hara 2012-09-10 08:49:24 UTC
(In reply to comment #2)
> If that is the fix, I can make the pull request. Do you want me to do it, or
> are you on it?

Of course I would welcome your contribution.
Comment 4 monarchdodra 2012-09-10 08:57:48 UTC
Will commit fix myself then.
Comment 5 Jonathan M Davis 2012-09-10 09:07:50 UTC
Both the annotation for pure and @safe need to go, because T's destructor and opCast could be impure or non-@safe. When the explicit annotations were added, clearly the destructor and opCast were not taken into account, since they're not called directly and are easily forgotten.
Comment 6 Kenji Hara 2012-09-10 09:15:20 UTC
(In reply to comment #5)
> Both the annotation for pure and @safe need to go, because T's destructor and
> opCast could be impure or non-@safe. When the explicit annotations were added,
> clearly the destructor and opCast were not taken into account, since they're
> not called directly and are easily forgotten.

Oh, that's right. We should remove both 'pure' and '@safe', and need to depends on fully attribute inference.
Comment 7 Dmitry Olshansky 2012-09-10 12:44:17 UTC
Sorry, it had regex somewhere in description so I jumped in ;)

https://github.com/D-Programming-Language/phobos/pull/783
Comment 8 monarchdodra 2012-09-10 13:59:53 UTC
Darn!

Kidding asside, thanks.

BTW, I just made touch on std.regex itself. Seems like you may have wanted to know: https://github.com/D-Programming-Language/phobos/pull/784
Comment 9 monarchdodra 2012-09-11 10:23:36 UTC
Fixed by Dmitry in
https://github.com/D-Programming-Language/phobos/pull/783
Comment 10 github-bugzilla 2012-09-14 19:12:04 UTC
Commit pushed to master at https://github.com/D-Programming-Language/phobos

https://github.com/D-Programming-Language/phobos/commit/7714a9c42e2cc8d794a5ee483fdd49d52995b3ac
Fix issue 8637

Compiler can and should deduce purity of templates