[GR-13908] [GR-13738] Function defaults and kwdefaults should be writeable.

PullRequest: graalpython/406

Author: fangerer

graalpython/com.oracle.graal.python.shell/src/com/oracle/graal/python/shell/GraalPythonMain.java

@@ -71,6 +71,7 @@ public static void main(String[] args) {
     private boolean noSite = false;
     private boolean stdinIsInteractive = System.console() != null;
     private boolean runLLI = false;
+    private boolean unbufferedIO = false;
     private VersionAction versionAction = VersionAction.None;
     private String sulongLibraryPath = null;
     private List<String> givenArguments;
@@ -89,7 +90,6 @@ protected List<String> preprocessArguments(List<String> givenArgs, Map<String, S
             String arg = arguments.get(i);
             switch (arg) {
                 case "-B":
-                    System.out.println("Warning: " + arg + " does nothing on GraalPython.");
                     break;
                 case "-c":
                     i += 1;
@@ -122,7 +122,7 @@ protected List<String> preprocessArguments(List<String> givenArgs, Map<String, S
                     break;
                 case "-O":
                 case "-OO":
-                    System.out.println("Warning: " + arg + " does nothing on GraalPython.");
+                case "-R":
                     break;
                 case "-q":
                     quietFlag = true;
@@ -184,15 +184,20 @@ protected List<String> preprocessArguments(List<String> givenArgs, Map<String, S
                     inputArgs.remove("-compile-truffle-immediately");
                     break;
                 case "-u":
-                    // TODO we currently don't support this option, but needs to be consumed
-                    // due pip/wheel installer.
+                    unbufferedIO = true;
                     break;
                 default:
                     if (!arg.startsWith("-")) {
                         inputFile = arg;
                         programArgs.add(inputFile);
                         break;
+                    } else if (!arg.startsWith("--") && arg.length() > 2) {
+                        // short arguments can be given together
+                        for (String optionChar : arg.substring(1).split("")) {
+                            arguments.add(i + 1, "-" + optionChar);
+                        }
                     } else {
+                        // possibly a polyglot argument
                         unrecognized.add(arg);
                     }
             }
@@ -340,6 +345,7 @@ protected void launch(Builder contextBuilder) {
             inspectFlag = inspectFlag || System.getenv("PYTHONINSPECT") != null;
             noUserSite = noUserSite || System.getenv("PYTHONNOUSERSITE") != null;
             verboseFlag = verboseFlag || System.getenv("PYTHONVERBOSE") != null;
+            unbufferedIO = unbufferedIO || System.getenv("PYTHONUNBUFFERED") != null;
         }
 
         // The unlikely separator is used because options need to be strings. See
@@ -359,6 +365,7 @@ protected void launch(Builder contextBuilder) {
         contextBuilder.option("python.NoUserSiteFlag", Boolean.toString(noUserSite));
         contextBuilder.option("python.NoSiteFlag", Boolean.toString(noSite));
         contextBuilder.option("python.IgnoreEnvironmentFlag", Boolean.toString(ignoreEnv));
+        contextBuilder.option("python.UnbufferedIO", Boolean.toString(unbufferedIO));
 
         sulongLibraryPath = System.getenv("SULONG_LIBRARY_PATH");
         if (sulongLibraryPath != null) {
@@ -492,29 +499,31 @@ protected String getLanguageId() {
     protected void printHelp(OptionCategory maxCategory) {
         print("usage: python [option] ... (-c cmd | file) [arg] ...\n" +
                         "Options and arguments (and corresponding environment variables):\n" +
-                        "-B     : don't write .py[co] files on import; also PYTHONDONTWRITEBYTECODE=x\n" +
+                        "-B     : on CPython, this disables writing .py[co] files on import;\n" +
+                        "         GraalPython does not use bytecode, and thus this flag has no effect\n" +
                         "-c cmd : program passed in as string (terminates option list)\n" +
                         // "-d : debug output from parser; also PYTHONDEBUG=x\n" +
                         "-E     : ignore PYTHON* environment variables (such as PYTHONPATH)\n" +
                         "-h     : print this help message and exit (also --help)\n" +
                         "-i     : inspect interactively after running script; forces a prompt even\n" +
                         "         if stdin does not appear to be a terminal; also PYTHONINSPECT=x\n" +
                         "-m mod : run library module as a script (terminates option list)\n" +
-                        "-O     : optimize generated bytecode slightly; also PYTHONOPTIMIZE=x\n" +
-                        "-OO    : remove doc-strings in addition to the -O optimizations\n" +
-                        // "-R : use a pseudo-random salt to make hash() values of various types
-                        // be\n" +
-                        // " unpredictable between separate invocations of the interpreter, as\n" +
-                        // " a defense against denial-of-service attacks\n" +
+                        "-O     : on CPython, this optimizes generated bytecode slightly;\n" +
+                        "         GraalPython does not use bytecode, and thus this flag has no effect\n" +
+                        "-OO    : remove doc-strings in addition to the -O optimizations;\n" +
+                        "         GraalPython does not use bytecode, and thus this flag has no effect\n" +
+                        "-R     : on CPython, this enables the use of a pseudo-random salt to make\n" +
+                        "         hash()values of various types be unpredictable between separate\n" +
+                        "         invocations of the interpreter, as a defense against denial-of-service\n" +
+                        "         attacks; GraalPython always enables this and the flag has no effect.\n" +
                         // "-Q arg : division options: -Qold (default), -Qwarn, -Qwarnall, -Qnew\n"
                         // +
                         "-q     : don't print version and copyright messages on interactive startup\n" +
                         "-s     : don't add user site directory to sys.path; also PYTHONNOUSERSITE\n" +
                         "-S     : don't imply 'import site' on initialization\n" +
                         // "-t : issue warnings about inconsistent tab usage (-tt: issue errors)\n"
                         // +
-                        // "-u : unbuffered binary stdout and stderr; also PYTHONUNBUFFERED=x\n" +
-                        // " see man page for details on internal buffering relating to '-u'\n" +
+                        "-u     : unbuffered binary stdout and stderr; also PYTHONUNBUFFERED=x\n" +
                         "-v     : verbose (trace import statements); also PYTHONVERBOSE=x\n" +
                         "         can be supplied multiple times to increase verbosity\n" +
                         "-V     : print the Python version number and exit (also --version)\n" +

graalpython/com.oracle.graal.python.test/src/graalpytest.py

@@ -201,7 +201,7 @@ def __enter__(self):
 
         def __exit__(self, exc_type, exc, traceback):
             if not exc_type:
-                assert False, "expected '%r' to raise '%r'" % (self.function, exc_type)
+                assert False, "expected '%r' to raise '%r'" % (self.function, self.exc_type)
             elif self.exc_type in exc_type.mro():
                 self.exception = exc
                 return True

graalpython/com.oracle.graal.python.test/src/tests/test_functions.py

@@ -1,4 +1,4 @@
-# Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+# Copyright (c) 2018, 2019, Oracle and/or its affiliates. All rights reserved.
 # DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
 #
 # The Universal Permissive License (UPL), Version 1.0
@@ -105,6 +105,134 @@ def test_constructor():
     import types
     func_copy = types.FunctionType(f.__code__, f.__globals__, f.__name__, f.__defaults__, f.__closure__)
 
-    assert func_copy(1, 2) == (1, 2, 10, (), {})
-    assert func_copy(1, 2, 3) == (1, 2, 3, (), {})
-    assert func_copy(1, 2, 3, 4, 5, x=2) == (1, 2, 3, (4, 5), {'x': 2})
+    assert func_copy(1, 2) == (1, 2, 10, (), {}), func_copy(1, 2)
+    assert func_copy(1, 2, 3) == (1, 2, 3, (), {}), func_copy(1, 2, 3)
+    assert func_copy(1, 2, 3, 4, 5, x=2) == (1, 2, 3, (4, 5), {'x': 2}), func_copy(1, 2, 3, 4, 5, x=2)
+
+
+def test_inner_function_with_defaults():
+    def make_func(sep):
+        def inner(sep=sep):
+            return sep
+        return inner
+
+    inner_a = make_func(",")
+    inner_b = make_func("\t")
+    assert inner_b() == "\t"
+    assert inner_a() == ","
+
+
+def test_inner_function_with_closure():
+    def make_func(sep):
+        def inner():
+            return sep
+        return inner
+
+    inner_a = make_func(",")
+    inner_b = make_func("\t")
+    assert inner_a() == ","
+    assert inner_b() == "\t"
+
+
+def test_inner_generator_with_defaults():
+    def make_func(sep):
+        def inner(sep=sep):
+            yield sep
+        return inner
+
+    inner_a = make_func(",")()
+    inner_b = make_func("\t")()
+    assert next(inner_b) == "\t"
+    assert next(inner_a) == ","
+
+
+def test_inner_generator_with_closure():
+    def make_func(sep):
+        closure_value = sep
+        def inner():
+            yield closure_value
+        return inner
+
+    inner_a = make_func(",")()
+    inner_b = make_func("\t")()
+    assert next(inner_b) == "\t"
+    assert next(inner_a) == ","
+
+
+def test_function_changes_defaults():
+    def foo(a):
+        return a
+
+    assert foo.__defaults__ is None
+    assert foo.__kwdefaults__ is None
+    assert_raises(TypeError, foo)
+
+    foo.__defaults__ = (1,)
+    assert foo() == 1
+
+    foo.__kwdefaults__ = {"a": 12}
+    assert foo() == 1
+
+    foo.__defaults__ = None
+    assert_raises(TypeError, foo)
+
+
+def test_function_changes_kwdefaults():
+    def foo(*args, x=1):
+        return x
+
+    assert foo.__defaults__ is None
+    assert foo.__kwdefaults__ == {"x": 1}
+    assert foo() == 1
+
+    foo.__kwdefaults__ = {"x": 32}
+    assert foo() == 32
+
+    foo.__kwdefaults__ = None
+    assert_raises(TypeError, foo)
+
+
+def test_code_change():
+    def foo():
+        return "foo"
+
+    def bar(a):
+        return "bar" + str(a)
+
+    assert foo() == "foo"
+    foo.__code__ = bar.__code__
+    assert foo(1) == "bar1"
+    assert_raises(TypeError, foo)
+
+
+def test_code_marshal_with_freevars():
+    import marshal
+    def foo():
+        x,y = 1,2
+        def bar():
+            return x,y
+        return bar
+
+    def baz():
+        x,y = 2,3
+        def bar():
+            return y,x
+        return bar
+
+    foobar_str = marshal.dumps(foo().__code__)
+    foobar_code = marshal.loads(foobar_str)
+    assert_raises(TypeError, exec, foobar_code)
+
+    bazbar = baz()
+    assert bazbar() == (3,2)
+
+    def assign_code(x, y):
+        if isinstance(y, type(assign_code)):
+            x.__code__ = y.__code__
+        else:
+            x.__code__ = y
+
+    assert_raises(ValueError, assign_code, foo, bazbar)
+    assert_raises(ValueError, assign_code, foo, foobar_code)
+    bazbar.__code__ = foobar_code
+    assert bazbar() == (2,3)

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/PythonLanguage.java

@@ -42,7 +42,6 @@
 import com.oracle.graal.python.builtins.objects.function.PArguments;
 import com.oracle.graal.python.builtins.objects.function.PBuiltinFunction;
 import com.oracle.graal.python.builtins.objects.function.PFunction;
-import com.oracle.graal.python.builtins.objects.function.PKeyword;
 import com.oracle.graal.python.builtins.objects.method.PBuiltinMethod;
 import com.oracle.graal.python.builtins.objects.method.PMethod;
 import com.oracle.graal.python.builtins.objects.module.PythonModule;
@@ -446,7 +445,7 @@ protected String toString(PythonContext context, Object value) {
         Object[] userArgs = PArguments.create(2);
         PArguments.setArgument(userArgs, 0, PNone.NONE);
         PArguments.setArgument(userArgs, 1, value);
-        Object res = InvokeNode.create(reprMethod).execute(null, userArgs, PKeyword.EMPTY_KEYWORDS);
+        Object res = InvokeNode.create(reprMethod).execute(null, userArgs);
         return res.toString();
     }
 

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/Builtin.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2017, 2018, Oracle and/or its affiliates.
+ * Copyright (c) 2017, 2019, Oracle and/or its affiliates.
  * Copyright (c) 2013, Regents of the University of California
  *
  * All rights reserved.
@@ -39,23 +39,23 @@
 
     PythonBuiltinClassType[] base() default {};
 
-    int fixedNumOfPositionalArgs() default 0;
-
     int minNumOfPositionalArgs() default 0;
 
-    int maxNumOfPositionalArgs() default 0;
+    int maxNumOfPositionalArgs() default -1;
 
     boolean isGetter() default false;
 
     boolean isSetter() default false;
 
     boolean takesVarArgs() default false;
 
+    boolean varArgsMarker() default false;
+
     boolean takesVarKeywordArgs() default false;
 
     String[] parameterNames() default {};
 
-    String[] keywordArguments() default {};
+    String[] keywordOnlyNames() default {};
 
     boolean isPublic() default true;
 

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/Python3Core.java

@@ -168,6 +168,7 @@
 import com.oracle.truffle.api.CompilerDirectives.CompilationFinal;
 import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
 import com.oracle.truffle.api.RootCallTarget;
+import com.oracle.truffle.api.Truffle;
 import com.oracle.truffle.api.TruffleFile;
 import com.oracle.truffle.api.TruffleLanguage.Env;
 import com.oracle.truffle.api.TruffleOptions;
@@ -629,7 +630,7 @@ private Source getSource(String basename, String prefix) {
 
     private void loadFile(String s, String prefix) {
         Source source = getSource(s, prefix);
-        Supplier<PCode> getCode = () -> factory.createCode((RootNode) getParser().parse(ParserMode.File, this, source, null));
+        Supplier<PCode> getCode = () -> factory.createCode(Truffle.getRuntime().createCallTarget((RootNode) getParser().parse(ParserMode.File, this, source, null)));
         RootCallTarget callTarget = getLanguage().cacheCode(source.getName(), getCode).getRootCallTarget();
         PythonModule mod = lookupBuiltinModule(s);
         if (mod == null) {

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/PythonBuiltins.java

@@ -28,7 +28,6 @@
 import static com.oracle.graal.python.nodes.SpecialAttributeNames.__DOC__;
 import static com.oracle.graal.python.nodes.SpecialMethodNames.__NEW__;
 
-import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
@@ -37,7 +36,6 @@
 import java.util.function.BiConsumer;
 
 import com.oracle.graal.python.builtins.objects.PNone;
-import com.oracle.graal.python.builtins.objects.function.Arity;
 import com.oracle.graal.python.builtins.objects.function.PBuiltinFunction;
 import com.oracle.graal.python.builtins.objects.type.PythonBuiltinClass;
 import com.oracle.graal.python.nodes.function.BuiltinFunctionRootNode;
@@ -79,14 +77,14 @@ public void initialize(PythonCore core) {
             Object builtinDoc = builtin.doc().isEmpty() ? PNone.NONE : builtin.doc();
             if (builtin.constructsClass().length > 0) {
                 assert !builtin.isGetter() && !builtin.isSetter() && !builtin.isClassmethod() && !builtin.isStaticmethod();
-                PBuiltinFunction newFunc = core.factory().createBuiltinFunction(__NEW__, null, createArity(builtin, declaresExplicitSelf), callTarget);
+                PBuiltinFunction newFunc = core.factory().createBuiltinFunction(__NEW__, null, numDefaults(builtin), callTarget);
                 for (PythonBuiltinClassType type : builtin.constructsClass()) {
                     PythonBuiltinClass builtinClass = core.lookupType(type);
                     builtinClass.setAttributeUnsafe(__NEW__, newFunc);
                     builtinClass.setAttribute(__DOC__, builtinDoc);
                 }
             } else {
-                PBuiltinFunction function = core.factory().createBuiltinFunction(builtin.name(), null, createArity(builtin, declaresExplicitSelf), callTarget);
+                PBuiltinFunction function = core.factory().createBuiltinFunction(builtin.name(), null, numDefaults(builtin), callTarget);
                 function.setAttribute(__DOC__, builtinDoc);
                 BoundBuiltinCallable<?> callable = function;
                 if (builtin.isGetter() || builtin.isSetter()) {
@@ -122,27 +120,14 @@ private void initializeEachFactoryWith(BiConsumer<NodeFactory<? extends PythonBu
         }
     }
 
-    private static Arity createArity(Builtin builtin, boolean declaresExplicitSelf) {
-        int minNumPosArgs = builtin.minNumOfPositionalArgs();
-        int maxNumPosArgs = Math.max(minNumPosArgs, builtin.maxNumOfPositionalArgs());
-        if (builtin.fixedNumOfPositionalArgs() > 0) {
-            minNumPosArgs = maxNumPosArgs = builtin.fixedNumOfPositionalArgs();
+    private static int numDefaults(Builtin builtin) {
+        String[] parameterNames = builtin.parameterNames();
+        int maxNumPosArgs = Math.max(builtin.minNumOfPositionalArgs(), parameterNames.length);
+        if (builtin.maxNumOfPositionalArgs() >= 0) {
+            maxNumPosArgs = builtin.maxNumOfPositionalArgs();
+            assert parameterNames.length == 0 : "either give all parameter names explicitly, or define the max number: " + builtin.name();
         }
-        if (!declaresExplicitSelf) {
-            // if we don't take the explicit self, we still need to accept it by arity
-            minNumPosArgs++;
-            maxNumPosArgs++;
-        }
-
-        List<Arity.KeywordName> keywordNames = new ArrayList<>();
-        for (String keywordArgument : builtin.keywordArguments()) {
-            keywordNames.add(new Arity.KeywordName(keywordArgument));
-            // the assumption here is that out Builtins do not take required keyword args,
-            // all supplied keyword args are before *args, **kwargs
-            maxNumPosArgs++;
-        }
-
-        return new Arity(builtin.name(), minNumPosArgs, maxNumPosArgs, builtin.takesVarKeywordArgs(), builtin.takesVarArgs(), builtin.parameterNames(), keywordNames.toArray(new Arity.KeywordName[0]));
+        return maxNumPosArgs - builtin.minNumOfPositionalArgs();
     }
 
     private void setBuiltinFunction(String name, BoundBuiltinCallable<?> function) {