Closed ribelo closed 2 years ago
Based on some discussion in the Slack channel, I think we should build on this a little more.
First, I think there are roughly 4 levels of safety (see the table below) that we can derive by considering the combinations of enabling/disabling type checking/native method calling: benign, safe, unsafe, and dangerous. I think what we should do is the following.
::m/use-native-methods
which by default is true.::m/unsafe
to #::m{:use-native-methods false, :no-type-check true}
.::m/dangerous
to #::m{:use-native-methods true, :no-type-check true}
.meander.match.ir.epsilon/unsafe?
meander.match.ir.epsilon/dangerous?
+-------------+----------------+----------------------------+-----------+
| type-check? | native-method? | Result | Name |
+-------------+----------------+----------------------------+-----------+
| true | false | (f object ,,,) | Slow |
+-------------+----------------+----------------------------+-----------+
| true | true | (.method ^Type object ,,,) | Safe |
+-------------+----------------+----------------------------+-----------+
| false | false | (f object ,,,) | Unsafe |
+-------------+----------------+----------------------------+-----------+
| false | true | (.method ^Type object ,,,) | Dangerous |
+-------------+----------------+----------------------------+-----------+
@ribelo By desugar I meant adding the following functions to the meander.environment.epsilon
namespace.
(defn desguar-unsafe [env]
(if (:meander.epsilon/unsafe env)
(assoc (dissoc :meander.epsilon/unsafe)
:meander.epsilon/no-type-check true
:meander.epsilon/use-native-methods false)
env))
(defn desguar-dangerous [env]
(if (:meander.epsilon/dangerous env)
(assoc (dissoc :meander.epsilon/unsafe)
:meander.epsilon/no-type-check true
:meander.epsilon/use-native-methods true)
env))
(defn desugar [env]
(-> env
desugar-unsafe
desugar-dangerous))
Then use the desugar
function in m.match/analyze-match-args
and similar functions where we initialize the environment such as here.
Additionally, I think we should add these predicates to meander.environment.epsilon
.
(defn unsafe? [env]
(and (true? (:meander.epsilon/no-type-check env))
(false? (:meander.epsilon/use-native-methods env))))
(defn dangerous? [env]
(and (true? (:meander.epsilon/no-type-check env))
(true? (:meander.epsilon/use-native-methods env))))
We update the predicates in meander.match.ir.epsilon
to use these.
@noprompt
I removed unsafe?
and dangerous?
and added native-method?
It seems to me that because we have desugar
, the above have no function. I also used desugar
in a different place than you suggested, because I wanted to use it as early as possible so that all functions that use env
already have updated variables.
I also noticed that neither find
nor search
use r.environment/default
, so I took the liberty of adding that.
EDIT: Fix markdown.
I also noticed that neither find nor search use r.environment/default, so I took the liberty of adding that.
@ribelo From what I can tell this seems good to me. The only thing left would be to delete those fields and the fast?
predicate and you can hit the merge button. :smile:
Disable type checks and bound checks and use fastest possible functions for lookup