This looks fine, but I'm just a little nervous about introducing new name clashes with this. Have you been able to rule that out as a possibility?

PS doesn't allow names starting with $. Is there another way that such a clash could occur?

(admittedly this is still "stringly-typed" programming, but I think it will do the trick in lieu of a much tricker solution)

Right, but the compiler already generates plenty of names with dollar signs in them in various situations.

Right, but the compiler already generates plenty of names with dollar signs in them in various situations.

I think $prime is the only thing that the compiler generates with a $, and it's used within an existing identifier.

I think $prime is the only thing that the compiler generates with a $, and it's used within an existing identifier.

there is a function called identCharToText that lists these. There are a few more. But we are safe.

None of these are used anymore since you can only introduce them with operators, which are desugared to the actual identifier it represents.

There is also $$name generated for builtins. But that's it.

For reference, the function that handles the name conversion generally is identToJS in CodeGen.JS.Common.

So you're saying there's absolutely no use of $ in generated names other than $prime, and reserved words in JS e.g. $$Math, $$null? Is that just in the JS output, or do we need to worry about other intermediate representations too?

I'd like to remove the unused code to do with how operators used to work, and actually I think I'd be slightly happier if we did that before doing this.

So you're saying there's absolutely no use of $ in generated names other than $prime, and reserved words in JS e.g. $$Math, $$null?

Yes

Is that just in the JS output, or do we need to worry about other intermediate representations too?

This guarantees no clash in JS output and intermediate representations (they don't use "$" to my knowledge). Other backends, however, would have to avoid the $unused symbol. But then, they have to do that with __unused now anyway. At the very least, I think this makes things slightly better.

I'd like to remove the unused code to do with how operators used to work, and actually I think I'd be slightly happier if we did that before doing this.

Might make a PR for this if nobody else has that waiting.

(they don't use "$" to my knowledge)

I've just been bitten too many times by thinking "I can't think of a way that this could go wrong, therefore it will not go wrong." Not using it to anyone's knowledge doesn't really count as any kind of guarantee, in my mind. I'll review this more thoroughly when I get a chance to try to verify that this is safe, but since this is a very small code change and it addresses a rare edge case that nobody has even run in to as far as we are aware, I'm probably not going to prioritise it (just FYI).

Might make a PR for this if nobody else has that waiting.

I'm not aware of anyone else working on that; please go ahead!

since this is a very small code change and it addresses a rare edge case that nobody has even run in to as far as we are aware, I'm probably not going to prioritise it (just FYI).

Yes, this makes sense.

Also, on reflection, there is indeed the issue that other backends might potentially exist for languages where something like $unused is significant. I mean, the overwhelming odds are that we're fine... But the real solution here would be to avoid using stringly-typed programming and optimize this stuff away through metadata/annotations or something like that.

I'm going to close this now and try to come up with a better solution :)

$ is only used in the names that are escaped (all that stuff is https://github.com/purescript/purescript/blob/master/src/Language/PureScript/CodeGen/JS/Common.hs) and for freshly generated names, that always have a number appended:

Ok great - I forgot that freshly generated names have a number at the end. In that case I’m happy with this.

I will resurrect this if I can't get something better to work. But I think there is a better way.

To introduce the new name type will take a little work, as currently "CoreImp" (aka, JS) uses Text rather than a real type for idents. I'd also like that approach more though, as it'll make optimising unuseds etc. less fragile, especially if we start doing it for empty class dictionaries and whatnot (although it depends - maybe we'll do that elsewhere anyway).

Not that it'd be hard to introduce a type for CoreImp names, just a little tedious 😉

I haven't done a thorough check, but it seems that so far C.__unused is only used for a few different things, at least some of which could be optimized away before hitting CoreImp. Perhaps all of them could be?

(Regarding the mentions of "other backends" - there's always going to be some mangling in some circumstances, and it will be different to the JS mangling, which is fine so long as it's injective - I think any principled approach is likely to be fine)

Another use of $ in variable names is in the tail call optimisation code