Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Macroify: str #10

Closed
lilactown opened this issue Aug 14, 2022 · 11 comments
Closed

Macroify: str #10

lilactown opened this issue Aug 14, 2022 · 11 comments

Comments

@lilactown
Copy link
Collaborator

str should emit "" + ... code.

I think we still need the core str function when the common (apply str ,,,) is used

@corasaurus-hex
Copy link
Collaborator

So there's another option here: string interpolation using template literals.

Note that there's a mild difference between the two syntaxes. Addition would coerce the expression to a primitive, which calls valueOf() in priority; on the other hand, template literal would coerce the expression to a string, which calls toString() in priority. If the expression has a @@toPrimitive method, string concatenation calls it with "default" as hint, while template literals use "string". This is important for objects that have different string and primitive representations — such as Temporal, whose valueOf() method throws.

It seems like we'd want string interpolation since it more closely maps to Clojure's str?

@corasaurus-hex
Copy link
Collaborator

I wrote some code to see the differences between the two options: https://gist.github.com/corasaurus-hex/2c03d8abc1c8827b44b59fbdc31a3622

I can't see any difference between them in practice except the ones noted in the paragraph I quoted.

So do we want the behavior of string concatenation or string interpolation? I'd vote for interpolation, both for forwards compatibility and it being the least surprising behavior.

@borkdude
Copy link
Member

@corasaurus-hex String interpolation is a different issue: this issue is just about generating more efficient code for (str foo bar) => cherry/str(foo,bar) vs foo + bar.
I think string interpolation should also be a built-in feature that compiles directly to JS string interpolation. We can support this using a reader template #i "foo ${name}". I'll make a different issue for that.

@corasaurus-hex
Copy link
Collaborator

corasaurus-hex commented Aug 17, 2022

The generated code for (str foo bar) can either be "" + foo + bar or it can be `${foo}${bar}` and I had intended to examine which was the better choice.

@borkdude
Copy link
Member

borkdude commented Aug 17, 2022 via email

@corasaurus-hex
Copy link
Collaborator

corasaurus-hex commented Aug 17, 2022

The main reasons, for me:

  • template interpolation calls .toString on the elements vs + which calls .valueOf on all the elements. I think that more matches with the spirit of str.
  • following from that, calling .toString on the elements makes it forwards compatible with Temporal.
  • cljs calls .toString on all the elements, too.

String interpolation via templates is just a means to an end for me, though, what I really care about is using .toString instead of .valueOf and just doing "" + ... will call .valueOf.

@borkdude
Copy link
Member

Right. How CLJS compiles (str "foo" "bar") is:

["foo","bar"].join('');

Does that match the .toString behavior? If so, then we could emit that too.

@borkdude
Copy link
Member

But then again, we could just handle that in the str function instead instead of inlining this, probably won't matter much.

@lilactown
Copy link
Collaborator Author

lilactown commented Aug 17, 2022

I don't have a strong preference. My hunch is that my usage of str would go down in application code if there was a reader conditional for doing string interpolation as in #32.

So instead of doing something like

(str "Hello, " name "!")

I would do

#i "Hello, ${name}!"

The only time I really need str then is for the (apply str ,,,) case I noted in the OP, which can't be expanded to infix anyway. So I think based on that we don't need to macroify it if we go with #32

@borkdude
Copy link
Member

Agreed

@corasaurus-hex
Copy link
Collaborator

Yes, Array.prototype.join calls .toString and not valueOf

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants