Merge pull request #514 from tjhance/arg_unpacking

support arg unpacking

src/analysis/scoping_analysis.cpp

@@ -218,6 +218,7 @@ struct ScopingAnalysis::ScopeNameUsage {
     StrSet read;
     StrSet written;
     StrSet forced_globals;
+    StrSet params;
     std::vector<AST_Name*> del_name_nodes;
 
     // Properties determined by looking at other scopes as well:
@@ -439,14 +440,24 @@ private:
     };
 };
 
+static void raiseGlobalAndLocalException(InternedString name, AST* node) {
+    assert(node->type == AST_TYPE::FunctionDef);
+    AST_FunctionDef* funcNode = ast_cast<AST_FunctionDef>(node);
+    char buf[1024];
+    snprintf(buf, sizeof(buf), "name '%s' is local and global", name.c_str());
+    raiseSyntaxError(buf, funcNode->lineno, funcNode->col_offset, "" /* file?? */, funcNode->name.str());
+}
+
 class NameCollectorVisitor : public ASTVisitor {
 private:
     AST* orig_node;
     ScopingAnalysis::NameUsageMap* map;
     ScopingAnalysis::ScopeNameUsage* cur;
     ScopingAnalysis* scoping;
+    bool currently_visiting_functiondef_args;
+
     NameCollectorVisitor(AST* node, ScopingAnalysis::NameUsageMap* map, ScopingAnalysis* scoping)
-        : orig_node(node), map(map), scoping(scoping) {
+        : orig_node(node), map(map), scoping(scoping), currently_visiting_functiondef_args(false) {
         assert(map);
         cur = (*map)[node];
         assert(cur);
@@ -457,6 +468,9 @@ public:
         assert(name == mangleName(name, cur->private_name, scoping->getInternedStrings()));
         cur->read.insert(name);
         cur->written.insert(name);
+        if (this->currently_visiting_functiondef_args) {
+            cur->params.insert(name);
+        }
     }
 
     void doRead(InternedString name) {
@@ -549,6 +563,10 @@ public:
     bool visit_global(AST_Global* node) override {
         for (int i = 0; i < node->names.size(); i++) {
             mangleNameInPlace(node->names[i], cur->private_name, scoping->getInternedStrings());
+            if (cur->params.find(node->names[i]) != cur->params.end()) {
+                // Throw an exception if a name is both declared global and a parameter
+                raiseGlobalAndLocalException(node->names[i], this->orig_node);
+            }
             cur->forced_globals.insert(node->names[i]);
         }
         return true;
@@ -577,8 +595,17 @@ public:
 
     bool visit_functiondef(AST_FunctionDef* node) override {
         if (node == orig_node) {
-            for (AST_expr* e : node->args->args)
+            this->currently_visiting_functiondef_args = true;
+
+            int counter = 0;
+            for (AST_expr* e : node->args->args) {
+                if (e->type == AST_TYPE::Tuple) {
+                    doWrite(scoping->getInternedStrings().get("." + std::to_string(counter)));
+                }
+                counter++;
                 e->accept(this);
+            }
+
             if (node->args->vararg.str().size()) {
                 mangleNameInPlace(node->args->vararg, cur->private_name, scoping->getInternedStrings());
                 doWrite(node->args->vararg);
@@ -587,6 +614,9 @@ public:
                 mangleNameInPlace(node->args->kwarg, cur->private_name, scoping->getInternedStrings());
                 doWrite(node->args->kwarg);
             }
+
+            this->currently_visiting_functiondef_args = false;
+
             for (AST_stmt* s : node->body)
                 s->accept(this);
             return true;

src/codegen/irgen/hooks.cpp

@@ -45,7 +45,7 @@
 namespace pyston {
 
 // TODO terrible place for these!
-ParamNames::ParamNames(AST* ast) : takes_param_names(true) {
+ParamNames::ParamNames(AST* ast, InternedStringPool& pool) : takes_param_names(true) {
     if (ast->type == AST_TYPE::Module || ast->type == AST_TYPE::ClassDef || ast->type == AST_TYPE::Expression
         || ast->type == AST_TYPE::Suite) {
         kwarg = "";
@@ -58,7 +58,8 @@ ParamNames::ParamNames(AST* ast) : takes_param_names(true) {
             if (arg->type == AST_TYPE::Name) {
                 args.push_back(ast_cast<AST_Name>(arg)->id.str());
             } else {
-                args.push_back("." + std::to_string(i + 1));
+                InternedString dot_arg_name = pool.get("." + std::to_string(i));
+                args.push_back(dot_arg_name.str());
             }
         }
 

src/core/cfg.cpp

@@ -2502,6 +2502,34 @@ CFG* computeCFG(SourceInfo* source, std::vector<AST_stmt*> body) {
         }
     }
 
+    if (source->ast->type == AST_TYPE::FunctionDef || source->ast->type == AST_TYPE::Lambda) {
+        // Unpack tuple arguments
+        // Tuple arguments get assigned names ".0", ".1" etc. So this
+        // def f(a, (b,c), (d,e)):
+        // would expand to:
+        // def f(a, .1, .2):
+        //     (b, c) = .1
+        //     (d, e) = .2
+        AST_arguments* args;
+        if (source->ast->type == AST_TYPE::FunctionDef) {
+            args = ast_cast<AST_FunctionDef>(source->ast)->args;
+        } else {
+            args = ast_cast<AST_Lambda>(source->ast)->args;
+        }
+        int counter = 0;
+        for (AST_expr* arg_expr : args->args) {
+            if (arg_expr->type == AST_TYPE::Tuple) {
+                InternedString arg_name = source->getInternedStrings().get("." + std::to_string(counter));
+                AST_Name* arg_name_expr
+                    = new AST_Name(arg_name, AST_TYPE::Load, arg_expr->lineno, arg_expr->col_offset);
+                visitor.pushAssign(arg_expr, arg_name_expr);
+            } else {
+                assert(arg_expr->type == AST_TYPE::Name);
+            }
+            counter++;
+        }
+    }
+
     for (int i = (skip_first ? 1 : 0); i < body.size(); i++) {
         if (!visitor.curblock)
             break;

src/core/types.h

@@ -230,7 +230,7 @@ struct ParamNames {
     std::vector<llvm::StringRef> args;
     llvm::StringRef vararg, kwarg;
 
-    explicit ParamNames(AST* ast);
+    explicit ParamNames(AST* ast, InternedStringPool& pool);
     ParamNames(const std::vector<llvm::StringRef>& args, llvm::StringRef vararg, llvm::StringRef kwarg);
     static ParamNames empty() { return ParamNames(); }
 
@@ -305,7 +305,7 @@ public:
           takes_varargs(takes_varargs),
           takes_kwargs(takes_kwargs),
           source(std::move(source)),
-          param_names(this->source->ast),
+          param_names(this->source->ast, this->source->getInternedStrings()),
           always_use_version(NULL),
           code_obj(NULL) {
         assert(num_args >= num_defaults);

test/tests/arg_unpacking.py

-# expected: fail
-
 def f((a,b)):
     print a,b
+    print sorted(list(locals().iteritems()))
 
 f(range(2))
 f((1, 2))
 
+def g(a, (b,c), d, (e,f), (g, (h, i), j)):
+    print a
+    print b
+    print c
+    print d
+    print e
+    print f
+    print g
+    print h
+    print i
+    print j
+    print sorted(list(locals().iteritems()))
+
+g(1, (2, 3), 4, (5, 6), (7, (8, 9), 10))
+
+def h((a,)):
+    print a
+h((3,))
+
+print 'testing lambda'
+f = lambda (a, b) : a + b
+print f((3, 4))
+
+f = lambda (a,) : a + 1
+print f((3,))

test/tests/global_and_local.py

-# expected: fail
-# - this particular check isn't implemented yet
-
 # I would have expected this to be valid, but cPython and pypy err out saying "name 'x' is local and global"
 
-print "first"
-
+try:
+    exec """
 x = 1
 def f(x):
     global x
@@ -12,3 +9,56 @@ def f(x):
 print "calling"
 f(2)
 print x
+"""
+except SyntaxError as e:
+    print e.message
+
+try:
+    exec """
+x = 1
+def f((x, y)):
+    global x
+
+print "calling"
+f(2)
+print x
+"""
+except SyntaxError as e:
+    print e.message
+
+try:
+    exec """
+def f(*args):
+    global args
+
+print "calling"
+f(2)
+print x
+"""
+except SyntaxError as e:
+    print e.message
+
+try:
+    exec """
+def f(*kwargs):
+    global kwargs
+
+print "calling"
+f(d=2)
+print x
+"""
+except SyntaxError as e:
+    print e.message
+
+try:
+    exec """
+class C(object):
+    def f(__a):
+        global __a
+
+print "calling"
+f(d=2)
+print x
+"""
+except SyntaxError as e:
+    print e.message

test/unittests/analysis.cpp

@@ -49,7 +49,7 @@ TEST_F(AnalysisTest, augassign) {
             ASSERT_TRUE(liveness->isLiveAtEnd(module->interned_strings->get("a"), block));
     }
 
-    std::unique_ptr<PhiAnalysis> phis = computeRequiredPhis(ParamNames(func), cfg, liveness.get(), scope_info);
+    std::unique_ptr<PhiAnalysis> phis = computeRequiredPhis(ParamNames(func, si->getInternedStrings()), cfg, liveness.get(), scope_info);
 }
 
 void doOsrTest(bool is_osr, bool i_maybe_undefined) {
@@ -93,7 +93,7 @@ void doOsrTest(bool is_osr, bool i_maybe_undefined) {
         entry_descriptor->args[iter_str] = NULL;
         phis = computeRequiredPhis(entry_descriptor, liveness.get(), scope_info);
     } else {
-        phis = computeRequiredPhis(ParamNames(func), cfg, liveness.get(), scope_info);
+        phis = computeRequiredPhis(ParamNames(func, si->getInternedStrings()), cfg, liveness.get(), scope_info);
     }
 
     // First, verify that we require phi nodes for the block we enter into.