<< Back to previous view

[MATCH-118] Simplify computation of necessary column Created: 06/Dec/16  Updated: 06/Dec/16

Status: Open
Project: core.match
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Task Priority: Minor
Reporter: daniel sutton Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: patch
Environment:

Fedora, CIDER, clojure 1.8


Attachments: Text File 0001-Increase-speed-for-necessary-column.patch    
Waiting On: David Nolen

 Description   

From match.clj:
;; IMPORTANT NOTE: this calculation is very very slow,
;; we should look at this more closely - David

This has to do with computing the necessary column, ie, the column that contains the most number of matches we will have to do. This patch simplifies this logic which is spread out over several functions and puts it into one location. Further, it improves complexity. When computing the necessity of a column, only patters above wildcards in a column need to be computed. the way the previous implementation worked was to compute the value for each entry in the pattern matrix and then check above each entry to see if there were wildcards. Now, we just group by column and then reduce on the column, adding up the same values as last time and immediately returning the accumulator with `(reduced acc)` if we hit a wildcard. This saves the repeated traversal of each column for each entry. A nice benefit is locality of everything inside of a single function.

I've timed this against master with the entries from `bench.clj` and was actually a little surprised to see that there was no time speedup, however.






[MATCH-117] Outdated dependencies Created: 02/Dec/16  Updated: 16/Dec/16

Status: Open
Project: core.match
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: daniel sutton Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: dependencies
Environment:

Emacs 25, CIDER


Attachments: Text File 0001-Update-clj-cljs-and-remove-nrepl-dependency.patch     Text File 0001-Update-clj-cljs-and-remove-nrepl-dependency.patch    
Patch: Code
Waiting On: David Nolen

 Description   

The Clojure version specified in core is 1.6, which is lower than the minimum supported version in CIDER (1.7)
The Clojurescript version is "0.0-2496" which seems rather outdated compared to the current "1.9.293"
The lein-cljsbuild is "1.0.4-snapshot" instead of current 1.1.4

Rather critically, this specifies its own version of cider-nrepl, a very out of date 0.8.1, rather than letting CIDER inject its current version (stable 0.14 or dev 0.15). These happen automatically now depending upon the version of CIDER you are running.

this just updates these versions in the project.clj file.



 Comments   
Comment by Enzzo Cavallo [ 05/Dec/16 5:30 AM ]

Please, bump tools.analyzer.jvm too.

[org.clojure/tools.analyzer.jvm "0.6.10"]

Comment by daniel sutton [ 05/Dec/16 7:23 AM ]

Patch added with `[org.clojure/tools.analyzer.jvm "0.6.10"]`

Comment by Alex Miller [ 06/Dec/16 10:04 AM ]

Generally we try to maintain the lowest possible minimum requirement for Clojure so that the lib can be used by as wide a group of users as possible. afaict, you don't need any new features in Clojure 1.7 or 1.8 and there is no reason to update this minimum version. You can still use it with CIDER, so that seems like a non-argument.

Comment by daniel sutton [ 07/Dec/16 10:25 AM ]

I was under the impression that the lib would use whatever versions of Clojure were in the project that loaded the lib. So updating the project.clj file affected development only. If this is incorrect I'd certainly agree with you.

CIDER doesn't officially support below 1.7 and prints a repl warning stating that nrepl and CIDER might have communication issues.

Comment by Alex Miller [ 16/Dec/16 11:38 AM ]

The lib is not in control of what version of Clojure it is used with - that's up to the user.

Pegging it to a lower version here (lowest supported) means that we are aware if the code starts using something newer (because of breakage). I don't know of any reason this project should not support back to Clojure 1.6.

As a separate issue, the project.clj file is helpful for dev (and cljs), but is not actually used when building or deploying the project - that's all controlled via the Maven pom.xml. So all dependency changes made in the project.clj dependencies also need to be mirrored in the pom.xml file.





[MATCH-115] AppPattern with single wildcard throws exception Created: 29/Jul/16  Updated: 29/Jul/16

Status: Open
Project: core.match
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Greg Chapman Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Example:

