ClojureScript

resolve-var for symbol with dot still wrong

Details

  • Type: Defect Defect
  • Status: Open Open
  • Priority: Major Major
  • Resolution: Unresolved
  • Affects Version/s: 1.7.145
  • Fix Version/s: 1.9.671
  • Component/s: None
  • 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.

Activity

Hide
Thomas Heller added a comment -

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

Show
Thomas Heller added a comment - 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
Hide
Thomas Heller added a comment -

FWIW I added a rather hackish workaround for this to shadow-cljs [1] and it already discovered several actual bugs in popular CLJS libraries as well as a couple problems in cljs.core itself. It is by no means a complete summary but these should probably be fixed prior to proceeding with this as it will produce lots of warnings otherwise. Still unsure how to fix this cleanly though.

https://dev.clojure.org/jira/browse/CLJS-2982
https://dev.clojure.org/jira/browse/CLJS-2983
https://dev.clojure.org/jira/browse/CLJS-2984

https://github.com/Day8/re-frame-10x/issues/219
https://github.com/nathanmarz/specter/issues/267

[1] https://github.com/thheller/shadow-cljs/blob/22944d9bba14a8428efbd52463a37eb636017609/src/main/shadow/build/cljs_hacks.cljc#L239-L308

Show
Thomas Heller added a comment - FWIW I added a rather hackish workaround for this to shadow-cljs [1] and it already discovered several actual bugs in popular CLJS libraries as well as a couple problems in cljs.core itself. It is by no means a complete summary but these should probably be fixed prior to proceeding with this as it will produce lots of warnings otherwise. Still unsure how to fix this cleanly though. https://dev.clojure.org/jira/browse/CLJS-2982 https://dev.clojure.org/jira/browse/CLJS-2983 https://dev.clojure.org/jira/browse/CLJS-2984 https://github.com/Day8/re-frame-10x/issues/219 https://github.com/nathanmarz/specter/issues/267 [1] https://github.com/thheller/shadow-cljs/blob/22944d9bba14a8428efbd52463a37eb636017609/src/main/shadow/build/cljs_hacks.cljc#L239-L308

People

Vote (3)
Watch (1)

Dates

  • Created:
    Updated: