[CLJS-2915] Tests fail if directory has a period (.) in the path Created: 16/Sep/18  Updated: 25/Sep/18

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 1.10.238
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Ray McDermott Assignee: Ray McDermott
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File CLJS-2915.patch    
Patch: Code

 Description   

Check out compiler into a cljs.dev directory and run lein test :only cljs.module-processing-tests.

Seems very similar to CLJS-2914.

$ lein test :only cljs.module-processing-tests

lein test cljs.module-processing-tests

lein test :only cljs.module-processing-tests/test-module-name-substitution

FAIL in (test-module-name-substitution) (module_processing_tests.clj:124)
expected: (= (str "goog.provide('my_calculator.core');" crlf "goog.require('cljs.core');" crlf "goog.require('" (absolute-module-path "src/test/cljs/calculator.js" true) "');" crlf) (compile (quote (ns my-calculator.core (:require [calculator :as calc :refer [subtract add] :rename {subtract sub}])))))
  actual: (not (= "goog.provide('my_calculator.core');\ngoog.require('cljs.core');\ngoog.require('module$Users$ray$dev$cljs.dev$clojurescript$src$test$cljs$calculator');\n" "goog.provide('my_calculator.core');\ngoog.require('cljs.core');\ngoog.require('module$Users$ray$dev$cljs_dev$clojurescript$src$test$cljs$calculator');\n"))

lein test :only cljs.module-processing-tests/test-module-name-substitution

FAIL in (test-module-name-substitution) (module_processing_tests.clj:129)
expected: (= output (compile (quote (calc/add 3 4))))
  actual: (not (= "module$Users$ray$dev$cljs.dev$clojurescript$src$test$cljs$calculator[\"default\"].add((3),(4));\n" "module$Users$ray$dev$cljs_dev$clojurescript$src$test$cljs$calculator[\"default\"].add((3),(4));\n"))

lein test :only cljs.module-processing-tests/test-module-name-substitution

FAIL in (test-module-name-substitution) (module_processing_tests.clj:130)
expected: (= output (compile (quote (calculator/add 3 4))))
  actual: (not (= "module$Users$ray$dev$cljs.dev$clojurescript$src$test$cljs$calculator[\"default\"].add((3),(4));\n" "module$Users$ray$dev$cljs_dev$clojurescript$src$test$cljs$calculator[\"default\"].add((3),(4));\n"))

lein test :only cljs.module-processing-tests/test-module-name-substitution

FAIL in (test-module-name-substitution) (module_processing_tests.clj:131)
expected: (= output (compile (quote (add 3 4))))
  actual: (not (= "module$Users$ray$dev$cljs.dev$clojurescript$src$test$cljs$calculator[\"default\"].add((3),(4));\n" "module$Users$ray$dev$cljs_dev$clojurescript$src$test$cljs$calculator[\"default\"].add((3),(4));\n"))

lein test :only cljs.module-processing-tests/test-module-name-substitution

FAIL in (test-module-name-substitution) (module_processing_tests.clj:132)
expected: (= (str (absolute-module-path "src/test/cljs/calculator.js" true) "[\"default\"].subtract((5),(4));" crlf) (compile (quote (sub 5 4))))
  actual: (not (= "module$Users$ray$dev$cljs.dev$clojurescript$src$test$cljs$calculator[\"default\"].subtract((5),(4));\n" "module$Users$ray$dev$cljs_dev$clojurescript$src$test$cljs$calculator[\"default\"].subtract((5),(4));\n"))

lein test :only cljs.module-processing-tests/commonjs-module-processing-preprocess-symbol

FAIL in (commonjs-module-processing-preprocess-symbol) (module_processing_tests.clj:191)
Processed modules are added to :js-module-index
expected: (= {"React" {:name (absolute-module-path "src/test/cljs/reactJS.js"), :module-type :commonjs}, "Circle" {:name (absolute-module-path "src/test/cljs/Circle.js"), :module-type :commonjs}} (:js-module-index (clojure.core/deref cenv)))
  actual: (not (= {"React" {:name "module$Users$ray$dev$cljs.dev$clojurescript$src$test$cljs$reactJS", :module-type :commonjs}, "Circle" {:name "module$Users$ray$dev$cljs.dev$clojurescript$src$test$cljs$Circle", :module-type :commonjs}} {"React" {:name "module$Users$ray$dev$cljs-dev$clojurescript$src$test$cljs$reactJS", :module-type :commonjs}, "Circle" {:name "module$Users$ray$dev$cljs-dev$clojurescript$src$test$cljs$Circle", :module-type :commonjs}}))

lein test :only cljs.module-processing-tests/es6-module-processing

FAIL in (es6-module-processing) (module_processing_tests.clj:97)
Processed modules are added to :js-module-index
expected: (= {"es6-hello" {:name (absolute-module-path "src/test/cljs/es6_hello.js"), :module-type :es6}} (:js-module-index (clojure.core/deref cenv)))
  actual: (not (= {"es6-hello" {:name "module$Users$ray$dev$cljs.dev$clojurescript$src$test$cljs$es6-hello", :module-type :es6}} {"es6-hello" {:name "module$Users$ray$dev$cljs-dev$clojurescript$src$test$cljs$es6-hello", :module-type :es6}}))

lein test :only cljs.module-processing-tests/test-cljs-1822

FAIL in (test-cljs-1822) (module_processing_tests.clj:162)
Processed modules are added to :js-module-index
expected: (= {"React" {:name (absolute-module-path "src/test/cljs/react-min.js"), :module-type :commonjs}, "Circle" {:name (absolute-module-path "src/test/cljs/Circle-min.js"), :module-type :commonjs}} (:js-module-index (clojure.core/deref cenv)))
  actual: (not (= {"React" {:name "module$Users$ray$dev$cljs.dev$clojurescript$src$test$cljs$react-min", :module-type :commonjs}, "Circle" {:name "module$Users$ray$dev$cljs.dev$clojurescript$src$test$cljs$Circle-min", :module-type :commonjs}} {"React" {:name "module$Users$ray$dev$cljs-dev$clojurescript$src$test$cljs$react-min", :module-type :commonjs}, "Circle" {:name "module$Users$ray$dev$cljs-dev$clojurescript$src$test$cljs$Circle-min", :module-type :commonjs}}))

lein test :only cljs.module-processing-tests/commonjs-module-processing

FAIL in (commonjs-module-processing) (module_processing_tests.clj:71)
Processed modules are added to :js-module-index
expected: (= {"React" {:name (absolute-module-path "src/test/cljs/reactJS.js"), :module-type :commonjs}, "Circle" {:name (absolute-module-path "src/test/cljs/Circle.js"), :module-type :commonjs}} (:js-module-index (clojure.core/deref cenv)))
  actual: (not (= {"React" {:name "module$Users$ray$dev$cljs.dev$clojurescript$src$test$cljs$reactJS", :module-type :commonjs}, "Circle" {:name "module$Users$ray$dev$cljs.dev$clojurescript$src$test$cljs$Circle", :module-type :commonjs}} {"React" {:name "module$Users$ray$dev$cljs-dev$clojurescript$src$test$cljs$reactJS", :module-type :commonjs}, "Circle" {:name "module$Users$ray$dev$cljs-dev$clojurescript$src$test$cljs$Circle", :module-type :commonjs}}))

Ran 5 tests containing 14 assertions.
9 failures, 0 errors.
Tests failed.


 Comments   
Comment by Ray McDermott [ 16/Sep/18 8:13 AM ]

It's looking for cljs_dev

Comment by Ray McDermott [ 25/Sep/18 4:51 PM ]

Building on the code fix for CLJS-2782 we can replace the periods with hyphens and then leverage the cond-> to selectively transform to underscores.

Comment by Ray McDermott [ 25/Sep/18 4:59 PM ]

Patch attached

Comment by Mike Fikes [ 25/Sep/18 9:06 PM ]

Hey Ray, this patch doesn't apply on master. Is the intent that this patch depend on the patch in CLJS-2782 being applied first? (Usually patches are independent, but in some cases a patch can depend on changes in another ticket if it needs to.)





[CLJS-2793] Instrumenting breaks function with varargs Created: 26/Jun/18  Updated: 25/Sep/18

Status: In Progress
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Critical
Reporter: Yuri Govorushchenko Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: None
Environment:

CLJS 1.10.238 (also reproducible in Lumo 1.9.0-alpha), 1.10.340


Attachments: Text File experiment.patch    

 Description   

Steps:

(require '[cljs.spec.alpha :as s]
         '[cljs.spec.test.alpha :as stest])
(defn foo [m & args] (prn :m m :args args))
(s/fdef foo)
(stest/instrument)
(foo {:x 1 :y 2})

Actual:

:m [:x 1] :args ([:y 2])

Expected:

:m {:x 1, :y 2} :args nil

Couldn't reproduce in Clojure 1.9.0 and CLJS 1.9.946.



 Comments   
Comment by Mike Fikes [ 14/Jul/18 4:37 PM ]

This one is fairly vexing, involving the same bit of code dealt with in CLJS-2397.

The attached experiment.patch, while somewhat hackish, appears to solve the issue, but only does so in the REPL; it fails in advanced with the added unit tests.

Comment by Andrea Richiardi [ 25/Sep/18 5:40 PM ]

Yes I still see this:

cljs.user> (require '[cljs.spec.alpha :as s] '[cljs.spec.test.alpha :as stest])
nil
cljs.user> (defn foo [m & args] (prn :m m :args args))
#'cljs.user/foo
cljs.user> (s/fdef foo)
cljs.user/foo
cljs.user> (stest/instrument)
[cljs.user/foo]
cljs.user> (foo {:x 1 :y 2})
nil
cljs.user> *clojurescript-version*
"1.10.339"

Other dep I have is:

org.clojure/test.check {:mvn/version "0.10.0-alpha3" :exclusions [org.clojure/clojure]}




[CLJS-2782] lein test fails if directory has hyphens Created: 16/Jun/18  Updated: 25/Sep/18

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None
Environment:

Master as of https://github.com/clojure/clojurescript/commit/7528dc79fea372fafa365ee80e4568a3af5f1526


Attachments: Text File CLJS-2782.patch    
Patch: Code

 Description   

Create a directory with hyphens in its name, and then clone clojurescript into it and run lein test.

lein test :only cljs.module-processing-tests/commonjs-module-processing-preprocess-symbol

FAIL in (commonjs-module-processing-preprocess-symbol) (module_processing_tests.clj:191)
Processed modules are added to :js-module-index
expected: (= {"React" {:name (absolute-module-path "src/test/cljs/reactJS.js"), :module-type :commonjs}, "Circle" {:name (absolute-module-path "src/test/cljs/Circle.js"), :module-type :commonjs}} (:js-module-index (clojure.core/deref cenv)))
  actual: (not (= {"React" {:name "module$Users$mfikes$Desktop$dir_with_hypens$clojurescript$src$test$cljs$reactJS", :module-type :commonjs}, "Circle" {:name "module$Users$mfikes$Desktop$dir_with_hypens$clojurescript$src$test$cljs$Circle", :module-type :commonjs}} {"React" {:name "module$Users$mfikes$Desktop$dir-with-hypens$clojurescript$src$test$cljs$reactJS", :module-type :commonjs}, "Circle" {:name "module$Users$mfikes$Desktop$dir-with-hypens$clojurescript$src$test$cljs$Circle", :module-type :commonjs}}))

lein test :only cljs.module-processing-tests/es6-module-processing

FAIL in (es6-module-processing) (module_processing_tests.clj:97)
Processed modules are added to :js-module-index
expected: (= {"es6-hello" {:name (absolute-module-path "src/test/cljs/es6_hello.js"), :module-type :es6}} (:js-module-index (clojure.core/deref cenv)))
  actual: (not (= {"es6-hello" {:name "module$Users$mfikes$Desktop$dir_with_hypens$clojurescript$src$test$cljs$es6-hello", :module-type :es6}} {"es6-hello" {:name "module$Users$mfikes$Desktop$dir-with-hypens$clojurescript$src$test$cljs$es6-hello", :module-type :es6}}))

lein test :only cljs.module-processing-tests/test-cljs-1822

FAIL in (test-cljs-1822) (module_processing_tests.clj:162)
Processed modules are added to :js-module-index
expected: (= {"React" {:name (absolute-module-path "src/test/cljs/react-min.js"), :module-type :commonjs}, "Circle" {:name (absolute-module-path "src/test/cljs/Circle-min.js"), :module-type :commonjs}} (:js-module-index (clojure.core/deref cenv)))
  actual: (not (= {"React" {:name "module$Users$mfikes$Desktop$dir_with_hypens$clojurescript$src$test$cljs$react-min", :module-type :commonjs}, "Circle" {:name "module$Users$mfikes$Desktop$dir_with_hypens$clojurescript$src$test$cljs$Circle-min", :module-type :commonjs}} {"React" {:name "module$Users$mfikes$Desktop$dir-with-hypens$clojurescript$src$test$cljs$react-min", :module-type :commonjs}, "Circle" {:name "module$Users$mfikes$Desktop$dir-with-hypens$clojurescript$src$test$cljs$Circle-min", :module-type :commonjs}}))

lein test :only cljs.module-processing-tests/commonjs-module-processing

FAIL in (commonjs-module-processing) (module_processing_tests.clj:71)
Processed modules are added to :js-module-index
expected: (= {"React" {:name (absolute-module-path "src/test/cljs/reactJS.js"), :module-type :commonjs}, "Circle" {:name (absolute-module-path "src/test/cljs/Circle.js"), :module-type :commonjs}} (:js-module-index (clojure.core/deref cenv)))
  actual: (not (= {"React" {:name "module$Users$mfikes$Desktop$dir_with_hypens$clojurescript$src$test$cljs$reactJS", :module-type :commonjs}, "Circle" {:name "module$Users$mfikes$Desktop$dir_with_hypens$clojurescript$src$test$cljs$Circle", :module-type :commonjs}} {"React" {:name "module$Users$mfikes$Desktop$dir-with-hypens$clojurescript$src$test$cljs$reactJS", :module-type :commonjs}, "Circle" {:name "module$Users$mfikes$Desktop$dir-with-hypens$clojurescript$src$test$cljs$Circle", :module-type :commonjs}}))

Perhaps related to CLJS-2703.



 Comments   
Comment by Ray McDermott [ 23/Sep/18 3:22 PM ]

Problem is that when munge uses the CLJ option, it emits the folder with `_` rather than `-`.

This constraint doesn't always seems to be applicable for CLJS.

Comment by Ray McDermott [ 23/Sep/18 3:26 PM ]

Note that the fix for CLJS-2703 does not enable the tests to pass

Comment by Ray McDermott [ 23/Sep/18 3:30 PM ]

One option could be to exclude the directory path from munging.

munge is already quite complex so having an additional special case might be undesirable.

Comment by Ray McDermott [ 23/Sep/18 3:39 PM ]

Another option is to have the tests support both munged and unmanaged paths locally.

It's also a little confusing but keeps the changes in the test code which is probably preferred.

Comment by Ray McDermott [ 23/Sep/18 3:49 PM ]

A mix of using the real / patched base directory works.

Seems like a hack solution but I'll create a patch for review.

Comment by Mike Fikes [ 24/Sep/18 2:05 PM ]

Hey Ray, you're right in that the change for CLJS-2703 broke this. That patch ended up fixing things for test-module-name-substitution by introducing a (string/replace $ "-" "_").

It turns out that test-module-name-substitution has the only places where the code? parameter in absolute-module-path is passed as true, so it seems that this could be leveraged to fix things. In short, the (string/replace $ "-" "_") introduced with CLJS-2703 could be changed to only conditionally do it by changing that offending line to instead be

(cond-> $ code? (string/replace "-" "_"))
Comment by Ray McDermott [ 25/Sep/18 3:17 PM ]

Ha - yeah, that's great and I've just tested it and works like a charm. I make a new patch.

Comment by Ray McDermott [ 25/Sep/18 3:25 PM ]

cond-> fix

Comment by Ray McDermott [ 25/Sep/18 3:43 PM ]

We can probably use the same fix approach for CLJS-2915.

I look forward to the emoji and unicode reports.

Comment by Mike Fikes [ 25/Sep/18 4:03 PM ]

CLJS-2782.patch LGTM. It is safe to apply as it only revises the test code like CLJS-2703. (As was deemed with CLJS-2703, this is probably only a problem with the test code and there is no real issue with the production code).

I tried it with a directory with hyphens and it worked for me, and it also passes in CI.





[CLJS-2652] cljs.main: Line breaks in long options for compile Created: 12/Mar/18  Updated: 25/Sep/18

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Trivial
Reporter: Mike Fikes Assignee: Ray McDermott
Resolution: Unresolved Votes: 0
Labels: newbie
Environment:

1.10.160



 Description   

Perhaps the -- should be kept with compile in the two spots in the help output:

init options only for --compile:
   -O, --optimizations level  Set optimization level, only effective with --
                              compile main option. Valid values are: none,
                              whitespace, simple, advanced
   -o, --output-to file       Set the output compiled file
   -w, --watch path           Continuously build, only effective with the --
                              compile main option





[CLJS-1474] Error if reserved symbol is defined Created: 21/Oct/15  Updated: 25/Sep/18

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Martin Klepsch Assignee: António Nuno Monteiro
Resolution: Unresolved Votes: 2
Labels: newbie

Attachments: Text File CLJS-1474.patch    
Patch: Code and Test

 Description   

Currently a definition like

(defn set! [] ...)

will not cause any warning. Any usage of it (without :as namespace aliasing) however will not use the defined var but the set! special form.

A warning seems appropriate.



 Comments   
Comment by António Nuno Monteiro [ 30/Jul/16 1:19 PM ]

Attached patch with fix and test.

Comment by Mike Fikes [ 31/Jul/16 2:09 PM ]

I know David suggested that a hard error is probably the right thing to do for this one, but one consequence is that the cljs.spec/def macro cannot be defined in bootstrap with this change. (I haven't investigated thoroughly, but this may simply be the result of macros being processed as ClojureScript in bootstrap, and thus being subject to this new guard.)

Regardless of the root cause, you'll see this if you try to run script/test-self-parity:

#error {:message "Could not eval cljs.spec", :data {:tag :cljs/analysis-error}, :cause #error {:message "Can't def special form at line 51 ", :data {:file nil, :line 51, :column 1, :tag :cljs/analysis-error}}}

For reference: Line 51 currently points at the def macro: https://github.com/clojure/clojurescript/blob/e2db5d9ff8cb6a099ebc2a8cd379385bf4649b38/src/main/cljs/cljs/spec.cljc#L51

Comment by Mike Fikes [ 14/Mar/18 7:23 PM ]

Patch no longer applies.

Comment by Ray McDermott [ 25/Sep/18 5:19 PM ]

This is assigned to António Nuno Monteiro but it seems like nothing has happened for a while.

Is it OK if I take it over?





[CLJS-2683] Suppress compiler options in watch log Created: 21/Mar/18  Updated: 25/Sep/18

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: newbie
Environment:

{:deps {org.clojure/clojurescript {:mvn/version "1.10.217"}}}


Attachments: Text File cljs-2683.patch     Text File CLJS-2683.patch    
Patch: Code

 Description   

If running watch via cljs.main compiler options logged for each watch event.

src/hello_world/core.cljs
(ns hello-world.core)
watch.clj
(require 'cljs.build.api)

(cljs.build.api/watch "src"
  {:main 'hello-world.core
   :output-to "out/main.js"})

If you

clj watch.clj

and tail -f out/watch.log and touch src/hello_world/core.cljs, you will see

Options passed to ClojureScript compiler: 

along with a fairly verbose options EDN dump, for every watch event.

The same happens with

clj -Srepro -m cljs.main -w src -c hello-world.core -r


 Comments   
Comment by Herald Reierskog [ 24/Sep/18 1:51 PM ]

I checked other uses of cljs.analyzer in the file, and there's no deeper level of logging. I guess just removing the log statement entirely is suitable?

Comment by Mike Fikes [ 24/Sep/18 9:54 PM ]

Note, Herald indicated via private Slack message to me that the CA has been signed.

Comment by Mike Fikes [ 24/Sep/18 10:12 PM ]

Herald, I don't think the log statement can be removed entirely. That would effectively undo CLJS-2436. For example, you want the compiler options to be logged when executing clj -m cljs.main -v. (In other words, outside of a watch-triggered build.)

I suspect that to fix this would involve somehow conditionally disabling the logging when compilation is occurring in response to a watch event: Perhaps in cljs.closure/watch, in the internally defined buildf, when it calls build it could assoc an extra "private" option onto the opts argument, like ::watch-triggered-build? true, and then additionally check that this particular option is falsey in the when test used for logging the options.

Comment by Herald Reierskog [ 25/Sep/18 1:58 PM ]

Thank you for the detailed instructions, Mike. I attached a new patch that only disables the log statement for watch-triggered builds.

Comment by Mike Fikes [ 25/Sep/18 3:33 PM ]

I tried CLJS-2683.patch out and it works for me. It also passes CI tests.





[CLJS-2921] int? predicates inconsistent with Clojure Created: 25/Sep/18  Updated: 25/Sep/18

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Mike Fikes Assignee: Mike Fikes
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File CLJS-2921-A.patch     Text File CLJS-2921-B.patch    
Patch: Code

 Description   

This ClojureScript change
https://github.com/clojure/clojurescript/commit/bcf60ce194e5292fbc5c4b2d89dfc5a7b886b94c
tracked this Clojure change
https://github.com/clojure/clojure/commit/20f67081b7654e44e960defb1e4e491c3a0c2c8b

A consequence is that int? is satisfied by goog.math.Integer which is not fixed-precision.
Also, it creates a situation where it is possible to have values that satisfy int? but don't satisfy integer? or number?. (Note that this also affects things like pos-int?.)

I'd propose two alternative approaches to fix this that we can debate:

Alternative A: Simply remove the goog.math items from int? and friends.

Alternative B: Have goog.math.Long values satisfy number?, integer?, and int? and have goog.math.Integer values satisfy number? and integer?.

In either case, the docstrings should be updated.

I'll attached two concrete patches for consideration / debate.



 Comments   
Comment by Francis Avila [ 25/Sep/18 12:35 PM ]

What should be the goal for the various numeric predicates in CLJS?

Is the goal to align with CLJ's meaning of these predicates? If so then:
number? true for typeof-number or goog.math.Integer or goog.math.Long
integer? true for goog.math.isInt/Number.isInteger or goog.math.Integer or goog.math.Long
int? true for goog.math.isInt/Number.isInteger or goog.math.Long

Is the goal to align with some notion of platform-primitiveness? If so then:
number? true for typeof-number or goog.math.Integer or goog.math.Long
integer? true for goog.math.isInt/Number.isInteger or goog.math.Integer or goog.math.Long
int? true for goog.math.isInt only

Is the goal to be a can-I-do-arithmetic-with-this type check? If so then:
number? true for typeof-number only
integer? true for goog.math.isInt (or maybe even Number.isSafeInteger)
int? same as integer?

In Java/CLJ these three goals are much more aligned with one another. Any number-as-identifier can also be used for clj arithmetic (so the widest check, number?, can also be used for arithmetic checking), and int? corresponds exactly to the host's most-primitive integer value-types (vs object-type).

CLJS lacks a number tower so it's more interesting.
Right now in CLJS number? appears to be designed as a predicate to test for whether arithmetic is possible, so it only allows js type-of===number. integer? is also an arithmetic check, but wants a non-fractional number (interestingly, still allowing unsafe integers, i.e. outside Number.MAX_SAFE_INTEGER, where normal integer arithmetic doesn't work--perhaps it should only allow safe integers). int? doesn't care about arithmetic, but it's still useful for seeing if a value could be a numeric identifier (e.g. a record id received via transit); but that's expressly not what CLJ's int? cares about.

Comment by Mike Fikes [ 25/Sep/18 2:41 PM ]

The attached A and B patches explore the two approaches.

Patch A could probably be applied as it exists now.

Patch B on the other hand seems to have lots of consequences related to whether something that satisfies number? can participate in arithmetic or be used in other places where numbers are currently used. I suspect we could make all of this work but we'd be giving up a lot of perf.

As one concrete example: Is this a valid program, and should it return true?

(zero? goog.math.Integer/ZERO)




[CLJS-2901] Return type inference on multi-arity and variadic defn Created: 12/Sep/18  Updated: 25/Sep/18

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Mike Fikes Assignee: Mike Fikes
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File CLJS-2901-0.patch     Text File CLJS-2901-1.patch     Text File CLJS-2901-2.patch     Text File CLJS-2901-3.patch     Text File CLJS-2901-4.patch    
Patch: Code

 Description   

CLJS-1997 currently only supports a single fixed arity defn.

I'm thinking that, during macroexpansion of defn we might be able to add :tag meta.

With multi-arity, we could infer tag as any if the types on different branches differ, or produce a union. If we make a union, there are some nice utility fns in CLJS-2869 that could be copied into the same place in the code and used. (These make it easier to work with tags as sets and to union them, and then collapse back down to a canonical representation.)



 Comments   
Comment by Mike Fikes [ 12/Sep/18 3:15 PM ]

Hey Martin, I was curious about the single arity variadic defn case and took a look at it. A potential way of doing this (without yet adding tests) is in CLJS-2901-0.patch.

I haven't yet looked at the other more complicated cases, but I still think that it might be possible by calling the analyzer during macroexpansion, unioning tags produced for each arity (using some of the utilities sitting in CLJS-2869), but I'm not yet sure.

If you think you are still interested in trying a patch for some of this, let me know. Otherwise feel free to assign this ticket back to me.

Comment by Mike Fikes [ 13/Sep/18 6:22 AM ]

A few things I thought of after having written CLJS-2901-0.patch:

  1. It doesn't handle the case where the user hints a variadic defn as in (defn ^string [& more] (.substring "abc" 1)).
  2. It might slow down compilation (one test is at https://github.com/mfikes/cljs-perf/)

If it slows down compilation, perhaps a fix would be to have it analyze exactly the same form that is produced for the variadic fn. (I think it is analyzing a slightly different form leaving the & in the arg list. If doing that maybe also turn off nowarn. I'm speculating that the way the patch is written, it will double analyze, but with the right revisions we can have it analyze once with to analyze calls (the analyzer marks forms it has analyzed), speeding up perf and triggering warnings only once.

Comment by Mike Fikes [ 13/Sep/18 8:03 AM ]

CLJS-2901-0.patch exhibits a speedup of 0.91 on https://github.com/mfikes/cljs-perf so perhaps there is indeed some perf-work to be done to ensure no regression.

Comment by Martin Kučera [ 18/Sep/18 4:30 AM ]

With multi-arity macroexpansion add union of each branch tags. Variadic macroexpansion now add tag.

I did'n measure performance because HyperV results of individual measurements are affected by the guest OS.

Comment by Martin Kučera [ 18/Sep/18 4:33 AM ]

Mike can you verify it, please?

Comment by Mike Fikes [ 18/Sep/18 6:15 AM ]

Thanks. I'm taking a look at it.

At some point, can you create a squashed patch that contains all of the changes? (No need to worry about my attribution.)

Comment by Martin Kučera [ 18/Sep/18 6:30 AM ]

Like this?

Comment by Mike Fikes [ 18/Sep/18 7:32 AM ]

High level comments (I haven't yet reviewed the actual content of the patch):

  • Patches can be done in a sequence with increasing numbers
  • Perf of CLJS-2901-1.patch may be OK. (Coal Mine compilation didn't slow down.)
  • Some of the helper methods could be revised to be exact copies of those being introduced in CLJS-2869
  • script/test-self-parity is failing with the patch
  • Large amounts of inadvertent whitespace reformatting is in the patch
Comment by Martin Kučera [ 20/Sep/18 4:44 AM ]

Helper functions from CLJS-2869 copied.
Overwritten with the copied functions.
test-self-parity now passed

Comment by Martin Kučera [ 20/Sep/18 4:48 AM ]

Check new patch, please.

Comment by Mike Fikes [ 20/Sep/18 6:21 AM ]

Can you make a squashed patch?

Comment by Martin Kučera [ 20/Sep/18 8:42 AM ]

patch squashed and excess whitespace changes removed

Comment by Mike Fikes [ 20/Sep/18 9:04 AM ]

This case doesn't appear to be handled correctly:

(defn foo (^boolean [x] true) ([x y] ""))

Then

(foo 1 2)

is inferred to be type boolean and, for example,

(if (foo 1 2) 1 2)

incorrectly evaluates to 2.

Comment by Martin Kučera [ 21/Sep/18 5:14 AM ]

fixed
need new performance test

Comment by Mike Fikes [ 21/Sep/18 7:08 AM ]

Great! I checked the behavior and it looks good for that case.

In terms of perf, the Coal Mine compilation tests (https://github.com/mfikes/cljs-perf) are still showing a speedup of around 1.0. But, I don't think those tests involve much multi-rarity and variadic function compilation.

You can look at the perf of compiling core and the test suite by doing script/clean, script/bootstrap, script/test and then looking for a line like

Compile sources, elapsed time: 13818.681484 msecs

With that, I'm consistently seeing a speedup of 0.95.

This is a sufficient slowdown where it may be worth trying to find an optimization. My immediate though is to try to avoid double analysis. (Avoid using no-warn and ensure that the forms being analyzed are exactly the same as what would be analyzed later. This is not currently the case with the variadic code, but if you look, you can see how the method signature can be adjusted so that this is indeed the case. If no-warn is removed, then you will definitely see the double-analysis occurring if you compile a multi-rarity or variadic function that has code that would cause a warn. And, if single-analysis is occurring, I'd expect any warns from the body to appear only once.

Comment by Mike Fikes [ 21/Sep/18 12:41 PM ]

I did a more careful test given the variability in the result you can get, letting the following run through 8 cycles:

unset NASHORN_HOME
unset GRAALVM_HOME
unset V8_HOME
unset SPIDERMONKEY_HOME
while true; do script/clean; script/bootstrap > /dev/null; script/test 2>&1 | grep "Compile sources"; done

With this for master I currently get a mean of 12272 ms with standard deviation of 342 ms. With the patch, I get a mean of 13012 ms and standard deviation of 362 ms.

With this the speedup is 0.94.

Comment by Mike Fikes [ 25/Sep/18 8:22 AM ]

I'll take a look to see if there might be a way to avoid the double analysis.





[CLJS-2794] with-meta should return identity when new meta is identical to prior Created: 27/Jun/18  Updated: 24/Sep/18

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 1.10.238
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Mike Fikes Assignee: Jacek Schae
Resolution: Unresolved Votes: 0
Labels: newbie


 Description   

For persistent objects, if meta is identical, with-meta should return the identical object rather than replacing meta and returning a new object.

Like CLJ-2362 but for ClojureScript.

For example,

(let [coll ^:foo [1]] 
  (identical? coll (with-meta coll (meta coll))))

now evaluates to true in Clojure 1.10.0-alpha5.

I suspect this can be achieved by updating all of pertinent IWithMeta implementations in cljs.core to be like the following revision for PersistentVector

IWithMeta
  (-with-meta [coll new-meta]
    (if (identical? meta new-meta)
      coll
      (PersistentVector. new-meta cnt shift root tail __hash)))

while also dealing with the trick that vec and set employ when using with-meta to force a new object to be created (see CLJS-2442).



 Comments   
Comment by Erik Assum [ 24/Sep/18 3:59 PM ]

Following a discussion on clojurians #clojure-dev, the second part of this ticket can be skipped (i.e ensuring that a new vector/set is returned for vec and set, respectively

Comment by Mike Fikes [ 24/Sep/18 4:10 PM ]

In fact, with CLJ-2362 which is in the current Clojure 1.10 alphas, you can see that the concern I raised at the end of the description (the need to force a new object to be created) is no longer one in Clojure. Erik has submitted CLJ-2405, a candidate doc string change.

The nice thing for this ticket is that nothing special need be done to handle the places where vec and set call (with-meta coll nil). And, thus, when using these as coercion functions on meta-less collections of the right type, they effectively act like the identity function.

There are a couple of assertions in test-cljs-2442 regarding identical? that will need to be removed.





[CLJS-1297] defrecord does not emit IKVReduce protocol Created: 03/Jun/15  Updated: 24/Sep/18

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 1.7.145
Fix Version/s: 1.9.671

Type: Defect Priority: Major
Reporter: Daniel Skarda Assignee: Samuel Miller
Resolution: Unresolved Votes: 2
Labels: newbie

Attachments: Text File CLJS-1297-19-July-2015.patch    
Patch: Code and Test

 Description   

Records are maps and in Clojure they support reduce-kv (IKVReduce protocol).
This is not true in ClojureScript:

(defrecord Foobar [x y])
 (reduce-kv assoc {} (Foobar. 1 2))

Fails wit Error: No protocol method IKVReduce.-kv-reduce defined for type : [object Object]



 Comments   
Comment by David Nolen [ 03/Jun/15 7:25 PM ]

Just seems like an oversight. Patch welcome, this one is a relatively easy one.

Comment by Daniel Skarda [ 04/Jun/15 2:53 AM ]

OK

I checked Clojure implementation. Records do not implement any reduce protocol on their own. For IKVReduce records use default implementation using reduce and destructuring. Is this approach OK?

Recently Alex Miller implemented many optimizations of reduce protocols in Clojure. Eg range returns an object which implements IReduce protocol so reduce (and transducers in general) can take advantage of it. Any plans for such optimizations in ClojureScript?

;;clojure/src/clj/clojure/core.clj:6523
;;slow path default
clojure.lang.IPersistentMap
(kv-reduce 
  [amap f init]
  (reduce (fn [ret [k v]] (f ret k v)) init amap))
Comment by David Nolen [ 04/Jun/15 9:05 AM ]

Going with the Clojure implementation is fine. Yes all of the optimizations in 1.7.0 are on the table for ClojureScript but these are separate issues from this one.

Comment by Samuel Miller [ 16/Jul/15 10:39 PM ]

Mind if I take this as my first cljs bug? Poking around quickly I think I know what needs to happen.

Comment by David Nolen [ 17/Jul/15 5:21 AM ]

Sure! Have you submitted your CA yet?

Comment by Samuel Miller [ 17/Jul/15 7:13 PM ]

Yes, I did yesterday.

Comment by Samuel Miller [ 20/Jul/15 9:52 PM ]

Here is a potential patch. I implemented a basic IKVreduce based on Daniel Skarda's comment. Note: I am a little fuzzy on macros still so please look over what I have. There is probably a better way. Also added a test for reduce-kv on records.

I ran the test on Linux on V8 and SpiderMonkey. I plan to get JSC and Nashorn working and tested this week but if someone wants to test them out before that would be great.

Comment by Sebastian Bensusan [ 23/Jul/15 6:45 PM ]

Experience report:

I just tested the patch in the Node Repl and it seems to work:

cljs.user=> (defrecord A [a b])
cljs.user/A
cljs.user=> (reduce-kv (fn [m k v] (assoc m k (inc v))) {} (A. 1 2))
{:a 2, :b 3}

and the provided tests passed in Spidermonkey, V8, and Nashorn (I don't have JSC installed).

For completeness: before applying the patch the same code fails with:

Error: No protocol method IKVReduce.-kv-reduce defined for type : [object Object]
Comment by David Nolen [ 10/Aug/15 10:22 PM ]

Is this the same approach taken by Clojure?

Comment by Samuel Miller [ 10/Aug/15 10:36 PM ]

You can see the relevant current Clojure code here...
https://github.com/clojure/clojure/blob/master/src/clj/clojure/core.clj#L6526
I think it is the same. I literally just tried to translate it over into CLJS. I might of understood something wrong though.

Comment by David Nolen [ 11/Aug/15 6:10 AM ]

Yes that's the slow path. Please use the implementation used by defrecord instead. If defrecord doesn't have one then this patch is OK.

Comment by Samuel Miller [ 11/Aug/15 8:48 PM ]

As far as I can tell there is no implementation on defrecord itself however there are separate implementations on the the java classes PersistentVector, PersistentArrayMap, PersistentTreeMap, and PersistenHashMap in pure java. I am not sure if you would want to do something similar for Clojurescript.

I can also spend some time trying to make a more performant version.

Comment by António Nuno Monteiro [ 27/Jul/16 7:38 AM ]

Confirmed that Clojure uses the slow path via the IPersistentMap implementation in defrecord
https://github.com/clojure/clojure/blob/d920ad/src/clj/clojure/core.clj#L6712

Patch still applies and can also confirm it works for me.

Comment by Mike Fikes [ 19/Nov/17 8:03 PM ]

CLJS-1297-19-July-2015.patch no longer applies.

Comment by Tommi Reiman [ 24/Sep/18 12:12 AM ]

Hi. What's the current status of this? Spec-tools fails to generate the JSON Schemas out of clojure.spec + records on cljs and there is a local fix proposed on that side: https://github.com/metosin/spec-tools/pull/130/files

Comment by Mike Fikes [ 24/Sep/18 9:09 AM ]

Hey Samuel, can you re-baseline your attached patch? (It no longer applies on master.)





[CLJS-2914] Tests fail if hypen in directory name Created: 16/Sep/18  Updated: 23/Sep/18  Resolved: 23/Sep/18

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Mike Fikes Assignee: Ray McDermott
Resolution: Duplicate Votes: 0
Labels: None


 Description   

Check out compiler into a foo-bar directory and run lein test :only cljs.module-processing-tests.

Seems very similar to CLJS-2703.

$ lein test :only cljs.module-processing-tests

lein test cljs.module-processing-tests

lein test :only cljs.module-processing-tests/commonjs-module-processing-preprocess-symbol

FAIL in (commonjs-module-processing-preprocess-symbol) (module_processing_tests.clj:191)
Processed modules are added to :js-module-index
expected: (= {"React" {:name (absolute-module-path "src/test/cljs/reactJS.js"), :module-type :commonjs}, "Circle" {:name (absolute-module-path "src/test/cljs/Circle.js"), :module-type :commonjs}} (:js-module-index (clojure.core/deref cenv)))
  actual: (not (= {"React" {:name "module$Users$mfikes$foo_bar$clojurescript$src$test$cljs$reactJS", :module-type :commonjs}, "Circle" {:name "module$Users$mfikes$foo_bar$clojurescript$src$test$cljs$Circle", :module-type :commonjs}} {"React" {:name "module$Users$mfikes$foo-bar$clojurescript$src$test$cljs$reactJS", :module-type :commonjs}, "Circle" {:name "module$Users$mfikes$foo-bar$clojurescript$src$test$cljs$Circle", :module-type :commonjs}}))

lein test :only cljs.module-processing-tests/es6-module-processing

FAIL in (es6-module-processing) (module_processing_tests.clj:97)
Processed modules are added to :js-module-index
expected: (= {"es6-hello" {:name (absolute-module-path "src/test/cljs/es6_hello.js"), :module-type :es6}} (:js-module-index (clojure.core/deref cenv)))
  actual: (not (= {"es6-hello" {:name "module$Users$mfikes$foo_bar$clojurescript$src$test$cljs$es6-hello", :module-type :es6}} {"es6-hello" {:name "module$Users$mfikes$foo-bar$clojurescript$src$test$cljs$es6-hello", :module-type :es6}}))

lein test :only cljs.module-processing-tests/test-cljs-1822

FAIL in (test-cljs-1822) (module_processing_tests.clj:162)
Processed modules are added to :js-module-index
expected: (= {"React" {:name (absolute-module-path "src/test/cljs/react-min.js"), :module-type :commonjs}, "Circle" {:name (absolute-module-path "src/test/cljs/Circle-min.js"), :module-type :commonjs}} (:js-module-index (clojure.core/deref cenv)))
  actual: (not (= {"React" {:name "module$Users$mfikes$foo_bar$clojurescript$src$test$cljs$react-min", :module-type :commonjs}, "Circle" {:name "module$Users$mfikes$foo_bar$clojurescript$src$test$cljs$Circle-min", :module-type :commonjs}} {"React" {:name "module$Users$mfikes$foo-bar$clojurescript$src$test$cljs$react-min", :module-type :commonjs}, "Circle" {:name "module$Users$mfikes$foo-bar$clojurescript$src$test$cljs$Circle-min", :module-type :commonjs}}))

lein test :only cljs.module-processing-tests/commonjs-module-processing

FAIL in (commonjs-module-processing) (module_processing_tests.clj:71)
Processed modules are added to :js-module-index
expected: (= {"React" {:name (absolute-module-path "src/test/cljs/reactJS.js"), :module-type :commonjs}, "Circle" {:name (absolute-module-path "src/test/cljs/Circle.js"), :module-type :commonjs}} (:js-module-index (clojure.core/deref cenv)))
  actual: (not (= {"React" {:name "module$Users$mfikes$foo_bar$clojurescript$src$test$cljs$reactJS", :module-type :commonjs}, "Circle" {:name "module$Users$mfikes$foo_bar$clojurescript$src$test$cljs$Circle", :module-type :commonjs}} {"React" {:name "module$Users$mfikes$foo-bar$clojurescript$src$test$cljs$reactJS", :module-type :commonjs}, "Circle" {:name "module$Users$mfikes$foo-bar$clojurescript$src$test$cljs$Circle", :module-type :commonjs}}))

Ran 5 tests containing 14 assertions.
4 failures, 0 errors.
Tests failed.


 Comments   
Comment by Mike Fikes [ 16/Sep/18 7:10 AM ]

As far as I can tell, this is not a new regression and existed at the time CLJS-2703 was fixed.

Comment by Mike Fikes [ 23/Sep/18 11:53 AM ]

I wonder if this is the same as CLJS-2782





[CLJS-2919] Git ignore package.json and package-lock.json Created: 21/Sep/18  Updated: 23/Sep/18

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: newbie

Attachments: Text File CLJS-2919.patch    
Patch: Code

 Description   

If you run the unit tests via script/test, then you end up with a package.json and package-lock.json file which are eligible to be staged by git (or whatever the correct term is), which makes it more difficult to then switch branches.

If you do a script/clean these files are cleaned up. But perhaps we can also add them to the .gitignore file.



 Comments   
Comment by Mike Fikes [ 23/Sep/18 2:58 PM ]

Jacek Schae shared via a private Slack DM that the CA has been signed.

Comment by Jacek Schae [ 23/Sep/18 2:59 PM ]

As per Mike (Fikes) guidance.





[CLJS-2920] Optimize qualified- / simple- ident? / keyword? / symbol? Created: 22/Sep/18  Updated: 23/Sep/18

Status: In Progress
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: newbie, performance

Attachments: Text File CLJS-2920-2.patch     Text File CLJS-2920.patch    
Patch: Code

 Description   

There are some optimizations that we can make for the simple-ident?, qualified-ident?, simple-symbol?, qualified-symbol?, simple-keyword?, and qualified-keyword? functions.

Two separate optimization can be done.

In places where boolean is used (the qualified- fns), we can instead use some? for a faster implementation:

(boolean (and (symbol? x) (namespace x) true))

can be converted to simply

(and (symbol? x) (some? (namespace x)))

Focusing on just V8 for now, for

(simple-benchmark [x 'a/b f #(qualified-symbol? x)] (f) 100000000)

and subtracting out the baseline cost for

(simple-benchmark [x 'a/b f (fn [] x)] (f) 100000000)

this yields a speedup of 1.74.

Since we know that we are dealing with keywords or symbols when applying namespace, we can furthermore use (.-ns x).

Alone this gives a speedup of 2.21 and when combined with the previous optimization as

(and (symbol? x) (some? (.-ns x)))

we see a combined speedup approaching infinity (the operation effectively becomes free, taking the same time as the baseline).

Note that we could perhaps use predicate-induced type inference along with a new macro for namespace that emits .-ns instead, but CLJS-2866 currently hinges on looking at the test in an if, while expressions like the above cause optimized js* output with short-circuiting &&.

These optimizations would be good because these functions are often used in spec-based generative code testing where performance can be an issue.

We have test coverage for these functions in cljs.predicates-test.

For reference here is the JavaScript currently emitted for qualified-symbol?

function cljs$core$qualified_symbol_QMARK_(x) {
  return cljs.core.boolean$(
    (function() {
      var and__8540__auto__ = x instanceof cljs.core.Symbol;
      if (and__8540__auto__) {
        var and__8540__auto____$1 = cljs.core.namespace(x);
        if (cljs.core.truth_(and__8540__auto____$1)) {
          return true;
        } else {
          return and__8540__auto____$1;
        }
      } else {
        return and__8540__auto__;
      }
    })()
  );
}

and here is what we get with the revisions

function cljs$user$qualified_symbol_QMARK_(x) {
  return x instanceof cljs.core.Symbol && !(x.ns == null);
}

(The compactness of this revised implementation means that it might be further inlinable via Closure for more speedups for whole-program optimization.)



 Comments   
Comment by Mike Fikes [ 22/Sep/18 9:49 PM ]

Note, Chris has submitted the CA (Same as @hlprmnky https://twitter.com/hlprmnky/status/1043661374830911489)

Comment by Chris Johnson Bidler [ 22/Sep/18 10:56 PM ]

Patch with changes as outlined in ticket. All tests pass.

Comment by Mike Fikes [ 23/Sep/18 6:59 AM ]

Hey Chris, Please attach a revised patch which doesn't update the def for *clojurescript-version*. (You can name the patch CLJS-2920-2.patch and it should be a standalone patch, not dependent on the previous patch.)

Comment by Chris Johnson Bidler [ 23/Sep/18 9:05 AM ]

As directed; thanks for catching that. For future reference, is resetting that `def` a manual cleanup step after running `lein uberjar`? If there's an automated thing I can do to protect myself from this mistake in future patch submissions, I would love to know about it.

Comment by Mike Fikes [ 23/Sep/18 10:54 AM ]

I'm not aware of an automated mechanism to clean up the *clojurescript-version* revision. I usually just try to avoid staging that particular change in the commit.

Comment by Mike Fikes [ 23/Sep/18 12:16 PM ]

CLJS-2920-2.patch LGTM and passes all Canary tests.





[CLJS-1562] WARN on hinted fn call type mismatch Created: 06/Feb/16  Updated: 23/Sep/18

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 1.7.228
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Mike Fikes Assignee: Mike Fikes
Resolution: Unresolved Votes: 0
Labels: None


 Description   

If a call is made to a function that has hinted arguments (especially {^boolean} and {^number}), with an expression that is known to not be of that type, emit a diagnostic type mismatch warning.

An example that should emit a warning is:

(defn f [^boolean b])
(f 0)


 Comments   
Comment by Mike Fikes [ 23/Sep/18 12:08 PM ]

If we infer parameter types then we can additionally do this even when unhinted. (Example https://gist.github.com/mfikes/1e2341b48b882587500547f6ba19279d)





[CLJS-1958] Macro :arglists include &form and &env Created: 27/Feb/17  Updated: 23/Sep/18

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Trivial
Reporter: Andrea Richiardi Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Minor issue, but nice to fix especially because of tooling. In lumo:

cljs.user=> (:arglists (get-in @lumo.repl/st [:cljs.analyzer/namespaces 'cljs.core$macros :defs 'when]))
([&form &env test & body])

I would like to filter out the first two arguments that are of course not needed.
A simple patch is here.

I can provide a patch once I figure out where is the right place to apply it. Of course if I am not doing it wrong.

Thanks!



 Comments   
Comment by Andrea Richiardi [ 27/Feb/17 12:26 PM ]

On second thought, if we put this filter in `load-analysis-cache!`, probably we will pay to much of a price for just a display/ide problem. The best thing I can think of is to filter the transit/json file before loading. So probably this is better to be inside `lumo`.

Comment by Andrea Richiardi [ 27/Feb/17 12:44 PM ]

After conversation with António Nuno Monteiro: this inconsistency might be solved with CLJS-1461 so this one here can be closed I guess.

Comment by Mike Fikes [ 23/Sep/18 12:06 PM ]

Perhaps related to CLJS-2852





[CLJS-2907] Hello world! not printed for QuickStart Created: 14/Sep/18  Updated: 23/Sep/18

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Mike Fikes Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

1.10.339
Mac OS
Safari



 Description   

When following Quick Start, just seeing a blank line at times:

Bad:

$ clj --main cljs.main --compile hello-world.core --repl

ClojureScript 1.10.339
cljs.user=>

Good:

$ clj --main cljs.main --compile hello-world.core --repl
Hello world!
ClojureScript 1.10.339
cljs.user=>


 Comments   
Comment by Mike Fikes [ 23/Sep/18 12:00 PM ]

Perhaps this is a duplicate of CLJS-2625





[CLJS-2728] Ability to disable macro spec checks Created: 08/Apr/18  Updated: 23/Sep/18

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Mike Fikes Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: newbie


 Description   

Akin to Clojure's new ability https://github.com/clojure/clojure/commit/3d9b356306db77946bdf4809baeb660f94cec846

Perhaps ClojureScript would employ a new compiler option to control this?



 Comments   
Comment by David Nolen [ 18/May/18 1:03 PM ]

Agreed.





[CLJS-2918] cljs.main: Extra out directory in path for user.cljs Created: 20/Sep/18  Updated: 23/Sep/18

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Mike Fikes Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

{:deps {org.clojure/clojurescript {:mvn/version "1.10.339"}}}



 Description   
src/user.cljs
(def a 3)

Things work

$ clj -Srepro -m cljs.main -d out -r
ClojureScript 1.10.339
cljs.user=> a
3
cljs.user=>

But if you look, you will see an extra out directory in the path for the user file:

out/out/cljs/user/userAE725FA.js


 Comments   
Comment by Mike Fikes [ 23/Sep/18 11:55 AM ]

Probably the same as CLJS-2753. (IIRC load-file is used for the user.cljs feature.)





[CLJS-2761] Warn if fn has more than 20 fixed arguments Created: 25/May/18  Updated: 23/Sep/18

Status: In Progress
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: David Nolen Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: newbie

Approval: Accepted

 Description   

Currently we do not warn if a function declares more than 20 fixed arguments.






[CLJS-2729] drop-last docstring refers to 'coll' but args refer to 's' Created: 08/Apr/18  Updated: 23/Sep/18

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: docstring, newbie

Attachments: Text File CLJS-2729.patch    
Patch: Code

 Description   

Same as CLJ-1826, but for ClojureScript.



 Comments   
Comment by Mike Fikes [ 21/Sep/18 1:47 PM ]

Hey Eugene, have you signed the CA? (I don't see your name listed here https://clojure.org/community/contributors).

Comment by Eugene Kostenko [ 23/Sep/18 8:28 AM ]

Hi Mike, yes, I have. I got my confirmation email on September 11, right before I submitted the patch.

Comment by Mike Fikes [ 23/Sep/18 8:29 AM ]

Thanks! I'll review.

Comment by Mike Fikes [ 23/Sep/18 11:37 AM ]

CLJS-2729.patch LGTM and all tests pass in CI.





[CLJS-2849] Make utility fn vars in cljs.spec.test.alpha private Created: 22/Jul/18  Updated: 22/Sep/18

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: newbie

Attachments: Text File CLJS-2849.patch    

 Description   

These utility fns in cljs.spec.test.alpha weren't intended for public use: distinct-by, get-host-port, get-ua-product, get-env



 Comments   
Comment by Julio Berina [ 07/Sep/18 12:12 PM ]

Here's the patch for this issue

Comment by Mike Fikes [ 22/Sep/18 5:56 PM ]

Hey Julio, have you signed the CA? (I don't see your name listed here https://clojure.org/community/contributors)

If not, see https://clojurescript.org/community/contributing

Comment by Julio Berina [ 22/Sep/18 6:06 PM ]

Yes. I have signed the CA since September 5. From what I saw online, it takes like a couple of months to update the contributor list

Comment by Mike Fikes [ 22/Sep/18 6:17 PM ]

Thanks, Julio. Your patch is probably fine. One aspect to consider is that defn- is typically used instead of private meta.

Comment by Julio Berina [ 22/Sep/18 6:21 PM ]

I initially thought of doing that because I thought it was the other way around (private meta over defn-). Thank you for letting me know. I'll remember that for next time.

Comment by Mike Fikes [ 22/Sep/18 7:17 PM ]

CLJS-2849.patch LGTM and passes tests.





[CLJS-2910] flatten docstring does not describe lazy result Created: 14/Sep/18  Updated: 22/Sep/18

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Trivial
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: newbie

Attachments: Text File 0001-CLJS-2910-Docstring-describe-lazy-result-for-flatten.patch    
Patch: Code

 Description   

Port CLJ-2122 to ClojureScript.



 Comments   
Comment by Mike Fikes [ 16/Sep/18 3:07 PM ]

Patch LGTM.





[CLJS-2735] Refine doc string for defmulti Created: 11/Apr/18  Updated: 22/Sep/18

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 1.10.238
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: docstring, newbie

Attachments: Text File CLJS-2735-2.patch     Text File CLJS-2735-3.patch     Text File CLJS-2735.patch    
Patch: Code

 Description   

CLJ-835 but for ClojureScript.

Perhaps we can copy Clojure's current docstring over en masse, replacing "Clojure" references to "ClojureScript", and ensuring that it is accurate for ClojureScript's defmulti semantics / behavior.



 Comments   
Comment by Pankaj Doharey [ 21/Sep/18 8:12 AM ]

Created a pull request on Github here https://github.com/clojure/clojurescript/pull/79

Comment by Pankaj Doharey [ 21/Sep/18 8:16 AM ]

Attached a CLJS-2735 patch in any case.

Comment by Mike Fikes [ 21/Sep/18 8:27 AM ]

Hey Pankaj, thanks for the contribution. Can you make a patch using the instructions here? https://clojurescript.org/community/patches

Comment by Pankaj Doharey [ 21/Sep/18 8:54 AM ]

Hi Mike, just uploaded a new patch based on the instructions on the given page.

Thanks.

Comment by Pankaj Doharey [ 22/Sep/18 12:37 PM ]

Update on the patch.

Comment by Mike Fikes [ 22/Sep/18 3:12 PM ]

Hey Pankaj, if you look at CLJ-835, you'll see the text to the right of :hierarchy that is in your patch should be removed.

Comment by Pankaj Doharey [ 22/Sep/18 3:22 PM ]

Yes will remove it.

Comment by Pankaj Doharey [ 22/Sep/18 3:29 PM ]

Updates docs string and removes incorrect :hierarchy text.

Comment by Mike Fikes [ 22/Sep/18 4:06 PM ]

CLJS-2735-3.patch LGTM





[CLJS-981] Better benchmarking infrastructure Created: 17/Jan/15  Updated: 22/Sep/18

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Task Priority: Minor
Reporter: David Nolen Assignee: Pankaj Doharey
Resolution: Unresolved Votes: 0
Labels: newbie


 Description   

We should use ProcessBuilder to run the various benchmark scripts and control which benchmarks we test and which engines we run. Benchmarks should produce EDN data that can be written to a file, loaded into Incanter, etc.



 Comments   
Comment by Pankaj Doharey [ 21/Sep/18 11:37 AM ]

Would it be alright to add a task :benchmark in lein in order to run these benchmarks using ProcessBuilder ?

Comment by Mike Fikes [ 22/Sep/18 6:54 AM ]

If we do anything, I'd suggest that we employ simple Clojure scripts, with no dependence on lein.

Comment by Pankaj Doharey [ 22/Sep/18 7:06 AM ]

That makes sense.





[CLJS-712] resolve-var for symbol with dot still wrong Created: 03/Dec/13  Updated: 22/Sep/18

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 1.7.145
Fix Version/s: 1.9.671

Type: Defect Priority: Major
Reporter: David Nolen Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None


 Description   

We need to recur on the first segment passing an new additional argument to resolve-var indicating that we should not try to resolve in the current namespace and instead warn.



 Comments   
Comment by Thomas Heller [ 22/Sep/18 4:42 AM ]

I just stumbled over this when trying to figure out why process.env.FOO (without js/) produces no warnings and noticed that pretty much all symbols with dots will resolve and sort of work accidentally. They also never produce any warnings.

When trying to fix this I stumbled over a lot of code that sort of relies on this behavior. Most prominently defrecord and exists?.

defrecord can easily be fixed since it uses cljs.core.MapEntry [1] instead of cljs.core/MapEntry. cljs.core.MapEntry resolves to cljs/core.MapEntry and pretty much all dotted symbols will resolve to the first segment turning into the namespace. This works since munge turns / into . later on.

exists? will split some.nested.Thing into checking some, some.nested, some.nested.Thing which again is some, some/nested, some/nested.Thing. I tried fixing this directly in cljs.analyzer/analyze-symbol by properly desugaring some.nested.Thing into (. (. some -nested) -Thing) but this is insufficient since macros can directly call cljs.analyzer/resolve-var with dotted symbols (eg. exists?).

At this point I'm unsure how to fix this. I do think this is worth fixing but it may produce lots of warnings in user code. Is that acceptable?

[1] https://github.com/clojure/clojurescript/blob/6062744a1600479d5b9c641db9fb15cbb





[CLJS-2886] count macro with specializations for string and array Created: 04/Sep/18  Updated: 21/Sep/18

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File CLJS-2886-2.patch     Text File CLJS-2886.patch    
Patch: Code and Test

 Description   

The count function, when applied to a value that can be statically inferred to be of string or array type, does some runtime checks that can be elided.

This enhancement proposes the introduction of a count macro which conditionally acts as an intrinsic driven by type inference, employing specializations for string and array types.



 Comments   
Comment by Mike Fikes [ 04/Sep/18 1:46 PM ]

With CLJS-2886.patch if you do

(simple-benchmark [x "abc"] (count x) 10000000)

in a Node REPL, it exhibits a speedup of 13.2.

Similarly, for

(simple-benchmark [x #js [1 2 3]] (count x) 10000000)

there is a speedup of 22.1.

Under :advanced the speedups are lower. For the string case:

V8: 1.6
SpiderMonkey: 1.9
JavaScriptCore: 1.9
Nashorn: 1.12
ChakraCore: 139.1 (not a typo)
GraalVM: 3.51

For the array case:

V8: 2.0
SpiderMonkey: 2.3
JavaScriptCore: 2.1
Nashorn: 0.72
ChakraCore: 54.5 (not a typo)
GraalVM: 5.6

Comment by Mike Fikes [ 04/Sep/18 3:48 PM ]

The 2nd patch fixes a few minor issues and also adds a new concept: The ability to suppress macroexpansion so that we can have a core macro call its runtime variant. Without this, the use of js/cljs.core.count trips up the externs inference logic. We really want to call cljs.core/count, but without it being treated as a macro recursively, so this is done in the 2nd patch with an extra bit of analyzer meta.

Comment by Mike Fikes [ 06/Sep/18 6:27 AM ]

In addition to general usage speedup illustrated, the code produced is much more amenable to Closure inlining. As a silly example (inc (count (subs "abcdefghijk" 1))) compiles to 11 (if subs is hinted with ^string in CLJS-2868).

Comment by Mike Fikes [ 21/Sep/18 2:22 PM ]

Compiler performance test: Letting the script below run 8 cycles and averaging the result shows a speedup of 0.997 relative to master. (In other words no significant compilation perf difference.)

$ unset NASHORN_HOME GRAALVM_HOME V8_HOME SPIDERMONKEY_HOME
$ while true; do script/clean; script/bootstrap > /dev/null; script/test 2>&1 | grep "Compile sources"; done

Also, when compiling the Coal Mine corpus with this patch, things don't slow down relative to master.





[CLJS-2911] partition-by runs infinite loop when one element of infinite partition is accessed Created: 14/Sep/18  Updated: 21/Sep/18  Resolved: 21/Sep/18

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: newbie

Attachments: Text File 0001-CLJS-2911-Avoid-infinite-loop-on-infinite-partitions.patch    
Patch: Code and Test

 Description   

Port CLJ-1764 to ClojureScript.



 Comments   
Comment by Mike Fikes [ 16/Sep/18 5:00 PM ]

Speedups for (simple-benchmark [r (range 100000)] (count (partition-by odd? r)) 10)

V8: 0.91
SpiderMonkey: 0.87
JavaScriptCore: 0.81
Nashorn: 0.96
ChakraCore: 0.87
GraalVM: 0.74

Comment by Mike Fikes [ 16/Sep/18 7:12 PM ]

Patch LGTM. It is a little surprising that changing a seq to lazy-seq slows things down so much for the benchmark above. Otherwise the change passes on all of the Canary tests.

Comment by David Nolen [ 21/Sep/18 2:37 PM ]

fixed https://github.com/clojure/clojurescript/commit/6062744a1600479d5b9c641db9fb15cbb1df023c





[CLJS-2869] Improve type inference of and / or Created: 21/Aug/18  Updated: 21/Sep/18

Status: In Progress
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Mike Fikes Assignee: Mike Fikes
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File CLJS-2869-0.patch     Text File CLJS-2869-1.patch     Text File CLJS-2869-2.patch     Text File CLJS-2869-3.patch     Text File CLJS-2869-4.patch     Text File CLJS-2869-5.patch    
Patch: Code and Test

 Description   

When non-simple-boolean arguments are passed to the and and or macros, the type inferred is simply due to the inferred type of the nested if expansion. Instead we know that we could infer the last type in an and and the first type in an or (while properly handling clj-nil and perhaps also boolean).

Concretely:

(or "a" :k 1)

can be inferred as type string and likewise

(and "a" :k 1)

can be inferred as type number.



 Comments   
Comment by Mike Fikes [ 21/Aug/18 10:42 PM ]

CLJS-2869-0.patch is an experimental WIP change that produces improvements to and / or inference as illustrated here:

form old inference new inference
(and nil 10) #{number clj-nil} clj-nil
(and "a" nil 10) #{number string clj-nil} clj-nil
(and 10 "a") #{number string} string
(and (even? (int (system-time))) "a" 10) #{boolean number string} #{boolean number}
(and "a" (even? (int (system-time))) 10) #{boolean number string} #{boolean number}
(and (when (even? (int (system-time))) "a") 10) #{number string clj-nil} #{number clj-nil}
(let [x ^any []] (and x 10)) #{any number} #{boolean number clj-nil}
(let [x ^any []] (and :kw (if x "3" 10))) #{cljs.core/Keyword number string} #{number string}
(or nil 10) #{number clj-nil} number
(or "a" nil 10) #{number string clj-nil} string
(or 10 "a") #{number string} number
(or (even? (int (system-time))) "a" 10) #{boolean number string} #{boolean string}
(or "a" (even? (int (system-time))) 10) #{boolean number string} string
(or (when (even? (int (system-time))) "a") 10) #{number string clj-nil} #{number string}
(or (when (even? (int (system-time))) (even? (int (* 10 (system-time))))) 10)) #{boolean number clj-nil} #{boolean number}
(let [x ^any []] (or x 10)) #{any number} any
(let [x ^any []] (or (if x "3" 10) :kw)) #{cljs.core/Keyword number string} #{number string}
Comment by Mike Fikes [ 24/Aug/18 12:05 PM ]

The core functionality is the two functions cljs.analyzer/infer-and and cljs.analyzer/infer-or. Even though they look a bit complex, they are exhaustively tested out to arity 4, comparing to a reference impl that produces type tags based on the actual behavior of the and and or macros.

Comment by Mike Fikes [ 24/Aug/18 6:00 PM ]

I haven't done any specific perf tests to ensure that the extra work done during macroexpansion of and and or doesn't slow down compilation, but one metric is that a coal-mine run (more than a quarter million lines of ClojureScript being compiled) didn't show any slowdown with this patch.

Comment by Mike Fikes [ 26/Aug/18 6:54 AM ]

I failed to realize that the semantics of the seq tag is "a sequence or nil" and the patch needs to accommodate this.

Comment by Mike Fikes [ 26/Aug/18 11:03 AM ]

CLJS-2869-2.patch accommodates semantics of the seq tag.

Comment by Mike Fikes [ 27/Aug/18 6:37 AM ]

I'm curious if the same results can be achieved by putting the inference logic directly into if, adding handling for the special cases of (if x x foo) and (if x foo x). Will take a look as this would lead to a broader result.

Comment by Mike Fikes [ 29/Aug/18 7:19 PM ]

CLJS-2869-3.patch pushes the inference down into if. It turns out that the non-if inference developed in the previous patches is still used, but in the special case where the and / or macros don't expand to if forms.

Comment by Mike Fikes [ 30/Aug/18 6:19 AM ]

Need to deal with a regression: Previously boolean was being "merged" into seq tags in a way that this would not result in a checked if in bar:

(defn foo [x] (if x (seq [1 2 3]) true))

(defn bar [x] (if (foo x) 1 2))
Comment by Mike Fikes [ 30/Aug/18 11:14 AM ]

CLJS-2869-4.patch exhibits correct behavior, but is not ready yet as it is showing a speedup of 0.69 relative to master (when compiling Coal Mine code).

Comment by Mike Fikes [ 30/Aug/18 9:06 PM ]

Actually, perf on this patch is good. I was inadvertently comparing against master which itself has a speedup of around 1.44 as a result of some recently landed optimizations, which explains the mistaken apparent slowdown of the patch when compared to master.

I re-tested the perf of the patch relative to its previous commit and the perf was about the same (I tested twice and the first test actually showed a speedup of 1.03 and the second a speedup of 1.02).

Comment by Mike Fikes [ 31/Aug/18 8:41 PM ]

CLJS-2869-5.patch rebaselines

Comment by Mike Fikes [ 21/Sep/18 7:38 AM ]

When running script/test, CLJS-2869-5.patch is showing a speedup of 0.91. Assigning to me to revisit.





[CLJS-2916] Optimize code gen for empty queue Created: 18/Sep/18  Updated: 21/Sep/18

Status: In Progress
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: newbie, performance

Attachments: Text File CLJS-2916.patch    
Patch: Code and Test

 Description   

If you have #queue [] in your code, the code generated is

cljs.core.into.call(null,cljs.core.PersistentQueue.EMPTY,cljs.core.PersistentVector.EMPTY)

when it could, for the empty vector case generate

cljs.core.PersistentQueue.EMPTY

See https://github.com/clojure/clojurescript/blob/f289ffee2270567f7976d45012a0a52c38eb6488/src/main/clojure/cljs/tagged_literals.cljc#L21



 Comments   
Comment by Colin Kahn [ 19/Sep/18 1:01 PM ]

Here's a go at doing this one. Not sure if the tests are how they should be.

Comment by Mike Fikes [ 19/Sep/18 1:11 PM ]

Hey Colin. Thanks for the contribution! Have you signed the CA? I don't see your name listed here https://clojure.org/community/contributors

If not, please see https://clojurescript.org/community/contributing

Comment by Colin Kahn [ 19/Sep/18 1:12 PM ]

Hi Mike,

Just did this morning, got the confirmation in my email.

Comment by Mike Fikes [ 19/Sep/18 1:13 PM ]

Great, I'll review the patch.

Comment by Mike Fikes [ 19/Sep/18 1:27 PM ]

Hey Colin,

For your 2nd test, did you perhaps mean to check this?

(instance? PersistentQueue #queue [1 2 3])

In terms of actually testing the patch, the only thing I can think of is to inspect the generated code. Looking at the string produced by

(binding [*print-fn-bodies* true] (pr-str (fn [] #queue [])))

seems like a bit of a hack, but you could check if the string produced contains PersistentVector and if so, fail the test.

Comment by Colin Kahn [ 19/Sep/18 2:01 PM ]

Yes, definitely meant to check the type. Oddly I didn't get a failure from that in the test report. I was curious if there's a way to run just a subset of the tests?

What do you think of using with-redefs?

(with-redefs [into (fn [& _] (throw (ex-info "Inefficiently created PersistentQueue" {})))] #queue [])
Comment by Mike Fikes [ 19/Sep/18 2:43 PM ]

Hi Colin,

I'm not aware of a way to just run a subset of the tests. FWIW, Travis CI fails the build https://travis-ci.org/mfikes/clojurescript/builds/430674720

I definitely like your with-redefs approach. Much more clever than my string hack.

Comment by Colin Kahn [ 19/Sep/18 5:12 PM ]

Hi Mike,

I updated the attached patch.

Those tests don't actually get run when you do 'lein test', looks like Travis does a build and runs them using jsc. I replicated locally, not sure if there are commands to make that easier.

Comment by Mike Fikes [ 19/Sep/18 5:21 PM ]

Thanks. I'll take a look at the revised patch. These tests are run via script/test. More info at https://clojurescript.org/community/running-tests

Comment by Mike Fikes [ 19/Sep/18 5:45 PM ]

With the patch #queue [] is free.

Benchmarking: (simple-benchmark [f (fn [] #queue [])] (f) 1e8)

Before:

Benchmarking with V8
[f (fn [] (cljs.core/into cljs.core.PersistentQueue.EMPTY []))], (f), 100000000 runs, 442 msecs
Benchmarking with SpiderMonkey
[f (fn [] (cljs.core/into cljs.core.PersistentQueue.EMPTY []))], (f), 100000000 runs, 988 msecs
Benchmarking with JavaScriptCore
[f (fn [] (cljs.core/into cljs.core.PersistentQueue.EMPTY []))], (f), 100000000 runs, 2041 msecs
Benchmarking with Nashorn
[f (fn [] (cljs.core/into cljs.core.PersistentQueue.EMPTY []))], (f), 100000000 runs, 47257 msecs
Benchmarking with ChakraCore
[f (fn [] (cljs.core/into cljs.core.PersistentQueue.EMPTY []))], (f), 100000000 runs, 3040 msecs
Benchmarking with GraalVM
[f (fn [] (cljs.core/into cljs.core.PersistentQueue.EMPTY []))], (f), 100000000 runs, 296 msecs

After:

Benchmarking with V8
[f (fn [] cljs.core.PersistentQueue.EMPTY)], (f), 100000000 runs, 0 msecs
Benchmarking with SpiderMonkey
[f (fn [] cljs.core.PersistentQueue.EMPTY)], (f), 100000000 runs, 0 msecs
Benchmarking with JavaScriptCore
[f (fn [] cljs.core.PersistentQueue.EMPTY)], (f), 100000000 runs, 0 msecs
Benchmarking with Nashorn
[f (fn [] cljs.core.PersistentQueue.EMPTY)], (f), 100000000 runs, -1 msecs
Benchmarking with ChakraCore
[f (fn [] cljs.core.PersistentQueue.EMPTY)], (f), 100000000 runs, -5 msecs
Benchmarking with GraalVM
[f (fn [] cljs.core.PersistentQueue.EMPTY)], (f), 100000000 runs, -1 msecs
Comment by Mike Fikes [ 19/Sep/18 6:33 PM ]

CLJS-2916.patch of 19/Sep/18 6:12 PM LGTM.

It passes all tests, including Canary tests. Perf looks good given the previous comment.





[CLJS-2917] cljs.main: Failure to load user.cljs if temp out dir Created: 20/Sep/18  Updated: 20/Sep/18

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Mike Fikes Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

{:deps {org.clojure/clojurescript {:mvn/version "1.10.339"}}}



 Description   
src/user.cljs
(def a 3)
$ clj -Srepro -m cljs.main
java.lang.IllegalArgumentException: /var/folders/gx/nymj3l7x4zq3gxb97v2zwzb40000gn/T/out6255758032123683761473031144485066/cljs/user/userAE725FA.js is not a relative path
	at clojure.java.io$as_relative_path.invokeStatic(io.clj:414)
	at clojure.java.io$file.invokeStatic(io.clj:426)
	at clojure.java.io$file.invoke(io.clj:418)
	at cljs.closure$compile_file.invokeStatic(closure.clj:633)
	at cljs.closure$compile_file.invoke(closure.clj:625)
	at cljs.closure$fn__5175.invokeStatic(closure.clj:721)
	at cljs.closure$fn__5175.invoke(closure.clj:715)
	at cljs.closure$fn__5088$G__5081__5095.invoke(closure.clj:543)
	at cljs.closure$compile.invokeStatic(closure.clj:595)
	at cljs.closure$compile.invoke(closure.clj:592)
	at cljs.repl$load_file$fn__6454.invoke(repl.cljc:601)
	at cljs.repl$load_file.invokeStatic(repl.cljc:600)
	at cljs.repl$load_file.invoke(repl.cljc:592)
	at cljs.repl$repl_STAR_$maybe_load_user_file__6612.invoke(repl.cljc:958)
	at cljs.repl$repl_STAR_$fn__6621$fn__6622.invoke(repl.cljc:989)
	at cljs.repl$repl_STAR_$fn__6621.invoke(repl.cljc:982)
	at cljs.compiler$with_core_cljs.invokeStatic(compiler.cljc:1289)
	at cljs.compiler$with_core_cljs.invoke(compiler.cljc:1278)
	at cljs.repl$repl_STAR_.invokeStatic(repl.cljc:979)
	at cljs.repl$repl_STAR_.invoke(repl.cljc:855)
	at cljs.cli$repl_opt.invokeStatic(cli.clj:305)
	at cljs.cli$repl_opt.invoke(cli.clj:292)
	at cljs.cli$main.invokeStatic(cli.clj:638)
	at cljs.cli$main.doInvoke(cli.clj:625)
	at clojure.lang.RestFn.invoke(RestFn.java:410)
	at clojure.lang.AFn.applyToHelper(AFn.java:154)
	at clojure.lang.RestFn.applyTo(RestFn.java:132)
	at clojure.core$apply.invokeStatic(core.clj:659)
	at clojure.core$apply.invoke(core.clj:652)
	at cljs.main$_main.invokeStatic(main.clj:61)
	at cljs.main$_main.doInvoke(main.clj:52)
	at clojure.lang.RestFn.invoke(RestFn.java:397)
	at clojure.lang.AFn.applyToHelper(AFn.java:152)
	at clojure.lang.RestFn.applyTo(RestFn.java:132)
	at clojure.lang.Var.applyTo(Var.java:702)
	at clojure.core$apply.invokeStatic(core.clj:657)
	at clojure.main$main_opt.invokeStatic(main.clj:317)
	at clojure.main$main_opt.invoke(main.clj:313)
	at clojure.main$main.invokeStatic(main.clj:424)
	at clojure.main$main.doInvoke(main.clj:387)
	at clojure.lang.RestFn.applyTo(RestFn.java:137)
	at clojure.lang.Var.applyTo(Var.java:702)
	at clojure.main.main(main.java:37)
ClojureScript 1.10.339
cljs.user=>

If, on the other hand you specify an output directory (via -d) things work.






[CLJS-2904] Default :npm-deps to false Created: 13/Sep/18  Updated: 19/Sep/18  Resolved: 14/Sep/18

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Critical
Reporter: David Nolen Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None


 Comments   
Comment by Mike Fikes [ 14/Sep/18 7:59 AM ]

Documentation PR: https://github.com/clojure/clojurescript-site/pull/264

Comment by David Nolen [ 14/Sep/18 8:09 AM ]

fixed https://github.com/clojure/clojurescript/commit/c2f5aba2f59cf30dbc13c968d8ee3f0e4359d38c

Comment by Mike Fikes [ 16/Sep/18 5:30 PM ]

The Reagent unit tests in Canary started failing with this change with the root cause being reliance on the implicit configuration of :npm-deps. The fix in this case was to configure an explicit :npm-deps {}.

Comment by Mike Fikes [ 19/Sep/18 6:54 AM ]

I've updated the documentation PR https://github.com/clojure/clojurescript-site/pull/264 to indicate the previous default value.





[CLJS-2633] Regression on scoped npm-modules Created: 08/Mar/18  Updated: 17/Sep/18

Status: In Progress
Project: ClojureScript
Component/s: None
Affects Version/s: 1.10.238
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Aleš Roubíček Assignee: Aleš Roubíček
Resolution: Unresolved Votes: 2
Labels: js-modules

Attachments: Zip Archive string-requires-npm-deps.zip    
Approval: Vetted

 Description   

I have regression in npm-deps resolution in 1.10.x for material-components-web. For sub-dependencies it doesn't resolve the module path correctly. When I require eg. @material/snackbar it fails with Uncaught Error: Undefined nameToPath for module$$material$base$component.

Steps to reproduce:

yarn add "@cljs-oss/module-deps" "@material/snackbar"

cat <<EOF > deps.edn
{:deps {org.clojure/clojurescript {:mvn/version "1.10.145"}}}
EOF

clj -m cljs.main -d out -e "(require '[\"@material/snackbar\"])"

Resolution worked in 1.9.946



 Comments   
Comment by David Nolen [ 08/Mar/18 12:31 PM ]

This ticket just needs more information. A good first step in this report would be a git bisect. Then someone needs to determine wether this is because of ClojureScript or Google Closure. If the later there's not much we can do.

Comment by Aleš Roubíček [ 09/Mar/18 12:08 AM ]

I can reproduce the regression even on 1.10.63, to narrow the window. I'll prepare backwards compatible repro and bisect after that.

Comment by Aleš Roubíček [ 09/Mar/18 1:23 AM ]

Bisected it and it was introduced in #CLJS-2389 (GCC update)

Comment by Aleš Roubíček [ 09/Mar/18 6:51 AM ]

Code for reproduction

Comment by Aleš Roubíček [ 09/Mar/18 6:55 AM ]

When head -2 out/node_modules/@material/snackbar/index.js ends with goog.require("module$$material$base$index"); it is the broken case.

Comment by David Nolen [ 11/Mar/18 2:31 PM ]

I can reproduce and see that the dependency index file `cljs_deps.js` does not look right, so not surprised it's not working.

Comment by David Nolen [ 11/Mar/18 3:13 PM ]

Digging in further this may be a Closure issue and we'll have to wait for an upstream fix.

Comment by David Nolen [ 11/Mar/18 5:58 PM ]

This is a Closure Compiler issue. We need to submit a patch and then make the necessary changes for the next Closure release, this change https://github.com/swannodette/closure-compiler/commit/58012d3f1068aa588a47dc34ec6f39413aa59e62 fixes the module name issue for me.

Comment by Aleš Roubíček [ 12/Mar/18 2:09 AM ]

Excellent, thank you David.

Comment by David Nolen [ 12/Mar/18 9:31 AM ]

PR Closure Compiler https://github.com/google/closure-compiler/pull/2847

Comment by David Nolen [ 24/Mar/18 1:12 PM ]

Can we get a non-trivial expression added to this ticket that should work after the require?

Comment by David Nolen [ 25/Mar/18 3:56 AM ]

I figured out how to test this - it does in fact seemed resolved with master + my Closure Compiler PR.

Comment by Aleš Roubíček [ 25/Mar/18 6:32 AM ]

Sorry, I didn't responded earlier, I had Code Retreat yesterday. It is DOM component, so the non-trivial example needs to run in browser REPL with modified index.html. I think, if emitted goog.require is correct, everything else will work fine. Thank you very much for the fix. I hope your GCC PR will be accepted soon. 1.10 looks very solid and fast.

Comment by David Nolen [ 25/Mar/18 6:46 AM ]

The Closure Compiler PR now has a test case. It might need some small tweaks after it gets reviewed but hopefully this fix can make it into the May release at the latest.

Comment by Thomas Mattacchione [ 21/Jul/18 10:47 AM ]

It seems that the PR to google closure has been reviewed https://github.com/google/closure-compiler/pull/2847

Comment by Aleš Roubíček [ 17/Sep/18 10:24 AM ]

It seems that it works in latest cljs (1.10.339).





[CLJS-2376] Add support for ES6 default imports/exports Created: 02/Oct/17  Updated: 17/Sep/18

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Thomas Heller Assignee: Juho Teperi
Resolution: Unresolved Votes: 14
Labels: npm-deps

Attachments: Text File CLJS-2376-1.patch    

 Description   

ES6 has special syntax for using "default" imports and there is currently no equivalent for CLJS when using imported ES6 code.

import * as name from "module-name";
(:require ["module-name" :as name])

import { export } from "module-name";
(:require ["module-name" :refer (export)])

import { export as alias } from "module-name";
(:require ["module-name" :refer (export) :rename {export alias}])

import defaultExport from "module-name";
import defaultExport, { export [ , [...] ] } from "module-name";
import defaultExport, * as name from "module-name";
;; no easy access to defaultExport

I'm proposing to add a :default to the ns :require.

(:require ["module-name" :as mod :default foo])

This makes it much more convenient to use rewritten ES6 code from CLJS. If "module-name" has a default export you currently have to write mod/default everywhere since they is no easy way to alias the default.

(:require ["material-ui/RaisedButton" :as RaisedButton])
;; :as is incorrect and the user must now use
RaisedButton/default

(:require ["material-ui/RaisedButton" :default RaisedButton])
;; would allow direct use of
RaisedButton

Internally the Closure compiler (and ES6 code rewritten by babel) will rewrite default exports to a .default property, so :default really is just a convenient way to access it.

The long version that already works is

(:require ["material-ui/RaisedButton" :refer (default) :rename {default RaisedButton}])

When ES6 becomes more widespread we should have convenient way to correctly refer to "default" exports.

[1] https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/import



 Comments   
Comment by Juho Teperi [ 10/Jan/18 2:25 PM ]

First take on the implementation. This implementation rewrites :default name to {{:refer [default] :rename {default name}}} at the lowest level (parse-require-spec), so I think other functions don't need to be changed.

I didn't update the require spec validation errors yet.

Comment by David Nolen [ 12/Feb/18 6:25 AM ]

It seems to me the most desirable syntax for ES6 default imports is the following:

(:require ["material-ui/RaisedButton" :refer [RaisedButton]])

I don't see why we couldn't just make this work?

Comment by Thomas Heller [ 12/Feb/18 6:55 AM ]

I don't understand that suggestion. The default export has no name, the .default property is only used in interop by transpilers. In actual JS a live reference is used. How would you map RaisedButton back to default?

["material-ui/RaisedButton" :default foo] would be valid. The JS module may also actually have an `export { RaisedButton }` in addition to `export default ...`, which would lead to a conflict if you "guess" the wrong name. Default exports are not aliases for the module itself, that is separate.

I made a reference translation table [1] and adding the :default alias is the only way to reliably map all ES6 import/export features. I don't see a way to make :refer work that would not be totally non-intuitive and unreliable.

import defaultExport, * as name from "module-name";

[1] https://shadow-cljs.github.io/docs/UsersGuide.html#_using_npm_packages

Comment by David Nolen [ 12/Feb/18 8:55 AM ]

Let's leave rhetoric out of this discussion please and focus on the technical bits. I just don't want to include another directive in the ns form and I would prefer an approach that avoids that. From my point of view there has not been enough exploration of that alternative.

This ticket is mostly about convenience, and I think we need to stew on this one for a bit longer before deciding on anything.

Comment by jcr [ 24/Mar/18 6:45 AM ]

Is there a reason we can't use metadata to deal with it? I.e.

(:require ["material-ui/RaisedButton" :refer [^:default RaisedButton]])
(:require ["module-name" :as mod :refer [^:default foo])
(:require ["module-name" :refer [^:default foo, bar, baz])

This doesn't require additional ns directives and doesn't cause any ambiguities either.

Since this wasn't used for macros (i.e. we have :require-macros instead of plain :require + ^:macros or :refer [^:macro foo]), I assume there may be some nuances I am not aware of that prevent implementing it. Are there any?

Comment by Thomas Heller [ 26/Apr/18 5:49 PM ]

The ^:default hint seems far more complicated to me from an implementation standpoint.

There is no equivalent for default import/export in Java/JVM and Clojure. There is also no equivalent in CommonJS even. It is strictly about ES6. Therefore bolting it onto "other" methods seems incorrect to me.

The reason it should be separate IMHO is that using the .default property is not strictly correct and the fact that it exists at all is purely for compatibility reasons with CommonJS. In strict ES6 they don't exist as part of the "Object" created by :as.

;; a.js
export let a = 1;
export let b = 2;
export default 3;

;; b.js
import x, * as y from "./a.js"

// these exist
y.a
y.b
// this is NOT valid in ES6 and does not exist
y.default
// instead x must be used
x

We certainly rely on the .default property existing for the time being given that CLJS does not emit ES6. webpack already changed how the default exports are handled however and I do expect the Closure Compiler to do something similar in the future.

Comment by Thomas Heller [ 26/Apr/18 6:06 PM ]

Turns out I'm actually wrong. Re-read the spec and .default is actually defined as a getter, so it is accessible. Nevermind my previous post then.

Doesn't change my opinion about the usefulness of :default as a simpler aliasing method though.

Comment by Paulus Esterhazy [ 17/Sep/18 8:46 AM ]

I ran into ES6 default exports, which are common these days. Folks stumble over `default` in the code, e.g.: https://github.com/pesterhazy/cljs-spa-example/issues/13. I wanted to report my findings.

As explained in the description of this issue, you can use a combination of `:refer` and `:rename` to get rid of the references to `:default` in the code, moving it into the NS declaration. It's not the most elegant solution but gets the job done and works with global exports: https://github.com/pesterhazy/cljs-spa-example/pull/14/files

Note that AIUI because `default` is a strict keyword in ES3, you need to set the following compiler option for this to work:

```
:language-out :ecmascript5
```

Otherwise `default` gets munged to `default$`





[CLJS-2912] Reuse seq in some Created: 14/Sep/18  Updated: 16/Sep/18

Status: In Progress
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: newbie

Attachments: Text File 0001-CLJS-2912-Reuse-seq-in-some.patch    
Patch: Code

 Description   

Port CLJ-1654 to ClojureScript.



 Comments   
Comment by Mike Fikes [ 16/Sep/18 4:28 PM ]

Speedups for (simple-benchmark [coll [3 3 5 5 7 7 7 6 9]] (some even? coll) 10000000):

V8: 0.81
SpiderMonkey: 1.08
JavaScriptCore: 1.20
Nashorn: 1.41
ChakraCore: 1.04
GraalVM: 2.70

Comment by Mike Fikes [ 16/Sep/18 5:57 PM ]

Patch LGTM and passes all Canary tests.





[CLJS-2913] improvements to exception messages and printing Created: 15/Sep/18  Updated: 15/Sep/18

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Mike Fikes Assignee: Mike Fikes
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Port CLJ-2373 to ClojureScript.






[CLJS-2909] clojure.walk/postwalk does not preserve MapEntry type objects Created: 14/Sep/18  Updated: 15/Sep/18

Status: In Progress
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File CLJS-2909.patch    
Patch: Code and Test

 Description   

Port CLJ-2031 to ClojureScript.



 Comments   
Comment by Mike Fikes [ 15/Sep/18 1:42 PM ]

CLJS-2909.patch passes Canary tests.





[CLJS-2908] Don't show quick prompt if :verbose or :repl-verbose Created: 14/Sep/18  Updated: 14/Sep/18

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

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

Attachments: Text File CLJS-2908.patch    
Patch: Code

 Description   

With CLJS-2897, if you specify -co '{:verbose true}' or -co '{:repl-verbose true}' then the prompt will be printed prior to much of the verbose output (and lost in the noise).






[CLJS-2906] cljs.main: Crash when with default REPL Created: 14/Sep/18  Updated: 14/Sep/18  Resolved: 14/Sep/18

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Blocker
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-2906.patch    
Patch: Code

 Description   

Regression caused with CLJS-2897 :

$ clj -m cljs.main
Exception in thread "main" java.lang.StringIndexOutOfBoundsException: String index out of range: -10
	at java.lang.String.substring(String.java:1931)
	at clojure.core$subs.invokeStatic(core.clj:4926)
	at clojure.core$subs.invoke(core.clj:4921)
	at cljs.cli$repl_name.invokeStatic(cli.clj:293)
	at cljs.cli$repl_name.invoke(cli.clj:292)
	at cljs.cli$fast_initial_prompt_QMARK_.invokeStatic(cli.clj:297)
	at cljs.cli$fast_initial_prompt_QMARK_.invoke(cli.clj:295)
	at cljs.cli$repl_opt.invokeStatic(cli.clj:314)
	at cljs.cli$repl_opt.invoke(cli.clj:299)
	at cljs.cli$main.invokeStatic(cli.clj:649)
	at cljs.cli$main.doInvoke(cli.clj:636)
	at clojure.lang.RestFn.invoke(RestFn.java:410)
	at clojure.lang.AFn.applyToHelper(AFn.java:154)
	at clojure.lang.RestFn.applyTo(RestFn.java:132)
	at clojure.core$apply.invokeStatic(core.clj:659)
	at clojure.core$apply.invoke(core.clj:652)
	at cljs.main$_main.invokeStatic(main.clj:61)
	at cljs.main$_main.doInvoke(main.clj:52)
	at clojure.lang.RestFn.invoke(RestFn.java:397)
	at clojure.lang.AFn.applyToHelper(AFn.java:152)
	at clojure.lang.RestFn.applyTo(RestFn.java:132)
	at clojure.lang.Var.applyTo(Var.java:702)
	at clojure.core$apply.invokeStatic(core.clj:657)
	at clojure.main$main_opt.invokeStatic(main.clj:317)
	at clojure.main$main_opt.invoke(main.clj:313)
	at clojure.main$main.invokeStatic(main.clj:424)
	at clojure.main$main.doInvoke(main.clj:387)
	at clojure.lang.RestFn.applyTo(RestFn.java:137)
	at clojure.lang.Var.applyTo(Var.java:702)
	at clojure.main.main(main.java:37)


 Comments   
Comment by David Nolen [ 14/Sep/18 12:16 PM ]

fixed https://github.com/clojure/clojurescript/commit/71f57714e6fc2f591d9de22cbfcfa009500e6742





[CLJS-2903] Add flag to append short content SHA to output file name Created: 13/Sep/18  Updated: 14/Sep/18

Status: In Progress
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

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


 Description   

Currently users that want unique file names from code-splits to avoid HTTP caching need to jump through a lot of hoops. One simple fix is to just add a new flag `:fingerprint`.



 Comments   
Comment by Thomas Heller [ 14/Sep/18 3:22 AM ]

cljs.loader needs the hashed output names for its module uris config. The might not be possible with the way it is currently configured via the :const injection since hash is only known after :advanced.

FWIW shadow-cljs supported this feature [1] for a long while and the loader is configured by prepending a global var shadow$loader = <json>; variable AFTER optimizations and the loader impl then accessing that global to configure itself. [2]

It used to append to shadow.loader.setup(...); call instead but that was problematic whenever something tried to access the loader before the setup call happened.

[1] https://shadow-cljs.github.io/docs/UsersGuide.html#NameHashing
[2] https://github.com/thheller/shadow-cljs/blob/master/src/main/shadow/loader.js#L7-L15





[CLJS-2898] cljs.main: ClojureScript version should not be displayed if there are inits Created: 09/Sep/18  Updated: 14/Sep/18

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File CLJS-2898-2.patch     Text File CLJS-2898.patch    
Patch: Code

 Description   

ClojureScript:

$ clj -m cljs.main -e 3 -r
3
ClojureScript 1.10.339
cljs.user=>

Clojure:

$ clj -e 3 -r
3
user=>


 Comments   
Comment by Mike Fikes [ 14/Sep/18 9:18 AM ]

CLJS-2898-2.patch rebaselines





[CLJS-2883] Instrumentation fails compilation with a large number of spec'd functions Created: 02/Sep/18  Updated: 14/Sep/18  Resolved: 14/Sep/18

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Jeaye Wilkerson Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None
Environment:

{:deps {org.clojure/clojurescript {:mvn/version "1.10.312"}
org.clojure/test.check {:mvn/version "0.9.0"}}}


Attachments: Text File CLJS-2883.patch    
Patch: Code
Approval: Accepted

 Description   

My team's product can no longer compile with instrumentation, since we get a "Method too large" error. By disabling the call to `instrument`, this compilation error is obviated. In the actual product, we have around 300 spec'd functions.

I have reproduced this in 21 lines here: https://github.com/jeaye/instrument-too-many Rather than having a large project with lots of spec'd fns, I just generate some with a macro. In practice, this is actually happening to my normal fns.

Repro, without external repo or downstream tooling:

src/instrument_too_many/macro.cljc
(ns instrument-too-many.macro
  (:require [clojure.spec.alpha :as s]))

(defmacro make-fns [fn-count]
  `(do
     ~@(for [i (range fn-count)]
         (let [fn-name (symbol (str "fn-" i))]
           `(do
              (defn ~fn-name []
                (str ~fn-name))
              (s/fdef ~fn-name))))))
$ clj -m cljs.main
ClojureScript 1.10.312
cljs.user=> (require 'clojure.spec.alpha 'clojure.spec.test.alpha)

cljs.user=> (require-macros 'instrument-too-many.macro)

cljs.user=> (instrument-too-many.macro/make-fns 2000)
cljs.user/fn-1999
cljs.user=> (clojure.spec.test.alpha/instrument)
clojure.lang.ExceptionInfo: java.lang.RuntimeException: Method code too large!, compiling:(NO_SOURCE_PATH:0:0) at line 1 <cljs repl> {:file "<cljs repl>", :line 1, :column 1, :root-source-info {:source-type :fragment, :source-form (clojure.spec.test.alpha/instrument)}, :tag :cljs/analysis-error}
	at clojure.core$ex_info.invokeStatic(core.clj:4739)
	at clojure.core$ex_info.invoke(core.clj:4739)
	at cljs.analyzer$error.invokeStatic(analyzer.cljc:706)
	at cljs.analyzer$error.invoke(analyzer.cljc:702)
	at cljs.analyzer$macroexpand_1.invokeStatic(analyzer.cljc:3375)
	at cljs.analyzer$macroexpand_1.invoke(analyzer.cljc:3371)
	at cljs.analyzer$analyze_seq.invokeStatic(analyzer.cljc:3408)
	at cljs.analyzer$analyze_seq.invoke(analyzer.cljc:3388)
	at cljs.analyzer$analyze_form.invokeStatic(analyzer.cljc:3578)
	at cljs.analyzer$analyze_form.invoke(analyzer.cljc:3574)
	at cljs.analyzer$analyze_STAR_.invokeStatic(analyzer.cljc:3628)
	at cljs.analyzer$analyze_STAR_.invoke(analyzer.cljc:3619)
	at cljs.analyzer$analyze.invokeStatic(analyzer.cljc:3649)
	at cljs.analyzer$analyze.invoke(analyzer.cljc:3631)
	at cljs.analyzer$analyze_seq.invokeStatic(analyzer.cljc:3411)
	at cljs.analyzer$analyze_seq.invoke(analyzer.cljc:3388)
	at cljs.analyzer$analyze_form.invokeStatic(analyzer.cljc:3578)
	at cljs.analyzer$analyze_form.invoke(analyzer.cljc:3574)
	at cljs.analyzer$analyze_STAR_.invokeStatic(analyzer.cljc:3628)
	at cljs.analyzer$analyze_STAR_.invoke(analyzer.cljc:3619)
	at cljs.analyzer$analyze.invokeStatic(analyzer.cljc:3649)
	at cljs.analyzer$analyze.invoke(analyzer.cljc:3631)
	at cljs.analyzer$analyze_seq.invokeStatic(analyzer.cljc:3411)
	at cljs.analyzer$analyze_seq.invoke(analyzer.cljc:3388)
	at cljs.analyzer$analyze_form.invokeStatic(analyzer.cljc:3578)
	at cljs.analyzer$analyze_form.invoke(analyzer.cljc:3574)
	at cljs.analyzer$analyze_STAR_.invokeStatic(analyzer.cljc:3628)
	at cljs.analyzer$analyze_STAR_.invoke(analyzer.cljc:3619)
	at cljs.analyzer$analyze.invokeStatic(analyzer.cljc:3649)
	at cljs.analyzer$analyze.invoke(analyzer.cljc:3631)
	at cljs.analyzer$analyze.invokeStatic(analyzer.cljc:3640)
	at cljs.analyzer$analyze.invoke(analyzer.cljc:3631)
	at cljs.analyzer$analyze.invokeStatic(analyzer.cljc:3638)
	at cljs.analyzer$analyze.invoke(analyzer.cljc:3631)
	at cljs.analyzer$analyze_let_binding_init.invokeStatic(analyzer.cljc:1904)
	at cljs.analyzer$analyze_let_binding_init.invoke(analyzer.cljc:1902)
	at cljs.analyzer$analyze_let_bindings_STAR_.invokeStatic(analyzer.cljc:1924)
	at cljs.analyzer$analyze_let_bindings_STAR_.invoke(analyzer.cljc:1913)
	at cljs.analyzer$analyze_let_bindings.invokeStatic(analyzer.cljc:1955)
	at cljs.analyzer$analyze_let_bindings.invoke(analyzer.cljc:1954)
	at cljs.analyzer$analyze_let.invokeStatic(analyzer.cljc:1970)
	at cljs.analyzer$analyze_let.invoke(analyzer.cljc:1965)
	at cljs.analyzer$fn__1847.invokeStatic(analyzer.cljc:1991)
	at cljs.analyzer$fn__1847.invoke(analyzer.cljc:1989)
	at clojure.lang.MultiFn.invoke(MultiFn.java:251)
	at cljs.analyzer$analyze_seq_STAR_.invokeStatic(analyzer.cljc:3381)
	at cljs.analyzer$analyze_seq_STAR_.invoke(analyzer.cljc:3379)
	at cljs.analyzer$analyze_seq_STAR__wrap.invokeStatic(analyzer.cljc:3386)
	at cljs.analyzer$analyze_seq_STAR__wrap.invoke(analyzer.cljc:3384)
	at cljs.analyzer$analyze_seq.invokeStatic(analyzer.cljc:3410)
	at cljs.analyzer$analyze_seq.invoke(analyzer.cljc:3388)
	at cljs.analyzer$analyze_form.invokeStatic(analyzer.cljc:3578)
	at cljs.analyzer$analyze_form.invoke(analyzer.cljc:3574)
	at cljs.analyzer$analyze_STAR_.invokeStatic(analyzer.cljc:3628)
	at cljs.analyzer$analyze_STAR_.invoke(analyzer.cljc:3619)
	at cljs.analyzer$analyze.invokeStatic(analyzer.cljc:3649)
	at cljs.analyzer$analyze.invoke(analyzer.cljc:3631)
	at cljs.analyzer$analyze_seq.invokeStatic(analyzer.cljc:3411)
	at cljs.analyzer$analyze_seq.invoke(analyzer.cljc:3388)
	at cljs.analyzer$analyze_form.invokeStatic(analyzer.cljc:3578)
	at cljs.analyzer$analyze_form.invoke(analyzer.cljc:3574)
	at cljs.analyzer$analyze_STAR_.invokeStatic(analyzer.cljc:3628)
	at cljs.analyzer$analyze_STAR_.invoke(analyzer.cljc:3619)
	at cljs.analyzer$analyze.invokeStatic(analyzer.cljc:3649)
	at cljs.analyzer$analyze.invoke(analyzer.cljc:3631)
	at cljs.analyzer$analyze.invokeStatic(analyzer.cljc:3640)
	at cljs.analyzer$analyze.invoke(analyzer.cljc:3631)
	at cljs.analyzer$analyze.invokeStatic(analyzer.cljc:3638)
	at cljs.analyzer$analyze.invoke(analyzer.cljc:3631)
	at cljs.analyzer$parse_invoke_STAR_$ana_expr__2343.invoke(analyzer.cljc:3187)
	at clojure.core$map$fn__5587.invoke(core.clj:2747)
	at clojure.lang.LazySeq.sval(LazySeq.java:40)
	at clojure.lang.LazySeq.seq(LazySeq.java:49)
	at clojure.lang.RT.seq(RT.java:528)
	at clojure.lang.LazilyPersistentVector.create(LazilyPersistentVector.java:44)
	at clojure.core$vec.invokeStatic(core.clj:377)
	at clojure.core$vec.invoke(core.clj:367)
	at cljs.analyzer$parse_invoke_STAR_.invokeStatic(analyzer.cljc:3189)
	at cljs.analyzer$parse_invoke_STAR_.invoke(analyzer.cljc:3136)
	at cljs.analyzer$parse_invoke.invokeStatic(analyzer.cljc:3194)
	at cljs.analyzer$parse_invoke.invoke(analyzer.cljc:3192)
	at cljs.analyzer$analyze_seq_STAR_.invokeStatic(analyzer.cljc:3382)
	at cljs.analyzer$analyze_seq_STAR_.invoke(analyzer.cljc:3379)
	at cljs.analyzer$analyze_seq_STAR__wrap.invokeStatic(analyzer.cljc:3386)
	at cljs.analyzer$analyze_seq_STAR__wrap.invoke(analyzer.cljc:3384)
	at cljs.analyzer$analyze_seq.invokeStatic(analyzer.cljc:3410)
	at cljs.analyzer$analyze_seq.invoke(analyzer.cljc:3388)
	at cljs.analyzer$analyze_form.invokeStatic(analyzer.cljc:3578)
	at cljs.analyzer$analyze_form.invoke(analyzer.cljc:3574)
	at cljs.analyzer$analyze_STAR_.invokeStatic(analyzer.cljc:3628)
	at cljs.analyzer$analyze_STAR_.invoke(analyzer.cljc:3619)
	at cljs.analyzer$analyze.invokeStatic(analyzer.cljc:3649)
	at cljs.analyzer$analyze.invoke(analyzer.cljc:3631)
	at cljs.analyzer$analyze.invokeStatic(analyzer.cljc:3640)
	at cljs.analyzer$analyze.invoke(analyzer.cljc:3631)
	at cljs.analyzer$analyze.invokeStatic(analyzer.cljc:3638)
	at cljs.analyzer$analyze.invoke(analyzer.cljc:3631)
	at cljs.analyzer$fn__1828.invokeStatic(analyzer.cljc:1883)
	at cljs.analyzer$fn__1828.invoke(analyzer.cljc:1879)
	at clojure.lang.MultiFn.invoke(MultiFn.java:251)
	at cljs.analyzer$analyze_seq_STAR_.invokeStatic(analyzer.cljc:3381)
	at cljs.analyzer$analyze_seq_STAR_.invoke(analyzer.cljc:3379)
	at cljs.analyzer$analyze_seq_STAR__wrap.invokeStatic(analyzer.cljc:3386)
	at cljs.analyzer$analyze_seq_STAR__wrap.invoke(analyzer.cljc:3384)
	at cljs.analyzer$analyze_seq.invokeStatic(analyzer.cljc:3410)
	at cljs.analyzer$analyze_seq.invoke(analyzer.cljc:3388)
	at cljs.analyzer$analyze_form.invokeStatic(analyzer.cljc:3578)
	at cljs.analyzer$analyze_form.invoke(analyzer.cljc:3574)
	at cljs.analyzer$analyze_STAR_.invokeStatic(analyzer.cljc:3628)
	at cljs.analyzer$analyze_STAR_.invoke(analyzer.cljc:3619)
	at cljs.analyzer$analyze.invokeStatic(analyzer.cljc:3649)
	at cljs.analyzer$analyze.invoke(analyzer.cljc:3631)
	at cljs.analyzer$analyze.invokeStatic(analyzer.cljc:3640)
	at cljs.analyzer$analyze.invoke(analyzer.cljc:3631)
	at cljs.analyzer$analyze.invokeStatic(analyzer.cljc:3638)
	at cljs.analyzer$analyze.invoke(analyzer.cljc:3631)
	at cljs.analyzer$fn__1696$fn__1720.invoke(analyzer.cljc:1471)
	at cljs.analyzer$fn__1696.invokeStatic(analyzer.cljc:1471)
	at cljs.analyzer$fn__1696.invoke(analyzer.cljc:1419)
	at clojure.lang.MultiFn.invoke(MultiFn.java:251)
	at cljs.analyzer$analyze_seq_STAR_.invokeStatic(analyzer.cljc:3381)
	at cljs.analyzer$analyze_seq_STAR_.invoke(analyzer.cljc:3379)
	at cljs.analyzer$analyze_seq_STAR__wrap.invokeStatic(analyzer.cljc:3386)
	at cljs.analyzer$analyze_seq_STAR__wrap.invoke(analyzer.cljc:3384)
	at cljs.analyzer$analyze_seq.invokeStatic(analyzer.cljc:3410)
	at cljs.analyzer$analyze_seq.invoke(analyzer.cljc:3388)
	at cljs.analyzer$analyze_form.invokeStatic(analyzer.cljc:3578)
	at cljs.analyzer$analyze_form.invoke(analyzer.cljc:3574)
	at cljs.analyzer$analyze_STAR_.invokeStatic(analyzer.cljc:3628)
	at cljs.analyzer$analyze_STAR_.invoke(analyzer.cljc:3619)
	at cljs.analyzer$analyze.invokeStatic(analyzer.cljc:3649)
	at cljs.analyzer$analyze.invoke(analyzer.cljc:3631)
	at cljs.repl$evaluate_form$__GT_ast__6412.invoke(repl.cljc:516)
	at cljs.repl$evaluate_form.invokeStatic(repl.cljc:518)
	at cljs.repl$evaluate_form.invoke(repl.cljc:498)
	at cljs.repl$eval_cljs.invokeStatic(repl.cljc:688)
	at cljs.repl$eval_cljs.invoke(repl.cljc:681)
	at cljs.repl$repl_STAR_$read_eval_print__6593.invoke(repl.cljc:973)
	at cljs.repl$repl_STAR_$fn__6599$fn__6608.invoke(repl.cljc:1014)
	at cljs.repl$repl_STAR_$fn__6599.invoke(repl.cljc:1013)
	at cljs.compiler$with_core_cljs.invokeStatic(compiler.cljc:1289)
	at cljs.compiler$with_core_cljs.invoke(compiler.cljc:1278)
	at cljs.repl$repl_STAR_.invokeStatic(repl.cljc:976)
	at cljs.repl$repl_STAR_.invoke(repl.cljc:855)
	at cljs.cli$repl_opt.invokeStatic(cli.clj:305)
	at cljs.cli$repl_opt.invoke(cli.clj:292)
	at cljs.cli$main.invokeStatic(cli.clj:638)
	at cljs.cli$main.doInvoke(cli.clj:625)
	at clojure.lang.RestFn.invoke(RestFn.java:410)
	at clojure.lang.AFn.applyToHelper(AFn.java:154)
	at clojure.lang.RestFn.applyTo(RestFn.java:132)
	at clojure.core$apply.invokeStatic(core.clj:659)
	at clojure.core$apply.invoke(core.clj:652)
	at cljs.main$_main.invokeStatic(main.clj:61)
	at cljs.main$_main.doInvoke(main.clj:52)
	at clojure.lang.RestFn.invoke(RestFn.java:397)
	at clojure.lang.AFn.applyToHelper(AFn.java:152)
	at clojure.lang.RestFn.applyTo(RestFn.java:132)
	at clojure.lang.Var.applyTo(Var.java:702)
	at clojure.core$apply.invokeStatic(core.clj:657)
	at clojure.main$main_opt.invokeStatic(main.clj:317)
	at clojure.main$main_opt.invoke(main.clj:313)
	at clojure.main$main.invokeStatic(main.clj:424)
	at clojure.main$main.doInvoke(main.clj:387)
	at clojure.lang.RestFn.applyTo(RestFn.java:137)
	at clojure.lang.Var.applyTo(Var.java:702)
	at clojure.main.main(main.java:37)
Caused by: java.lang.RuntimeException: Method code too large!, compiling:(NO_SOURCE_PATH:0:0)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:7010)
	at clojure.lang.Compiler.analyze(Compiler.java:6773)
	at clojure.lang.Compiler.eval(Compiler.java:7059)
	at clojure.lang.Compiler.eval(Compiler.java:7025)
	at clojure.core$eval.invokeStatic(core.clj:3206)
	at clojure.core$eval.invoke(core.clj:3202)
	at cljs.spec.test.alpha$instrument.invokeStatic(alpha.cljc:113)
	at cljs.spec.test.alpha$instrument.invoke(alpha.cljc:69)
	at clojure.lang.AFn.applyToHelper(AFn.java:165)
	at clojure.lang.AFn.applyTo(AFn.java:144)
	at clojure.core$apply.invokeStatic(core.clj:661)
	at clojure.core$apply.invoke(core.clj:652)
	at cljs.analyzer$macroexpand_1_STAR_$fn__2380.invoke(analyzer.cljc:3328)
	at cljs.analyzer$macroexpand_1_STAR_.invokeStatic(analyzer.cljc:3327)
	at cljs.analyzer$macroexpand_1_STAR_.invoke(analyzer.cljc:3314)
	... 163 more
Caused by: java.lang.RuntimeException: Method code too large!
	at clojure.asm.MethodWriter.getSize(MethodWriter.java:1872)
	at clojure.asm.ClassWriter.toByteArray(ClassWriter.java:775)
	at clojure.lang.Compiler$ObjExpr.compile(Compiler.java:4661)
	at clojure.lang.Compiler$FnExpr.parse(Compiler.java:4099)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:7001)
	... 177 more
cljs.user=>


 Comments   
Comment by Mike Fikes [ 03/Sep/18 12:23 AM ]

Workaround: Instrument via separate calls, one per namespace. Presuming that no single namespace has a sufficiently large number of instrumentable functions that can provoke the issue, this would work around via a divide and conquer approach.

Example: Using the test code, simulate 4 namespaces, each which contains 500 instrumentable functions:

$ clj -m cljs.main
cljs.user=> (ns test1.core)

test1.core=> (require 'clojure.spec.alpha)
nil
test1.core=> (require-macros 'instrument-too-many.macro)
nil
test1.core=> (instrument-too-many.macro/make-fns 500)
test1.core/fn-499
test1.core=> (ns test2.core)

test2.core=> (require 'clojure.spec.alpha)
nil
test2.core=> (require-macros 'instrument-too-many.macro)
nil
test2.core=> (instrument-too-many.macro/make-fns 500)
test2.core/fn-499
test2.core=> (ns test3.core)

test3.core=> (require 'clojure.spec.alpha)
nil
test3.core=> (require-macros 'instrument-too-many.macro)
nil
test3.core=> (instrument-too-many.macro/make-fns 500)
test3.core/fn-499
test3.core=> (ns test4.core)

test4.core=> (require 'clojure.spec.alpha)
nil
test4.core=> (require-macros 'instrument-too-many.macro)
nil
test4.core=> (instrument-too-many.macro/make-fns 500)
test4.core/fn-499

Now, instead of calling arity-0 clojure.spec.test.alpha/instrument, call it 4 times, passing each namespace separately. In the example below, count is used simply to reduce the amount of output being displayed.

test4.core=> (in-ns 'cljs.user)
nil
cljs.user=> (require 'clojure.spec.test.alpha)
nil
cljs.user=> (count (clojure.spec.test.alpha/instrument 'test1.core))
500
cljs.user=> (count (clojure.spec.test.alpha/instrument 'test2.core))
500
cljs.user=> (count (clojure.spec.test.alpha/instrument 'test3.core))
500
cljs.user=> (count (clojure.spec.test.alpha/instrument 'test4.core))
500

Likewise, unstrument ing can be done on a per-namespace basis, but for this example, it appears to work, perhaps because the macroexpansion ends up being a bit smaller:

cljs.user=> (count (clojure.spec.test.alpha/unstrument))
2000
Comment by Mike Fikes [ 03/Sep/18 9:22 AM ]

With CLJS-2883.patch:

cljs.user=> (require 'clojure.spec.alpha 'clojure.spec.test.alpha)
nil
cljs.user=> (require-macros 'instrument-too-many.macro)
nil
cljs.user=> (instrument-too-many.macro/make-fns 2000)
cljs.user/fn-1999
cljs.user=> (count (clojure.spec.test.alpha/instrument))
2000
Comment by David Nolen [ 14/Sep/18 7:47 AM ]

fixed https://github.com/clojure/clojurescript/commit/1a155adb0984de26b9606d8aa7c0e1388567eb0b





[CLJS-2897] cljs.main: Display initial REPL prompt sooner Created: 09/Sep/18  Updated: 14/Sep/18  Resolved: 14/Sep/18

Status: Resolved
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-2897.patch    
Patch: Code

 Description   

In certain situations, it is possible to display the initial REPL prompt sooner while the JavaScript engine is initialized in the background. This altered timing gives a subtle impression of responsiveness without changing actual behavior.



 Comments   
Comment by Mike Fikes [ 09/Sep/18 11:04 AM ]

CLJS-2897.patch does this for all the shipping REPLs except the browser REPL. (For the browser REPL, if this logic is applied, then you see the prompt prior t the browser even launching.) It also skips displaying the initial prompt sooner if there are any inits. This ensures that any -e or -i output will continue to display prior to the initial prompt.

Comment by Mike Fikes [ 12/Sep/18 4:06 PM ]

Give this one a try. It seems that typical behavior is to get to the prompt is about 4 seconds while the engine continues to initialize for another 3 seconds.

Feel free of course to decline if this behavior is undesired or not worth the extra complexity.

Comment by David Nolen [ 14/Sep/18 7:46 AM ]

fixed https://github.com/clojure/clojurescript/commit/0a0ff32f9b479a002d7875b327d89e6a66d26f82





[CLJS-2905] NodeJS Foreign Libs Behave Differently With Optimizations Created: 13/Sep/18  Updated: 14/Sep/18

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 1.10.238
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Garrett Hopper Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

When using `:foreign-libs` in a `:target :nodejs` build, the foreign lib bundle must define variables as `this.EXAMPLE = "value"` instead of just `EXAMPLE = "value"` for `:global-exports` to work with `:optimizations :simple`. However, when `:optimizations :none` is set, the `EXAMPLE = "value"` `:global-exports` works just fine.






[CLJS-2902] Warn on type hint mismatch with inferred Created: 13/Sep/18  Updated: 13/Sep/18

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Mike Fikes Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

If you have code like

(let [^string x 3] )

we would generally prefer the type hint over the inferred type. But in cases where we have inferred a type and the type is inconsistent with the type hint, it may be useful to emit a diagnostic in that case.

If the inferred type is a superset of the hint, then we shouldn't warn. For example, if we infer any then no warning should be emitted. Also, if, for example we infer #{clj-nil string} and we have a hint of string we can take the stance that the developer is essentially saying that clj-nil can't happen.

But, on the other hand, if we infer number and there is a hint of string then clearly something is amiss.






[CLJS-2868] Add ^string hints Created: 19/Aug/18  Updated: 12/Sep/18

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Mike Fikes Assignee: Martin Kučera
Resolution: Unresolved Votes: 1
Labels: newbie

Attachments: Text File CLJS-2868.patch    
Patch: Code

 Description   

There exist functions that are not automatically inferred as returning strings. Hinting these would help in the event that their return values are passed to numeric functions, or for code gen with CLJS-2865.

The set I believe needs explicit hints comprises:

subs
pr-str
prn-str
print-str
munge-str
clojure.string/reverse
clojure.string/replace
clojure.string/replace-first
clojure.string/join
clojure.string/upper-case
clojure.string/lower-case
clojure.string/capitalize
clojure.string/trim
clojure.string/triml
clojure.string/trimr
clojure.string/trim-newline
clojure.string/escape


 Comments   
Comment by Martin Kučera [ 06/Sep/18 2:36 AM ]

It's my first contribution, tell me if anything wrong!

Comment by Mike Fikes [ 07/Sep/18 1:55 PM ]

Confirmed with Martin Kučera via Slack that CA has been signed.

Comment by Mike Fikes [ 07/Sep/18 2:28 PM ]

Hey Martin, the way that subs is hinted in the patch leaves it as being inferred as any.

You can add a test in cljs.analyzer-tests like the following to see this.

(deftest test-cljs-2688
  (is (= (e/with-compiler-env test-cenv
           (:tag (analyze test-env '(subs "a" 0))))
        'string)))

With this in place you can check that it fails via

lein test :only cljs.analyzer-tests/test-cljs-2688

You can fix the issue by simply adding a single hint as in

(defn ^string subs
  "Returns the substring of s beginning at start inclusive, and ending
  at end (defaults to length of string), exclusive."
  ([s start] (.substring s start))
  ([s start end] (.substring s start end)))

I'd suggest adding is checks for each function of interest, and because of outward type hint propagation, you may find that some hints are unnecessary if the function's implementation involves something that can be inferred as being of type string. For example, a single hint on prn-str-with-opts might take care of pr-str, prn-str, or any other uses of prn-str-with-opts in the codebase.

Comment by Martin Kučera [ 08/Sep/18 12:10 AM ]

Ok, I’ll create test for each function and then add type hints when necessary.

Comment by Mike Fikes [ 12/Sep/18 3:17 PM ]

Martin discovered that some of these cases involve variadic functions, and in those cases this ticket would ideally depend upon work in CLJS-2901 so that we can get away with fewer hints.





[CLJS-2900] Tag hash-map in PersistentHashSet definition as not-native Created: 11/Sep/18  Updated: 11/Sep/18

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Thomas Mulvaney Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

PersistentHashSet wraps a PAM/PHM named `hash-map` for its functionality. Tagging the field as `not-native` allows the protocols methods it implements to be called directly.






[CLJS-2899] The :testing-contexts value does not survive async macro Created: 09/Sep/18  Updated: 09/Sep/18

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Andrea Richiardi Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

It seems like the :testing-contexts is not preserved in the test environment after (or within better) a `cljs.test/async` test.

Repro:

cljs.user=> (require '[cljs.test :as test :refer-macros [deftest is testing async]])
cljs.user=> (deftest test-testing-contexts
       #_=>   (testing "not there"
       #_=>     (async done
       #_=>            (js/setTimeout #(do (is (= 0 1))
       #_=>                                (done))))))

cljs.user=> (test/test-var #'cljs.user/test-testing-contexts)
#object[Object]
cljs.user=> 
FAIL in (test-testing-contexts) (at applyHandler (<eval>:NaN:28)
expected: (= 0 1)
  actual: (not (= 0 1))

I have also inspected, using test/testing-contexts-str, the env before and after async and:

cljs.user=> (deftest test-testing-contexts
       #_=>   (testing "not there"
       #_=>     (println "outside ->" (test/testing-contexts-str))
       #_=>     (async done
       #_=>            (println "inside -> " (test/testing-contexts-str))
       #_=>            (js/setTimeout #(do (is (= 0 1))
       #_=>                                (done))))))

cljs.user=> (test/test-var #'cljs.user/test-testing-contexts)
outside -> not there
inside ->  
#object[Object]
cljs.user=> 
FAIL in (test-testing-contexts) (at applyHandler (<eval>:NaN:28)
expected: (= 0 1)
  actual: (not (= 0 1))





[CLJS-2878] Update Closure Compiler to v20180805 Created: 01/Sep/18  Updated: 08/Sep/18  Resolved: 08/Sep/18

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Mike Fikes Assignee: Mike Fikes
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-2878.patch    
Approval: Accepted

 Comments   
Comment by Mike Fikes [ 01/Sep/18 8:25 AM ]

If you apply CLJS-2878.patch, tests pass on macOS and Windows but not on Linux (CI) with the following.

Note that Canary's Linux-based test with the latest Closure Compiler snapshot is passing.

Options passed to ClojureScript compiler: {:output-dir "builds/out-adv", :closure-warnings {:non-standard-jsdoc :off, :global-this :off, :check-types :off, :check-variables :off}, :closure-defines {"process.env.NODE_ENV" "production"}, :static-fns true, :ups-libs nil, :optimize-constants true, :closure-module-roots [], :install-deps true, :language-out :es5, :optimizations :advanced, :ups-foreign-libs [{:file "calculator_global.js", :provides ["thirdparty.calculator"]}], :parallel-build true, :verbose true, :aot-cache false, :preloads [process.env], :ignore-js-module-exts [".css"], :output-to :print, :preamble ["cljs/imul.js"], :output-wrapper true, :ups-externs ["externs.js"], :opts-cache "cljsc_opts.edn", :foreign-libs [{:file "src/test/cljs/calculator_global.js", :provides ["calculator"], :global-exports {calculator Calculator}} {:file "src/test/cljs/es6_dep.js", :module-type :es6, :provides ["es6_calc"]} {:file "src/test/cljs/calculator.js", :module-type :commonjs, :provides ["calculator"]} {:file "src/test/cljs/es6_default_hello.js", :provides ["es6_default_hello"], :module-type :es6}], :cache-analysis-format :transit, :compiler-stats true, :language-in :es6, :emit-constants true, :npm-deps {:lodash "4.17.4"}}
module.js:478
    throw err;
    ^

Error: Cannot find module '@cljs-oss/module-deps'
    at Function.Module._resolveFilename (module.js:476:15)
    at Function.Module._load (module.js:424:25)
    at Module.require (module.js:504:17)
    at require (internal/module.js:20:19)
    at [eval]:8:13
    at ContextifyScript.Script.runInThisContext (vm.js:25:33)
    at Object.runInThisContext (vm.js:97:38)
    at Object.<anonymous> ([eval]-wrapper:6:22)
    at Module._compile (module.js:577:32)
    at evalScript (bootstrap_node.js:358:27)

module.js:478
    throw err;
    ^

Error: Cannot find module '@cljs-oss/module-deps'
    at Function.Module._resolveFilename (module.js:476:15)
    at Function.Module._load (module.js:424:25)
    at Module.require (module.js:504:17)
    at require (internal/module.js:20:19)
    at [eval]:8:13
    at ContextifyScript.Script.runInThisContext (vm.js:25:33)
    at Object.runInThisContext (vm.js:97:38)
    at Object.<anonymous> ([eval]-wrapper:6:22)
    at Module._compile (module.js:577:32)
    at evalScript (bootstrap_node.js:358:27)

module.js:478
    throw err;
    ^

Error: Cannot find module '@cljs-oss/module-deps'
    at Function.Module._resolveFilename (module.js:476:15)
    at Function.Module._load (module.js:424:25)
    at Module.require (module.js:504:17)
    at require (internal/module.js:20:19)
    at [eval]:8:13
    at ContextifyScript.Script.runInThisContext (vm.js:25:33)
    at Object.runInThisContext (vm.js:97:38)
    at Object.<anonymous> ([eval]-wrapper:6:22)
    at Module._compile (module.js:577:32)
    at evalScript (bootstrap_node.js:358:27)

Copying file:/home/travis/build/mfikes/clojurescript/src/test/cljs/es6_dep.js to builds/out-adv/src/test/cljs/es6_dep.js
Copying file:/home/travis/build/mfikes/clojurescript/src/test/cljs/calculator.js to builds/out-adv/src/test/cljs/calculator.js
Copying file:/home/travis/build/mfikes/clojurescript/src/test/cljs/es6_default_hello.js to builds/out-adv/src/test/cljs/es6_default_hello.js
Process JS modules, elapsed time: 342.036667 msecs
Analyzing file:/home/travis/build/mfikes/clojurescript/src/main/cljs/cljs/core.cljs
Compiling /home/travis/build/mfikes/clojurescript/src/main/cljs/cljs/core.cljs to builds/out-adv/cljs/core.js, elapsed time: 4510.117779 msecs
Compiling src/test/cljs/cljs/spec/test/test_ns1.cljs to builds/out-adv/cljs/spec/test/test_ns1.js, elapsed time: 2.013643 msecs
Compiling /home/travis/build/mfikes/clojurescript/src/main/cljs/clojure/string.cljs to builds/out-adv/clojure/string.js, elapsed time: 90.262169 msecs
Compiling /home/travis/build/mfikes/clojurescript/src/main/cljs/clojure/walk.cljs to builds/out-adv/clojure/walk.js, elapsed time: 18.90655 msecs
Compiling /home/travis/build/mfikes/clojurescript/src/main/cljs/cljs/pprint.cljs to builds/out-adv/cljs/pprint.js, elapsed time: 1253.513518 msecs
Copying jar:file:/home/travis/build/mfikes/clojurescript/lib/test.check.jar!/clojure/test/check/results.cljc to builds/out-adv/clojure/test/check/results.cljc
Compiling builds/out-adv/clojure/test/check/results.cljc to builds/out-adv/clojure/test/check/results.js, elapsed time: 29.831141 msecs
Copying jar:file:/home/travis/build/mfikes/clojurescript/lib/tools.reader-1.3.0.jar!/cljs/tools/reader/impl/utils.cljs to builds/out-adv/cljs/tools/reader/impl/utils.cljs
Compiling builds/out-adv/cljs/tools/reader/impl/utils.cljs to builds/out-adv/cljs/tools/reader/impl/utils.js, elapsed time: 78.122021 msecs
Copying jar:file:/home/travis/build/mfikes/clojurescript/lib/tools.reader-1.3.0.jar!/cljs/tools/reader/reader_types.cljs to builds/out-adv/cljs/tools/reader/reader_types.cljs
Compiling builds/out-adv/cljs/tools/reader/reader_types.cljs to builds/out-adv/cljs/tools/reader/reader_types.js, elapsed time: 127.64787 msecs
Copying jar:file:/home/travis/build/mfikes/clojurescript/lib/tools.reader-1.3.0.jar!/cljs/tools/reader/impl/inspect.cljs to builds/out-adv/cljs/tools/reader/impl/inspect.cljs
Compiling builds/out-adv/cljs/tools/reader/impl/inspect.cljs to builds/out-adv/cljs/tools/reader/impl/inspect.js, elapsed time: 21.999863 msecs
Copying jar:file:/home/travis/build/mfikes/clojurescript/lib/tools.reader-1.3.0.jar!/cljs/tools/reader/impl/errors.cljs to builds/out-adv/cljs/tools/reader/impl/errors.cljs
Compiling builds/out-adv/cljs/tools/reader/impl/errors.cljs to builds/out-adv/cljs/tools/reader/impl/errors.js, elapsed time: 137.602692 msecs
Copying jar:file:/home/travis/build/mfikes/clojurescript/lib/tools.reader-1.3.0.jar!/cljs/tools/reader/impl/commons.cljs to builds/out-adv/cljs/tools/reader/impl/commons.cljs
Compiling builds/out-adv/cljs/tools/reader/impl/commons.cljs to builds/out-adv/cljs/tools/reader/impl/commons.js, elapsed time: 47.857357 msecs
Compiling /home/travis/build/mfikes/clojurescript/src/main/cljs/cljs/test.cljs to builds/out-adv/cljs/test.js, elapsed time: 581.324756 msecs
Copying jar:file:/home/travis/build/mfikes/clojurescript/lib/tools.reader-1.3.0.jar!/cljs/tools/reader.cljs to builds/out-adv/cljs/tools/reader.cljs
Copying jar:file:/home/travis/build/mfikes/clojurescript/lib/test.check.jar!/clojure/test/check/clojure_test/assertions.cljc to builds/out-adv/clojure/test/check/clojure_test/assertions.cljc
Compiling src/test/cljs/clojure/walk_test.cljs to builds/out-adv/clojure/walk_test.js, elapsed time: 122.27173 msecs
Compiling builds/out-adv/clojure/test/check/clojure_test/assertions.cljc to builds/out-adv/clojure/test/check/clojure_test/assertions.js, elapsed time: 160.32143 msecs
Compiling builds/out-adv/cljs/tools/reader.cljs to builds/out-adv/cljs/tools/reader.js, elapsed time: 366.146303 msecs
Compiling src/test/cljs/data_readers_test/core.cljc to builds/out-adv/data_readers_test/core.js, elapsed time: 1.896901 msecs
Copying jar:file:/home/travis/build/mfikes/clojurescript/lib/tools.reader-1.3.0.jar!/cljs/tools/reader/edn.cljs to builds/out-adv/cljs/tools/reader/edn.cljs
Compiling /home/travis/build/mfikes/clojurescript/src/test/cljs/cljs/array_access/beta.cljs to builds/out-adv/cljs/array_access/beta.js, elapsed time: 21.851356 msecs
Compiling /home/travis/build/mfikes/clojurescript/src/test/cljs/cljs/array_access/alpha.cljs to builds/out-adv/cljs/array_access/alpha.js, elapsed time: 1.701452 msecs
Compiling src/test/cljs/cljs/spec/test/test_ns2.cljs to builds/out-adv/cljs/spec/test/test_ns2.js, elapsed time: 3.214807 msecs
Compiling builds/out-adv/cljs/tools/reader/edn.cljs to builds/out-adv/cljs/tools/reader/edn.js, elapsed time: 213.68345 msecs
Copying jar:file:/home/travis/build/mfikes/clojurescript/lib/test.check.jar!/clojure/test/check/impl.cljc to builds/out-adv/clojure/test/check/impl.cljc
Compiling builds/out-adv/clojure/test/check/impl.cljc to builds/out-adv/clojure/test/check/impl.js, elapsed time: 1.719057 msecs
Copying jar:file:/home/travis/build/mfikes/clojurescript/lib/test.check.jar!/clojure/test/check/rose_tree.cljc to builds/out-adv/clojure/test/check/rose_tree.cljc
Compiling /home/travis/build/mfikes/clojurescript/src/main/cljs/cljs/stacktrace.cljc to builds/out-adv/cljs/stacktrace.js, elapsed time: 261.569784 msecs
Copying jar:file:/home/travis/build/mfikes/clojurescript/lib/test.check.jar!/clojure/test/check/random/longs/bit_count_impl.cljs to builds/out-adv/clojure/test/check/random/longs/bit_count_impl.cljs
Compiling builds/out-adv/clojure/test/check/random/longs/bit_count_impl.cljs to builds/out-adv/clojure/test/check/random/longs/bit_count_impl.js, elapsed time: 7.951325 msecs
Copying jar:file:/home/travis/build/mfikes/clojurescript/lib/test.check.jar!/clojure/test/check/random/longs.cljs to builds/out-adv/clojure/test/check/random/longs.cljs
Compiling builds/out-adv/clojure/test/check/random/longs.cljs to builds/out-adv/clojure/test/check/random/longs.js, elapsed time: 36.891666 msecs
Compiling builds/out-adv/clojure/test/check/rose_tree.cljc to builds/out-adv/clojure/test/check/rose_tree.js, elapsed time: 138.145448 msecs
Copying jar:file:/home/travis/build/mfikes/clojurescript/lib/test.check.jar!/clojure/test/check/random/doubles.cljs to builds/out-adv/clojure/test/check/random/doubles.cljs
Compiling builds/out-adv/clojure/test/check/random/doubles.cljs to builds/out-adv/clojure/test/check/random/doubles.js, elapsed time: 73.923033 msecs
Compiling /home/travis/build/mfikes/clojurescript/src/main/cljs/cljs/reader.cljs to builds/out-adv/cljs/reader.js, elapsed time: 252.358915 msecs
Copying jar:file:/home/travis/build/mfikes/clojurescript/lib/test.check.jar!/clojure/test/check/random.cljs to builds/out-adv/clojure/test/check/random.cljs
Compiling /home/travis/build/mfikes/clojurescript/src/test/cljs/cljs/tagged_literals_test.cljs to builds/out-adv/cljs/tagged_literals_test.js, elapsed time: 44.25205 msecs
Compiling builds/out-adv/clojure/test/check/random.cljs to builds/out-adv/clojure/test/check/random.js, elapsed time: 55.651549 msecs
Copying jar:file:/home/travis/build/mfikes/clojurescript/lib/test.check.jar!/clojure/test/check/generators.cljc to builds/out-adv/clojure/test/check/generators.cljc
Compiling builds/out-adv/clojure/test/check/generators.cljc to builds/out-adv/clojure/test/check/generators.js, elapsed time: 437.105159 msecs
Copying jar:file:/home/travis/build/mfikes/clojurescript/lib/test.check.jar!/clojure/test/check/properties.cljc to builds/out-adv/clojure/test/check/properties.cljc
Copying jar:file:/home/travis/build/mfikes/clojurescript/lib/test.check.jar!/clojure/test/check.cljc to builds/out-adv/clojure/test/check.cljc
Compiling builds/out-adv/clojure/test/check/properties.cljc to builds/out-adv/clojure/test/check/properties.js, elapsed time: 68.246025 msecs
Compiling builds/out-adv/clojure/test/check.cljc to builds/out-adv/clojure/test/check.js, elapsed time: 69.870508 msecs
Compiling /home/travis/build/mfikes/clojurescript/src/main/cljs/cljs/spec/gen/alpha.cljs to builds/out-adv/cljs/spec/gen/alpha.js, elapsed time: 554.761197 msecs
Compiling /home/travis/build/mfikes/clojurescript/src/test/cljs/cljs/hash_map_test.cljs to builds/out-adv/cljs/hash_map_test.js, elapsed time: 355.878808 msecs
Compiling /home/travis/build/mfikes/clojurescript/src/test/cljs/cljs/predicates_test.cljs to builds/out-adv/cljs/predicates_test.js, elapsed time: 48.145784 msecs
Compiling src/test/cljs/cljs/ns_test/bar.cljs to builds/out-adv/cljs/ns_test/bar.js, elapsed time: 5.392888 msecs
Compiling /home/travis/build/mfikes/clojurescript/src/main/cljs/clojure/set.cljs to builds/out-adv/clojure/set.js, elapsed time: 99.974164 msecs
Compiling src/test/cljs/cljs/set_equiv_test.cljs to builds/out-adv/cljs/set_equiv_test.js, elapsed time: 85.066112 msecs
Compiling src/test/cljs/cljs/binding_test_other_ns.cljs to builds/out-adv/cljs/binding_test_other_ns.js, elapsed time: 1.583184 msecs
Compiling /home/travis/build/mfikes/clojurescript/src/test/cljs/cljs/binding_test.cljs to builds/out-adv/cljs/binding_test.js, elapsed time: 49.698315 msecs
Compiling /home/travis/build/mfikes/clojurescript/src/test/cljs/cljs/keyword_other.cljs to builds/out-adv/cljs/keyword_other.js, elapsed time: 1.585948 msecs
Compiling src/test/cljs/cljs/keyword_test.cljs to builds/out-adv/cljs/keyword_test.js, elapsed time: 53.564496 msecs
Compiling /home/travis/build/mfikes/clojurescript/src/test/cljs/cljs/apply_test.cljs to builds/out-adv/cljs/apply_test.js, elapsed time: 425.710602 msecs
Compiling /home/travis/build/mfikes/clojurescript/src/main/cljs/clojure/core/reducers.cljs to builds/out-adv/clojure/core/reducers.js, elapsed time: 173.721872 msecs
Compiling /home/travis/build/mfikes/clojurescript/src/test/cljs/cljs/reducers_test.cljs to builds/out-adv/cljs/reducers_test.js, elapsed time: 53.381312 msecs
Copying jar:file:/home/travis/build/mfikes/clojurescript/lib/test.check.jar!/clojure/test/check/clojure_test.cljc to builds/out-adv/clojure/test/check/clojure_test.cljc
Compiling /home/travis/build/mfikes/clojurescript/src/main/cljs/cljs/spec/alpha.cljs to builds/out-adv/cljs/spec/alpha.js, elapsed time: 1443.596345 msecs
Compiling builds/out-adv/clojure/test/check/clojure_test.cljc to builds/out-adv/clojure/test/check/clojure_test.js, elapsed time: 73.281347 msecs
Compiling /home/travis/build/mfikes/clojurescript/src/main/cljs/cljs/spec/test/alpha.cljs to builds/out-adv/cljs/spec/test/alpha.js, elapsed time: 1312.809931 msecs
WARNING: baz is a single segment namespace at line 1 /home/travis/build/mfikes/clojurescript/src/test/cljs/baz.cljs
Compiling /home/travis/build/mfikes/clojurescript/src/test/cljs/baz.cljs to builds/out-adv/baz.js, elapsed time: 2.683303 msecs
Compiling /home/travis/build/mfikes/clojurescript/src/test/cljs/foo/ns_shadow_test.cljs to builds/out-adv/foo/ns_shadow_test.js, elapsed time: 71.628221 msecs
Compiling src/test/cljs/cljs/syntax_quote_test.cljs to builds/out-adv/cljs/syntax_quote_test.js, elapsed time: 53.676944 msecs
WARNING: No such namespace: cljs.spec.test.test-macros$macros, could not locate cljs/spec/test/test_macros$macros.cljs, cljs/spec/test/test_macros$macros.cljc, or JavaScript source providing "cljs.spec.test.test-macros$macros" (Please check that namespaces with dashes use underscores in the ClojureScript file name) at line 51 src/test/cljs/cljs/spec/test_test.cljs
Compiling /home/travis/build/mfikes/clojurescript/src/test/cljs/clojure/string_test.cljs to builds/out-adv/clojure/string_test.js, elapsed time: 1872.248777 msecs
Compiling src/test/cljs/cljs/spec/test_test.cljs to builds/out-adv/cljs/spec/test_test.js, elapsed time: 590.610678 msecs
Compiling /home/travis/build/mfikes/clojurescript/src/test/cljs/cljs/ns_test/foo.cljs to builds/out-adv/cljs/ns_test/foo.js, elapsed time: 41.874633 msecs
Compiling /home/travis/build/mfikes/clojurescript/src/test/cljs/cljs/array_access_test.cljs to builds/out-adv/cljs/array_access_test.js, elapsed time: 647.274414 msecs
Compiling src/test/cljs/cljs/reader_test.cljs to builds/out-adv/cljs/reader_test.js, elapsed time: 755.252612 msecs
Compiling src/test/cljs/cljs/seqs_test.cljs to builds/out-adv/cljs/seqs_test.js, elapsed time: 1267.853872 msecs
Compiling src/test/cljs/cljs/hashing_test.cljs to builds/out-adv/cljs/hashing_test.js, elapsed time: 243.651164 msecs
Compiling /home/travis/build/mfikes/clojurescript/src/test/cljs/cljs/new_new_test.cljs to builds/out-adv/cljs/new_new_test.js, elapsed time: 717.278849 msecs
Compiling /home/travis/build/mfikes/clojurescript/src/test/cljs/cljs/pprint_test.cljs to builds/out-adv/cljs/pprint_test.js, elapsed time: 5922.541317 msecs
Compile sources, elapsed time: 19353.57812 msecs
Exception in thread "main" clojure.lang.ExceptionInfo: failed compiling file:/home/travis/build/mfikes/clojurescript/src/test/cljs/cljs/npm_deps_test.cljs {:file #object[java.io.File 0x5f72aaed "/home/travis/build/mfikes/clojurescript/src/test/cljs/cljs/npm_deps_test.cljs"]}, compiling:(/home/travis/build/mfikes/clojurescript/bin/../bin/cljsc.clj:22:1)
	at clojure.lang.Compiler.load(Compiler.java:7526)
	at clojure.lang.Compiler.loadFile(Compiler.java:7452)
	at clojure.main$load_script.invokeStatic(main.clj:278)
	at clojure.main$script_opt.invokeStatic(main.clj:338)
	at clojure.main$script_opt.invoke(main.clj:333)
	at clojure.main$main.invokeStatic(main.clj:424)
	at clojure.main$main.doInvoke(main.clj:387)
	at clojure.lang.RestFn.applyTo(RestFn.java:137)
	at clojure.lang.Var.applyTo(Var.java:702)
	at clojure.main.main(main.java:37)
Caused by: clojure.lang.ExceptionInfo: failed compiling file:/home/travis/build/mfikes/clojurescript/src/test/cljs/cljs/npm_deps_test.cljs {:file #object[java.io.File 0x5f72aaed "/home/travis/build/mfikes/clojurescript/src/test/cljs/cljs/npm_deps_test.cljs"]}
	at clojure.core$ex_info.invokeStatic(core.clj:4739)
	at clojure.core$ex_info.invoke(core.clj:4739)
	at cljs.compiler$compile_file$fn__4775.invoke(compiler.cljc:1663)
	at cljs.compiler$compile_file.invokeStatic(compiler.cljc:1623)
	at cljs.compiler$compile_file.invoke(compiler.cljc:1599)
	at cljs.closure$compile_file.invokeStatic(closure.clj:647)
	at cljs.closure$compile_file.invoke(closure.clj:625)
	at cljs.closure$eval7125$fn__7126.invoke(closure.clj:721)
	at cljs.closure$eval7028$fn__7029$G__7017__7036.invoke(closure.clj:543)
	at cljs.closure$eval7131$fn__7132.invoke(closure.clj:730)
	at cljs.closure$eval7028$fn__7029$G__7017__7036.invoke(closure.clj:543)
	at cljs.closure$compile_task$fn__7235.invoke(closure.clj:1030)
	at cljs.closure$compile_task.invokeStatic(closure.clj:1028)
	at cljs.closure$compile_task.invoke(closure.clj:1020)
	at cljs.closure$parallel_compile_sources$fn__7241.invoke(closure.clj:1058)
	at clojure.lang.AFn.applyToHelper(AFn.java:152)
	at clojure.lang.AFn.applyTo(AFn.java:144)
	at clojure.core$apply.invokeStatic(core.clj:657)
	at clojure.core$with_bindings_STAR_.invokeStatic(core.clj:1965)
	at clojure.core$with_bindings_STAR_.doInvoke(core.clj:1965)
	at clojure.lang.RestFn.invoke(RestFn.java:425)
	at clojure.lang.AFn.applyToHelper(AFn.java:156)
	at clojure.lang.RestFn.applyTo(RestFn.java:132)
	at clojure.core$apply.invokeStatic(core.clj:661)
	at clojure.core$bound_fn_STAR_$fn__5471.doInvoke(core.clj:1995)
	at clojure.lang.RestFn.invoke(RestFn.java:397)
	at clojure.lang.AFn.run(AFn.java:22)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
	at java.lang.Thread.run(Thread.java:748)
Caused by: clojure.lang.ExceptionInfo: No such namespace: lodash/array, could not locate lodash_SLASH_array.cljs, lodash_SLASH_array.cljc, or JavaScript source providing "lodash/array" (Please check that namespaces with dashes use underscores in the ClojureScript file name) in file /home/travis/build/mfikes/clojurescript/src/test/cljs/cljs/npm_deps_test.cljs {:tag :cljs/analysis-error}
	at clojure.core$ex_info.invokeStatic(core.clj:4739)
	at clojure.core$ex_info.invoke(core.clj:4739)
	at cljs.analyzer$error.invokeStatic(analyzer.cljc:718)
	at cljs.analyzer$error.invoke(analyzer.cljc:714)
	at cljs.analyzer$error.invokeStatic(analyzer.cljc:716)
	at cljs.analyzer$error.invoke(analyzer.cljc:714)
	at cljs.analyzer$analyze_deps.invokeStatic(analyzer.cljc:2338)
	at cljs.analyzer$analyze_deps.invoke(analyzer.cljc:2312)
	at cljs.analyzer$ns_side_effects.invokeStatic(analyzer.cljc:3741)
	at cljs.analyzer$ns_side_effects.invoke(analyzer.cljc:3736)
	at cljs.analyzer$analyze_STAR_$fn__3218.invoke(analyzer.cljc:3861)
	at clojure.lang.PersistentVector.reduce(PersistentVector.java:341)
	at clojure.core$reduce.invokeStatic(core.clj:6747)
	at clojure.core$reduce.invoke(core.clj:6730)
	at cljs.analyzer$analyze_STAR_.invokeStatic(analyzer.cljc:3861)
	at cljs.analyzer$analyze_STAR_.invoke(analyzer.cljc:3851)
	at cljs.analyzer$analyze.invokeStatic(analyzer.cljc:3880)
	at cljs.analyzer$analyze.invoke(analyzer.cljc:3863)
	at cljs.compiler$emit_source.invokeStatic(compiler.cljc:1485)
	at cljs.compiler$emit_source.invoke(compiler.cljc:1463)
	at cljs.compiler$compile_file_STAR_$fn__4744.invoke(compiler.cljc:1566)
	at cljs.compiler$with_core_cljs.invokeStatic(compiler.cljc:1383)
	at cljs.compiler$with_core_cljs.invoke(compiler.cljc:1372)
	at cljs.compiler$compile_file_STAR_.invokeStatic(compiler.cljc:1550)
	at cljs.compiler$compile_file_STAR_.invoke(compiler.cljc:1543)
	at cljs.compiler$compile_file$fn__4775.invoke(compiler.cljc:1648)
	... 27 more
Comment by Mike Fikes [ 08/Sep/18 7:04 PM ]

It appears that the root cause of the CI problems was that the Travis CI box was temporarily running older versions of NPM/Node. You can repro the problem with a default install of Ubuntu 14.04, and you can even repro it without this patch.

Comment by Mike Fikes [ 08/Sep/18 7:31 PM ]

fixed https://github.com/clojure/clojurescript/commit/9e1ff2232da00dcda7fb495784e7d5a1d72573a7





[CLJS-2895] Cull source maps Created: 06/Sep/18  Updated: 08/Sep/18

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Mike Fikes Assignee: Mike Fikes
Resolution: Unresolved Votes: 1
Labels: None

Attachments: Text File CLJS-2895-0.patch    

 Description   

When generating source maps, we emit a lot of extra data that is not needed for mapping stacktraces.

Planck culls a lot of this information out while still being able to successfully map stacktraces, with a big perf gain: Planck loads the smaller source map files instantly rather than seconds. This has been in place for a year now with no reports of issues.

I'm thinking that we may be able to do the same in ClojureScript proper, when writing out source maps. It's worth trying an experiment where we optionally strip out this information using the function that had been developed for Planck, included here for reference:

(defn- strip-source-map
  "Strips a source map down to the minimal representation needed for mapping
  stacktraces. This means we only need the :line and :col fields, we only need
  the last element in each vector of such maps, and we can eliminate
  duplicates, taking the smallest col number for each unique value."
  [sm]
  (into {}
    (map (fn [[row cols]]
           [row (->> cols
                  (map (fn [[col frames]]
                         [col [(select-keys (peek frames) [:line :col])]]))
                  (sort-by first)
                  (distinct-by second)
                  (into {}))]))
    sm))


 Comments   
Comment by Mike Fikes [ 07/Sep/18 9:18 PM ]

We are currently emitting source map line/col information for every AST, regardless of op type. While this guarantees that we have comprehensive source mapping information, it may be a much larger set than we need for many common uses.

Some op types can clearly be omitted, like :no-op for example. With a little experimentation, you can see that source mapping for the limited purpose of mapping stack traces is possible with just two or three op types. Dirac DevTools makes more extensive use of source mapping information in order to properly identify locals, binding forms, etc. within the source code.

The attached patch limits source map line/col emission to those tags which are needed for stack trace mapping, and for some simple uses with Dirac (which is a superset of those needed for stack trace mapping). The fundamental problem with this strategy is identifying the right subset of ops for which we emit line/col information.

But, this might be worth it if we can successfully identify a minimal set that meets general needs. With the attached patch, we get a 12% performance boost relative to current master when compiling Coal Mine in non-parallel mode.

With this patch, the size of the source maps written to disk are smaller: For cljs.core it is 432567 bytes instead of 640411 bytes.

Attaching this patch for feedback. If we can find a suitable subset, this might work out. If this proves too difficult, perhaps a compiler option could be introduced to control whether we emit a a tiny subset sufficient for stack trace mapping or slightly larger for debugging (Dirac), or all ops.

Comment by Antonin Hildebrand [ 08/Sep/18 9:47 AM ]

Dirac relies on source maps in two ways:
1) indirectly: standard Chrome DevTools (or V8) code uses source maps in various places (e.g. mapping console message line/col info back to original sources, or showing proper lines in the debugger, mapping names of local variables in the debugger, etc.)
2) directly: Dirac uses source map info from DevTools to provide code completion in its REPL prompt (it relies on "names" list in associated source maps as defined in source map specs[1])

I have pretty good test coverage for #2 so I would spot any regressions with code completion. But it is unclear to me how pruned source maps could affect DevTools/V8 itself.

Also please be aware that there are known source maps issues in DevTools related to ClojureScript (or other transpiled languages). Dirac tried to patch them but there might still be some outstanding bugs.
https://github.com/binaryage/dirac/issues/53

Are you aware of https://github.com/sokra/source-map-visualization? This tool proved to be helpful when I was debugging source map issues with Dirac. I think this could help to determine how to generate minimal source maps with relevant info.

[1] https://sourcemaps.info/spec.html





[CLJS-2894] Optimize source map gen-col counting Created: 06/Sep/18  Updated: 07/Sep/18  Resolved: 06/Sep/18

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Mike Fikes Assignee: Mike Fikes
Resolution: Completed Votes: 1
Labels: None

Attachments: Text File CLJS-2894-2.patch     Text File CLJS-2894.patch    
Approval: Accepted

 Description   

When source maps are generated, this line https://github.com/clojure/clojurescript/blob/4f3707624846a2cf0345859e41370ec172da73c4/src/main/clojure/cljs/compiler.cljc#L209-L211
which updates :gen-col, is evidently called frequently enough where it can account for 10 to 20% of the perf when compiling large codebases like Coal Mine.



 Comments   
Comment by Mike Fikes [ 06/Sep/18 12:57 PM ]

CLJS-2894.patch moves :gen-col out to a separate dynamic var which holds an AtomicLong. This should have sufficient threading properties when :parallel-build true, as this dynamic var is effectively a thread-local.

For self-hosted, it roughly keeps the previous approach, with some mild optimizations (especially avoiding count on string).

Comment by Mike Fikes [ 06/Sep/18 1:28 PM ]

CLJS-2894-2.patch fixes a self-host bug in the first patch and tweaks a little whitespace formatting.

Comment by Mike Fikes [ 06/Sep/18 8:21 PM ]

fixed https://github.com/clojure/clojurescript/commit/83866aaf597f183877c0cf586c002f3b8b51d487

Comment by Mike Fikes [ 07/Sep/18 8:05 PM ]

See https://github.com/binaryage/dirac/issues/81





[CLJS-2867] Inferred return type of namespace is string Created: 19/Aug/18  Updated: 07/Sep/18

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Unresolved Votes: 1
Labels: newbie

Attachments: Text File CLJS-2867.patch    
Patch: Code

 Description   

The namespace function is inferred to return type string, even though it can return nil.

I believe that this is due to an inappropriate ^string in the INamed protocol definition.



 Comments   
Comment by Martin Kučera [ 06/Sep/18 8:00 AM ]

Corrected type inference of the INamed protocol method namespace to #{string clj-nil}.

Comment by Mike Fikes [ 07/Sep/18 1:56 PM ]

Confirmed with Martin Kučera via Slack that CA has been signed.

Comment by Mike Fikes [ 07/Sep/18 2:34 PM ]

I've confirmed that this patch works correctly. Forms like (namespace :ab/c) are now inferred as returning #{string clj-nil}





[CLJS-2896] Allow parallel analysis and compilation Created: 07/Sep/18  Updated: 07/Sep/18  Resolved: 07/Sep/18

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Mike Fikes Assignee: Mike Fikes
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-2896.patch    
Patch: Code
Approval: Accepted

 Description   

If is possible to allow compilation to occur concurrently with analysis in the following sense: Analysis is done causing a set of ASTs to be emitted (compiled), with the rough pattern being to analyze a form, compile it, analyze the next, compile it, etc. Instead, the emission can be done in a separate thread, with things joining up at the end prior to source map emission, etc.

This strategy can be employed if :parallel-build true, and in that case, experiments are showing a 10% speedup on large codebases (Coal Mine with lots of files). What generally happens in the Coal Mine case is that, near the end, a few straggler files exist and things ramp down towards single-threaded behavior. With parallel analysis and compilation, this ramp-down phase is much quicker, shaving off time.

With smaller codebases, this extra granularity can more effectively make use of the available processors.



 Comments   
Comment by Mike Fikes [ 07/Sep/18 12:26 PM ]

For smaller codebases, I'm seeing more speedup: ClojureScript's unit tests exhibit a speedup of 1.16; Planck's compilation exhibits 1.2

Comment by Mike Fikes [ 07/Sep/18 12:39 PM ]

fixed https://github.com/clojure/clojurescript/commit/fc66a5a558b2749c93e1d29605bb69d58f0ad36f





[CLJS-2893] seq calls alength on string Created: 04/Sep/18  Updated: 06/Sep/18  Resolved: 06/Sep/18

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Mike Fikes Assignee: Mike Fikes
Resolution: Completed Votes: 1
Labels: newbie

Attachments: Text File CLJS-2893.patch    
Patch: Code
Approval: Accepted

 Description   

The implementation of seq calls alength on string values and should instead use .-length. (See CLJS-2335).

https://github.com/clojure/clojurescript/blob/4f3707624846a2cf0345859e41370ec172da73c4/src/main/cljs/cljs/core.cljs#L1217



 Comments   
Comment by Oliver Eidel [ 05/Sep/18 3:11 AM ]

this is my first contribution, so I hope I did everything right. tell me if I missed anything!

Comment by Mike Fikes [ 05/Sep/18 5:54 AM ]

Great! And I see your name is listed here https://clojure.org/community/contributors.

Comment by Mike Fikes [ 06/Sep/18 9:41 PM ]

fixed https://github.com/clojure/clojurescript/commit/0598b93e309150a979d3a738ed6cb6a8558dde05

Thanks for your contribution Oliver!





[CLJS-2890] fspec role in problem path is not useful Created: 04/Sep/18  Updated: 06/Sep/18  Resolved: 06/Sep/18

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Mike Fikes Assignee: Mike Fikes
Resolution: Completed Votes: 0
Labels: spec

Attachments: Text File CLJS-2890.patch    
Patch: Code
Approval: Accepted

 Description   

Port CLJ-2392 to ClojureScript.



 Comments   
Comment by Mike Fikes [ 06/Sep/18 9:34 PM ]

fixed https://github.com/clojure/clojurescript/commit/47553d8a3173ad4ebfcbfc557b73ecb44ac468b6





[CLJS-2887] Improve names in core macro specs Created: 04/Sep/18  Updated: 06/Sep/18  Resolved: 06/Sep/18

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Mike Fikes Assignee: Mike Fikes
Resolution: Completed Votes: 0
Labels: spec

Attachments: Text File CLJS-2887.patch    
Patch: Code
Approval: Accepted

 Description   

Port CLJ-2395 to ClojureScript.



 Comments   
Comment by Mike Fikes [ 06/Sep/18 9:12 PM ]

fixed https://github.com/clojure/clojurescript/commit/ef32778989f7ba2311a1e8a5d99c30e6805f5719





[CLJS-2891] stop including data in ex-info message Created: 04/Sep/18  Updated: 06/Sep/18  Resolved: 06/Sep/18

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Mike Fikes Assignee: Mike Fikes
Resolution: Completed Votes: 0
Labels: spec

Attachments: Text File CLJS-2891.patch    
Patch: Code
Approval: Accepted

 Description   

Port https://github.com/clojure/spec.alpha/commit/f23ea614b3cb658cff0044a027cacdd76831edcf to ClojureScript and subsume CLJS-2892 which introduced ex-info usage.



 Comments   
Comment by Mike Fikes [ 06/Sep/18 9:12 PM ]

fixed https://github.com/clojure/clojurescript/commit/5f0fabc65ae7ba201b32cc513a1e5931a80a2bf7





[CLJS-2889] Improve sorting on problem printing Created: 04/Sep/18  Updated: 06/Sep/18  Resolved: 06/Sep/18

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Mike Fikes Assignee: Mike Fikes
Resolution: Completed Votes: 0
Labels: spec

Attachments: Text File CLJS-2889.patch    
Patch: Code
Approval: Accepted

 Description   

Port CLJ-2393 to ClojureScript.



 Comments   
Comment by Mike Fikes [ 04/Sep/18 7:50 PM ]

Before:

cljs.user=> (let [[a :as b c] [1 2 3]] a)
clojure.lang.ExceptionInfo: Call to cljs.core/let did not conform to spec:
In: [0 0] val: [a :as b c] fails spec: :cljs.core.specs.alpha/local-name at: [:args :bindings :binding :sym] predicate: simple-symbol?
In: [0 0 3] val: (c) fails spec: :cljs.core.specs.alpha/seq-binding-form at: [:args :bindings :binding :seq] predicate: (cat :elems (* :cljs.core.specs.alpha/binding-form) :rest (? (cat :amp #{(quote &)} :form :cljs.core.specs.alpha/binding-form)) :as (? (cat :as #{:as} :sym :cljs.core.specs.alpha/local-name))),  Extra input
In: [0 0] val: [a :as b c] fails spec: :cljs.core.specs.alpha/map-bindings at: [:args :bindings :binding :map] predicate: map?
In: [0 0] val: [a :as b c] fails spec: :cljs.core.specs.alpha/map-special-binding at: [:args :bindings :binding :map] predicate: map?
 at line 1 <cljs repl> {:file "<cljs repl>", :line 1, :column 1, :root-source-info {:source-type :fragment, :source-form (let [[a :as b c] [1 2 3]] a)}, :tag :cljs/analysis-error}
Comment by Mike Fikes [ 04/Sep/18 8:04 PM ]

After:

For the particular example above, this involves Clojure generating the message, but after the change in ClojureScript, you can easily see the corresponding error update in self-hosted ClojureScript (in this case generated via Planck built with the change):

cljs.user=> (let [[a :as b c] [1 2 3]] a)
            ^
Call to cljs.core$macros/let did not conform to spec:
In: [0 0 3] val: (c) fails spec: :cljs.core.specs.alpha/seq-binding-form at: [:args :bindings :binding :seq] predicate: (cat :elems (* :cljs.core.specs.alpha/binding-form) :rest (? (cat :amp #{(quote &)} :form :cljs.core.specs.alpha/binding-form)) :as (? (cat :as #{:as} :sym :cljs.core.specs.alpha/local-name))),  Extra input
In: [0 0] val: [a :as b c] fails spec: :cljs.core.specs.alpha/local-name at: [:args :bindings :binding :sym] predicate: simple-symbol?
In: [0 0] val: [a :as b c] fails spec: :cljs.core.specs.alpha/map-bindings at: [:args :bindings :binding :map] predicate: map?
In: [0 0] val: [a :as b c] fails spec: :cljs.core.specs.alpha/map-special-binding at: [:args :bindings :binding :map] predicate: map?

The salient aspect is that the complaint about c bubbles up to the top.

Comment by Mike Fikes [ 06/Sep/18 9:11 PM ]

fixed https://github.com/clojure/clojurescript/commit/033d19cdaf53f979034c3b60329f71d1509204c9





[CLJS-2888] Printing of spec problems buries the failing predicate which should be more prominent Created: 04/Sep/18  Updated: 06/Sep/18  Resolved: 06/Sep/18

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Mike Fikes Assignee: Mike Fikes
Resolution: Completed Votes: 0
Labels: spec

Attachments: Text File CLJS-2888.patch    
Patch: Code
Approval: Accepted

 Description   

Port CLJ-2391 to ClojureScript.



 Comments   
Comment by Mike Fikes [ 06/Sep/18 9:11 PM ]

fixed https://github.com/clojure/clojurescript/commit/2f73857f0af13fb386cedbc4da21f4e95e05fdfc





[CLJS-2884] Support for GraalJS RC6 Created: 03/Sep/18  Updated: 06/Sep/18  Resolved: 06/Sep/18

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Mike Fikes Assignee: Mike Fikes
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-2884.patch    
Patch: Code and Test
Approval: Accepted

 Description   

With RC 6 a couple of things changed:

This enhancement ticket asks that we update to RC6 and update the CI tests to use this release candidate.



 Comments   
Comment by Mike Fikes [ 06/Sep/18 9:06 PM ]

fixed https://github.com/clojure/clojurescript/commit/3e28f7b9703a8995a58c7a285b05aa36daf76ddd





[CLJS-2866] Predicate-induced type inference Created: 19/Aug/18  Updated: 06/Sep/18

Status: In Progress
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Mike Fikes Assignee: Mike Fikes
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File CLJS-2866-0.patch     Text File CLJS-2866-1.patch     Text File CLJS-2866-2.patch     Text File CLJS-2866-3.patch    

 Description   

This pattern seems to be fairly frequent:

(if (string? x)
  (foo x)
  (bar x))

It would be nice if the compiler could, based on the use of the predicate, infer that x is of type string in the then branch, and effectively behave as if the code were explicitly hinted:

(if (string? x)
  (foo ^string x)
  (bar x))

This would be useful in situations where the type could affect warnings or code gen. As a motivating example, consider the source for name

(defn name
  "Returns the name String of a string, symbol or keyword."
  [x]
  (if (implements? INamed x)
    (-name ^not-native x)
    (if (string? x)
      x
      (throw (js/Error. (str "Doesn't support name: " x))))))

If x were inferred to be of type string in the then branch of the code above, then via function return type inference, name would be inferred to return type string (because -name is hinted).

Then, changes like CLJS-2864 could make use of this information, with expressions like (str (namespace x) "/" (name x)) expanding to

(js* "[~{},~{},~{}].join('')" (namespace x) "/" (name x))

(avoiding an unnecessary call to cljs.core.str.cljs$core$IFn$_invoke$arity$1, being much more efficient).

This kind of inference could be done for other core predicates, and even for code involving implements?. You could imagine that the expicit hint here could be removed:

(if (implements? ISeq coll)
      (-first ^not-native coll)
      ...


 Comments   
Comment by Mike Fikes [ 19/Aug/18 8:46 AM ]

Added CLJS-2866-0.patch, a baby-step experimental change that does this just for cljs.core/string? and for simple symbol locals. Interested in thoughts on the direction of this idea.

Comment by Mike Fikes [ 20/Aug/18 10:18 AM ]

Node from David in slack: "we definitely want type inference to flow along based on conditionals"

Comment by Mike Fikes [ 20/Aug/18 4:54 PM ]

CLJS-2866-1.patch fleshes out the capability so that it covers a lot of what we might want in the end, and is at a point where it is reviewable.

There is one issue that will need to be resolved: A guard (not= 'cljs.core/IPrintWithWriter tag) is in the type-check-induced-tag function. Without it, the tests in cljs.extend-to-native-test fail. (It appears that extend-type / extend-protocol do type hinting on their methods and perhaps something with this new logic breaks things, and we will need to get to the bottom of that.)

Comment by Mike Fikes [ 20/Aug/18 5:57 PM ]

CLJS-2866-2.patch removes the guard that is in CLJS-2866-1.patch but instead illustrates suppressing the static dispatch that occurs by using ^any. Without this you get an error like this

cljs.user=> (extend-type object
  IPrintWithWriter
  (-pr-writer [obj writer _]
      (write-all writer "#object[custom-print-cljs-2812]")))
nil
cljs.user=> #js {}
repl:13
throw e__8737__auto__;
^

TypeError: obj.cljs$core$IPrintWithWriter$_pr_writer$arity$3 is not a function
at Object.cljs$core$pr_writer_impl [as pr_writer_impl] (/Users/mfikes/out/cljs/core.js:32916:12)
...

It's not clear to me how we can solve this one in a clean way. We may have to put satisfies? inference behind a compiler flag that is off by default?

Comment by Mike Fikes [ 21/Aug/18 12:28 PM ]

Assigning to David to take a look at the compiler emission for this case.

I found that, if you take current master and simply add a type hint in pr-writer-impl:

(satisfies? IPrintWithWriter obj)
        (-pr-writer ^cljs.core/IPrintWithWriter obj writer opts)

then at the REPL you can repro the failure with

cljs.user=> (extend-type object
  IPrintWithWriter
  (-pr-writer [obj writer _]
      (write-all writer "#object[custom-print-cljs-2812]")))
nil
cljs.user=> #js {}
repl:13
throw e__8712__auto__;
^

TypeError: obj.cljs$core$IPrintWithWriter$_pr_writer$arity$3 is not a function
    at Object.cljs$core$pr_writer_impl [as pr_writer_impl] (/Users/mfikes/out/cljs/core.js:32916:12)
...

The latest patch puts an ^any type hint in that spot to suppress the inferred type, but if we can figure out a way to fix the compiler emission for this case then perhaps we can just let it infer ^cljs.core/IPrintWithWriter.

Comment by Mike Fikes [ 06/Sep/18 9:41 AM ]

There is a problem with the patch where this code

(fn [form]
  (while true
        (let [curr form]
          (when (sequential? curr)
            (reverse curr)))))

causes this to be emitted

function (form){
while(true){
var curr_18941 = form;
if(cljs.core.sequential_QMARK_.call(null,curr_18941)){
cljs.core.reverse.call(null,curr);
} else {
}

continue;

break;
}
}

Which is wrong in that curr is placed in the code instead of curr_18941.

Assigning back to me to look at this.

Comment by Mike Fikes [ 06/Sep/18 9:04 PM ]

CLJS-2866-3.patch adds a test for the last issue. The root cause is that the strategy of analyzing the then branch with a tag added to the local messes with the System/identityHashCode logic used to key bindings placed in the *lexical-renames* map (because assoc ing the :tag creates a new entry in env). The CLJS-2866-3.patch is working around this for now by placing an extra :identity key in the AST. We should think about the best way to solve this; this is just an interim fix not meant to ultimately be applied. As an aside: Using System/identityHashCode adds a remote chance of collision (as hash codes are not guaranteed to be unique, while the :identity value being added in the last patch is a unique counter value.





[CLJS-2873] Improved inference for loop / recur Created: 26/Aug/18  Updated: 05/Sep/18

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Mike Fikes Assignee: Mike Fikes
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File CLJS-2873-0.patch    

 Description   

Currently, type inference in loop / recur is done as if a let construct were being used. Critically, the inferred types of the bound locals is assumed to be that implied by their inits, with the types of the recur expressions not being considered.

CLJS-1561 attempts to address this situation by warning if the code recurs with a different type. But, it is common and legal practice to have code that results in the type varying. (For example, initial bindings might be nil, with things breaking out of the loop / recur when a non-nil value is found.)

Perhaps the right thing to do is to infer that the types of each local are the sum of the types derived from their inits and the types of the recur expressions.

As a concrete example, consider this code:

(loop [x nil]
  (let [y (and x 3)]
    (if y
      y
      (recur "a"))))

Currently x is inferred to be of type clj-nil. A consequence is that (with CLJS-2869) y will initially be inferred to be of type clj-nil, and, since it is the return value, the entire loop / recur is inferred to be of type clj-nil.

Instead, perhaps x should be inferred to be of type #{clj-nil string} by taking the sum of the types inferred by the init and the recur expression. Then (with CLJS-2869), y would be inferred to be of type number and the entire loop / recur would be correctly inferred to be of type number.

In order to infer the types of the recur expressions, you need to start with the types initially inferred by the initial bindings. I'd propose that this produce an initial set of types, and then the entire thing can be analyzed again (using the sum types), to deduce the correct types. Then, after this second analysis, a second sum can be done, and if it differs you could either do additional analysis cycles until things converge, or (in the name of compiler perf), just throw in the towel for non-converging situations and conclude that the type is any.

The same arguments above would apply to function arguments when recur is used, but unhinted arguments start off as type any and therefore cannot grow to encompass more types, and hinted arguments have their types specified.






[CLJS-2209] case docstring should explain constants may be evaluated (cljs only) Created: 10/Jul/17  Updated: 05/Sep/18

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 1.9.660
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Phill Wolf Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: docstring, newbie

Approval: Vetted

 Description   

The ClojureScript docstring for case says, "The test-constants are not evaluated." But that statement is not complete. The ClojureScript "Differences from Clojure" page <https://www.clojurescript.org/about/differences> says ":const metadata: ... causes case test constants which are symbols resolving to ^:const Vars to be inlined with their values". The case docstring should reflect that. Related discussion at <https://groups.google.com/d/msg/clojure/u1RZsmjbQ64/p7B9eRwuAQAJ>.






[CLJS-2808] Make cljs.repl/special-doc-map accessible to self-host Created: 05/Jul/18  Updated: 05/Sep/18

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Andrea Richiardi Assignee: Andrea Richiardi
Resolution: Unresolved Votes: 0
Labels: newbie


 Description   

It would be a good thing to move cljs.repl/special-doc-map and cljs.repl/repl-special-doc-map to a place that is also accessible to self-host environments.

At the moment both Planck and Lumo just copy them in their code base. This is error prone and requires manual maintenance, together with being a bit hard to handle in downstream tooling - the tool now needs to know the three places where the maps are placed.

I can provide a patch so I am going to ask a) if my proposal makes sense b) what is the best way to shared them between the clj and cljs world.



 Comments   
Comment by Andrea Richiardi [ 09/Aug/18 10:45 PM ]

Any comment, feedback, deniable denial on this one ? I really think these should be somewhat public so that tooling avoids replicating them.

Comment by David Nolen [ 10/Aug/18 2:26 PM ]

Go for it!





[CLJS-2892] throw ex-info on macroexpand spec error with ex-data Created: 04/Sep/18  Updated: 04/Sep/18  Resolved: 04/Sep/18

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Mike Fikes Assignee: Mike Fikes
Resolution: Duplicate Votes: 0
Labels: spec


 Description   

Port https://github.com/clojure/spec.alpha/commit/b753c005a5e5301cbbe8400ba8987b339a08199d to ClojureScript.



 Comments   
Comment by Mike Fikes [ 04/Sep/18 9:55 PM ]

Subsumed into CLJS-2891.





[CLJS-2854] With :global-exports and browser REPL, need to require twice Created: 01/Aug/18  Updated: 04/Sep/18

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Mike Fikes Assignee: Mike Fikes
Resolution: Unresolved Votes: 0
Labels: None
Environment:

{:deps {org.clojure/clojurescript {:mvn/version "1.10.238"}}}
and
{:deps {org.clojure/clojurescript {:mvn/version "1.10.339"}}}
and current master:
{:deps {org.clojure/clojurescript {:git/url "https://github.com/clojure/clojurescript" :sha "3123aa32851c01682cf076e0e3b497e890b26922"}}}


Attachments: Text File CLJS-2854-0.patch     Text File CLJS-2854-1.patch     Text File CLJS-2854-2.patch    
Patch: Code

 Description   
mylib.js
mylib = { abc: 3 }
co.edn
{:foreign-libs [{:file "mylib.js"
                 :provides ["my-lib"]
                 :global-exports {my-lib mylib}}]}

Repro:

$ clj -m cljs.main -co co.edn -r
ClojureScript 1.10.339
cljs.user=> (require 'my-lib)

cljs.user=> my-lib/abc
TypeError: Cannot read property 'abc' of undefined
	 (<NO_SOURCE_FILE>)
cljs.user=> (require 'my-lib)

cljs.user=> my-lib/abc
3

This doesn't occur in the Node and Nashorn REPLs.



 Comments   
Comment by Mike Fikes [ 01/Aug/18 6:35 PM ]

The problem is that browser requires are asynchronous (at least in the REPL), so the bit of code that sets up the global export races with the require code loading.

CLJS-2854-0.patch explores a way to solve this (without yet including tests, etc), by conditionally queueing code (when in the browser REPL) and then executing it after the load queue drains.

Comment by Mike Fikes [ 08/Aug/18 8:44 AM ]

CLJS-2854-1.patch introduces a mechanism whereby REPLs can indicate whether they have a way to queue post load activity, and has the browser REPL participate in this mechanism. The names used could be cleaned up, especially if this leads to public API that REPLs could use.

Comment by Mike Fikes [ 09/Aug/18 6:49 AM ]

With this code

src/foo/core.cljs
(ns foo.core
 (:require my-lib))

(def x my-lib/abc)

things work with the patch:

$ clj -m cljs.main -co co.edn -r
cljs.user=> (require 'foo.core)

cljs.user=> foo.core/x
3
Comment by David Nolen [ 10/Aug/18 4:43 PM ]

To clarify the problem a bit - the issue is that (require ...) forms in the REPL are implicitly in cljs.user, this means that direct requires of libraries with global exports will race because that lib's export mapping comes right after that lib's goog.require, however that will be an async load, so it won't work. This doesn't affect the transitive case.

Comment by Mike Fikes [ 10/Aug/18 5:44 PM ]

CLJS-2854-2.patch tries to keep the logic in the compiler code by simply using a timeout to try to avoid the race.

Comment by Mike Fikes [ 04/Sep/18 9:00 PM ]

CLJS-2854-2.patch no longer applies.





[CLJS-2865] Optimize string expression concatenation Created: 19/Aug/18  Updated: 04/Sep/18

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File CLJS-2865-2.patch     Text File CLJS-2865-3.patch     Text File CLJS-2865.patch    
Patch: Code and Test

 Description   

Now that we have function return type inference (CLJS-1997), we can see that functions like namespace return strings. Additionally, we can easily make it so that expressions involving str are inferred as returning strings. With those two in place, with a mild extension to CLJS-2314, we can eliminate unnecessary string coercions in str macro expansions where arguments are inferred to be strings.

This leads to performance gains which are akin to checked-if elimination (but perhaps even greater):

(defn foo [x y]
  (str (+ x y)))

(simple-benchmark [x :foo/bar] (str (namespace x) "-" (foo 3 2)) 10000000)

Yields these speedups under :advanced

V8: 3.2
SpiderMonkey: 1.3
JavaScriptCore: 4.2
Nashorn: 2.5
ChakraCore: 3.2
GraalVM: 1.3

This could be great for UIs that are heavy in string-concatenation, as well as for the self-hosted compiler.

Details:

Before:

Benchmarking with V8
[x :foo/bar], (str (namespace x) "-" (foo 3 2)), 10000000 runs, 1686 msecs
Benchmarking with SpiderMonkey
[x :foo/bar], (str (namespace x) "-" (foo 3 2)), 10000000 runs, 1845 msecs
Benchmarking with JavaScriptCore
[x :foo/bar], (str (namespace x) "-" (foo 3 2)), 10000000 runs, 7973 msecs
Benchmarking with Nashorn
[x :foo/bar], (str (namespace x) "-" (foo 3 2)), 10000000 runs, 37874 msecs
Benchmarking with ChakraCore
[x :foo/bar], (str (namespace x) "-" (foo 3 2)), 10000000 runs, 4899 msecs
Benchmarking with GraalVM
[x :foo/bar], (str (namespace x) "-" (foo 3 2)), 10000000 runs, 955 msecs

After:

Benchmarking with V8
[x :foo/bar], (str (namespace x) "-" (foo 3 2)), 10000000 runs, 518 msecs
Benchmarking with SpiderMonkey
[x :foo/bar], (str (namespace x) "-" (foo 3 2)), 10000000 runs, 1340 msecs
Benchmarking with JavaScriptCore
[x :foo/bar], (str (namespace x) "-" (foo 3 2)), 10000000 runs, 1859 msecs
Benchmarking with Nashorn
[x :foo/bar], (str (namespace x) "-" (foo 3 2)), 10000000 runs, 15276 msecs
Benchmarking with ChakraCore
[x :foo/bar], (str (namespace x) "-" (foo 3 2)), 10000000 runs, 1496 msecs
Benchmarking with GraalVM
[x :foo/bar], (str (namespace x) "-" (foo 3 2)), 10000000 runs, 683 msecs


 Comments   
Comment by Mike Fikes [ 20/Aug/18 5:43 AM ]

Assigning back to me to look into handling elimination of coercions for nil. This is also feasible (as js/undefined also works out), and would handle the important pattern of

(str "x" (when test "y"))
Comment by Mike Fikes [ 20/Aug/18 9:02 AM ]

CLJS-2865-2.patch adds support for avoiding coercion for inferred clj-nil, along with some tests. (This patch also passes all the Canary tests.)

Comment by Mike Fikes [ 04/Sep/18 8:47 PM ]

CLJS-2865-3.patch is the same as CLJS-2865-2.patch except that it uses the parameter name form in a few places where ast was previously used.





[CLJS-2885] DCE-friendly cljs.pprint Created: 04/Sep/18  Updated: 04/Sep/18

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Mike Fikes Assignee: Mike Fikes
Resolution: Unresolved Votes: 1
Labels: None

Attachments: Text File CLJS-2885.patch    
Patch: Code

 Description   

If you simply require cljs.pprint in your code, then you end up with an extra 30 KB of JavaScript after Closure processing and typical gzip compression.

This ticket seeks to see if there is anything we can do to make this namespace more DCE-friendly.



 Comments   
Comment by Mike Fikes [ 04/Sep/18 11:32 AM ]

The attached patch essentially eliminates all code in the case that cljs.pprint is required but never used. (Some small amount of code remains owing to the need to preserve some public defmulti s.) But, as soon as something like pprint or cl-format is used, most of the code is brought back in as non-dead.

In terms of runtime performance, at the REPL, pprint ing

{:a 2, :b [1 2 3], :c {:x 1, :y :kw}, :d [1 :ab {:x 1}], :e "string"}

is about 10% faster, perhaps owing to the use of case constructs over defmulti. The same is about 20% faster under :advanced. (Perhaps the revisions are amenable to Closure optimization.)





[CLJS-2820] cljs.loader not found when running cljs.main with these options Created: 13/Jul/18  Updated: 03/Sep/18

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Khalid Jebbari Assignee: Khalid Jebbari
Resolution: Unresolved Votes: 0
Labels: None
Environment:

Clojure 1.9.0
ClojureScript 1.10.339
Linux
Java 1.8



 Description   
deps.edn
{:deps {org.clojure/clojurescript {:mvn/version "1.10.339"}}
 :aliases {:bug {:main-opts ["-m" "cljs.main" "-co" "bug.cljs.edn" "--repl"]}}}
bug.cljs.edn
{:main          bug.core
 :output-dir    "target/js"
 :optimizations :none
 :libs          ["bug/jslib.js"]
 :modules       {:cljs-base {:output-to "target/js/bug.js"}
                 :lazy-loaded {:entries #{bug.wrapper}
                               :output-to "target/js/bug-wrapper.js"}}}
src/bug/core.cljs
(ns bug.core
  (:require [bug.loader :as bug]))

(enable-console-print!)

(bug/load)
src/bug/jslib.js
goog.provide("bug.jslib");

bug.jslib.start = function() {
    console.log("I've been lazy-loaded successfully!")
};
src/bug/loader.cljs
(ns bug.loader
  (:require [cljs.loader :as loader]))

(defn load []
      (loader/load :lazy-loaded
                   (fn []
                       ((resolve 'bug.wrapper/start)))))
src/bug/wrapper.cljs
(ns bug.wrapper
  (:require [bug.jslib :as jslib]
            [cljs.loader :as loader]))

(enable-console-print!)

(defn start []
      (jslib/start))

(loader/set-loaded! :lazy-loaded)

Repro:

$ clj -A:bug
java.io.FileNotFoundException: /private/tmp/cljs-2820/target/js/cljs/loader.js (No such file or directory)
	at java.io.FileInputStream.open0(Native Method)
	at java.io.FileInputStream.open(FileInputStream.java:195)
	at java.io.FileInputStream.<init>(FileInputStream.java:138)
	at clojure.java.io$fn__10972.invokeStatic(io.clj:238)
	at clojure.java.io$fn__10972.invoke(io.clj:235)
	at clojure.java.io$fn__10881$G__10874__10888.invoke(io.clj:69)
	at clojure.java.io$fn__10942.invokeStatic(io.clj:165)
	at clojure.java.io$fn__10942.invoke(io.clj:165)
	at clojure.java.io$fn__10894$G__10870__10901.invoke(io.clj:69)
	at clojure.java.io$reader.invokeStatic(io.clj:102)
	at clojure.java.io$reader.doInvoke(io.clj:86)
	at clojure.lang.RestFn.invoke(RestFn.java:410)
	at cljs.repl$load_sources.invokeStatic(repl.cljc:217)
	at cljs.repl$load_sources.invoke(repl.cljc:209)
	at cljs.repl$load_namespace.invokeStatic(repl.cljc:251)
	at cljs.repl$load_namespace.invoke(repl.cljc:233)
	at cljs.repl$load_dependencies$fn__6358.invoke(repl.cljc:259)
	at clojure.core$map$fn__5587.invoke(core.clj:2747)
	at clojure.lang.LazySeq.sval(LazySeq.java:40)
	at clojure.lang.LazySeq.seq(LazySeq.java:49)
	at clojure.lang.RT.seq(RT.java:528)
	at clojure.core$seq__5124.invokeStatic(core.clj:137)
	at clojure.core$apply.invokeStatic(core.clj:652)
	at clojure.core$mapcat.invokeStatic(core.clj:2775)
	at clojure.core$mapcat.doInvoke(core.clj:2775)
	at clojure.lang.RestFn.invoke(RestFn.java:423)
	at cljs.repl$load_dependencies.invokeStatic(repl.cljc:259)
	at cljs.repl$load_dependencies.invoke(repl.cljc:254)
	at cljs.repl$evaluate_form.invokeStatic(repl.cljc:561)
	at cljs.repl$evaluate_form.invoke(repl.cljc:498)
	at cljs.repl$repl_STAR_$fn__6610.invoke(repl.cljc:950)
	at cljs.repl$repl_STAR_$fn__6621$fn__6622.invoke(repl.cljc:987)
	at cljs.repl$repl_STAR_$fn__6621.invoke(repl.cljc:982)
	at cljs.compiler$with_core_cljs.invokeStatic(compiler.cljc:1289)
	at cljs.compiler$with_core_cljs.invoke(compiler.cljc:1278)
	at cljs.repl$repl_STAR_.invokeStatic(repl.cljc:979)
	at cljs.repl$repl_STAR_.invoke(repl.cljc:855)
	at cljs.cli$repl_opt.invokeStatic(cli.clj:305)
	at cljs.cli$repl_opt.invoke(cli.clj:292)
	at cljs.cli$main.invokeStatic(cli.clj:636)
	at cljs.cli$main.doInvoke(cli.clj:625)
	at clojure.lang.RestFn.applyTo(RestFn.java:139)
	at clojure.core$apply.invokeStatic(core.clj:659)
	at clojure.core$apply.invoke(core.clj:652)
	at cljs.main$_main.invokeStatic(main.clj:61)
	at cljs.main$_main.doInvoke(main.clj:52)
	at clojure.lang.RestFn.applyTo(RestFn.java:137)
	at clojure.lang.Var.applyTo(Var.java:702)
	at clojure.core$apply.invokeStatic(core.clj:657)
	at clojure.main$main_opt.invokeStatic(main.clj:317)
	at clojure.main$main_opt.invoke(main.clj:313)
	at clojure.main$main.invokeStatic(main.clj:424)
	at clojure.main$main.doInvoke(main.clj:387)
	at clojure.lang.RestFn.applyTo(RestFn.java:137)
	at clojure.lang.Var.applyTo(Var.java:702)
	at clojure.main.main(main.java:37)
ClojureScript 1.10.339
cljs.user=>

Indeed when I go into target/js/cljs I see loader.cljs but not loader.js. It seems like it's not compiled to js for a reason I can't understand.

In the REPL when I try to

(require '[trackers.core :as t])
I get the exact same error.



 Comments   
Comment by David Nolen [ 14/Jul/18 6:19 PM ]

Any chance we can get something minimal without all these outside file references we won't have access to? Thanks.

Comment by Khalid Jebbari [ 16/Jul/18 4:26 AM ]

Here you go https://github.com/DjebbZ/bug-cljs-2820
Bare minimum and bug happens. All instructions in the README.

Comment by Mike Fikes [ 16/Jul/18 7:40 AM ]

Updated description with copy of minimal repro (so no GitHub reference necessary).

Comment by Khalid Jebbari [ 21/Jul/18 3:11 PM ]

Thank you Mike, sorry for not having inlined everything necessary from the start.

Comment by Khalid Jebbari [ 03/Sep/18 11:41 AM ]

It doesn't seem related to the :libs option at all, since I tried another variant with no Javascript file. The only changes necessary to the repro case above are:

1. Remove the :libs option from bug.cljs.edn

bug.cljs.edn
{:main          bug.core
 :output-dir    "target/js"
 :optimizations :none
 :modules       {:cljs-base {:output-to "target/js/bug.js"}
                 :lazy-loaded {:entries #{bug.wrapper}
                               :output-to "target/js/bug-wrapper.js"}}}

2. Change bug.wrapper.cljs so that it doesn't call the js namespace but directly prints to the console

bug.wrapper.cljs
(ns bug.wrapper
  (:require [cljs.loader :as loader]))

(enable-console-print!)

(defn start []
      (println "I, cljs, have been lazy-loaded successfully"))

(loader/set-loaded! :lazy-loaded)

I did run rm -rf target/ .cpcache/ before re-running clj -A:bug. Hence, unless I misunderstood something, I think code-splitting is broken?

Comment by Khalid Jebbari [ 03/Sep/18 11:43 AM ]

Sorry I did not specify the output with the changes: it's exactly the same as before (no loader.js file)





[CLJS-2882] Self-hosted: Optimize string printing during compilation Created: 02/Sep/18  Updated: 02/Sep/18

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: bootstrap

Attachments: Text File CLJS-2882.patch    
Patch: Code

 Description   

Optimize string printing during compilation.

Like https://github.com/clojure/clojurescript/commit/f8b4125cbef671143b241881afdfc0195cf36480 but for self-hosted ClojureScript, directly using *print-fn*.

This leads to a speedup of 1.03 downstream in Planck when compiling Andare.






[CLJS-2881] cl-format character directive with \space fails Created: 02/Sep/18  Updated: 02/Sep/18

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File CLJS-2881.patch    
Patch: Code and Test

 Description   

Clojure:

user=> (cl-format nil "~@c" \space)
"\\space"

ClojureScript:

cljs.user=> (cl-format nil "~@c" \space)
"\\ "

See https://github.com/clojure/clojure/blob/master/doc/clojure/pprint/CommonLispFormat.markdown#differences-from-the-common-lisp-format-function



 Comments   
Comment by Mike Fikes [ 02/Sep/18 11:35 AM ]

The broken fn, print-char is only used in the case when the character directive is used, so adding handling for \space in that function shouldn't be capable of breaking anything else in the cl-format code.





[CLJS-2880] cl-format octal and Unicode character directives fail Created: 02/Sep/18  Updated: 02/Sep/18

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File CLJS-2880.patch    
Patch: Code and Test

 Description   

Using octal and Unicode cl-format character directives fails with "Error: Badly formed parameters in format directive".

(See https://github.com/clojure/clojure/blob/master/doc/clojure/pprint/CommonLispFormat.markdown#differences-from-the-common-lisp-format-function)

(cl-format nil "~'o@c" \a)
(cl-format nil "~'u@c" \a)


 Comments   
Comment by Mike Fikes [ 02/Sep/18 9:59 AM ]

This is a cut-n-dry defect involving extra spaces in the port here:
https://github.com/clojure/clojurescript/blob/a93ac962c97845a07aa64cbfe95abcca63b5ae9b/src/main/cljs/cljs/pprint.cljs#L1364-L1365

Compare with original Clojure implementation:
https://github.com/clojure/clojure/blob/4ef4b1ed7a2e8bb0aaaacfb0942729252c2c3091/src/clj/clojure/pprint/cl_format.clj#L507-L508





[CLJS-1963] cljs.analyzer/load-core is called for every analyzed form Created: 01/Mar/17  Updated: 01/Sep/18  Resolved: 01/Sep/18

Status: Resolved
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Thomas Heller Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None


 Description   

In cljs.analyzer/analyze-form the load-core fn is called. load-core guards against doing its work multiple times. It then always calls (intern-macros 'cljs.core), which also checks whether it was called before. This ends up doing the checks very often. load-core should probably be called in a less frequent manner.

Performance impact is very minimal but I did a quick test in my work project and load-core is called 416671 times there (without cache) when 1 would be enough.



 Comments   
Comment by Mike Fikes [ 25/Jun/17 4:22 PM ]

Previously, it used to not always call (intern-macros 'cljs.core), with this changing with this commit: https://github.com/clojure/clojurescript/commit/7025bd212fb925cb90db680aa7a5eb3f4c0de4bb

Comment by Thomas Heller [ 01/Sep/18 4:28 PM ]

https://github.com/clojure/clojurescript/commit/4f3707624846a2cf0345859e41370ec172da73c4





[CLJS-2879] Close analysis cache file Created: 01/Sep/18  Updated: 01/Sep/18

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File CLJS-2879.patch    
Patch: Code

 Description   

The FileOutputStream opened to write analysis cache is left open: https://github.com/clojure/clojurescript/blob/master/src/main/clojure/cljs/analyzer.cljc#L4266






[CLJS-2877] Use SourceFile/fromPath instead of SourceFile/fromFile Created: 30/Aug/18  Updated: 31/Aug/18  Resolved: 31/Aug/18

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Mike Fikes Assignee: Mike Fikes
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-2877.patch    
Patch: Code
Approval: Accepted

 Description   

A deprecated Closure method com.google.javascript.jscomp.SourceFile.fromFile is called here

https://github.com/clojure/clojurescript/blob/04e6bd9073daa905543d7decab95a2252c2e53e2/src/main/clojure/cljs/closure.clj#L117

which was recently removed on Closure master with

https://github.com/google/closure-compiler/commit/09128dd331a16f1b5537178cf1031e7e54fd414f



 Comments   
Comment by Mike Fikes [ 30/Aug/18 9:35 PM ]

This patch essentially does what the deprecated Closure forwarding code did, but skipping the internal use of their builder class, directly calling the method recommended in the deprecation notice.

Comment by Mike Fikes [ 31/Aug/18 1:28 PM ]

fixed https://github.com/clojure/clojurescript/commit/d2e4424cec693b59af67051aebefca971a477567





[CLJS-2872] Need reader conditional with emits micro-opt Created: 25/Aug/18  Updated: 31/Aug/18  Resolved: 31/Aug/18

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Mike Fikes Assignee: Mike Fikes
Resolution: Completed Votes: 0
Labels: bootstrap

Attachments: Text File CLJS-2872.patch    
Patch: Code
Approval: Accepted

 Description   

https://github.com/mfikes/clojurescript/commit/f8b4125cbef671143b241881afdfc0195cf36480 breaks self-hosted



 Comments   
Comment by Mike Fikes [ 31/Aug/18 1:26 PM ]

fixed https://github.com/clojure/clojurescript/commit/de3a60384933097e7eedff185a12dc6d1b10d00e





[CLJS-2876] Warn on vacuous if tests Created: 30/Aug/18  Updated: 30/Aug/18

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Mike Fikes Assignee: Mike Fikes
Resolution: Unresolved Votes: 0
Labels: None


 Description   

If the test of an if has a tag of clj-nil then the else branch will always be taken. If this is the result of something that is not the constant nil, we can warn.

Likewise if the test of an if has a tag that cannot yield a falsey value (for example #{number string}), then the then branch will always be taken and we can likewise warn that the test is vacuous.






[CLJS-2875] Eliminate dead code for clj-nil and types that don't admit falsey Created: 30/Aug/18  Updated: 30/Aug/18

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Mike Fikes Assignee: Mike Fikes
Resolution: Unresolved Votes: 0
Labels: None


 Description   

If the test of an if has a tag of clj-nil then the else branch will always be taken so we can replace such code with the else branch.

Likewise if the test of an if has a tag that cannot yield a falsey value (for example #{number string}), then the then branch will always be taken and we can replace such code with the then branch.

We already have dead code elimination for constants, this ticket asks that we extend it to cases where we can definitely determine a branch will be taken based on inferred types.






[CLJS-2874] Eliminate checked if for clj-nil Created: 30/Aug/18  Updated: 30/Aug/18

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Mike Fikes Assignee: Mike Fikes
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Currently seq and boolean tags are considered "safe" for elimination of checked if.

Consider this code:

(defn foo [x] (if x true nil))

(defn bar [x] (if (foo x) 1 2))

In this case foo is inferred as #{boolean clj-nil}, but bar will involve a call to cljs.core.truth_. It is safe to eliminate the checked if in this case as clj-nil will be interpreted by JavaScript correctly as false.

Note: CLJS-2869 has some utility code that makes this ticket easier to implement.






[CLJS-2871] Check for redundant nil? checks Created: 23/Aug/18  Updated: 23/Aug/18

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Mike Fikes Assignee: Mike Fikes
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Type inference is improving. We can add a check to see if nil? is being applied to an expression of type clj-nil and warn, and likewise we can check if it is being applied to a type that cannot be clj-nil and also warn.

(One thing that could preclude this sort of thing is nil checks in tests.)






[CLJS-2870] Wrong provides/requires for ES6 modules with index.js Created: 21/Aug/18  Updated: 21/Aug/18

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Gary Verhaegen Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

macOS 10.13.6, java 1.8.0_161



 Description   

This is related to CLJS-2205 and CLJS-2836, though I believe it is slightly different and I have additional (hopefully useful) information to provide.

The "semantic-ui-react" node module, when used as an npm-deps, produces code that fails to load. Minimal reproducing case I could come up with:

build_opts.edn
{:npm-deps {:react "16.2.0"
            :react-dom "16.2.0"
            :semantic-ui-react "0.82.2"}
 :install-deps true}
deps.edn
{:deps {org.clojure/clojurescript {:mvn/version "1.10.339"}}}

Note: I see exactly the same issue with current master, which at the time of writing is commit 81a1ea12 installing locally as 1.10.394.

src/hello_world/core.cljs
(ns hello-world.core
  (:require semantic-ui-react))
 
(println "Hello world!")

Files organized as:

$ tree
.
├── build_opts.edn
├── deps.edn
└── src
    └── hello_world
        └── core.cljs

Running:

clj -m cljs.main -co build_opts.edn -c hello-world.core

produces code that fails to load in the following way. First, modules that have an index.js file seem to be missing a provide statement; for example:

out/node_modules/semantic-ui-react/dist/es/addons/Confirm/index.js
goog.provide("module$Users$gary$sparrho$cljs_bug_report$node_modules$semantic_ui_react$dist$es$addons$Confirm$index");
goog.provide("module$semantic_ui_react$dist$es$addons$Confirm$index");
goog.provide("module$semantic_ui_react$dist$es$addons$Confirm");
goog.require("module$Users$gary$sparrho$cljs_bug_report$node_modules$semantic_ui_react$dist$es$addons$Confirm$Confirm");
var module$Users$gary$sparrho$cljs_bug_report$node_modules$semantic_ui_react$dist$es$addons$Confirm$index={get default(){return module$Users$gary$sparrho$cljs_bug_report$node_modules$semantic_ui_react$dist$es$addons$Confirm$Confirm["default"]}}

gets required elsewhere as the "full path" without the final "index":

out/node_modules/semantic-ui-react/dist/es/index.js (elided)
...
goog.require("module$Users$gary$sparrho$cljs_bug_report$node_modules$semantic_ui_react$dist$es$addons$Confirm");
...

which produces an error saying that nameToPath cannot find the module "module$Users$gary$sparrho$cljs_bug_report$node_modules$semantic_ui_react$dist$es$addons$Confirm". I managed to fix that by patching the ClojureScript compiler (adding the additional goog.provides to cljs.closure/add-converted-source if the current source ends in index, mirroring the patch in CLJS-2205), only to find out that there is a second problem (which makes me doubt my approach is the proper way to fix this) in that the code produced actually then goes and tries to use the module with its full name, including the index part:

out/node_modules/semantic-ui-react/dist/es/index.js (elided)
...
goog.require("module$Users$gary$sparrho$cljs_bug_report$node_modules$semantic_ui_react$dist$es$addons$Confirm");
...
var module$Users$gary$sparrho$cljs_bug_report$node_modules$semantic_ui_react$dist$es$index={get Confirm(){return module$Users$gary$sparrho$cljs_bug_report$node_modules$semantic_ui_react$dist$es$addons$Confirm$index["default"]},
...

which throws an error saying that the variable module$Users$gary$sparrho$cljs_bug_report$node_modules$semantic_ui_react$dist$es$addons$Confirm$index is not defined.

I am completely unfamiliar with all of the ClojureScript compiler, the Google Closure compiler or the ES6 module system, which makes it very hard for me to investigate further. I have verified that at the point of generating the requires and provides calls (cljs.closure/add-converted-source) the Closure object representing the dependencies does not contain the index part at the end, which is consistent with the goog.requires calls that are generated. The goog.provides calls seem to be generated independently of the Closure compiler, based on analysis of the js files done in src/main/cljs/cljs/module_deps.js. What I cannot figure out (and therefore where I am blocked) is what piece of code generates the JS code that tries to access the "wrong" variable name. Maybe this is a bug in ClojureScript not generating the right goog.requires calls, or maybe this is a bug in Closure not using the same name for the JS variables as the symbols it generates on its Require objects.






[CLJS-2864] Optimize str macro for single arity case Created: 18/Aug/18  Updated: 18/Aug/18

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Unresolved Votes: 1
Labels: None

Attachments: Text File CLJS-2864.patch    
Patch: Code and Test

 Description   

The array construction and join can be avoided for the single arity case in the str macro and this leads to a speedup of roughly 2 for the :none case. In :advanced you can see that Closure doesn't elide the array / join, but the perf difference is negligible, so this optimization would largely be for self-hosted environments (both for the self-hosted compiler itself, where there are many single-arity str calls, and for end-user code), or perhaps environments where people are running scripts via cljs.main.

Before:

Node:

cljs.user=> (time (dotimes [_ 10000000] (str 1)))
"Elapsed time: 556.863925 msecs"

Safari:

cljs.user=> (time (dotimes [_ 10000000] (str 1)))
"Elapsed time: 3530.000000 msecs"

After:

Node:

cljs.user=> (time (dotimes [_ 10000000] (str 1)))
"Elapsed time: 285.108083 msecs"

Safari:

cljs.user=> (time (dotimes [_ 10000000] (str 1)))
"Elapsed time: 1765.000000 msecs"





[CLJS-2863] Need to reject incorrect fn with fixed arity of more params than variadic Created: 18/Aug/18  Updated: 18/Aug/18

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Mike Fikes Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   
ClojureScript 1.10.339
cljs.user=> (defn foo ([x] [:single x]) ([& xs] [:variadic xs]))
#'cljs.user/foo
cljs.user=> (foo 1)
[:single 1]
cljs.user=> (foo 1 2)
[:variadic 1]

Note that the variadic call provides the wrong answer.

It appears that the right thing to do is not "make it work" but to reject the code as incorrect. See Clojure:

user=> (defn foo ([x] [:single x]) ([& xs] [:variadic xs]))
CompilerException java.lang.RuntimeException: Can't have fixed arity function with more params than variadic function, compiling:(NO_SOURCE_PATH:1:1)





[CLJS-1960] Require CommonJS modules directly from a ClojureScript namespace Created: 27/Feb/17  Updated: 16/Aug/18  Resolved: 16/Aug/18

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: 1.9.655

Type: Enhancement Priority: Minor
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Completed Votes: 3
Labels: None

Attachments: Text File CLJS-1960.patch    
Patch: Code

 Comments   
Comment by Andrea Richiardi [ 27/Feb/17 11:34 PM ]

I will comment again after I'll try the patch, but this is very good stuff. One thing I was pondering about is the name of the option. It could be more generic, for instance :js-deps, so that we kind of abstract from the package manager. I know I know everybody knows what npm is...Anyways, my two cents.

Comment by Andrea Richiardi [ 01/Mar/17 5:09 PM ]

Ok I tried the new patch and I detected that it basically throws (sorry it is a bit verbose):

Error: Cannot find module '/home/arichiardi/git/rest-resources-viz/home/arichiardi/.boot/cache/tmp/glu/-d8mh6z/main.out/cljs$node_modules.js' from '/home/arichiardi/git/rest-resources-viz'

But it looks like the file is there, and if I escape the $ with a \$, the file can be read

Comment by David Nolen [ 10/Mar/17 1:09 PM ]

fixed https://github.com/clojure/clojurescript/commit/777d41b9b6fe83c3d29fc51ee3ddbdfeff4f803b





[CLJS-2457] For non-seqable types, seq evals to object info when running tests Created: 03/Jan/18  Updated: 16/Aug/18  Resolved: 16/Aug/18

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: test

Attachments: Text File CLJS-2457.patch    
Patch: Code and Test
Approval: Accepted

 Description   

Observing on master and 1.9.946, and as far back as 1.7.28:

(require '[clojure.test :refer [deftest]])

(deftype FooSeq [])

(deftype BarSeq []
  ASeq)

(deftype BazSeq []
  ICounted (-count [_] 0)
  ISequential)

(defprotocol IFooSeq)

(deftype QuuxSeq []
  IFooSeq)

(deftest test-seq
  (prn (seqable? (->FooSeq)))
  (prn (seq (->FooSeq)))
  (prn (seqable? (->BarSeq)))
  (prn (seq (->BarSeq)))
  (prn (seqable? (->BazSeq)))
  (prn (seq (->BazSeq)))
  (prn (seqable? (->QuuxSeq)))
  (prn (seq (->QuuxSeq))))

If you do this in a REPL and call (test-seq) you will get the second test form throwing as expected:

cljs.user=> (test-seq)
false

ERROR in (test-seq) (Error:NaN:NaN)
Uncaught exception, not in assertion.
expected: nil
  actual: #object[Error Error: [object Object] is not ISeqable]
nil

Note that this also behaves properly if you are in a REPL and put this code in a namespace and load it via require while in the REPL:

Have src/foo/core.cljs containing

(ns foo.core
 (:require [clojure.test :refer [deftest]]))

(deftype FooSeq [])

(deftype BarSeq []
  ASeq)

(deftype BazSeq []
  ICounted (-count [_] 0)
  ISequential)

(defprotocol IFooSeq)

(deftype QuuxSeq []
  IFooSeq)

(deftest test-seq
  (prn (seqable? (->FooSeq)))
  (prn (seq (->FooSeq)))
  (prn (seqable? (->BarSeq)))
  (prn (seq (->BarSeq)))
  (prn (seqable? (->BazSeq)))
  (prn (seq (->BazSeq)))
  (prn (seqable? (->QuuxSeq)))
  (prn (seq (->QuuxSeq))))

and then try it at the REPL:

$ java -cp cljs.jar:src clojure.main -m cljs.repl.node
ClojureScript Node.js REPL server listening on 51614
To quit, type: :cljs/quit
cljs.user=> (require 'foo.core)
nil
cljs.user=> (foo.core/test-seq)
false

ERROR in (test-seq) (Error:NaN:NaN)
Uncaught exception, not in assertion.
expected: nil
  actual: #object[Error Error: [object Object] is not ISeqable]
nil

If instead you drop the code (apart from the require form) into the bottom of the cljs.core-test namespace (or, the top of cljs.primitives-test) and run script/test-simple or script/test-self-parity you will see some concerning output. Also note the odd true and false return values.

true
()
false
(["cljs$lang$protocol_mask$partition0$" 32] ["cljs$lang$protocol_mask$partition1$" 0])
false
(["cljs$lang$protocol_mask$partition0$" 16777218] ["cljs$lang$protocol_mask$partition1$" 0] ["cljs$core$ICounted$_count$arity$1" #object[Function]])
true
(["cljs$core_test$IFooSeq$" #js {}])

A simpler case is (prn (seq #js {:a 1 :b 2})) which will print

(["a" 1] ["b" 2])
.



 Comments   
Comment by Mike Fikes [ 03/Jan/18 2:27 PM ]

Root cause is this test intermixing with other things https://github.com/clojure/clojurescript/commit/009db3c7dc45d2a99afd5d89720bca7d2047219d

The reason I wrote this ticket was a result of one assertion in CLJS-2455 failing.

Comment by Mike Fikes [ 03/Jan/18 3:18 PM ]

Attached patch moves the manipulation of object to a separate namespace that is loaded last, and makes the manipulation occur at runtime.

Comment by David Nolen [ 03/Jan/18 8:22 PM ]

fixed https://github.com/clojure/clojurescript/commit/9f04cb2abec3d6425b35c001d6cd311119570a17





[CLJS-2861] Self-host: :checked-arrays not working Created: 14/Aug/18  Updated: 16/Aug/18  Resolved: 16/Aug/18

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Mike Fikes Assignee: Mike Fikes
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-2861.patch    
Patch: Code and Test
Approval: Accepted

 Description   

Working with 1.10.238:

$ clj -Sdeps '{:deps {org.clojure/clojurescript {:mvn/version "1.10.238"}}}' -m cljs.main -re node -r
ClojureScript 1.10.238
cljs.user=> (require 'cljs.js)
nil
cljs.user=> (cljs.js/eval-str (cljs.js/empty-state) "(aget #js {:foo 1} \"foo\")" nil 
{:eval cljs.js/js-eval :context :expr :checked-arrays :warn} prn)
WARNING: cljs.core/aget, arguments must be an array followed by numeric indices, got [object string] instead (consider goog.object/get for object access) at line 1
Error: Assert failed: (or (array? array) (js/goog.isArrayLike array))
    ...
{:ns cljs.user, :value 1}
nil

Regressed with 1.10.339:

$ clj -Sdeps '{:deps {org.clojure/clojurescript {:mvn/version "1.10.339"}}}' -m cljs.main -re node -r
ClojureScript 1.10.339
cljs.user=> (require 'cljs.js)
nil
cljs.user=> (cljs.js/eval-str (cljs.js/empty-state) "(aget #js {:foo 1} \"foo\")" nil 
{:eval cljs.js/js-eval :context :expr :checked-arrays :warn} prn)
{:ns cljs.user, :value 1}
nil


 Comments   
Comment by Mike Fikes [ 14/Aug/18 6:11 PM ]

regressed with https://github.com/clojure/clojurescript/commit/c37442ce331431a8c6203e29556fa949ad78dfc7

Comment by Mike Fikes [ 14/Aug/18 8:05 PM ]

A typo causes (set! *unchecked-arrays* true) to not be treated purely as an intrisic, emitting this code in cljs/core.js
where the first assignment to _STAR_unchecked_arrays_STAR_ is expected (associated with the def) and the second is associated with the
(set! *unchecked-arrays* true) instruction to turn off checked arrays for the cljs.core namespace (which should not have emitted any code).

cljs.core._STAR_clojurescript_version_STAR_ = "1.10.339";
cljs.core._STAR_unchecked_if_STAR_ = false;
cljs.core._STAR_unchecked_arrays_STAR_ = false;
cljs.core._STAR_warn_on_infer_STAR_ = false;
cljs.core._STAR_unchecked_arrays_STAR_ = true;

A consequence is that *unchecked-arrays* is left as true, and in self-hosted ClojureScript this essentially turns
checked arrays off (until some non-core namespace is loaded by self-hosted ClojureScript, which, as a post-load side-effect
resets the var to false per its file-scope semantics).

Comment by Mike Fikes [ 16/Aug/18 7:47 PM ]

fixed https://github.com/clojure/clojurescript/commit/81a1ea127974d43a6166fbdae33bcaa296fe9156





[CLJS-2862] Externs inference warning when extending Object Created: 16/Aug/18  Updated: 16/Aug/18

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Thomas Heller Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None


 Description   
(ns demo.app)

(set! *warn-on-infer* true)

(deftype Foo []
  Object
  (bar [this] :bar))

produces an inference warning when compiling.

WARNING: Cannot infer target type in expression (. (. Foo -prototype) -bar) at line 5 .../src/demo/app.cljs

Instead it should collect the bar property and generate externs for it since it is very likely that extending Object will be used to implement JS interfaces (eg. React lifecycle methods).






[CLJS-2718] Setting *warn-on-infer* in REPL throws a SyntaxError Created: 29/Mar/18  Updated: 14/Aug/18  Resolved: 29/Mar/18

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Critical
Reporter: Miikka Koskinen Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None
Environment:

ClojureScript 1.10.238



 Description   
% clj --main cljs.main 
ClojureScript 1.10.238
cljs.user=> (set! *warn-on-infer* true)
SyntaxError: Unexpected token ';'
	 (<NO_SOURCE_FILE>)
	 clojure$browser$repl$evaluate_javascript (clojure/browser/repl.cljs:118:4)
	 (goog/messaging/abstractchannel.js:141:21)
	 (goog/net/xpc/crosspagechannel.js:734:19)
	 (goog/net/xpc/nativemessagingtransport.js:321:23)
	 (goog/events/events.js:744:25)
	 (goog/events/events.js:870:34)


 Comments   
Comment by David Nolen [ 29/Mar/18 8:22 PM ]

fixed https://github.com/clojure/clojurescript/commit/c37442ce331431a8c6203e29556fa949ad78dfc7

Comment by Mike Fikes [ 14/Aug/18 6:12 PM ]

see regression CLJS-2718





[CLJS-2860] Suppress warnings for qualified references to transitively loaded Closure libs Created: 11/Aug/18  Updated: 11/Aug/18

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Mike Fikes Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

{:deps {org.clojure/clojurescript {:mvn/version "1.10.339"}}}



 Description   

This ticket asks that analyzer warnings be suppressed when using transitively loaded Closure libs via qualified references.

co.edn
{:libs ["libs/mylib.js"]}
libs/mylib.js
goog.provide('my_lib.core')

my_lib.core = {
  add: function(x, y) { return x + y; }
}
src/foo/core.cljs
(ns foo.core
 (:require my-lib.core
           clojure.set))

Here is an example illustrating the concept:

$ clj -m cljs.main -co co.edn -re node -r
ClojureScript 1.10.339
cljs.user=> (clojure.set/intersection #{1 2 3} #{2 4 5})
WARNING: Use of undeclared Var clojure.set/intersection at line 1 <cljs repl>
...
cljs.user=> (require 'foo.core)
nil
cljs.user=> (clojure.set/intersection #{1 2 3} #{2 4 5})
#{2}
cljs.user=> (clojure.set/intersection)
WARNING: Wrong number of args (0) passed to clojure.set/intersection at line 1 <cljs repl>
cljs.user=> (my-lib.core/add 2 3)
WARNING: No such namespace: my-lib.core, could not locate my_lib/core.cljs, my_lib/core.cljc, or JavaScript source providing "my-lib.core" (Please check that namespaces with dashes use underscores in the ClojureScript file name) at line 1 <cljs repl>
WARNING: Use of undeclared Var my-lib.core/add at line 1 <cljs repl>
5
cljs.user=> (in-ns 'foo.core)
nil
foo.core=> (my-lib.core/add 2 3)
5

In the above, you can see that when a transitively loaded ClojureScript namespace (in this example clojure.set) is used via qualified symbols, the analyzer works properly.

This characteristic evidently doesn't currently apply to Closure libs, so (correctly functioning) source code would need to be revised in this example simply in order to avoid analyzer warnings (and the source code would need to be aware that a namespace is implemented by a Closure lib instead of a ClojureScript namespace).

The argument for this enhancement is that it leads to consistency and make it possible to cleanly use Closure libs in valid scenarios where transitive dependencies are used. (Normally it is a bad idea to not directly indicate your dependencies in a given namespace, but an arguably copacetic use is when you are using a namespace that exposes macros that expand to use code that the namespace transitively loads. This could occur, for example with macros that want to use libs defined via :libs or even libs in Closure Library.)






[CLJS-2859] Graal.JS: Enable high-res timers by default, allow user-configuration Created: 10/Aug/18  Updated: 10/Aug/18  Resolved: 10/Aug/18

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Mike Fikes Assignee: Mike Fikes
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-2859.patch    
Patch: Code
Approval: Accepted

 Description   

Enable the high-resolution timers by default in the graaljs REPL environment, and allow users to override or otherwise configure options via --repl-opt.



 Comments   
Comment by Mike Fikes [ 10/Aug/18 8:31 AM ]

With the patch:

$ clj -m cljs.main -re graaljs -r
cljs.user=> (time (count (filter odd? (range 1e6))))
"Elapsed time: 2941.132765 msecs"
500000

Disabling it via -ro:

$ clj -m cljs.main -re graaljs -ro '{"js.precise-time" "false"}' -r
cljs.user=> (time (count (filter odd? (range 1e6))))
"Elapsed time: 2378.000000 msecs"
500000

There are a whole slew of options that the user could configure via this mechanism:

$ js --help:languages

Language options:
  JavaScript:
    --js.annex-b=<Boolean>                       Enable ECMAScript Annex B features. (default:true)
    --js.array-sort-inherited=<Boolean>          implementation-defined behavior in Array.protoype.sort: sort inherited keys? (default:true)
    --js.atomics=<Boolean>                       ES2017 Atomics (default:true)
    --js.const-as-var                            parse const declarations as a var (default:false)
    --js.direct-byte-buffer                      Use direct (off-heap) byte buffer for typed arrays. (default:false)
    --js.ecmascript-version=<Integer>            ECMAScript Version. (default:10)
    --js.function-statement-error                Treat hoistable function statements in blocks as an error (in ES5 mode) (default:false)
    --js.intl-402                                Enable ECMAScript Internationalization API (default:false)
    --js.java-package-globals=<Boolean>          provide Java package globals: Packages, java, javafx, javax, com, org, edu. (default:true)
    --js.nashorn-compat                          provide compatibility with the OpenJDK Nashorn engine (default:false)
    --js.parse-only                              Only parse source code, do not run it. (default:false)
    --js.precise-time                            High-resolution timestamps via performance.now() (default:false)
    --js.regexp-static-result=<Boolean>          provide last RegExp match in RegExp global var, e.g. RegExp.$1 (default:true)
    --js.scripting                               enable scripting features (Nashorn compatibility option) (default:false)
    --js.shared-array-buffer=<Boolean>           ES2017 SharedArrayBuffer (default:true)
    --js.shebang=<Boolean>                       support files starting with #! (default:true)
    --js.stack-trace-limit=<Integer>             number of stack frames to capture (default:10)
    --js.strict                                  run in strict mode (default:false)
    --js.syntax-extensions=<Boolean>             enable Nashorn syntax extensions (default:true)
    --js.timezone=<String>                       Set custom timezone. (default:)
    --js.v8-compat                               provide compatibility with the Google V8 engine (default:false)

An example involving js.intl-402 is in the associated site PR https://github.com/clojure/clojurescript-site/pull/256

Comment by Mike Fikes [ 10/Aug/18 5:21 PM ]

fixed https://github.com/clojure/clojurescript/commit/dab61a6f2d66a6353003724745dd55b0ef93d216





[CLJS-2738] Can't use aws-amplify or aws-sdk with npm-deps Created: 15/Apr/18  Updated: 09/Aug/18

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Quang Van Assignee: Unassigned
Resolution: Unresolved Votes: 5
Labels: npm-deps


 Description   

If I put aws-amplify in :npm-deps it seems to require aws-sdk, but when I put aws-sdk in there, it outputs this error:

Compiling build :dev to "resources/public/js/main.js" from ["src"]...
events.js:182
      throw er; // Unhandled 'error' event
      ^

Error: module not found: "fs" from file /home/quang/dev/org-re-frame/node_modules/aws-amplify/node_modules/aws-sdk/lib/util.js
    at onresolve (/home/quang/dev/org-re-frame/node_modules/@cljs-oss/module-deps/index.js:181:30)
    at onResolve (/home/quang/dev/org-re-frame/node_modules/enhanced-resolve/lib/Resolver.js:70:11)
    at innerCallback (/home/quang/dev/org-re-frame/node_modules/enhanced-resolve/lib/Resolver.js:143:22)
    at callbackWrapper (/home/quang/dev/org-re-frame/node_modules/enhanced-resolve/lib/createInnerCallback.js:10:21)
    at next (/home/quang/dev/org-re-frame/node_modules/tapable/lib/Tapable.js:249:35)
    at innerCallback (/home/quang/dev/org-re-frame/node_modules/enhanced-resolve/lib/Resolver.js:143:22)
    at callbackWrapper (/home/quang/dev/org-re-frame/node_modules/enhanced-resolve/lib/createInnerCallback.js:10:21)
    at next (/home/quang/dev/org-re-frame/node_modules/tapable/lib/Tapable.js:249:35)
    at resolver.doResolve.createInnerCallback (/home/quang/dev/org-re-frame/node_modules/enhanced-resolve/lib/DescriptionFilePlugin.js:41:24)
    at callbackWrapper (/home/quang/dev/org-re-frame/node_modules/enhanced-resolve/lib/createInnerCallback.js:10:21)


 Comments   
Comment by Jeffrey Stanton [ 26/Apr/18 2:51 PM ]

I've also run into the same issue. Here's a simple repro:

deps.edn
{:deps {org.clojure/clojurescript {:mvn/version "1.10.238"}}}
src/foo.cljs
(ns foo (:require ["aws-sdk" :as sdk]))
(defn -main [] (println "hello world"))

then run:

$ clj -m cljs.main -co '{:install-deps true, :npm-deps {:aws-sdk "2.229.1"}}' -m foo
...
Error: module not found: "fs" from file /Users/jeff/repro/node_modules/aws-sdk/lib/util.js
    at onresolve (/Users/jeff/repro/node_modules/@cljs-oss/module-deps/index.js:181:30)
    at onResolve (/Users/jeff/repro/node_modules/enhanced-resolve/lib/Resolver.js:70:11)
    at innerCallback (/Users/jeff/repro/node_modules/enhanced-resolve/lib/Resolver.js:143:22)
    at callbackWrapper (/Users/jeff/repro/node_modules/enhanced-resolve/lib/createInnerCallback.js:10:21)
    ...
Comment by Mike Fikes [ 05/May/18 7:35 PM ]

The big picture is not completely clear to me for this one, but it appears that the NPM version of the library is not intended to be used directly from the browser runtime, and instead you need to build it so that it is packaged for the browser.

The root problem is that the code is attempting to use Node's fs code, but from the browser.

You can see that the code can be compiled and executed if you target Node: If you modify Jeffrey's minimal repro to add

(set! *main-cli-fn* -main)

to the bottom of src/foo.cljs then things will compile and execute in Node:

$ clj -m cljs.main -co '{:install-deps true, :npm-deps {:aws-sdk "2.229.1"}}' -O simple -o main.js -t node -c foo
WARNING: foo is a single segment namespace at line 1 /private/tmp/aws/src/foo.cljs
$ node main.js
hello world

For the browser case, if you look at the code in node_modules/aws-sdk/lib/util.js you can see that it has conditional branches for browser use, but I suspect that this is intended to be used as described here https://github.com/aws/aws-sdk-js#in-the-browser but if you really want to use the code directly from its NPM dependency, you have to use that NPM dependency to build the JavaScript intended to be used in the browser as detailed here: https://docs.aws.amazon.com/sdk-for-javascript/v2/developer-guide/building-sdk-for-browsers.html

If this can be confirmed, then this issue can be resolved as a non-issue, and it is just a matter of correct use of the library.

Comment by David Whittington [ 05/May/18 8:23 PM ]

I can confirm that it works in the browser with the AWS recommended browser build process. I was hoping it would also work as a node dep, but I think you're probably right that it's just not intended to be used that way.

Comment by Christian Paul Dehli [ 09/Aug/18 9:55 PM ]

So adding `aws-sdk` the recommended way does work, however there are many AWS packages that pull in `aws-sdk` as a dependency. One example is https://github.com/awslabs/aws-mobile-appsync-sdk-js/tree/master/packages/aws-appsync. This means that these packages won't run in ClojureScript because they throw the same error as above. I have confirmed that in a regular JavaScript project, `aws-sdk` is added correctly (an example is here: https://github.com/aws-samples/aws-mobile-appsync-events-starter-react/).

Like Mike said, it looks to be coming from this line in the code: https://github.com/aws/aws-sdk-js/blob/ebe83921863f1eb020b6a07ef471f2017cd58550/lib/util.js#L39. process.browser is what's indicating to aws-sdk whether or not it's a browser environment.

My guess would be if we set `process.browser` to true for when building to non-node environments it would solve this problem (https://nolanlawson.com/2017/01/09/how-to-write-a-javascript-package-for-both-node-and-the-browser/)





[CLJS-2781] Validate `:global-exports` Created: 16/Jun/18  Updated: 08/Aug/18

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: David Nolen Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File CLJS-2781.patch    
Patch: Code and Test

 Description   

Currently we don't validate `:global-exports`. It must map known provides to some symbol that represents a JavaScript global var.



 Comments   
Comment by Mike Fikes [ 07/Aug/18 9:44 AM ]

Note, the attached patch makes one change that falls a little out of the scope of the ticket: In the case that :provides is implicit (because :file represents a directory), and :global-exports is specified, the previous code would copy the entire global exports map into each of the generated foreign lib specs. This was revised to copy only the sub-map relevant to the associated file. While this seems like the correct thing to do when generating the expansion, it is also necessary with the new validation logic: Without this change any non-relevant global exports that happen to be copied would cause the validation logic to conclude that there is a missing provide.

For example

{:file "libs"
 :global-exports {foo foo-g bar bar-g}}

would be expanded into several lib specs, one for each file found in "libs", with the previous implementation copying the entire map into each. The revised code only copies the submap

:global-exports {foo foo-g}

into the spec related to :file "libs/foo.js".

Example validation messages:

Explicit case:

java.lang.Exception: Foreign lib global exports not provided: ["missing-lib" "another-lib"] not found in :provides in {:file "libs/mylib.js", :provides ["my-lib"], :global-exports {my-lib mylib, missing-lib missing, another-lib another}}
...

Explicit case, with namespaces:

java.lang.Exception: Foreign lib global exports not provided: ["missing-lib" "another-ns/another-lib"] not found in :provides in {:file "libs/mylib.js", :provides ["some-ns/my-lib"], :global-exports {some-ns/my-lib mylib, missing-lib missing, another-ns/another-lib another}}
...

Implicit (directory) case where only libs/other.js is present

java.lang.Exception: Foreign lib global exports not implicitly provided. Files ["libs/missing.js" "libs/another.js"] not found for library spec {:file "libs", :global-exports {other other-g, missing missing-g, another another-g}}
...




[CLJS-2858] :global-exports causes error Created: 07/Aug/18  Updated: 08/Aug/18

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: Next
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Colin Kahn Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None
Environment:

Mac OSX



 Description   

When trying to package a javascript library using `:global-exports` I'm getting an error:

Exception in thread "main" java.lang.IllegalArgumentException: contains? not supported on type: clojure.lang.Cons
        at clojure.lang.RT.contains(RT.java:846)
        at clojure.core$contains_QMARK_.invokeStatic(core.clj:1484)
        at clojure.core$contains_QMARK_.invoke(core.clj:1476)
        at cljs.analyzer$dep_has_global_exports_QMARK_.invokeStatic(analyzer.cljc:782)
...

Library repository: https://github.com/colinkahn/deckgl-cljs

To reproduce:

clj -Sdeps '{:deps {org.clojure/clojurescript {:mvn/version "1.10.339"} org.clojars.protocol55/deckgl {:mvn/version "0.1.1"}}}' -m cljs.main -e "(require 'com.uber.deckgl) (println com.uber.deckgl/DeckGL)"

Same library with `:global-exports` removed:

clj -Sdeps '{:deps {org.clojure/clojurescript {:mvn/version "1.10.339"} org.clojars.protocol55/deckgl {:mvn/version "1.0.0"}}}' -m cljs.main -e "(require 'com.uber.deckgl) (println js/deck.DeckGL)"


 Comments   
Comment by Mike Fikes [ 07/Aug/18 4:34 PM ]

Thank-you Colin!

Can you include any relevant files directly in the description?

(See https://clojurescript.org/community/reporting-issues)

Comment by Mike Fikes [ 07/Aug/18 4:37 PM ]

Oh, never mind. I see to repro you only need to issue a clj command.

Comment by Mike Fikes [ 07/Aug/18 6:02 PM ]

Root cause is that, even though the name of the file is deps.cljs it is read by the compiler as if it were an EDN file. In other words, symbols should not be quoted.

The file included in the JAR looks like:

{:foreign-libs [{:file "deckgl/deckgl.min.js"
                 :provides ["com.uber.deckgl"]
                 :global-exports '{com.uber.deckgl deck}}]
 :externs ["deckgl/externs/deckgl.js"]}

This causes contains? to be applied to (quote {com.uber.deckgl deck}) with the stacktrace seen.

Hey David, what's your opinion on this one? Should we handle this via documentation on clojurescript.org, clarifying that deps.cljs is actually meant to contain EDN?

Comment by Colin Kahn [ 07/Aug/18 6:25 PM ]

Ouch, yeah that is pretty unintuitive, especially if you already know the rules for each file type. I was using cljsjs as a reference too, but now realizing that the outputted result must be just edn: https://github.com/cljsjs/packages/blob/master/react/build.boot#L31

deps.cljs is a bit mysterious, would be nice to have a thorough explanation of what are the valid options inside of it.

Supporting an alternate name like jslibs.edn would probably add some clarity.





[CLJS-2857] Math with syntax quote Created: 06/Aug/18  Updated: 06/Aug/18

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Mike Fikes Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   
cljs.user=> `Math/foo
java.lang.Math/foo

Expect instead that this would yield Math/foo.






[CLJS-2262] Correct comment that *warn-on-infer* is file-scope Created: 19/Jul/17  Updated: 06/Aug/18  Resolved: 06/Aug/18

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: 1.9.854
Fix Version/s: None

Type: Defect Priority: Trivial
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: documentation

Attachments: Text File CLJS-2262.patch    
Patch: Code

 Description   

I had accidentally written a comment in the code next to *warn-on-infer* indicating that it has global scope.



 Comments   
Comment by David Nolen [ 24/Jul/17 12:26 AM ]

fixed https://github.com/clojure/clojurescript/commit/3037f04cdc7d8cc977842e9f129ef9f3aee70796





[CLJS-2256] Generated code doesn't add newline after sourceMappingURL comment Created: 17/Jul/17  Updated: 06/Aug/18  Resolved: 06/Aug/18

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: 1.9.854

Type: Defect Priority: Major
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-2256.patch    
Patch: Code

 Description   

Currently generated code doesn't output a newline after the `sourceMappingURL` comment, which makes code transforms (e.g. Webpack bundling) output code on that line that also ends up being commented out



 Comments   
Comment by David Nolen [ 18/Jul/17 12:39 AM ]

fixed https://github.com/clojure/clojurescript/commit/62926332a78e89935f5aa263195fc29cc297c3b7





[CLJS-2856] Transitive foreign libs not loaded in non-browser REPLs Created: 05/Aug/18  Updated: 05/Aug/18

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Mike Fikes Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

{:deps {org.clojure/clojurescript {:mvn/version "1.10.339"}}}



 Description   
mylib.js
mylib = { abc: 3 }
other.js
other = { def: 17 }
co.edn
{:foreign-libs [{:file "mylib.js"
                 :provides ["my-lib"]
                 :requires ["other-lib"]}
                {:file "other.js"
                 :provides ["other-lib"]}]}

Working, with browser REPL:

$ clj -m cljs.main -co co.edn -r
ClojureScript 1.10.339
cljs.user=> (require 'my-lib)

cljs.user=> js/other
#js {:def 17}

Faling with Node:

$ clj -m cljs.main -co co.edn -re node -r
ClojureScript 1.10.339
cljs.user=> (require 'my-lib)
nil
cljs.user=> js/other
repl:13
throw e__6464__auto__;
^

ReferenceError: other is not defined
...





[CLJS-2853] nth is documented as working on Matchers and Lists Created: 30/Jul/18  Updated: 04/Aug/18

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: docstring, newbie
Environment:

{:deps {org.clojure/clojurescript {:mvn/version "1.10.339"}}}


Attachments: Text File 0001-CLJS-2853-Remove-wrongly-carried-over-docs-from-Cloj.patch    
Patch: Code

 Description   
cljs.user=> (doc nth)
-------------------------
cljs.core/nth
([coll n] [coll n not-found])
  Returns the value at the index. get returns nil if index out of
  bounds, nth throws an exception unless not-found is supplied.  nth
  also works for strings, arrays, regex Matchers and Lists, and,
  in O(n) time, for sequences.

Lists is a carryover from the Clojure docstring, and likely also is regex Matchers.






[CLJS-2852] Clojure imparity: ns-publics returns different arglists for macros Created: 30/Jul/18  Updated: 03/Aug/18  Resolved: 03/Aug/18

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Martin Klepsch Assignee: Mike Fikes
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-2852.patch    
Patch: Code and Test
Approval: Accepted

 Description   
;; test-sources/codox_test/macro.cljc
(ns codox-test.macro)

(defmacro test2
  [a & xs]
  `(reduce + ~a ~(vec xs)))
(let [file (java.io.File. "test-sources/codox_test/macro.cljc")]
  (let [state (cljs.analyzer.api/empty-state)]
    (binding [cljs.analyzer/*analyze-deps* false]
      (cljs.analyzer.api/analyze-file state file {}))
    (->> (cljs.analyzer.api/ns-publics state 'codox-test.macro)
         (map (fn [[name m]] [name (:arglists m)]))
         (into {}))))

returns

{test2 ([&form &env a & xs])}

contrary, clojure's ns-publics:

(->> (vals (ns-publics 'codox-test.macro))
     (map meta)
     (map (fn [m] [(:name m) (:arglists m)]))
     (into {}))
{test2 ([a & xs])}

I didn't notice this issue with macros that don't use {& rest} style arglists.



 Comments   
Comment by Martin Klepsch [ 30/Jul/18 3:14 PM ]

This also results in different arglists metadata for clj/cljs

(doto (defmacro with-config-override [opts & body]
        `(with-config-override* ~opts (fn [] ~@body)))
  (alter-meta! assoc :arglists '([{:keys [switches secret-keys override-switches] :as opts-override} & body])))
Comment by Mike Fikes [ 03/Aug/18 2:57 PM ]

fixed https://github.com/clojure/clojurescript/commit/f13c08c90f27aa2a71694859f288e9b5f2407c73





[CLJS-2725] Doc on spec keywords Created: 05/Apr/18  Updated: 03/Aug/18  Resolved: 03/Aug/18

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Mike Fikes Assignee: Mike Fikes
Resolution: Completed Votes: 1
Labels: None

Attachments: Text File CLJS-2725.patch    
Patch: Code
Approval: Accepted

 Description   

For example, in Clojure:

user=> (doc :clojure.core.specs.alpha/arg-list)
-------------------------
:clojure.core.specs.alpha/arg-list
Spec
  (and vector? (cat :args (* :clojure.core.specs.alpha/binding-form) :varargs (? (cat :amp #{(quote &)} :form :clojure.core.specs.alpha/binding-form))))
nil


 Comments   
Comment by Mike Fikes [ 03/Aug/18 2:54 PM ]

fixed https://github.com/clojure/clojurescript/commit/b369fe14ec1b899e4b65fbbfe7efdd000a491ee7





[CLJS-2665] Port clojure.spec.test.alpha/enumerate-namespace Created: 15/Mar/18  Updated: 03/Aug/18  Resolved: 03/Aug/18

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Mike Fikes Assignee: Mike Fikes
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-2665.patch    
Patch: Code and Test
Approval: Accepted

 Description   

Make ClojureScript consistent with https://github.com/clojure/spec.alpha/commit/452a916a0842cb413acd6d177e5bbc4b323bb058



 Comments   
Comment by Mike Fikes [ 22/Jul/18 1:34 PM ]

Looks like all we need do is add an enumerate-namespace function to address the missing bits (the other stuff appears to have been done on master at this point).

Comment by Mike Fikes [ 03/Aug/18 2:53 PM ]

fixed https://github.com/clojure/clojurescript/commit/8614498e6c4a0613a168bf30f02756b629575871





[CLJS-2694] Remove direct references to Closure DiagnosticsGroups Created: 23/Mar/18  Updated: 03/Aug/18

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Trivial
Reporter: Thomas Heller Assignee: Thomas Heller
Resolution: Unresolved Votes: 1
Labels: None

Attachments: Text File CLJS-2694.patch    
Patch: Code
Approval: Accepted

 Description   

These change frequently with Closure Compiler releases and keeping them in sync manually breaks if a user wants to pick a specific Closure version.

Instead the warning types can be loaded at runtime by looking at DiagnosticsGroups registry itself.



 Comments   
Comment by Mike Fikes [ 03/Aug/18 2:51 PM ]

Patch need rebaseline





[CLJS-2848] Default explain printer prints root val and spec Created: 21/Jul/18  Updated: 03/Aug/18  Resolved: 03/Aug/18

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: 1.10.238
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Mike Fikes Assignee: Mike Fikes
Resolution: Completed Votes: 0
Labels: spec

Attachments: Text File CLJS-2848.patch    
Patch: Code
Approval: Accepted

 Description   

Port CLJ-2171



 Comments   
Comment by Mike Fikes [ 03/Aug/18 2:48 PM ]

fixed https://github.com/clojure/clojurescript/commit/d2cc14af6b26cd06bbd4d871123d4dd82c8ca4e0





[CLJS-2846] [spec] s/tuple explain-data :pred problem Created: 21/Jul/18  Updated: 03/Aug/18  Resolved: 03/Aug/18

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: 1.10.238
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Mike Fikes Assignee: Mike Fikes
Resolution: Completed Votes: 0
Labels: spec

Attachments: Text File CLJS-2846.patch    
Patch: Code and Test
Approval: Accepted

 Description   

Port CLJ-2176



 Comments   
Comment by Mike Fikes [ 03/Aug/18 2:46 PM ]

fixed https://github.com/clojure/clojurescript/commit/6152a3165b3196cc25e0929f6b2367d2761f15ac





[CLJS-2847] s/coll-of and s/every gen is very slow if :kind specified without :into Created: 21/Jul/18  Updated: 03/Aug/18  Resolved: 03/Aug/18

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: 1.10.238
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Mike Fikes Assignee: Mike Fikes
Resolution: Completed Votes: 0
Labels: spec

Attachments: Text File CLJS-2847.patch    
Patch: Code
Approval: Accepted

 Description   

Port CLJ-2103.



 Comments   
Comment by Mike Fikes [ 21/Jul/18 5:18 PM ]

Before:

cljs.user=> (time (dorun (gen/sample (s/gen (s/coll-of int? :kind list?)) 1000)))
"Elapsed time: 18424.869714 msecs"

After:

cljs.user=> (time (dorun (gen/sample (s/gen (s/coll-of int? :kind list?)) 1000)))
"Elapsed time: 943.532827 msecs"
Comment by Mike Fikes [ 03/Aug/18 2:43 PM ]

fixed https://github.com/clojure/clojurescript/commit/a6885a0cb21274fcab180cc82c4b791ee96197c5





[CLJS-2841] [spec] instrument exception doesn't contain function name in ex-data Created: 21/Jul/18  Updated: 03/Aug/18  Resolved: 03/Aug/18

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: 1.10.238
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Mike Fikes Assignee: Mike Fikes
Resolution: Completed Votes: 0
Labels: spec

Attachments: Text File CLJS-2841.patch    
Patch: Code
Approval: Accepted

 Description   

Port CLJ-2166 to ClojureScript.



 Comments   
Comment by Mike Fikes [ 03/Aug/18 2:42 PM ]

fixed https://github.com/clojure/clojurescript/commit/4d408e1aae6961611200abb9f839838ebf6966a8





[CLJS-2842] [spec] Clarify s/every docstring for :kind Created: 21/Jul/18  Updated: 03/Aug/18  Resolved: 03/Aug/18

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: 1.10.238

Type: Enhancement Priority: Major
Reporter: Mike Fikes Assignee: Mike Fikes
Resolution: Completed Votes: 0
Labels: spec

Attachments: Text File CLJS-2842.patch    
Patch: Code
Approval: Accepted

 Description   

Port CLJ-2111



 Comments   
Comment by Mike Fikes [ 03/Aug/18 2:40 PM ]

fixed https://github.com/clojure/clojurescript/commit/387ae34f7d667853c6995b0f0c406455d02e9fb3





[CLJS-2845] [spec] generate random subsets of or'd required keys in map specs Created: 21/Jul/18  Updated: 03/Aug/18  Resolved: 03/Aug/18

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: 1.10.238
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Mike Fikes Assignee: Mike Fikes
Resolution: Completed Votes: 0
Labels: spec

Attachments: Text File CLJS-2845.patch    
Patch: Code and Test
Approval: Accepted

 Description   

Port CLJ-2046



 Comments   
Comment by Mike Fikes [ 03/Aug/18 2:39 PM ]

fixed https://github.com/clojure/clojurescript/commit/1261aed07d5e11b38d5deafcf6afa4a7abe4b346





[CLJS-2844] [spec] Add support for undefining a spec Created: 21/Jul/18  Updated: 03/Aug/18  Resolved: 03/Aug/18

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: 1.10.238
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Mike Fikes Assignee: Mike Fikes
Resolution: Completed Votes: 0
Labels: spec

Attachments: Text File CLJS-2844.patch    
Patch: Code and Test
Approval: Accepted

 Description   

Port CLJ-2060



 Comments   
Comment by Mike Fikes [ 03/Aug/18 2:33 PM ]

fixed https://github.com/clojure/clojurescript/commit/7187f8f91ee2cc35b41e22899c6103c86674dd2b





[CLJS-2840] [spec] s/keys explain-data :pred problem Created: 21/Jul/18  Updated: 03/Aug/18  Resolved: 03/Aug/18

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: 1.10.238
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Mike Fikes Assignee: Mike Fikes
Resolution: Completed Votes: 0
Labels: spec

Attachments: Text File CLJS-2840.patch    
Patch: Code and Test
Approval: Accepted

 Description   

Port CLJ-2177 to ClojureScript.



 Comments   
Comment by Mike Fikes [ 03/Aug/18 2:27 PM ]

fixed https://github.com/clojure/clojurescript/commit/e58cf89115f7f08354ba09caeb63eff08b142f01





[CLJS-2839] [spec] s/& explain-data :pred problem Created: 21/Jul/18  Updated: 03/Aug/18  Resolved: 03/Aug/18

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: 1.10.238

Type: Defect Priority: Major
Reporter: Mike Fikes Assignee: Mike Fikes
Resolution: Completed Votes: 0
Labels: spec

Attachments: Text File CLJS-2839.patch    
Patch: Code and Test
Approval: Accepted

 Description   

Port CLJ-2178 to ClojureScript.



 Comments   
Comment by Mike Fikes [ 03/Aug/18 2:20 PM ]

fixed https://github.com/clojure/clojurescript/commit/455724c99afb000d853af6838065d62ce6661d2b





[CLJS-2838] [spec] s/& does not check preds if regex matches empty collection Created: 21/Jul/18  Updated: 03/Aug/18  Resolved: 03/Aug/18

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: 1.10.238
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Mike Fikes Assignee: Mike Fikes
Resolution: Completed Votes: 0
Labels: spec

Attachments: Text File CLJS-2838.patch    
Patch: Code and Test
Approval: Accepted

 Description   

Port CLJ-2182 to ClojureScript.



 Comments   
Comment by Mike Fikes [ 03/Aug/18 2:18 PM ]

fixed https://github.com/clojure/clojurescript/commit/0ec3870d8e301f2d3c99c7dd9714f3fb101ba5c9





[CLJS-2825] Eliminate unnecessary ^boolean annotations Created: 15/Jul/18  Updated: 03/Aug/18

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

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

Attachments: Text File CLJS-2825.patch    
Patch: Code and Test

 Description   

With CLJS-1997, many of the ^boolean annotations in core could be removed.

If we do this, we should add tests that truth checks are not added for the functions where ^boolean is removed.



 Comments   
Comment by Mike Fikes [ 20/Jul/18 11:24 PM ]

CLJS-2825.patch removes 65 ^boolean annotations that are on functions, adding tests that ensure truth_ is not called when applications of these functions to simple values are used as if tests.

There remain ^boolean annotations on some functions that are also macros. These are not removed because I couldn't think of a way to call them as functions in a way that would involve truth_.

Comment by David Nolen [ 03/Aug/18 2:06 PM ]

Thanks, I'm a bit confused how inference outward propagation can work for goog/isString?

Comment by Mike Fikes [ 03/Aug/18 2:16 PM ]

Hey David, are you perhaps referring to the goog/isString? that appears in the patch?

Here it is in context: https://github.com/mfikes/clojurescript/commit/d1f89b0e74ffa7fcdf5b3ec86c9808e1fb934919#diff-6db96e000d4412edf3aa45181efd6ebbL268

The patch preserves the ^boolean annotation on the string? predicate.





[CLJS-2837] [spec] `cat` specs should verify value is sequential Created: 21/Jul/18  Updated: 03/Aug/18  Resolved: 03/Aug/18

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Mike Fikes Assignee: Mike Fikes
Resolution: Completed Votes: 0
Labels: spec
Environment:

{:deps {org.clojure/clojurescript {:mvn/version "1.10.339"}}}


Attachments: Text File CLJS-2837.patch    
Patch: Code and Test
Approval: Accepted

 Description   

Port CLJ-2183 to ClojureScript.



 Comments   
Comment by Mike Fikes [ 21/Jul/18 10:48 AM ]

Note, the patch does not port the use of res to mres because doing so causes the predicate data to exhibit gensyms related to the anonymous function literal arguments.

Behavior with the patch:

cljs.user=> (require '[clojure.spec.alpha :as s])
nil
cljs.user=> (s/def ::kv (s/cat :k keyword? :v any?))
:cljs.user/kv
cljs.user=> (s/explain ::kv {"foo" "bar"})
val: {"foo" "bar"} fails spec: :cljs.user/kv predicate: (or (nil? %) (sequential? %))
:cljs.spec.alpha/spec  :cljs.user/kv
:cljs.spec.alpha/value  {"foo" "bar"}
Comment by Mike Fikes [ 03/Aug/18 2:09 PM ]

fixed https://github.com/clojure/clojurescript/commit/c40a475ff42273d4d355486104d56c1ec5d795d3





[CLJS-2541] binding not made in parallel Created: 21/Feb/18  Updated: 03/Aug/18  Resolved: 03/Aug/18

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: 1.9.908
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Mike Fikes Assignee: Mike Fikes
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-2541-2.patch     Text File CLJS-2541-3.patch     Text File CLJS-2541.patch    
Patch: Code and Test
Approval: Accepted

 Description   

The docstring for binding indicates "The new bindings are made in parallel (unlike let); all init-exprs are evaluated before the vars are bound to their new values."

You can observe this in Clojure:

user=> (def ^:dynamic *a* 1)
#'user/*a*
user=> (def ^:dynamic *b* nil)
#'user/*b*
user=> (binding [*a* 10 *b* (inc *a*)] *b*)
2

but in ClojureScript bindings are made in serial as if with let:

cljs.user=> (def ^:dynamic *a* 1)
#'cljs.user/*a*
cljs.user=> (def ^:dynamic *b* nil)
#'cljs.user/*b*
cljs.user=> (binding [*a* 10 *b* (inc *a*)] *b*)
11


 Comments   
Comment by Mike Fikes [ 22/Feb/18 9:15 AM ]

The defect also extends to with-redefs.

The attached patch ensures the correct evaluation order by placing values into temporary vectors.

Currently, if you macroexpand the example given in the ticket description, you get:

(let* [*a*28 *a*
       *b*29 *b*]
  (set! *a* 10)
  (set! *b* (inc *a*))
  (try
   *b*
   (finally
    (set! *b* *b*29)
    (set! *a* *a*28))))

The patch revises the expansion so that you instead get:

(let* [orig-vals__30 [*a* *b*]
       temp-vals__31 [10 (inc *a*)]]
  (set! *a* (temp-vals__31 0))
  (set! *b* (temp-vals__31 1))
  (try
   *b*
   (finally
    (set! *b* (orig-vals__30 1))
    (set! *a* (orig-vals__30 0)))))

The patch also fixes a place in cljs.pprint which has an invalid binding form. (It attempts to bind a var but provides no value.)

Additionally, the patch fixes places in cljs.js where the binding of r/*alias-map* was established using a function that itself relies on bindings that are established earlier in the binding form list.

Comment by Mike Fikes [ 19/Jul/18 3:12 PM ]

CLJS-2541-2.patch is essentially the same as the first patch but uses JavaScript arrays instead of persistent vectors for temporary storage.

Comment by Mike Fikes [ 19/Jul/18 4:47 PM ]

CLJS-2541-3.patch maintains the necessary logic but unrolls everything (avoiding allocation) and ends up being much closer to the original implementation.

(let* [*a*-orig-val__18906 *a* 
       *b*-orig-val__18907 *b* 
       *a*-temp-val__18908 10 
       *b*-temp-val__18909 (inc *a*)] 
  (set! *a* *a*-temp-val__18908) 
  (set! *b* *b*-temp-val__18909) 
  (try *b* 
    (finally 
      (set! *b* *b*-orig-val__18907) 
      (set! *a* *a*-orig-val__18906))))
Comment by Mike Fikes [ 03/Aug/18 2:06 PM ]

fixed https://github.com/clojure/clojurescript/commit/a93ac962c97845a07aa64cbfe95abcca63b5ae9b





[CLJS-2831] Add a graaljs REPL environment Created: 17/Jul/18  Updated: 03/Aug/18  Resolved: 03/Aug/18

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Mike Fikes Assignee: Mike Fikes
Resolution: Completed Votes: 2
Labels: None

Attachments: Text File CLJS-2831.patch    
Patch: Code and Test
Approval: Accepted

 Description   

Graal.JS is a new JavaScript engine https://github.com/graalvm/graaljs and with CLJS-2745, we are testing ClojureScript compatibility with Graal.js.

ClojureScript could ship with a new REPL environment (compatible with cljs.main's --repl-env feature).

With this new REPL environment, Graal.js's polyglot capability can be enabled by default, thus allowing direct interaction with Truffle languages.

Here is an example, based on a spike into such a feature:

clj -m cljs.main -re graaljs -r
cljs.user=> (reduce + (range 1 101))
5050
cljs.user=> (.eval js/Polyglot "R" "sum(1:100)")
5050
cljs.user=> (.eval js/Polyglot "ruby" "(1..100).reduce(:+)")
5050
cljs.user=> (.eval js/Polyglot "python" "sum(x for x in range(1, 101))")
5050
cljs.user=> (.eval js/Polyglot "js" "(100 * (100 + 1)) / 2")
5050

Over the long term, the graaljs REPL environment could be considered a modern replacement for the use cases currently satisfied by the nashorn REPL environment.

Nashorn will evidently be deprecated with Java 11: http://hg.openjdk.java.net/jdk/jdk11/rev/fb7800b66c92



 Comments   
Comment by Mike Fikes [ 03/Aug/18 2:02 PM ]

fixed https://github.com/clojure/clojurescript/commit/04e6bd9073daa905543d7decab95a2252c2e53e2





[CLJS-2832] Bad code gen for `((not empty?) "foo")` when compiled with no optimizations Created: 18/Jul/18  Updated: 03/Aug/18  Resolved: 03/Aug/18

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: 1.9.908
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Matt Freer Assignee: Mike Fikes
Resolution: Completed Votes: 1
Labels: clojurescript
Environment:

macOS Sierra (10.12.6).
ClojureScript 1.9.946


Attachments: Text File CLJS-2832.patch    
Patch: Code and Test
Approval: Accepted

 Description   

Using a ClojureScript (1.9.946) REPL If I run:

[(true? ((comp not empty?) "foo"))
 (false? ((comp not empty?) ""))
 (true? ((not empty?) "foo"))
 (false? ((not empty?) ""))]

I get the result: [true true true true]

I would have expected this code to raise an exception. The first two expressions are valid, however the last two expressions don't use comp and just apply not to empty?, which returns false

Interestingly if I run this code in a Clojure (not a ClojureScript) REPL, I get an error that I would expect:

java.lang.Boolean cannot be cast to clojure.lang.IFn

If I compile this code with the ClojureScript compiler using advanced optimizations then I get a runtime error,
which is more like what I would expect:

Uncaught TypeError: b.call is not a function

So when compiled under non-optimization, ((not empty?) "foo") seems to produce unexpected results.



 Comments   
Comment by Mike Fikes [ 18/Jul/18 7:44 AM ]

Note: Even though CLJS-2832.patch has tests, we are currently really relying on the self-host tests for automated verification (because they pass regardless in :advanced mode without the patch).

Comment by Mike Fikes [ 18/Jul/18 10:01 AM ]

Just to be extra sure, I ran this patch through Coal Mine (which uses :optimizations :none), and it passed

Ran 47695 tests containing 184114 assertions.
0 failures, 0 errors.
Comment by Mike Fikes [ 03/Aug/18 1:58 PM ]

fixed https://github.com/clojure/clojurescript/commit/9923fadc659dc1694c88371d46791da8881c026b





[CLJS-2833] Update to Closure Compiler v20180716 Created: 18/Jul/18  Updated: 03/Aug/18  Resolved: 03/Aug/18

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Mike Fikes Assignee: Mike Fikes
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-2833.patch    
Patch: Code
Approval: Accepted

 Description   

Closure Compiler v20180716 was released a couple of days ago.

Ratiionale: We have some tickets surrounding NPM deps processing that improve with later builds of Closure Compiler. (See, for example, CLJS-2757)



 Comments   
Comment by Mike Fikes [ 03/Aug/18 1:55 PM ]

fixed https://github.com/clojure/clojurescript/commit/cbb4896e2afe52d786529c7a5f81c64f3fd394cc





[CLJS-2855] Browser REPL prints empty string after require Created: 02/Aug/18  Updated: 03/Aug/18  Resolved: 03/Aug/18

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Mike Fikes Assignee: Mike Fikes
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-2855.patch    
Patch: Code and Test
Approval: Accepted

 Description   

When you do a require in the browser REPL, you end up seeing an empty line being printed instead of nil like you do in Node, Nashorn (or Clojure) REPLs.

This is an artifact of implementation where (for some reason), the null returned for CLJS-2135 gets converted to the string "nil" for the non-browser REPLs. The browser REPL on the other hand simply calls str on the value, and thus causes the empty string to be printed.



 Comments   
Comment by Mike Fikes [ 02/Aug/18 2:35 PM ]

While this may seem an odd way to fix this, here are arguments for doing it this way, rather than patching the browser REPL code:

  1. When evaluating, REPLs wrap the form in pr-str, so returning "nil" is consistent with (pr-str nil).
  2. This fix is limited in scope to what gets printed after a require (or require-macros) form is evaluated, and avoids messing with how returned values in general are handled.
  3. This approach has a side effect of fixing an issue with the Node REPL where it would inadvertently print nil when doing a -e with a require form. The unit tests are updated to reflect this corrected behavior.
  4. Of interest, this fixes things downstream in Figwheel, so it properly prints nil after a require form in that REPL.
Comment by Mike Fikes [ 03/Aug/18 1:52 PM ]

fixed https://github.com/clojure/clojurescript/commit/e78177a73b3ff4dc7d9a7a16ec0c1da768eee953





[CLJS-2851] Self-assignment in def produces code breaking Closure :advanced Created: 30/Jul/18  Updated: 30/Jul/18

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Thomas Heller Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   
(def x x)

This code executes fine in JS but does not produce anything useful and I wonder if we should check for this. When running :advanced compilation it will fail with a cryptic error message like:

Exception in thread "main" java.lang.IllegalStateException: ASSIGN 5 [length: 23] [source_file: .../test/foo.js] is not the parent of GETPROP 5 [length: 10] [source_file: .../test/foo.js]
        at com.google.common.base.Preconditions.checkState(Preconditions.java:733)
        at com.google.javascript.rhino.Node.replaceChild(Node.java:900)
        at com.google.javascript.jscomp.AggressiveInlineAliases.inlineGlobalAliasIfPossible(AggressiveInlineAliases.java:557)
        at com.google.javascript.jscomp.AggressiveInlineAliases.inlineAliases(AggressiveInlineAliases.java:180)
        at com.google.javascript.jscomp.AggressiveInlineAliases.process(AggressiveInlineAliases.java:123)
        at com.google.javascript.jscomp.PhaseOptimizer$NamedPass.process(PhaseOptimizer.java:303)
        at com.google.javascript.jscomp.PhaseOptimizer.process(PhaseOptimizer.java:230)
        at com.google.javascript.jscomp.Compiler.performOptimizations(Compiler.java:2480)
        at com.google.javascript.jscomp.Compiler$3.call(Compiler.java:808)
        at com.google.javascript.jscomp.Compiler$3.call(Compiler.java:804)
        at com.google.javascript.jscomp.CompilerExecutor$2.call(CompilerExecutor.java:102)
        at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
        at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1135)
        at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
        at java.base/java.lang.Thread.run(Thread.java:844)

The code is unusable anyways and probably always written by mistake. This mistake came up a couple of times when shadow-cljs users migrated some code from CLJSJS that used to pull some global into the local namespace.

(ns demo.foo
  (:require [cljsjs.foo]))

(def foo js/foo)

rewritten to

(ns demo.foo
  (:require ["that-npm-foo" :as foo]))

(def foo foo)

The mistake made is thinking that the RHS of the def refers to the ns alias foo but it actually refers to the LHS.






[CLJS-2850] Support webpack node lib replacements Created: 29/Jul/18  Updated: 29/Jul/18

Status: Open
Project: Clo