Mangling makeover #1517

Kodiologist · 2018-02-26T19:10:54Z

Reopened for the new merge policy.

Closes Find a way to keep the "original" symbol before the interop munging #360
Closes add option to hy2py that converts symbols to valid python identifiers #970 (no longer necessary)
Closes clean up rules for ? #1115 (debatably)
Closes Should keywords be mangled? #1168 (keywords are now indeed mangled when used for keyword arguments)
Closes Rethink mangling #1173 (or at least, changes the situation drastically)

This pull request overhauls mangling such that HySymbols remember their original names (it is no longer the case that (= 'a-b 'a_b)) and all Python variables have names that are legal Python identifiers. The latter accomplishes most of what we need to ensure that hy2py produces real Python code.

The major changes are:

Mangling happens at compile-time, not read-time.
Mangling rules are different.
- *foo* → FOO and foo! → foo_bang are gone.
  - The first one seems misleading, and the second rarely useful.
- The Punycode transformation (which was only enabled for Python 2 anyway) is gone.
- foo-bar → foo_bar and foo? → is_foo remain.
- Names that still aren't legal Python identifiers (either because they contain illegal characters or because they coincide with a Python reserved word) are prepended with hyx_, and each illegal character is replaced with ΔfooΔ (or for Python 2, XfooX), where foo is the Unicode character name in lowercase with spaces replaced by underscores and hyphens replaced by "H"es. If the character doesn't have a name, we use "U" followed by its code point in lowercase hexadecimal.
- Any added hyx_ or is_ is added after any leading underscores (because leading underscores can have magical effects in Python).
The names of keyword arguments are fully mangled.

The below things remain to be done, but I figured I'd get feeback before continuing.

Watch out if the REPL variable _ is confused with the shadow subtraction operator -
~~In the unmangle function, undo hyx_ when necessary; otherwise, the distinct variables "hyx_X61X" and "a" will both unmangle to "a"~~ Not worth it.
Perhaps make the mangle function stringify the input, so the caller doesn't have to
Rename the mangling functions mangle and unmangle and add them to core (fixes add mangle function to public api #1336)
- Testing
- Documentation
Document mangling in general
Use unmangle instead of hyify in core
NEWS

vodik · 2018-02-26T20:13:22Z

hy/_compat.py

+    if x.rstrip() != x:
+        return False
+    import tokenize as T
+    from StringIO import StringIO


You should prefer io.StringIO/io.BytesIO over the StringIO package

This means that a HySymbol remembers its original name. That is, `a-b` and `a_b` are different symbols although `(setv a-b 1)` and `(setv a_b 1)` set the same variable (namely, `a_b`). Most of the edits in this commit are to switch underscores to hyphens in places where mangling hasn't happened yet. I removed some lexer tests since the lexer no longer does any mangling.

`_`, as a variable, is now the shadow subtraction operator.

gilch · 2018-03-13T19:29:48Z

This really should have had a link to #1458 where much of the original discussion happened.

One-approval merges should say so here in the conversation, for the record.
As a courtesy I'd like to ask that you give a one day warning before such merges. I'd have requested that in the original rule change, but never got the chance.

Now that we've got the mangling makeover format settled, it's time to rethink the gensym format as discussed in #1458. I'd have mentioned this in time for the merge had I gotten warning.

Kodiologist · 2018-03-13T20:13:00Z

This really should have had a link to #1458 where much of the original discussion happened.

Fair enough.

One-approval merges should say so here in the conversation, for the record.

It isn't hard to figure out if something was validly merged (and, if so, under what rules it was merged), is it?

As a courtesy I'd like to ask that you give a one day warning before such merges.

But the creation of a PR is itself a 14-day warning. In theory, if you wanted a 1-day warning, you could set an alarm for yourself for 13 days hence (we could even write a bot that does this automatically; in Hy, no less), but why wait to do whatever it is you were going to do?

Now that we've got the mangling makeover format settled, it's time to rethink the gensym format as discussed in #1458. I'd have mentioned this in time for the merge had I gotten warning.

No worries, that can be done in a new issue or PR. This PR is already big, anyway.

Kodiologist · 2018-03-13T20:14:14Z

we could even write a bot that does this automatically

Come to think of it, this could be useful as a reminder to PR authors to check that the PR is in a mergeworthy state.

vodik · 2018-03-13T22:33:06Z

Heads up, we'll have to be careful with this at release time. I found I had to flush all my compiled pyc files.

Kodiologist · 2018-03-13T22:37:48Z

"Be careful" in the sense of warning users to remove stale bytecode? NEWS could be a good place to put such a warning.

vodik · 2018-03-14T19:39:01Z

Yes.

gilch · 2018-03-14T21:43:00Z

Shouldn't the installer replace all the stale .pyc's automatically? It's fine to warn users about the issue #1324 since it will come up in their own development. But we shouldn't ask the users to do an extra installation step that we could have automated.

vodik · 2018-03-14T22:00:33Z

Well, its not only about Hy, right?

Lets say I package Hy for Arch Linux (which there exists a package in AUR), and some other useful Hy library/tool (honestly don't know of any, but for examples sake, suppose there is).

Because the best practice for packaging Python like this is to also package the .pyc files, the eventual update to Hy to 0.15 would also mean all other Hy packages have to be rebuilt.

Not the worst thing in the world to require, but its a kind of change that really needs to be announced.

I'll put something up for adding it to the NEWS file tonight.

vodik · 2018-03-24T23:45:32Z

tests/native_tests/mangling.hy

+  (setv out (eval `(do
+    (setv ~sym 10)
+    [foo? is_foo])))
+  (assert (= out [10 10])))


@Kodiologist what's the rational for late mangling?

Just asking because its easier to make #1504 nice if I mangle symbols before comparing:

(defmacro foo [symbol] (if (= symbol 'foobar?) ... ...))

Means this macro work on either 'foobar? and is_foobar. Not sure if there's something I'm missing, so asking.

So that you have access to the literal symbol that was entered, in case you want it. If you want to compare things post-mangling, you can always write code like (= (mangle symbol) (mangle 'foobar?)). By contrast, before this PR, when symbols were mangled at read-time, there was no way to tell whether the user entered foobar? or is_foobar, if you wanted to do that. After all, not every symbol ends up as a Python name, so not every symbol eventually needs to be mangled.

The specific feature request was #360.

ekaschalk · 2018-06-06T19:43:22Z

The new mangle has different behavior on the empty string, namely an assertion error whereas before it returned back the empty string.

Is there a reasoning for this? Note that unmangle on an empty string doesn't fail, returning back an empty string.

If not, the blank string assertion error would be helpful to have dropped.

Kodiologist · 2018-06-06T20:12:19Z

I'm not sure what unmangle should do with an empty string, but mangle errors on an empty string because it's supposed to always return a valid Python identifier, and we have no mangling rule to produce a Python identifier for the empty symbol.

Kodiologist requested a review from refi64 February 26, 2018 19:34

vodik approved these changes Feb 26, 2018

View reviewed changes

Kodiologist force-pushed the mangling-makeover branch 4 times, most recently from 2c543a8 to 027dc47 Compare March 5, 2018 00:47

Kodiologist added 14 commits March 13, 2018 14:55

Overhaul mangling rules

52edad2

Mangle names that coincide with Python keywords

d252bb0

Mangle keyword arguments

0c816f2

Mangle macro names

ccb3ba6

Mangle special forms

0c8c5dc

Remove an obsolete test

ebc9bda

Add mangle and unmangle as core functions

85968e7

Use *1 instead of _ for REPL history

3c97d29

`_`, as a variable, is now the shadow subtraction operator.

Replace hyify with unmangle

6875ae0

Spin off syntax documentation from api.rst

4d77dd0

Document mangling

eda0b89

Update NEWS

ca06294

Use io.StringIO instead of the StringIO module

4c5dea0

Kodiologist force-pushed the mangling-makeover branch from 027dc47 to 4c5dea0 Compare March 13, 2018 19:00

Kodiologist merged commit b023ebd into hylang:master Mar 13, 2018

gilch mentioned this pull request Mar 13, 2018

Rethink gensym format #1531

Closed

This was referenced Mar 22, 2018

Add more recent repl history globals #1341

Closed

No implicit conversions between HySymbol and HyKeyword and str #1504

Closed

vodik reviewed Mar 24, 2018

View reviewed changes

This was referenced Apr 25, 2018

hy2py produces symbols not parsable by python #1597

Closed

hy2py syntax error with toolz.partial #1600

Closed

Kodiologist mentioned this pull request Jun 28, 2018

Suggestion on let reimplementation #1648

Closed

berquist mentioned this pull request Dec 26, 2018

Remove outdated reference to earmuffs in docs #1714

Closed

Kodiologist deleted the mangling-makeover branch January 20, 2019 14:42

Kodiologist restored the mangling-makeover branch January 20, 2019 14:42

Kodiologist deleted the mangling-makeover branch January 20, 2019 14:44

Kodiologist mentioned this pull request Jan 18, 2021

Add clojure style destucturing #1940

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Mangling makeover #1517

Mangling makeover #1517

Kodiologist commented Feb 26, 2018 •

edited

Loading

vodik Feb 26, 2018

Kodiologist Mar 5, 2018

gilch commented Mar 13, 2018

Kodiologist commented Mar 13, 2018

Kodiologist commented Mar 13, 2018

vodik commented Mar 13, 2018

Kodiologist commented Mar 13, 2018

vodik commented Mar 14, 2018

gilch commented Mar 14, 2018

vodik commented Mar 14, 2018

vodik Mar 24, 2018

Kodiologist Mar 24, 2018

Kodiologist Mar 24, 2018

vodik Mar 25, 2018

ekaschalk commented Jun 6, 2018

Kodiologist commented Jun 6, 2018

Mangling makeover #1517

Mangling makeover #1517

Conversation

Kodiologist commented Feb 26, 2018 • edited Loading

vodik Feb 26, 2018

Choose a reason for hiding this comment

Kodiologist Mar 5, 2018

Choose a reason for hiding this comment

gilch commented Mar 13, 2018

Kodiologist commented Mar 13, 2018

Kodiologist commented Mar 13, 2018

vodik commented Mar 13, 2018

Kodiologist commented Mar 13, 2018

vodik commented Mar 14, 2018

gilch commented Mar 14, 2018

vodik commented Mar 14, 2018

vodik Mar 24, 2018

Choose a reason for hiding this comment

Kodiologist Mar 24, 2018

Choose a reason for hiding this comment

Kodiologist Mar 24, 2018

Choose a reason for hiding this comment

vodik Mar 25, 2018

Choose a reason for hiding this comment

ekaschalk commented Jun 6, 2018

Kodiologist commented Jun 6, 2018

Kodiologist commented Feb 26, 2018 •

edited

Loading