Add function version of vswap!

Description

Volatiles and vswap! were added in 1.7 as performant mechanisms to achieve uncoordinated mutation to the language.
given the fact that their addition was a performance-centric one, vswap! was implemented as a macro rather than a normal function to avoid runtime dereference of the function var and the optional apply overhead in case of multiple args.

However this:
-is not necessary
-breaks the api parallelism between volatile/atom swap!/vswap! reset!/vreset!
-makes impossible certain use cases (vswaps! in update-in forms)
-is potentially confusing given that swap! is a function

Infact the macro can be replaced with a function with :inline metadata.
This is a strictly additive change that will make so that for all the current valid usages of vswap! nothing will change, it will still be macroexpanded by the inliner and additionally since it is now a function it can be used in HOF contexts where it's not unusual to see swap! used.

Environment

None

Activity

Show:
Nicola Mometto
July 14, 2015, 2:10 AM

Sorry, I mixed the fix version with the affected field

Greg Chapman
March 9, 2016, 5:41 PM

An additional drawback to the current macro is that it will result in double evaluation of the vol expression:

Alex Miller
March 10, 2016, 3:22 PM

I don't think Rich is interested in expanding the use of inline.

Re Greg's comment, that would be considered if you want to make a second ticket.

Nicola Mometto
March 10, 2016, 6:31 PM

Not really sure I understand the adversion to using inline – it works perfectly fine and this is the exact use-case for it: a function that we want to inline for performance reason.

I really fail to see the reasoning behind making what should be a function, a macro, just for the sake of not using inline.

import
March 19, 2016, 2:59 PM

Comment made by: pbwolf

Adding :inline to existing functions may be deplored as an unprincpled, sloppy, desperate, tragic compromise, and a slippery slope. But vswap! is the opposite case: The macro is what already exists, somewhat idiosyncratically. Pairing it with a function would make it more wholesome, not less.

Assignee

Unassigned

Reporter

Nicola Mometto

Labels

None

Approval

None

Patch

Code

Affects versions

Priority

Major