Repository: python/mypy
Pull request: [mypyc] always add implicit None return type to __init__ method
### Description

Fixes https://github.com/mypyc/mypyc/issues/792.

Always give `__init__` an implicit `None` return type so that programs like
```
class C:
    def __init__(self):  # missing `-> None` annotation
        pass
```
compile.

## Test Plan

Included a test program that doesn't compile with the existing application code.

Problem statement:
C compilation fails if '__init__' has no return type
This program doesn't compile:

```py
class C:
    def __init__(self):
        pass
```

Here's the output:

```
...
x86_64-linux-gnu-gcc -pthread -Wno-unused-result -Wsign-compare -DNDEBUG -g -fwrapv -O2 -Wall -g -fstack-protector-strong -Wformat -Werror=format-security -g -fwrapv -O2 -g -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -fPIC -I/home/jukka/src/mypy/mypyc/lib-rt -I/home/jukka/venv/mypy/include -I/usr/include/python3.8 -c build/__native.c -o build/temp.linux-x86_64-3.8/build/__native.o -O3 -Werror -Wno-unused-function -Wno-unused-label -Wno-unreachable-code -Wno-unused-variable -Wno-unused-command-line-argument -Wno-unknown-warning-option -Wno-unused-but-set-variable
build/__native.c: In function ‘CPyDef_C’:
build/__native.c:111:16: error: initialization of ‘char’ from ‘PyObject *’ {aka ‘struct _object *’} makes integer from pointer without a cast [-Werror=int-conversion]
  111 |     char res = CPyDef_C_____init__(self);
      |                ^~~~~~~~~~~~~~~~~~~
...
```

Maybe we should always give `__init__` an implicit `None` return type.

Hints from issue discussion:
I'd like to give this one a try. I think a special case for `__init__` in `mypyc.irbuild.mapper.py` could be a way to solve this.

Commit message: add FuncDef.is_module_level_func and don't apply implicit `-> None` annotation to module-level __init__ functions

Patch to review:
diff --git a/mypy/nodes.py b/mypy/nodes.py
index 0571788bf002..c768c28bc1ba 100644
--- a/mypy/nodes.py
+++ b/mypy/nodes.py
@@ -683,6 +683,10 @@ def __init__(self,
     def name(self) -> str:
         return self._name
 
+    @property
+    def is_module_level_func(self) -> bool:
+        return self.info == FUNC_NO_INFO
+
     def accept(self, visitor: StatementVisitor[T]) -> T:
         return visitor.visit_func_def(self)
 
diff --git a/mypy/suggestions.py b/mypy/suggestions.py
index b66ba6d6118d..8e50cfe1a760 100644
--- a/mypy/suggestions.py
+++ b/mypy/suggestions.py
@@ -437,7 +437,7 @@ def get_suggestion(self, mod: str, node: FuncDef) -> PyAnnotateSignature:
         if self.no_errors and orig_errors:
             raise SuggestionFailure("Function does not typecheck.")
 
-        is_method = bool(node.info) and not node.is_static
+        is_method = not node.is_module_level_func and not node.is_static
 
         with strict_optional_set(graph[mod].options.strict_optional):
             guesses = self.get_guesses(
diff --git a/mypyc/irbuild/mapper.py b/mypyc/irbuild/mapper.py
index 364e650aa5dc..76158b48c7fb 100644
--- a/mypyc/irbuild/mapper.py
+++ b/mypyc/irbuild/mapper.py
@@ -132,7 +132,12 @@ def fdef_to_sig(self, fdef: FuncDef) -> FuncSignature:
         else:
             # Handle unannotated functions
             arg_types = [object_rprimitive for arg in fdef.arguments]
-            ret = object_rprimitive
+            # We at least know the return type for __init__ methods will be None.
+            is_init_method = fdef.name == '__init__' and not fdef.is_module_level_func
+            if is_init_method:
+                ret = none_rprimitive
+            else:
+                ret = object_rprimitive
 
         args = [RuntimeArg(arg_name, arg_type, arg_kind)
                 for arg_name, arg_kind, arg_type in zip(fdef.arg_names, fdef.arg_kinds, arg_types)]
diff --git a/mypyc/test-data/run-classes.test b/mypyc/test-data/run-classes.test
index 273fd18d5c3f..a900e6364d1f 100644
--- a/mypyc/test-data/run-classes.test
+++ b/mypyc/test-data/run-classes.test
@@ -374,6 +374,11 @@ o = object()
 c1.d = o
 assert c1.d is o
 
+[case testInitMethodWithMissingNoneReturnAnnotation]
+class C:
+    def __init__(self):
+        pass
+
 [case testConstructClassWithDefaultConstructor]
 class C:
     a: int
diff --git a/mypyc/test-data/run-functions.test b/mypyc/test-data/run-functions.test
index 5dd5face6b0e..5e2f4a4ba17d 100644
--- a/mypyc/test-data/run-functions.test
+++ b/mypyc/test-data/run-functions.test
@@ -928,6 +928,15 @@ def f(x):
 from native import f
 assert f(3) == 6
 
+[case testUnannotatedModuleLevelInitFunction]
+# Ensure that adding an implicit `-> None` annotation only applies to `__init__`
+# _methods_ specifically (not module-level `__init__` functions).
+def __init__():
+    return 42
+[file driver.py]
+from native import __init__
+assert __init__() == 42
+
 [case testComplicatedArgs]
 from typing import Tuple, Dict
 


Code review comments:
diff_hunk: @@ -132,7 +132,12 @@ def fdef_to_sig(self, fdef: FuncDef) -> FuncSignature:
         else:
             # Handle unannotated functions
             arg_types = [object_rprimitive for arg in fdef.arguments]
-            ret = object_rprimitive
+            # We at least know the return type for __init__ methods will be None.
+            is_init_method = fdef.name == '__init__' and not fdef.is_module_level_func
path: mypyc/irbuild/mapper.py
text: @user1:
I think that you can just use `... and bool(fdef.info)`. Your suggestion to use a property is reasonable, but we have many checks where we do the equivalent of `if fdef.info` so it would add some inconsistency.

@author:
Okay thank you that makes sense. Applying this update and the suggestions above now.

diff_hunk: @@ -683,6 +683,10 @@ def __init__(self,
     def name(self) -> str:
         return self._name
 
+    @property
+    def is_module_level_func(self) -> bool:
+        return self.info == FUNC_NO_INFO
path: mypy/nodes.py
text: @author:
@user1 my instinct was to "hide" the use of `FUNC_NO_INFO`. I'm not sure if that was a good instinct.

@user1:
Actually I think that this is unnecessary (see below).

diff_hunk: @@ -437,7 +437,7 @@ def get_suggestion(self, mod: str, node: FuncDef) -> PyAnnotateSignature:
         if self.no_errors and orig_errors:
             raise SuggestionFailure("Function does not typecheck.")
 
-        is_method = bool(node.info) and not node.is_static
+        is_method = not node.is_module_level_func and not node.is_static
path: mypy/suggestions.py
text: @user1:
Let's leave this as it was, since using an `info` attribute as a boolean is a common idiom.