Skip to content
This repository was archived by the owner on Oct 12, 2022. It is now read-only.
/ druntime Public archive

Comments

Qualify free(void*) as pure#1718

Closed
nordlow wants to merge 1 commit intodlang:masterfrom
nordlow:pure-free
Closed

Qualify free(void*) as pure#1718
nordlow wants to merge 1 commit intodlang:masterfrom
nordlow:pure-free

Conversation

@nordlow
Copy link
Contributor

@nordlow nordlow commented Dec 21, 2016

@mathias-lang-sociomantic
Copy link
Contributor

What ? free mutate global state.

@nordlow
Copy link
Contributor Author

nordlow commented Dec 21, 2016

Yes, via mutable indirections via input arguments.

That's how D defines weak purity.

Andrei has already approved this.

realloc has already been purified thanks to a recent fix DMD.

@wilzbach
Copy link
Contributor

realloc has already been purified.

See also: #1683

@nordlow
Copy link
Contributor Author

nordlow commented Dec 21, 2016

See also: #1719

@andralex
Copy link
Member

This is dependent upon dlang/dlang.org#1528, which makes void free(void*) a non-strongly pure function and requires it to be called compulsively.

@schveiguy
Copy link
Member

schveiguy commented Dec 23, 2016

which makes void free(void*) a non-strongly pure function

it should already be! it accepts a parameter with mutable indirection, the very reason we have weak-pure.

Note that the issue hasn't ever been that free itself would be omitted, it's that one typically casts away immutability to free something. So the wrapping function may accept an immutable indirected item, and then the wrapper function is never called.

For example:

pure void foo(immutable int *x) { .free(cast()x); }

This now may be elided by the compiler, and the memory is never freed. Think of how this may apply e.g. to a reference counting struct that has an immutable item it's tracking.

@andralex
Copy link
Member

@schveiguy the cast makes the compiler wash its hands so I think we're fine - the programmer needs to take special measures to make sure the desired effect is attained

@andralex
Copy link
Member

hmmm I just realized destructors must be always called even if they seem pure...

@schveiguy
Copy link
Member

the programmer needs to take special measures to make sure the desired effect is attained

This isn't possible. You can't enforce this effect unless you can somehow mark the type itself as special to the compiler.

@andralex
Copy link
Member

pure void* foo(immutable int *x) { .free(cast()x); return cast()x; }

@andralex
Copy link
Member

andralex commented Dec 23, 2016

oh, can actually return null: pure void* foo(immutable int *x) { .free(cast()x); return null; }

@schveiguy
Copy link
Member

schveiguy commented Dec 23, 2016

At some level, you will just have an immutable int *. Every pure function that accepts this and might call foo with it will have to either be superficially qualified as weak-pure, or possibly create a mess.

Not to mention that if you throw in implied purity in templates, you now have to contend with the compiler both eliding your call and qualifying your function so it can do so.

And I'm not 100% sure your workaround would work anyway -- you are ignoring obviously new memory as far as the compiler is concerned. It can safely elide the call as no side effects happen. What you need is a mutable parameter.

@andralex
Copy link
Member

Not to mention that if you throw in implied purity in templates, you now have to contend with the compiler both eliding your call and qualifying your function so it can do so.

It cannot by the rules proposed in dlang/dlang.org#1528, even if the result is not used.

Also: @schveiguy you can always hoist the cast into the call: instead of defining foo to take an immutable int* define it to take a mutable int* and have the caller insert the cast.

@schveiguy
Copy link
Member

The cast cannot happen inside pure code. But it will, someone will do it. Once it happens inside pure code, then that pure function could potentially be hiding this bug.

I think making free pure is a recipe for subtle elusive bugs. Let someone instead work around the impurity that is obvious rather than fight the pure optimizations that are hidden by the compiler. In other words, if you have to call a non-pure function inside pure code, there's ways to do it. They aren't pretty, but that cost is appropriate.

It cannot by the rules proposed in dlang/dlang.org#1528,

That's not how I read it. I read that it cannot be memoized, but this eliding is not due to memoization, it's due to having no side effects. There's no explicit statement that says calls to weakly pure functions cannot be elided under any circumstances.

@andralex
Copy link
Member

That's not how I read it.

Then I need to improve it!

@andralex
Copy link
Member

dlang/dlang.org@4a80b16

@schveiguy
Copy link
Member

Then I need to improve it!

It doesn't matter. A strong pure function can call a weak pure one, and then that call can be elided. As long as the cast happens inside a strong-pure function at any level, the trap is set.

@andralex
Copy link
Member

@schveiguy I see your point. @nordlow fistfight it out with @schveiguy

@nordlow
Copy link
Contributor Author

nordlow commented Jan 10, 2017

@WalterBright, do you oppose to @schveiguy's last objection?

A code example proving you right would be in place, @schveiguy ;)

@JackStouffer
Copy link
Contributor

@andralex @nordlow I know I'm late to the party, but issues with a pure free aside, malloc and friends can set errno to ENOMEM, meaning they break Andrei's statement,

Pure functions are functions that cannot access global/static mutable state, except if their arguments contain pointers to such

So, if we need to remove pure from malloc, which we really should, I don't see much point to marking free as pure.

@nemanja-boric-sociomantic
Copy link
Contributor

@andralex doesn't seem to be clear on errno issue right now, see #1683 (comment)

Then we need to figure what the impact of it setting a global is; I'm unclear on that right now.

@andralex
Copy link
Member

Not to worry. One thing we can do is write a pure wrapper for malloc that sets errno back to whatever it was if the original malloc returns null. That achieves purity albeit in a roundabout way.

@JackStouffer
Copy link
Contributor

@andralex Then we need to mark the wrappers as pure, and not the C functions.

@andralex
Copy link
Member

correct

@JackStouffer
Copy link
Contributor

@andralex #1735

@schveiguy
Copy link
Member

errno may be something that we can accept as modifiable by pure. After all, errno is really a return code, only valid for the last call, just done in a really clunky way.

I believe both malloc and free can be considered pure as long as they are properly matched. You can't make them pure in general, but under certain circumstances they can be used in pure code (for instance in Reference Counting).

Note that malloc generates data, it does not alter it. But it requires free to properly clean up. I'm much less concerned about malloc and elision, because it just won't generate the data that then doesn't need to be freed! And technically, free can't be elided either. It's just that I forsee people casting to "fix" memory leaks which will unexpectedly result in bugs that are hard to find and difficult to fix (and ironically result in memory leaks).

A code example proving you right would be in place

This is one of those things where one-in-a-million times it will break. But it will break at some point. A good "real-world" example is hard to find, but I'm sure if we allow this PR to go through, we will encounter some. Then there will be the usual "you're just doing it wrong" pushback, until finally we have to painfully remove the pure attribute.

The cautious and lower-risk path forward is to use escapes (as has been done in dlang/phobos#4832) for specific situations and see if we can make it work. If we can, then reexamining ways to add pure attribute for general use without issues might be in order. I still don't think adding pure on the C prototype is a good idea.

@andralex
Copy link
Member

andralex commented Jan 11, 2017

OK so this is not required anymore because it has a local workaround. I'm not sure about @schveiguy's concerns but if there's no urgency we should move cautiously. OK to close for now?

@nordlow
Copy link
Contributor Author

nordlow commented Jan 12, 2017

Ok, with me to go with local qualified imports for now.

@schveiguy
Copy link
Member

Closing this, as #1836 seems likely to get in.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants