Get rid of the __init__ return suppression automagic and replace with a definit macro

[gilch]

This kind of automagic makes me uncomfortable. It's also complicating the compiler and seems to be the cause of #1533.

Python has a very dynamic runtime. You can rewrite existing classes on the fly because they're objects too. You can introducing new methods and delete old ones. Metaclasses may need to do this kind of rewriting internally to function.

But the __init__ automagic to return None only works in a defclass form. Not if __init__ was declared as a function outside and the attached later.

Worse, if some metaclass other than type requires an __init__ method to actually return something, it may be impossible to use in Hy.

I'm also worried this may cause issues with decorators. Does the automagic even work if __init__ is decorated? And even worse, what if does work, but the decorator requires a return value in the decorated method? It would be difficult to use that in Hy.

I was opposed to this automagic from the start, but we did add this for a reason. Usually automatic returns are more convenient in Hy. But that's an error in __init__ so it's a pain to remember to always end __init__ methods with None in Hy (if the last form happens to return None, as many do, you might not notice the problem right away).

Explicit is better than implicit. Let's get rid of the automagic and just use a simple core definit macro instead, like this,

(defmacro definit [argslist &rest body]
  `(defn __init__ ~argslist ~@body None))

Which you'd use like

(defclass Foo []
  (definit [self x]
    (setv self.x x))

It returns None as required by the standard type metaclass, and definit is easier to type, than def __init__ so it'll get used, instead of forgotten.

[Kodiologist]

I share your discomfort with __init__ rewriting, at least to some degree, but I feel like it wouldn't be much easier to remember to use definit than to include the explicit None.

[vodik]

I did start working on #1482, musing outloud, I wonder if a metadata attribute could help here:

(defmacro definit [argslist &rest body]
  `(defn ^:noreturn __init__ ~argslist ~@body))

[gilch]

@vodik I like your thinking, but that's even harder to remember and longer to type the noreturn explicitly. Wrapping it in a definit macro like that seems equivalent to my original proposal.

In almost all the cases when you'd want to suppress the auto return, you could explicitly return None instead. The main exception is generators in Python 2. A metadata option like that would have been a great fit, I think. But it changed in Python 3. I think a return None at the end is equivalent now. So I don't think we need this ability in the compiler anymore.

[gilch]

wouldn't be much easier to remember to use definit than to include the explicit None.

@Kodiologist I think putting it first instead of last helps a lot, mentally, especially if it's easy to form a habit. Programmers have limited working memory. You have to think hierarchically and do small pieces at a time.

The hardest part of writing a method, in terms of cognitive load, is usually the body. When you're thinking at the "method body" level, you're not thinking at the "class outline" level anymore. Remembering to add None at the end of the body, as a finishing touch, after you've already offloaded your "class-level" cache, is probably the hardest time to do it. By comparison, figuring out that you have to write an __init__ method is easy. If your habit in Hy is to write a definit method, you'll think of that first, in the "class level" and not have to worry about remembering it while you write the body.

But if you've got a better idea for how to do this without automagic, I'd like to hear it. I do think the explicit None is less bad than the current automagic. But it is still bad, which is why we tried to fix it with the automagic in the first place.

[Kodiologist]

I don't have any better ideas, no. Although, I guess it's not a big problem if you forget the None or the definit, because you'll get a clear error message (TypeError: __init__() should return None) when you try to instantiate the class.

[gilch]

Worse, if some metaclass other than type requires an init method to actually return something, it may be impossible to use in Hy.
...
And even worse, what if does work, but the decorator requires a return value in the decorated method? It would be difficult to use that in Hy.

Just use an explicit (return). We have those now, but not when I initially objected to the automagic. Remembering to do this would still be a pain, but these are not impossible cases anymore.

The existence of an early return form means the macro is probably better rendered as

