Merge pull request #275 from tjhance/keyword-params

src/analysis/function_analysis.cpp

@@ -222,10 +222,12 @@ private:
     typedef DefinednessAnalysis::DefinitionLevel DefinitionLevel;
 
     CFG* cfg;
-    const SourceInfo::ArgNames& arg_names;
+    ScopeInfo* scope_info;
+    const ParamNames& arg_names;
 
 public:
-    DefinednessBBAnalyzer(CFG* cfg, const SourceInfo::ArgNames& arg_names) : cfg(cfg), arg_names(arg_names) {}
+    DefinednessBBAnalyzer(CFG* cfg, ScopeInfo* scope_info, const ParamNames& arg_names)
+        : cfg(cfg), scope_info(scope_info), arg_names(arg_names) {}
 
     virtual DefinitionLevel merge(DefinitionLevel from, DefinitionLevel into) const {
         assert(from != DefinednessAnalysis::Undefined);
@@ -330,13 +332,13 @@ public:
 void DefinednessBBAnalyzer::processBB(Map& starting, CFGBlock* block) const {
     DefinednessVisitor visitor(starting);
 
-    if (block == cfg->getStartingBlock() && arg_names.args) {
-        for (auto e : (*arg_names.args))
-            visitor._doSet(e);
-        if (arg_names.vararg.str().size())
-            visitor._doSet(arg_names.vararg);
-        if (arg_names.kwarg.str().size())
-            visitor._doSet(arg_names.kwarg);
+    if (block == cfg->getStartingBlock()) {
+        for (auto e : arg_names.args)
+            visitor._doSet(scope_info->internString(e));
+        if (arg_names.vararg.size())
+            visitor._doSet(scope_info->internString(arg_names.vararg));
+        if (arg_names.kwarg.size())
+            visitor._doSet(scope_info->internString(arg_names.kwarg));
     }
 
     for (int i = 0; i < block->body.size(); i++) {
@@ -351,11 +353,11 @@ void DefinednessBBAnalyzer::processBB(Map& starting, CFGBlock* block) const {
     }
 }
 
-DefinednessAnalysis::DefinednessAnalysis(const SourceInfo::ArgNames& arg_names, CFG* cfg, ScopeInfo* scope_info)
+DefinednessAnalysis::DefinednessAnalysis(const ParamNames& arg_names, CFG* cfg, ScopeInfo* scope_info)
     : scope_info(scope_info) {
     Timer _t("DefinednessAnalysis()", 10);
 
-    results = computeFixedPoint(cfg, DefinednessBBAnalyzer(cfg, arg_names), false);
+    results = computeFixedPoint(cfg, DefinednessBBAnalyzer(cfg, scope_info, arg_names), false);
 
     for (const auto& p : results) {
         RequiredSet required;
@@ -384,8 +386,7 @@ const DefinednessAnalysis::RequiredSet& DefinednessAnalysis::getDefinedNamesAtEn
     return defined_at_end[block];
 }
 
-PhiAnalysis::PhiAnalysis(const SourceInfo::ArgNames& arg_names, CFG* cfg, LivenessAnalysis* liveness,
-                         ScopeInfo* scope_info)
+PhiAnalysis::PhiAnalysis(const ParamNames& arg_names, CFG* cfg, LivenessAnalysis* liveness, ScopeInfo* scope_info)
     : definedness(arg_names, cfg, scope_info), liveness(liveness) {
     Timer _t("PhiAnalysis()", 10);
 
@@ -463,8 +464,7 @@ LivenessAnalysis* computeLivenessInfo(CFG* cfg) {
     return new LivenessAnalysis(cfg);
 }
 
-PhiAnalysis* computeRequiredPhis(const SourceInfo::ArgNames& args, CFG* cfg, LivenessAnalysis* liveness,
-                                 ScopeInfo* scope_info) {
+PhiAnalysis* computeRequiredPhis(const ParamNames& args, CFG* cfg, LivenessAnalysis* liveness, ScopeInfo* scope_info) {
     return new PhiAnalysis(args, cfg, liveness, scope_info);
 }
 }

src/analysis/function_analysis.h

@@ -66,7 +66,7 @@ private:
     ScopeInfo* scope_info;
 
 public:
-    DefinednessAnalysis(const SourceInfo::ArgNames& args, CFG* cfg, ScopeInfo* scope_info);
+    DefinednessAnalysis(const ParamNames& param_names, CFG* cfg, ScopeInfo* scope_info);
 
     DefinitionLevel isDefinedAtEnd(InternedString name, CFGBlock* block);
     const RequiredSet& getDefinedNamesAtEnd(CFGBlock* block);
@@ -82,7 +82,7 @@ private:
     std::unordered_map<CFGBlock*, const RequiredSet> required_phis;
 
 public:
-    PhiAnalysis(const SourceInfo::ArgNames&, CFG* cfg, LivenessAnalysis* liveness, ScopeInfo* scope_info);
+    PhiAnalysis(const ParamNames&, CFG* cfg, LivenessAnalysis* liveness, ScopeInfo* scope_info);
 
     bool isRequired(InternedString name, CFGBlock* block);
     bool isRequiredAfter(InternedString name, CFGBlock* block);
@@ -93,7 +93,7 @@ public:
 };
 
 LivenessAnalysis* computeLivenessInfo(CFG*);
-PhiAnalysis* computeRequiredPhis(const SourceInfo::ArgNames&, CFG*, LivenessAnalysis*, ScopeInfo* scope_Info);
+PhiAnalysis* computeRequiredPhis(const ParamNames&, CFG*, LivenessAnalysis*, ScopeInfo* scope_Info);
 }
 
 #endif

src/analysis/scoping_analysis.cpp

@@ -106,6 +106,7 @@ public:
     bool saveInClosure(InternedString name) override { return false; }
 
     InternedString mangleName(InternedString id) override { return id; }
+    InternedString internString(llvm::StringRef s) override { abort(); }
 };
 
 struct ScopingAnalysis::ScopeNameUsage {
@@ -221,6 +222,8 @@ public:
     InternedString mangleName(const InternedString id) override {
         return pyston::mangleName(id, usage->private_name, usage->scoping->getInternedStrings());
     }
+
+    InternedString internString(llvm::StringRef s) override { return usage->scoping->getInternedStrings().get(s); }
 };
 
 class NameCollectorVisitor : public ASTVisitor {

src/analysis/scoping_analysis.h

@@ -38,6 +38,7 @@ public:
     virtual bool saveInClosure(InternedString name) = 0;
 
     virtual InternedString mangleName(InternedString id) = 0;
+    virtual InternedString internString(llvm::StringRef) = 0;
 };
 
 class ScopingAnalysis {

src/analysis/type_analysis.cpp

@@ -670,7 +670,7 @@ public:
         return changed;
     }
 
-    static PropagatingTypeAnalysis* doAnalysis(CFG* cfg, const SourceInfo::ArgNames& arg_names,
+    static PropagatingTypeAnalysis* doAnalysis(CFG* cfg, const ParamNames& arg_names,
                                                const std::vector<ConcreteCompilerType*>& arg_types,
                                                SpeculationLevel speculation, ScopeInfo* scope_info) {
         Timer _t("PropagatingTypeAnalysis::doAnalysis()");
@@ -681,30 +681,25 @@ public:
 
         assert(arg_names.totalParameters() == arg_types.size());
 
-        if (arg_names.args) {
-            TypeMap& initial_types = starting_types[cfg->getStartingBlock()];
-            int i = 0;
+        TypeMap& initial_types = starting_types[cfg->getStartingBlock()];
+        int i = 0;
 
-            for (; i < arg_names.args->size(); i++) {
-                AST_expr* arg = (*arg_names.args)[i];
-                RELEASE_ASSERT(arg->type == AST_TYPE::Name, "");
-                AST_Name* arg_name = ast_cast<AST_Name>(arg);
-                initial_types[arg_name->id] = unboxedType(arg_types[i]);
-            }
-
-            if (arg_names.vararg.str().size()) {
-                initial_types[arg_names.vararg] = unboxedType(arg_types[i]);
-                i++;
-            }
+        for (; i < arg_names.args.size(); i++) {
+            initial_types[scope_info->internString(arg_names.args[i])] = unboxedType(arg_types[i]);
+        }
 
-            if (arg_names.kwarg.str().size()) {
-                initial_types[arg_names.kwarg] = unboxedType(arg_types[i]);
-                i++;
-            }
+        if (arg_names.vararg.size()) {
+            initial_types[scope_info->internString(arg_names.vararg)] = unboxedType(arg_types[i]);
+            i++;
+        }
 
-            assert(i == arg_types.size());
+        if (arg_names.kwarg.size()) {
+            initial_types[scope_info->internString(arg_names.kwarg)] = unboxedType(arg_types[i]);
+            i++;
         }
 
+        assert(i == arg_types.size());
+
         std::unordered_set<CFGBlock*> in_queue;
         std::priority_queue<CFGBlock*, std::vector<CFGBlock*>, CFGBlockMinIndex> queue;
         queue.push(cfg->getStartingBlock());
@@ -785,9 +780,9 @@ public:
 
 
 // public entry point:
-TypeAnalysis* doTypeAnalysis(CFG* cfg, const SourceInfo::ArgNames& arg_names,
-                             const std::vector<ConcreteCompilerType*>& arg_types, EffortLevel::EffortLevel effort,
-                             TypeAnalysis::SpeculationLevel speculation, ScopeInfo* scope_info) {
+TypeAnalysis* doTypeAnalysis(CFG* cfg, const ParamNames& arg_names, const std::vector<ConcreteCompilerType*>& arg_types,
+                             EffortLevel::EffortLevel effort, TypeAnalysis::SpeculationLevel speculation,
+                             ScopeInfo* scope_info) {
     // if (effort == EffortLevel::INTERPRETED) {
     // return new NullTypeAnalysis();
     //}

src/analysis/type_analysis.h

@@ -42,7 +42,7 @@ public:
     virtual BoxedClass* speculatedExprClass(AST_expr*) = 0;
 };
 
-TypeAnalysis* doTypeAnalysis(CFG* cfg, const SourceInfo::ArgNames& arg_names,
+TypeAnalysis* doTypeAnalysis(CFG* cfg, const ParamNames& param_names,
                              const std::vector<ConcreteCompilerType*>& arg_types, EffortLevel::EffortLevel effort,
                              TypeAnalysis::SpeculationLevel speculation, ScopeInfo* scope_info);
 }

src/codegen/ast_interpreter.cpp

@@ -189,21 +189,19 @@ void ASTInterpreter::initArguments(int nargs, BoxedClosure* _closure, BoxedGener
     for (int i = 3; i < nargs; ++i)
         argsArray.push_back(args[i - 3]);
 
+    const ParamNames& param_names = compiled_func->clfunc->param_names;
+
     int i = 0;
-    if (source_info->arg_names.args) {
-        for (AST_expr* e : *source_info->arg_names.args) {
-            RELEASE_ASSERT(e->type == AST_TYPE::Name, "not implemented");
-            AST_Name* name = (AST_Name*)e;
-            doStore(name->id, argsArray[i++]);
-        }
+    for (auto& name : param_names.args) {
+        doStore(source_info->getInternedStrings().get(name), argsArray[i++]);
     }
 
-    if (source_info->arg_names.vararg.str().size()) {
-        doStore(source_info->arg_names.vararg, argsArray[i++]);
+    if (!param_names.vararg.str().empty()) {
+        doStore(source_info->getInternedStrings().get(param_names.vararg), argsArray[i++]);
     }
 
-    if (source_info->arg_names.kwarg.str().size()) {
-        doStore(source_info->arg_names.kwarg, argsArray[i++]);
+    if (!param_names.kwarg.str().empty()) {
+        doStore(source_info->getInternedStrings().get(param_names.kwarg), argsArray[i++]);
     }
 }
 
@@ -268,8 +266,8 @@ void ASTInterpreter::eraseDeadSymbols() {
         source_info->liveness = computeLivenessInfo(source_info->cfg);
 
     if (source_info->phis == NULL)
-        source_info->phis
-            = computeRequiredPhis(source_info->arg_names, source_info->cfg, source_info->liveness, scope_info);
+        source_info->phis = computeRequiredPhis(compiled_func->clfunc->param_names, source_info->cfg,
+                                                source_info->liveness, scope_info);
 
     std::vector<InternedString> dead_symbols;
     for (auto&& it : sym_table) {

src/codegen/codegen.cpp

@@ -34,8 +34,7 @@ namespace pyston {
 DS_DEFINE_RWLOCK(codegen_rwlock);
 
 SourceInfo::SourceInfo(BoxedModule* m, ScopingAnalysis* scoping, AST* ast, const std::vector<AST_stmt*>& body)
-    : parent_module(m), scoping(scoping), ast(ast), cfg(NULL), liveness(NULL), phis(NULL), arg_names(ast, scoping),
-      body(body) {
+    : parent_module(m), scoping(scoping), ast(ast), cfg(NULL), liveness(NULL), phis(NULL), body(body) {
     switch (ast->type) {
         case AST_TYPE::ClassDef:
         case AST_TYPE::Lambda:

src/codegen/irgen.cpp

@@ -664,7 +664,7 @@ static void emitBBs(IRGenState* irstate, const char* bb_type, GuardList& out_gua
                 emitter->getBuilder()->SetInsertPoint(llvm_entry_blocks[source->cfg->getStartingBlock()]);
             }
 
-            generator->doFunctionEntry(source->arg_names, cf->spec->arg_types);
+            generator->doFunctionEntry(*irstate->getParamNames(), cf->spec->arg_types);
 
             // Function-entry safepoint:
             // TODO might be more efficient to do post-call safepoints?
@@ -1087,7 +1087,7 @@ static std::string getUniqueFunctionName(std::string nameprefix, EffortLevel::Ef
     return os.str();
 }
 
-CompiledFunction* doCompile(SourceInfo* source, const OSREntryDescriptor* entry_descriptor,
+CompiledFunction* doCompile(SourceInfo* source, ParamNames* param_names, const OSREntryDescriptor* entry_descriptor,
                             EffortLevel::EffortLevel effort, FunctionSpecialization* spec, std::string nameprefix) {
     Timer _t("in doCompile");
     Timer _t2;
@@ -1109,7 +1109,7 @@ CompiledFunction* doCompile(SourceInfo* source, const OSREntryDescriptor* entry_
     ////
     // Initializing the llvm-level structures:
 
-    int nargs = source->arg_names.totalParameters();
+    int nargs = param_names->totalParameters();
     ASSERT(nargs == spec->arg_types.size(), "%d %ld", nargs, spec->arg_types.size());
 
 
@@ -1157,8 +1157,8 @@ CompiledFunction* doCompile(SourceInfo* source, const OSREntryDescriptor* entry_
     TypeAnalysis::SpeculationLevel speculation_level = TypeAnalysis::NONE;
     if (ENABLE_SPECULATION && effort >= EffortLevel::MODERATE)
         speculation_level = TypeAnalysis::SOME;
-    TypeAnalysis* types = doTypeAnalysis(source->cfg, source->arg_names, spec->arg_types, effort, speculation_level,
-                                         source->getScopeInfo());
+    TypeAnalysis* types
+        = doTypeAnalysis(source->cfg, *param_names, spec->arg_types, effort, speculation_level, source->getScopeInfo());
 
     _t2.split();
 
@@ -1174,7 +1174,7 @@ CompiledFunction* doCompile(SourceInfo* source, const OSREntryDescriptor* entry_
         computeBlockSetClosure(full_blocks, partial_blocks);
     }
 
-    IRGenState irstate(cf, source, getGCBuilder(), dbg_funcinfo);
+    IRGenState irstate(cf, source, param_names, getGCBuilder(), dbg_funcinfo);
 
     emitBBs(&irstate, "opt", guards, GuardList(), types, entry_descriptor, full_blocks, partial_blocks);
 
@@ -1196,7 +1196,7 @@ CompiledFunction* doCompile(SourceInfo* source, const OSREntryDescriptor* entry_
         assert(deopt_full_blocks.size() || deopt_partial_blocks.size());
 
         irgen_us += _t2.split();
-        TypeAnalysis* deopt_types = doTypeAnalysis(source->cfg, source->arg_names, spec->arg_types, effort,
+        TypeAnalysis* deopt_types = doTypeAnalysis(source->cfg, *param_names, spec->arg_types, effort,
                                                    TypeAnalysis::NONE, source->getScopeInfo());
         _t2.split();
 

src/codegen/irgen.h

@@ -94,7 +94,7 @@ extern const std::string PASSED_GENERATOR_NAME;
 InternedString getIsDefinedName(InternedString name, InternedStringPool& interned_strings);
 bool isIsDefinedName(const std::string& name);
 
-CompiledFunction* doCompile(SourceInfo* source, const OSREntryDescriptor* entry_descriptor,
+CompiledFunction* doCompile(SourceInfo* source, ParamNames* param_names, const OSREntryDescriptor* entry_descriptor,
                             EffortLevel::EffortLevel effort, FunctionSpecialization* spec, std::string nameprefix);
 
 // A common pattern is to branch based off whether a variable is defined but only if it is

src/codegen/irgen/hooks.cpp

@@ -42,27 +42,37 @@
 namespace pyston {
 
 // TODO terrible place for these!
-SourceInfo::ArgNames::ArgNames(AST* ast, ScopingAnalysis* scoping) {
+ParamNames::ParamNames(AST* ast) : takes_param_names(true) {
     if (ast->type == AST_TYPE::Module || ast->type == AST_TYPE::ClassDef) {
-        args = NULL;
-        kwarg = vararg = scoping->getInternedStrings().get("");
-    } else if (ast->type == AST_TYPE::FunctionDef) {
-        AST_FunctionDef* f = ast_cast<AST_FunctionDef>(ast);
-        args = &f->args->args;
-        vararg = f->args->vararg;
-        kwarg = f->args->kwarg;
-    } else if (ast->type == AST_TYPE::Lambda) {
-        AST_Lambda* l = ast_cast<AST_Lambda>(ast);
-        args = &l->args->args;
-        vararg = l->args->vararg;
-        kwarg = l->args->kwarg;
+        kwarg = "";
+        vararg = "";
+    } else if (ast->type == AST_TYPE::FunctionDef || ast->type == AST_TYPE::Lambda) {
+        AST_arguments* arguments = ast->type == AST_TYPE::FunctionDef ? ast_cast<AST_FunctionDef>(ast)->args
+                                                                      : ast_cast<AST_Lambda>(ast)->args;
+        for (int i = 0; i < arguments->args.size(); i++) {
+            AST_expr* arg = arguments->args[i];
+            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));
+            }
+        }
+
+        vararg = arguments->vararg.str();
+        kwarg = arguments->kwarg.str();
     } else {
         RELEASE_ASSERT(0, "%d", ast->type);
     }
 }
 
+ParamNames::ParamNames(const std::vector<llvm::StringRef>& args, llvm::StringRef vararg, llvm::StringRef kwarg)
+    : takes_param_names(true) {
+    this->args = args;
+    this->vararg = vararg;
+    this->kwarg = kwarg;
+}
+
 InternedString SourceInfo::mangleName(InternedString id) {
-    // TODO should cache the results of this
     assert(ast);
     if (ast->type == AST_TYPE::Module)
         return id;
@@ -199,8 +209,7 @@ CompiledFunction* compileFunction(CLFunction* f, FunctionSpecialization* spec, E
             source->liveness = computeLivenessInfo(source->cfg);
 
         if (source->phis == NULL)
-            source->phis
-                = computeRequiredPhis(source->arg_names, source->cfg, source->liveness, source->getScopeInfo());
+            source->phis = computeRequiredPhis(f->param_names, source->cfg, source->liveness, source->getScopeInfo());
     }
 
 
@@ -209,7 +218,7 @@ CompiledFunction* compileFunction(CLFunction* f, FunctionSpecialization* spec, E
     if (effort == EffortLevel::INTERPRETED) {
         cf = new CompiledFunction(0, spec, true, NULL, NULL, effort, 0);
     } else {
-        cf = doCompile(source, entry, effort, spec, name);
+        cf = doCompile(source, &f->param_names, entry, effort, spec, name);
         compileIR(cf, effort);
     }
 
@@ -413,17 +422,26 @@ extern "C" char* reoptCompiledFunc(CompiledFunction* cf) {
     return (char*)reoptCompiledFuncInternal(cf)->code;
 }
 
-CLFunction* createRTFunction(int num_args, int num_defaults, bool takes_varargs, bool takes_kwargs) {
-    return new CLFunction(num_args, num_defaults, takes_varargs, takes_kwargs, NULL);
+CLFunction* createRTFunction(int num_args, int num_defaults, bool takes_varargs, bool takes_kwargs,
+                             const ParamNames& param_names) {
+    return new CLFunction(num_args, num_defaults, takes_varargs, takes_kwargs, param_names);
 }
 
-CLFunction* boxRTFunction(void* f, ConcreteCompilerType* rtn_type, int num_args) {
-    return boxRTFunction(f, rtn_type, num_args, 0, false, false);
+CLFunction* boxRTFunction(void* f, ConcreteCompilerType* rtn_type, int num_args, const ParamNames& param_names) {
+    assert(!param_names.takes_param_names || num_args == param_names.args.size());
+    assert(param_names.vararg.str() == "");
+    assert(param_names.kwarg.str() == "");
+
+    return boxRTFunction(f, rtn_type, num_args, 0, false, false, param_names);
 }
 
 CLFunction* boxRTFunction(void* f, ConcreteCompilerType* rtn_type, int num_args, int num_defaults, bool takes_varargs,
-                          bool takes_kwargs) {
-    CLFunction* cl_f = createRTFunction(num_args, num_defaults, takes_varargs, takes_kwargs);
+                          bool takes_kwargs, const ParamNames& param_names) {
+    assert(!param_names.takes_param_names || num_args == param_names.args.size());
+    assert(takes_varargs || param_names.vararg.str() == "");
+    assert(takes_kwargs || param_names.kwarg.str() == "");
+
+    CLFunction* cl_f = createRTFunction(num_args, num_defaults, takes_varargs, takes_kwargs, param_names);
 
     addRTFunction(cl_f, f, rtn_type);
     return cl_f;

src/codegen/irgen/irgenerator.cpp

@@ -2430,9 +2430,8 @@ public:
         return CLOSURE;
     }
 
-    void doFunctionEntry(const SourceInfo::ArgNames& arg_names,
-                         const std::vector<ConcreteCompilerType*>& arg_types) override {
-        assert(arg_names.totalParameters() == arg_types.size());
+    void doFunctionEntry(const ParamNames& param_names, const std::vector<ConcreteCompilerType*>& arg_types) override {
+        assert(param_names.totalParameters() == arg_types.size());
 
         auto scope_info = irstate->getScopeInfo();
 
@@ -2485,26 +2484,26 @@ public:
         }
 
         assert(AI == irstate->getLLVMFunction()->arg_end());
-        assert(python_parameters.size() == arg_names.totalParameters());
+        assert(python_parameters.size() == param_names.totalParameters());
 
-        if (arg_names.args) {
-            int i = 0;
-            for (; i < arg_names.args->size(); i++) {
-                loadArgument((*arg_names.args)[i], arg_types[i], python_parameters[i], UnwindInfo::cantUnwind());
-            }
-
-            if (arg_names.vararg.str().size()) {
-                loadArgument(arg_names.vararg, arg_types[i], python_parameters[i], UnwindInfo::cantUnwind());
-                i++;
-            }
+        int i = 0;
+        for (; i < param_names.args.size(); i++) {
+            loadArgument(internString(param_names.args[i]), arg_types[i], python_parameters[i],
+                         UnwindInfo::cantUnwind());
+        }
 
-            if (arg_names.kwarg.str().size()) {
-                loadArgument(arg_names.kwarg, arg_types[i], python_parameters[i], UnwindInfo::cantUnwind());
-                i++;
-            }
+        if (param_names.vararg.size()) {
+            loadArgument(internString(param_names.vararg), arg_types[i], python_parameters[i],
+                         UnwindInfo::cantUnwind());
+            i++;
+        }
 
-            assert(i == arg_types.size());
+        if (param_names.kwarg.size()) {
+            loadArgument(internString(param_names.kwarg), arg_types[i], python_parameters[i], UnwindInfo::cantUnwind());
+            i++;
         }
+
+        assert(i == arg_types.size());
     }
 
     void run(const CFGBlock* block) override {

src/codegen/irgen/irgenerator.h

@@ -55,6 +55,7 @@ class IRGenState {
 private:
     CompiledFunction* cf;
     SourceInfo* source_info;
+    ParamNames* param_names;
     GCBuilder* gc;
     llvm::MDNode* func_dbg_info;
 
@@ -63,9 +64,10 @@ private:
     int scratch_size;
 
 public:
-    IRGenState(CompiledFunction* cf, SourceInfo* source_info, GCBuilder* gc, llvm::MDNode* func_dbg_info)
-        : cf(cf), source_info(source_info), gc(gc), func_dbg_info(func_dbg_info), scratch_space(NULL), frame_info(NULL),
-          scratch_size(0) {
+    IRGenState(CompiledFunction* cf, SourceInfo* source_info, ParamNames* param_names, GCBuilder* gc,
+               llvm::MDNode* func_dbg_info)
+        : cf(cf), source_info(source_info), param_names(param_names), gc(gc), func_dbg_info(func_dbg_info),
+          scratch_space(NULL), frame_info(NULL), scratch_size(0) {
         assert(cf->func);
         assert(!cf->clfunc); // in this case don't need to pass in sourceinfo
     }
@@ -92,6 +94,8 @@ public:
     ScopeInfo* getScopeInfoForNode(AST* node);
 
     llvm::MDNode* getFuncDbgInfo() { return func_dbg_info; }
+
+    ParamNames* getParamNames() { return param_names; }
 };
 
 class GuardList {
@@ -195,8 +199,8 @@ public:
 
     virtual ~IRGenerator() {}
 
-    virtual void doFunctionEntry(const SourceInfo::ArgNames& arg_names,
-                                 const std::vector<ConcreteCompilerType*>& arg_types) = 0;
+    virtual void doFunctionEntry(const ParamNames& param_names, const std::vector<ConcreteCompilerType*>& arg_types)
+        = 0;
 
     virtual void giveLocalSymbol(InternedString name, CompilerVariable* var) = 0;
     virtual void copySymbolsFrom(SymbolTable* st) = 0;

src/core/types.h

@@ -219,6 +219,23 @@ public:
     void speculationFailed();
 };
 
+struct ParamNames {
+    bool takes_param_names;
+    std::vector<llvm::StringRef> args;
+    llvm::StringRef vararg, kwarg;
+
+    explicit ParamNames(AST* ast);
+    ParamNames(const std::vector<llvm::StringRef>& args, llvm::StringRef vararg, llvm::StringRef kwarg);
+    static ParamNames empty() { return ParamNames(); }
+
+    int totalParameters() const {
+        return args.size() + (vararg.str().size() == 0 ? 0 : 1) + (kwarg.str().size() == 0 ? 0 : 1);
+    }
+
+private:
+    ParamNames() : takes_param_names(false) {}
+};
+
 class BoxedModule;
 class ScopeInfo;
 class InternedStringPool;
@@ -236,20 +253,6 @@ public:
 
     ScopeInfo* getScopeInfo();
 
-    struct ArgNames {
-        const std::vector<AST_expr*>* args;
-        InternedString vararg, kwarg;
-
-        explicit ArgNames(AST* ast, ScopingAnalysis* scoping);
-
-        int totalParameters() const {
-            if (!args)
-                return 0;
-            return args->size() + (vararg.str().size() == 0 ? 0 : 1) + (kwarg.str().size() == 0 ? 0 : 1);
-        }
-    };
-
-    ArgNames arg_names;
     // TODO we're currently copying the body of the AST into here, since lambdas don't really have a statement-based
     // body and we have to create one.  Ideally, we'd be able to avoid the space duplication for non-lambdas.
     const std::vector<AST_stmt*> body;
@@ -269,6 +272,8 @@ public:
     bool takes_varargs, takes_kwargs;
 
     SourceInfo* source;
+    ParamNames param_names;
+
     FunctionList
         versions; // any compiled versions along with their type parameters; in order from most preferred to least
     std::unordered_map<const OSREntryDescriptor*, CompiledFunction*> osr_versions;
@@ -283,14 +288,17 @@ public:
 
     CLFunction(int num_args, int num_defaults, bool takes_varargs, bool takes_kwargs, SourceInfo* source)
         : num_args(num_args), num_defaults(num_defaults), takes_varargs(takes_varargs), takes_kwargs(takes_kwargs),
-          source(source) {
+          source(source), param_names(source->ast) {
+        assert(num_args >= num_defaults);
+    }
+    CLFunction(int num_args, int num_defaults, bool takes_varargs, bool takes_kwargs, const ParamNames& param_names)
+        : num_args(num_args), num_defaults(num_defaults), takes_varargs(takes_varargs), takes_kwargs(takes_kwargs),
+          source(NULL), param_names(param_names) {
         assert(num_args >= num_defaults);
     }
 
     int numReceivedArgs() { return num_args + (takes_varargs ? 1 : 0) + (takes_kwargs ? 1 : 0); }
 
-    // const std::vector<AST_expr*>* getArgNames();
-
     void addVersion(CompiledFunction* compiled) {
         assert(compiled);
         assert(compiled->spec);
@@ -306,10 +314,12 @@ public:
     }
 };
 
-CLFunction* createRTFunction(int num_args, int num_defaults, bool takes_varargs, bool takes_kwargs);
+CLFunction* createRTFunction(int num_args, int num_defaults, bool takes_varargs, bool takes_kwargs,
+                             const ParamNames& param_names = ParamNames::empty());
 CLFunction* boxRTFunction(void* f, ConcreteCompilerType* rtn_type, int nargs, int num_defaults, bool takes_varargs,
-                          bool takes_kwargs);
-CLFunction* boxRTFunction(void* f, ConcreteCompilerType* rtn_type, int nargs);
+                          bool takes_kwargs, const ParamNames& param_names = ParamNames::empty());
+CLFunction* boxRTFunction(void* f, ConcreteCompilerType* rtn_type, int nargs,
+                          const ParamNames& param_names = ParamNames::empty());
 void addRTFunction(CLFunction* cf, void* f, ConcreteCompilerType* rtn_type);
 void addRTFunction(CLFunction* cf, void* f, ConcreteCompilerType* rtn_type,
                    const std::vector<ConcreteCompilerType*>& arg_types);

src/runtime/builtin_modules/builtins.cpp

@@ -405,7 +405,9 @@ Box* notimplementedRepr(Box* self) {
     return boxStrConstant("NotImplemented");
 }
 
-Box* sorted(Box* obj) {
+Box* sorted(Box* obj, Box* key, Box* cmp, Box** args) {
+    Box* reverse = args[0];
+
     BoxedList* rtn = new BoxedList();
     for (Box* e : obj->pyElements()) {
         listAppendInternal(rtn, e);
@@ -1110,10 +1112,10 @@ void setupBuiltins() {
     builtins_module->giveAttr("enumerate", enumerate_cls);
 
 
-    CLFunction* sorted_func = createRTFunction(1, 0, false, false);
-    addRTFunction(sorted_func, (void*)sortedList, LIST, { LIST });
-    addRTFunction(sorted_func, (void*)sorted, LIST, { UNKNOWN });
-    builtins_module->giveAttr("sorted", new BoxedBuiltinFunctionOrMethod(sorted_func));
+    CLFunction* sorted_func = createRTFunction(4, 3, false, false, ParamNames({ "", "cmp", "key", "reverse" }, "", ""));
+    addRTFunction(sorted_func, (void*)sortedList, LIST, { LIST, UNKNOWN, UNKNOWN, UNKNOWN });
+    addRTFunction(sorted_func, (void*)sorted, LIST, { UNKNOWN, UNKNOWN, UNKNOWN, UNKNOWN });
+    builtins_module->giveAttr("sorted", new BoxedBuiltinFunctionOrMethod(sorted_func, { None, None, False }));
 
     builtins_module->giveAttr("True", True);
     builtins_module->giveAttr("False", False);
@@ -1124,8 +1126,9 @@ void setupBuiltins() {
     setupXrange();
     builtins_module->giveAttr("xrange", xrange_cls);
 
-    open_obj = new BoxedBuiltinFunctionOrMethod(boxRTFunction((void*)open, typeFromClass(file_cls), 2, 1, false, false),
-                                                { boxStrConstant("r") });
+    open_obj = new BoxedBuiltinFunctionOrMethod(
+        boxRTFunction((void*)open, typeFromClass(file_cls), 2, 1, false, false, ParamNames({ "name", "mode" }, "", "")),
+        { boxStrConstant("r") });
     builtins_module->giveAttr("open", open_obj);
 
     builtins_module->giveAttr(

src/runtime/objmodel.cpp

@@ -1753,8 +1753,8 @@ extern "C" bool nonzero(Box* obj) {
         ASSERT(isUserDefined(obj->cls) || obj->cls == classobj_cls || obj->cls == type_cls
                    || isSubclass(obj->cls, Exception) || obj->cls == file_cls || obj->cls == traceback_cls
                    || obj->cls == instancemethod_cls,
-               "%s.__nonzero__",
-               getTypeName(obj)); // TODO
+               "%s.__nonzero__", getTypeName(obj)); // TODO
+
         // TODO should rewrite these?
         return true;
     }
@@ -2312,19 +2312,14 @@ static std::string getFunctionName(CLFunction* f) {
     return "<unknown function>";
 }
 
-static void placeKeyword(const std::vector<AST_expr*>& arg_names, std::vector<bool>& params_filled,
-                         const std::string& kw_name, Box* kw_val, Box*& oarg1, Box*& oarg2, Box*& oarg3, Box** oargs,
-                         BoxedDict* okwargs, CLFunction* cl) {
+static void placeKeyword(const ParamNames& param_names, std::vector<bool>& params_filled, const std::string& kw_name,
+                         Box* kw_val, Box*& oarg1, Box*& oarg2, Box*& oarg3, Box** oargs, BoxedDict* okwargs,
+                         CLFunction* cl) {
     assert(kw_val);
 
     bool found = false;
-    for (int j = 0; j < arg_names.size(); j++) {
-        AST_expr* e = arg_names[j];
-        if (e->type != AST_TYPE::Name)
-            continue;
-
-        AST_Name* n = ast_cast<AST_Name>(e);
-        if (n->id.str() == kw_name) {
+    for (int j = 0; j < param_names.args.size(); j++) {
+        if (param_names.args[j].str() == kw_name && kw_name.size() > 0) {
             if (params_filled[j]) {
                 raiseExcHelper(TypeError, "%.200s() got multiple values for keyword argument '%s'",
                                getFunctionName(cl).c_str(), kw_name.c_str());
@@ -2512,8 +2507,8 @@ Box* callFunc(BoxedFunctionBase* func, CallRewriteArgs* rewrite_args, ArgPassSpe
         getArg(f->num_args + (f->takes_varargs ? 1 : 0), oarg1, oarg2, oarg3, oargs) = okwargs;
     }
 
-    const std::vector<AST_expr*>* arg_names = f->source ? f->source->arg_names.args : NULL;
-    if (arg_names == nullptr && argspec.num_keywords && !f->takes_kwargs) {
+    const ParamNames& param_names = f->param_names;
+    if (!param_names.takes_param_names && argspec.num_keywords && !f->takes_kwargs) {
         raiseExcHelper(TypeError, "%s() doesn't take keyword arguments", getFunctionName(f).c_str());
     }
 
@@ -2526,15 +2521,13 @@ Box* callFunc(BoxedFunctionBase* func, CallRewriteArgs* rewrite_args, ArgPassSpe
         int arg_idx = i + argspec.num_args;
         Box* kw_val = getArg(arg_idx, arg1, arg2, arg3, args);
 
-        if (!arg_names) {
+        if (!param_names.takes_param_names) {
             assert(okwargs);
             okwargs->d[boxStringPtr((*keyword_names)[i])] = kw_val;
             continue;
         }
 
-        assert(arg_names);
-
-        placeKeyword(*arg_names, params_filled, *(*keyword_names)[i], kw_val, oarg1, oarg2, oarg3, oargs, okwargs, f);
+        placeKeyword(param_names, params_filled, *(*keyword_names)[i], kw_val, oarg1, oarg2, oarg3, oargs, okwargs, f);
     }
 
     if (argspec.has_kwargs) {
@@ -2551,8 +2544,8 @@ Box* callFunc(BoxedFunctionBase* func, CallRewriteArgs* rewrite_args, ArgPassSpe
 
             BoxedString* s = static_cast<BoxedString*>(p.first);
 
-            if (arg_names) {
-                placeKeyword(*arg_names, params_filled, s->s, p.second, oarg1, oarg2, oarg3, oargs, okwargs, f);
+            if (param_names.takes_param_names) {
+                placeKeyword(param_names, params_filled, s->s, p.second, oarg1, oarg2, oarg3, oargs, okwargs, f);
             } else {
                 assert(okwargs);
 

test/tests/builtin_keywords.py

+f = open(**{"name": "/dev/null", "mode": "r"})
+print repr(f.read())

test/unittests/analysis.cpp

@@ -48,6 +48,6 @@ TEST_F(AnalysisTest, augassign) {
             ASSERT_TRUE(liveness->isLiveAtEnd(module->interned_strings->get("a"), block));
     }
 
-    PhiAnalysis* phis = computeRequiredPhis(SourceInfo::ArgNames(func, scoping), cfg, liveness, scope_info);
+    PhiAnalysis* phis = computeRequiredPhis(ParamNames(func), cfg, liveness, scope_info);
 }