[ClojureScript] Review of new property lookup syntax

[Fogus]

I have a branch at https://github.com/clojure/clojurescript/tree/prop-lookup
with support for the new ClojureScript property lookup syntax[1]. If
you have some time please have a look at the implementation and run
through some scenarios that you have in your own code to make sure
that it works (and breaks) as expected.

There is more work around macroexpand-1 that I need to double check,
but for the common uses of the dot operator the functionality is in
place.

Thanks!

[1]: http://dev.clojure.org/display/design/Unified+ClojureScript+and+Clojure+field+access+syntax

[Rich Hickey]

I added a comment to the ticket re: (.whatever x) -> (. x whatever) transformation being a (special) macro expansion.

Does this code reflect that?

Rich

> --
> You received this message because you are subscribed to the Google Groups "Clojure Dev" group.
> To post to this group, send email to cloju...@googlegroups.com.
> To unsubscribe from this group, send email to clojure-dev...@googlegroups.com.
> For more options, visit this group at http://groups.google.com/group/clojure-dev?hl=en.
>

[Michael Fogus]

Rich, 

Do you mean in cljs.compiler.macroexpand-1?  I have yet to touch that, but I plan to look at it whenever I get home tonight.  However, in the cases of (.-p o) -> (. o -p) and (.m o) -> (. o m) the relative action semantics (i.e. lookup and call) are the same.  Do you know of a representational case that I should be mindful of?

[David Powell]

On Sun, Oct 30, 2011 at 2:47 AM, Fogus <mef...@gmail.com> wrote:

I have a branch at https://github.com/clojure/clojurescript/tree/prop-lookup
with support for the new ClojureScript property lookup syntax[1].  If
you have some time please have a look at the implementation and run
through some scenarios that you have in your own code to make sure
that it works (and breaks) as expected.

The following code doesn't seem to work:

(defrecord A [x])

(->A 1)

It looks like all of the:

(. f cljs$lang$maxFixedArity)

stuff in cljs/core.cljs needs fixing to be field access.

Anyway - I quite like it.  Yes it adds this new '-' syntax, which is a bit magic-y, but it tidies up all of those odd cases of what is a field and what is a property.

-- 

Dave

[Michael Fogus]

> (->A 1)

Fixed. Thank you for catching that.
:F

[David Nolen]

This change broke the browser REPL. I took a stab at fixing the browser REPL up and encountered the following bug:

(def y (js-obj))

(set! (.-bar y) "woz")

(def x (js-obj))

(set! (.-foo x) y)

(.-bar (.-foo x))

java.lang.Error: Unknown dot form of [(.-foo x) -bar nil] with cassification [:cljs.compiler/list :cljs.compiler/property ()]

[Michael Fogus]

David,

Thank you for looking. I just pushed a fix to the branch.
:F

[David Nolen]

With the latest changes the following does not work:

(.-constructor "foo")

It seem like we're encountering the problem that Rich was hoping to avoid? Why do we need to handle cases?

David

:F

[Michael Fogus]

> Why do we need to handle cases?

Me and Alan are working through making this more generic and should
have something out later today.

Thanks
:F

[Alan Dipert]

Fogus and I didn't get further with CLJS, but we did take a swing at
property lookup syntax for Clojure. Please see
http://dev.clojure.org/jira/browse/CLJ-872.

Alan

[David Nolen]

I patched up prop-look up as well as fixed Browser REPL for the changes. Is there anything holding us up from merging this into master?

David

[David Nolen]

Hmm looks like I spoke too soon. Looks like apply was somehow broken by some of the changes around the property syntax. Anyone else seeing this as well?

David

[Michael Fogus]

I just fixed apply. The only thing keeping us from pushing this
prop-lookup behavior is:

- general announcement regarding its breaking nature and how to fix it
in the general case
- a co-release in some 1.4-alpha version of the same prop-lookup syntax
- (ideally) a utility or compiler util that can identify and warn the
old-style lookup cases in people's code
+ this might not be 100% comprehensive, but it might be worth exploring

Am I missing anything else?

:F

[David Nolen]

On Sun, Nov 13, 2011 at 8:44 PM, Michael Fogus <mef...@gmail.com> wrote:

I just fixed apply.  The only thing keeping us from pushing this
prop-lookup behavior is:

- general announcement regarding its breaking nature and how to fix it
in the general case

Yup.

 

- a co-release in some 1.4-alpha version of the same prop-lookup syntax

Is this necessary?

 

- (ideally) a utility or compiler util that can identify and warn the
old-style lookup cases in people's code

Clojure isn't changing in any breaking way, and ClojureScript is in alpha - do we need this?

 

 + this might not be 100% comprehensive, but it might be worth exploring

Am I missing anything else?

:F

David 

[Michael Fogus]

>> - a co-release in some 1.4-alpha version of the same prop-lookup syntax
>
> Is this necessary?

Define necessary.

> Clojure isn't changing in any breaking way, and ClojureScript is in alpha -
> do we need this?

If we can take some of the burden off people with existing code then
that's worthwhile. Minimally we can print a warning whenever the
compiler sees a (. target something) pattern that simply raises
awareness that it might not mean what it once meant.

[David Nolen]

On Sun, Nov 13, 2011 at 9:28 PM, Michael Fogus <mef...@gmail.com> wrote:

>> - a co-release in some 1.4-alpha version of the same prop-lookup syntax
>
> Is this necessary?

Define necessary.

What is benefit of synchronizing while ClojureScript is alpha? Perhaps there's an advantage that I'm not seeing. In the worst case it's just delaying a real benefit for active ClojureScript users.

Also since there's no ambiguity in Clojure, I don't see many people adopting the (.-prop x) form there.

 

> Clojure isn't changing in any breaking way, and ClojureScript is in alpha -
> do we need this?

If we can take some of the burden off people with existing code then
that's worthwhile.  Minimally we can print a warning whenever the
compiler sees a (. target something) pattern that simply raises
awareness that it might not mean what it once meant.

As soon as people realize that (.method x) works, they're going to get a lot of useless warnings moving forward. How long should this warning persist?

David 

[Rich Hickey]

On Nov 13, 2011, at 9:00 PM, David Nolen wrote:

>
> - a co-release in some 1.4-alpha version of the same prop-lookup syntax
>
> Is this necessary?

Yes. It's necessary so that ClojureScript implements Clojure on JS, and not some new language.

But, it is also not a big deal on the Clojure side, nor is generating an alpha.

Rich

[David Nolen]

Just tried this out and voted it up on JIRA. Seems to work well! A side note- I'm liking the fact that it's much easier to detect field access when browsing source with this syntax.

Anything holding up a 1.4.0-alpha1?

David

On Fri, Nov 4, 2011 at 5:27 PM, Alan Dipert <al...@dipert.org> wrote:

[David Nolen]

Anything blocking this patch from getting applied so we can cut 1.4.0-alpha3? It's tough working on ClojureScript patches/branches knowing that the current patches/branches will need to be fixed once this gets in.

David

[Fogus]

> Anything blocking this patch from getting applied so we can cut
> 1.4.0-alpha3?

Nothing on my end is needed. I'm ready for the required merge.