user=> (require '[clojure.core.match :as m])
nil
user=> (m/match [[1 2 3]] [(x :<< first)] x)
IllegalArgumentException No method in multimethod 'to-source' for dispatch value: null  clojure.lang.MultiFn.getFn (MultiFn.java:156)

It seems this can be fixed by adding a dummy to-source for wildcards:

user=> (defmethod m/to-source nil [pattern ocr]
  (if (m/wildcard-pattern? pattern)
    true
    (throw (AssertionError. (str "Don't know how to emit code for: " pattern)))))
#object[clojure.lang.MultiFn 0x4bb9f7d4 "clojure.lang.MultiFn@4bb9f7d4"]
user=> (m/match [[1 2 3]] [(x :<< first)] x)
1





[MATCH-113] Typo in match.clj Created: 15/Jun/16  Updated: 15/Jun/16

Status: Open
Project: core.match
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Greg Chapman Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Line 1710 of match.clj (0.3.0-alpha4) has this:

(defmethod groupable? [::predice ::predicate]
  [a b] (= (:gs a) (:gs b)))

I believe "predice" should be "predicate".






[MATCH-108] Upgrade org.clojure/tools.analyzer.jvm dependency to "0.6.6" Created: 20/May/15  Updated: 20/May/15

Status: Open
Project: core.match
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Task Priority: Minor
Reporter: Matt Foster Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Currently core.match depends on tools.analyzer.jvm "0.6.5", the latest release is "0.6.6". The outdated dependency on tools.analyzer.jvm conflicts with core.async's dependency on version "0.6.6". This can cause unexpected behavior when a project depends on both core.match and core.async.



 Comments   
Comment by David Nolen [ 20/May/15 3:03 PM ]

In the meantime you can use Leiningen or Maven to control what you get.





[MATCH-105] Regex: only attempt to match string values. Created: 06/Mar/15  Updated: 06/Mar/15

Status: Open
Project: core.match
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Pavel Repin Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File regex-match-safely.patch    
Patch: Code and Test

 Description   

Before this fix, matching a regex pattern against a non-string value would crash like so:

java.lang.ClassCastException: clojure.lang.Keyword cannot be cast to java.lang.CharSequence
 at clojure.core$re_matcher.invoke (core.clj:4460)
    clojure.core$re_matches.invoke (core.clj:4497)
...





[MATCH-103] Support for lists in patterns. Created: 18/Dec/14  Updated: 18/Dec/14

Status: Open
Project: core.match
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Jan-Paul Bultmann Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Currently it is not possible to use list literals with or without variables in a pattern.
Thus something like this

(match ['(1 2 3)]
       ['(a b c)] a)

will result in a failing assert.

AssertionError Invalid list syntax (a b c) in (quote (a b c)).

There is a workaround for this by using `:seq` and `:guard`, as in

(match ['(1 2 3)]
       [(([a b c] :seq) :guard list?)] a)

but it is rather tedious to write.

List matching would be very useful when writing macros and compilers.






[MATCH-72] complexity analysis Created: 18/Jun/13  Updated: 18/Jun/13

Status: Open
Project: core.match
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: David Nolen Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None


 Description   

For smaller matches avoid backtracking actually greatly improves performance at the potential cost of code size. Perhaps we can devise a heuristic such that we employ the the best option available? Would need some serious hammock time.






[MATCH-64] Improve match compile times Created: 15/Aug/12  Updated: 15/Jun/13

Status: Open
Project: core.match
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: David Nolen Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None





[MATCH-8] Implement (p|q)ba heuristics Created: 04/Sep/11  Updated: 04/Sep/11

Status: Open
Project: core.match
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: David Nolen Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None


 Description   

We should implement the rest of Maranget's suggested heuristics.

p - needed prefix
q - constructor prefix
b - branching factor
a - constructor arity






[MATCH-2] Matching Diagnostics Created: 04/Sep/11  Updated: 16/Jun/13

Status: Open
Project: core.match
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: David Nolen Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Communicate to the user precisely what failed to match. Conversation here: http://groups.google.com/group/clojure/browse_thread/thread/675456fba1712214. We adopt the behavior of condp since that is closer to what match does and will do (predicate dispatch)



 Comments   
Comment by David Nolen [ 05/Sep/11 10:40 AM ]

Continuing the conversation from GitHub - concerning your changes, we should probably show what current occurrence failed to match as well as the breadcrumb.

Comment by David Nolen [ 26/Sep/11 7:10 AM ]

Matching diagnostics will be complicated by MATCH-1. Clojure doesn't yet support simple data conveying exceptions. Will probably have to use a combination of proxy and definterface.

Comment by Ambrose Bonnaire-Sergeant [ 30/Sep/11 4:39 AM ]

I haven't yet kept up with your backtracking changes, and I just noticed this conversation, so I'll have to have another look at diagnostics with backtracking.

Comment by David Nolen [ 16/Jun/13 1:57 PM ]

Lowering priority. Happy to take a patch, but I don't think I'll be looking into this myself in the near future.





Generated at Mon Feb 27 17:06:34 CST 2017 using JIRA 4.4#649-r158309.