(defmacro definit [argslist &rest body]
  `(defn __init__ ~argslist ~@body (return)))

I'm still uncomfortable with the automagic for the other reasons though, like decorators and metaclasses. Look at this:

=> (defclass Foo []
... #@(identity
...    (defn __init__ [self]
...     2)))

compiles to

class Foo:

    @identity
    def __init__(self):
        return 2

The return wasn't suppressed. Is that expected? Good? Bad? I don't even know. Automagic is too confusing.

[gilch]

it's not a big problem if you forget the None or the definit, because you'll get a pretty clear error message

Yeah, I guess it's just not that bad even without any solution. It's easy to find and fix. It's just annoying when you forget. I do think it's worth a macro (because of the aforementioned cognitive load), but I'd approve just removing the automagic then.

Another option might be a defmethod macro, or something, which requires explicit (return)s. We could call it something shorter, like def.

(defmacro def [name argslist &rest body]
  `(defn ~name ~argslist ~@body (return)))

I really don't like this. There should be one-- and preferably only one --obvious way to do it. I'm just throwing that out there in case it inspires a better idea.

[vodik]

Just a note, I was hoping to bring def back as a way to declare a variable with type annotations/metadata.

[vodik]

I don't know if this is a big problem to worry about. 90% of __init__, at least in my cases, is setting attributes on self. setv returns None already. What else do people practically do?

Maybe the rewrite can be dropped any nobody will even notice 😉

[gilch]

hoping to bring def back as a way to declare a variable with type annotations/metadata.

That was my original plan, and I still support it. @Kodiologist suggested renaming defn to def to make switching between Python and Hy easier. I think that's nothing compared to putting the parenthesis in the wrong place and putting commas in when you shouldn't and getting a completely different symbol (e.g. foo is not foo,)--or forgetting to. In that case we'd probably use annv for annotations, since that's what it's called in the ast. But def is used for declarations in Clojure, and that's closer to what it originally meant in Hy.

setv returns None already

It didn't always. Maybe we wouldn't have bothered with the automagic if it had. But I'm not sure if that makes it better, since people will forget they always have to (return) or end with None explicitly. It's harder to build a habit if it's only sometimes. definit would be all the time, so you're not bothered when your last form happens not to return None on occasion.

What else do people practically do?

You can configure self in __init__ using doto and configuration methods. doto would return self in that case, which is not what we want as the auto-return form. But I do think it's better that doto itself return what it's configuring, since it's used in more than just __init__.

[vodik]

Guess the doto problem might not be a problem with hylang/hyrule#35? Though it probably still makes sense for that to have a result.

[allison-casey]

@Kodiologist with #2077 rewire_init has gotten even more complicated and i'm wondering how you feel about this now given our current refactor to do less magic in Hy. I tried deleting the init return suppression and it broke (besides the specific implicit-none test itself) no tests, of which include 53 defclass's 🤷‍♀️ It feels like the majority of the time you're not going to run into this issue unless you're doing something tricky and even if you do python's syntax error is pretty specific about whats happening so im leaning to just getting rid of it completely, but i want to know what you think

[gilch]

python's syntax error is pretty specific

This. I thought the magic had pretty dubious value to begin with (and especially now that we have (return)), not worth the cost of the extra complications it adds, but still, someone had to have been bothered enough by the error to create the magic in the first place. I thought a replacement would be easier to negotiate than a removal at the time, so that's the approach I took when starting this issue.

It seems like maybe Hy is moving in the direction of hugging Python closely, and moving conveniences elsewhere. This seems like probably the right approach to me, since it will become easier to make Hy itself stable that way. In that case, the definit would probably belong there, if anyone still cares to have it. That means removing the magic altogether in Hy proper.

[Kodiologist]

I tried deleting the init return suppression and it broke (besides the specific implicit-none test itself) no tests, of which include 53 defclass

That's surprising to me. I guess __init__s tend to end with setv forms, which always return None. If you have a decent-sized Hy codebase with defclasses, maybe try it with that, too. Overall, though, your suggestion makes sense.