ParamNames: only store AST_Name* or char* but not both

src/analysis/function_analysis.cpp

@@ -549,22 +549,15 @@ std::unique_ptr<PhiAnalysis> computeRequiredPhis(const ParamNames& args, CFG* cf
         initial_map[vreg] = DefinednessAnalysis::Undefined;
     }
 
-    auto maybe_add = [&](AST_Name* n) {
+    for (AST_Name* n : args.allArgsAsName()) {
         ScopeInfo::VarScopeType vst = n->lookup_type;
         assert(vst != ScopeInfo::VarScopeType::UNKNOWN);
         assert(vst != ScopeInfo::VarScopeType::GLOBAL); // global-and-local error
         if (vst == ScopeInfo::VarScopeType::NAME)
-            return;
+            continue;
         assert(n->vreg >= 0);
         initial_map[n->vreg] = DefinednessAnalysis::Defined;
-    };
-
-    for (auto e : args.arg_names)
-        maybe_add(e);
-    if (args.vararg_name)
-        maybe_add(args.vararg_name);
-    if (args.kwarg_name)
-        maybe_add(args.kwarg_name);
+    }
 
     assert(initial_map.numVregs() == vreg_info.getTotalNumOfVRegs());
 

src/analysis/type_analysis.cpp

@@ -859,30 +859,15 @@ TypeAnalysis* doTypeAnalysis(CFG* cfg, const ParamNames& arg_names, const std::v
 
     TypeMap initial_types(cfg->getVRegInfo().getTotalNumOfVRegs());
     int i = 0;
-
-    auto maybe_add = [&](AST_Name* n) {
+    for (AST_Name* n : arg_names.allArgsAsName()) {
         ScopeInfo::VarScopeType vst = n->lookup_type;
         assert(vst != ScopeInfo::VarScopeType::UNKNOWN);
         assert(vst != ScopeInfo::VarScopeType::GLOBAL); // global-and-local error
-        if (vst == ScopeInfo::VarScopeType::NAME)
-            return;
-        initial_types[n->vreg] = unboxedType(arg_types[i]);
+        if (vst != ScopeInfo::VarScopeType::NAME)
+            initial_types[n->vreg] = unboxedType(arg_types[i]);
+        ++i;
     };
 
-    for (; i < arg_names.args.size(); i++) {
-        maybe_add(arg_names.arg_names[i]);
-    }
-
-    if (arg_names.vararg.size()) {
-        maybe_add(arg_names.vararg_name);
-        i++;
-    }
-
-    if (arg_names.kwarg.size()) {
-        maybe_add(arg_names.kwarg_name);
-        i++;
-    }
-
     assert(i == arg_types.size());
 
     return PropagatingTypeAnalysis::doAnalysis(speculation, scope_info, std::move(initial_types),

src/codegen/ast_interpreter.cpp

@@ -272,26 +272,21 @@ void ASTInterpreter::initArguments(BoxedClosure* _closure, BoxedGenerator* _gene
 
     const ParamNames& param_names = getMD()->param_names;
 
-    // make sure the AST_Name nodes are set
-    assert(param_names.args.size() == param_names.arg_names.size());
-    assert(param_names.vararg.empty() == (param_names.vararg_name == NULL));
-    assert(param_names.kwarg.empty() == (param_names.kwarg_name == NULL));
-
     int i = 0;
-    for (auto& name : param_names.arg_names) {
+    for (auto& name : param_names.argsAsName()) {
         doStore(name, Value(incref(getArg(i++, arg1, arg2, arg3, args)), 0));
     }
 
-    if (param_names.vararg_name)
-        doStore(param_names.vararg_name, Value(incref(getArg(i++, arg1, arg2, arg3, args)), 0));
+    if (param_names.has_vararg_name)
+        doStore(param_names.varArgAsName(), Value(incref(getArg(i++, arg1, arg2, arg3, args)), 0));
 
-    if (param_names.kwarg_name) {
+    if (param_names.has_kwarg_name) {
         Box* val = getArg(i++, arg1, arg2, arg3, args);
         if (!val)
             val = createDict();
         else
             Py_INCREF(val);
-        doStore(param_names.kwarg_name, Value(val, 0));
+        doStore(param_names.kwArgAsName(), Value(val, 0));
     }
     assert(i == param_names.totalParameters());
 }
@@ -314,13 +309,9 @@ void ASTInterpreter::startJITing(CFGBlock* block, int exit_offset, llvm::DenseSe
     // small optimization: we know that the passed arguments in the entry block are non zero
     if (block == block->cfg->getStartingBlock() && block->predecessors.empty()) {
         auto param_names = getMD()->param_names;
-        for (auto&& arg : param_names.arg_names) {
+        for (auto&& arg : param_names.allArgsAsName()) {
             known_non_null_vregs.insert(arg->vreg);
         }
-        if (param_names.vararg_name)
-            known_non_null_vregs.insert(param_names.vararg_name->vreg);
-        if (param_names.kwarg_name)
-            known_non_null_vregs.insert(param_names.kwarg_name->vreg);
     }
     jit = code_block->newFragment(block, exit_offset, std::move(known_non_null_vregs));
 }

src/codegen/codegen.cpp

@@ -40,22 +40,22 @@ namespace pyston {
 FunctionMetadata::FunctionMetadata(int num_args, bool takes_varargs, bool takes_kwargs,
                                    std::unique_ptr<SourceInfo> source)
     : code_obj(NULL),
-      num_args(num_args),
-      takes_varargs(takes_varargs),
-      takes_kwargs(takes_kwargs),
       source(std::move(source)),
       param_names(this->source->ast, this->source->getInternedStrings()),
+      takes_varargs(takes_varargs),
+      takes_kwargs(takes_kwargs),
+      num_args(num_args),
       times_interpreted(0),
       internal_callable(NULL, NULL) {
 }
 
 FunctionMetadata::FunctionMetadata(int num_args, bool takes_varargs, bool takes_kwargs, const ParamNames& param_names)
     : code_obj(NULL),
-      num_args(num_args),
-      takes_varargs(takes_varargs),
-      takes_kwargs(takes_kwargs),
       source(nullptr),
       param_names(param_names),
+      takes_varargs(takes_varargs),
+      takes_kwargs(takes_kwargs),
+      num_args(num_args),
       times_interpreted(0),
       internal_callable(NULL, NULL) {
 }
@@ -272,7 +272,6 @@ llvm::JITEventListener* makeRegistryListener() {
     return new RegistryEventListener();
 }
 
-
 FunctionSpecialization::FunctionSpecialization(ConcreteCompilerType* rtn_type) : rtn_type(rtn_type) {
     accepts_all_inputs = true;
     boxed_return_value = (rtn_type->llvmType() == UNKNOWN->llvmType());

src/codegen/irgen.cpp

@@ -1024,7 +1024,7 @@ static std::string getUniqueFunctionName(std::string nameprefix, EffortLevel eff
 }
 
 std::pair<CompiledFunction*, llvm::Function*> doCompile(FunctionMetadata* md, SourceInfo* source,
-                                                        ParamNames* param_names,
+                                                        const ParamNames* param_names,
                                                         const OSREntryDescriptor* entry_descriptor, EffortLevel effort,
                                                         ExceptionStyle exception_style, FunctionSpecialization* spec,
                                                         llvm::StringRef nameprefix) {

src/codegen/irgen.h

@@ -138,9 +138,11 @@ extern const std::string PASSED_GENERATOR_NAME;
 InternedString getIsDefinedName(InternedString name, InternedStringPool& interned_strings);
 bool isIsDefinedName(llvm::StringRef name);
 
-std::pair<CompiledFunction*, llvm::Function*>
-doCompile(FunctionMetadata* md, SourceInfo* source, ParamNames* param_names, const OSREntryDescriptor* entry_descriptor,
-          EffortLevel effort, ExceptionStyle exception_style, FunctionSpecialization* spec, llvm::StringRef nameprefix);
+std::pair<CompiledFunction*, llvm::Function*> doCompile(FunctionMetadata* md, SourceInfo* source,
+                                                        const ParamNames* param_names,
+                                                        const OSREntryDescriptor* entry_descriptor, EffortLevel effort,
+                                                        ExceptionStyle exception_style, FunctionSpecialization* spec,
+                                                        llvm::StringRef nameprefix);
 
 // A common pattern is to branch based off whether a variable is defined but only if it is
 // potentially-undefined.  If it is potentially-undefined, we have to generate control-flow

src/codegen/irgen/hooks.cpp

@@ -56,11 +56,9 @@ namespace pyston {
 
 // TODO terrible place for these!
 ParamNames::ParamNames(AST* ast, InternedStringPool& pool)
-    : vararg_name(NULL), kwarg_name(NULL), takes_param_names(true) {
+    : all_args_contains_names(1), takes_param_names(1), has_vararg_name(0), has_kwarg_name(0) {
     if (ast->type == AST_TYPE::Module || ast->type == AST_TYPE::ClassDef || ast->type == AST_TYPE::Expression
         || ast->type == AST_TYPE::Suite) {
-        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;
@@ -68,32 +66,57 @@ ParamNames::ParamNames(AST* ast, InternedStringPool& pool)
             AST_expr* arg = arguments->args[i];
             if (arg->type == AST_TYPE::Name) {
                 AST_Name* name = ast_cast<AST_Name>(arg);
-                arg_names.push_back(name);
-                args.push_back(name->id.s());
+                all_args.emplace_back(name);
             } else {
                 InternedString dot_arg_name = pool.get("." + std::to_string(i));
-                arg_names.push_back(new AST_Name(dot_arg_name, AST_TYPE::Param, arg->lineno, arg->col_offset));
-                args.push_back(dot_arg_name.s());
+                all_args.emplace_back(new AST_Name(dot_arg_name, AST_TYPE::Param, arg->lineno, arg->col_offset));
             }
         }
 
-        vararg_name = arguments->vararg;
-        if (vararg_name)
-            vararg = vararg_name->id.s();
+        auto vararg_name = arguments->vararg;
+        if (vararg_name) {
+            has_vararg_name = 1;
+            all_args.emplace_back(vararg_name);
+        }
 
-        kwarg_name = arguments->kwarg;
-        if (kwarg_name)
-            kwarg = kwarg_name->id.s();
+        auto kwarg_name = arguments->kwarg;
+        if (kwarg_name) {
+            has_kwarg_name = 1;
+            all_args.emplace_back(kwarg_name);
+        }
     } else {
         RELEASE_ASSERT(0, "%d", ast->type);
     }
 }
 
-ParamNames::ParamNames(const std::vector<llvm::StringRef>& args, llvm::StringRef vararg, llvm::StringRef kwarg)
-    : vararg_name(NULL), kwarg_name(NULL), takes_param_names(true) {
-    this->args = args;
-    this->vararg = vararg;
-    this->kwarg = kwarg;
+ParamNames::ParamNames(const std::vector<const char*>& args, const char* vararg, const char* kwarg)
+    : all_args_contains_names(0),
+      takes_param_names(1),
+      has_vararg_name(vararg && *vararg),
+      has_kwarg_name(kwarg && *kwarg) {
+    all_args.reserve(args.size() + has_vararg_name + has_kwarg_name);
+    for (auto&& arg : args) {
+        all_args.emplace_back(arg);
+    }
+    if (has_vararg_name)
+        all_args.emplace_back(vararg);
+    if (has_kwarg_name)
+        all_args.emplace_back(kwarg);
+}
+
+std::vector<const char*> ParamNames::allArgsAsStr() const {
+    std::vector<const char*> ret;
+    ret.reserve(all_args.size());
+    if (all_args_contains_names) {
+        for (auto&& arg : all_args) {
+            ret.push_back(arg.name->id.c_str());
+        }
+    } else {
+        for (auto&& arg : all_args) {
+            ret.push_back(arg.str);
+        }
+    }
+    return ret;
 }
 
 InternedString SourceInfo::mangleName(InternedString id) {

src/codegen/irgen/irgenerator.cpp

@@ -45,7 +45,7 @@ extern "C" void dumpLLVM(void* _v) {
 }
 
 IRGenState::IRGenState(FunctionMetadata* md, CompiledFunction* cf, llvm::Function* func, SourceInfo* source_info,
-                       std::unique_ptr<PhiAnalysis> phis, ParamNames* param_names, GCBuilder* gc,
+                       std::unique_ptr<PhiAnalysis> phis, const ParamNames* param_names, GCBuilder* gc,
                        llvm::MDNode* func_dbg_info, RefcountTracker* refcount_tracker)
     : md(md),
       cf(cf),
@@ -2982,16 +2982,17 @@ public:
         assert(python_parameters.size() == param_names.totalParameters());
 
         int i = 0;
-        for (; i < param_names.args.size(); i++) {
-            loadArgument(param_names.arg_names[i], arg_types[i], python_parameters[i], UnwindInfo::cantUnwind());
+        for (auto&& arg : param_names.argsAsName()) {
+            loadArgument(arg, arg_types[i], python_parameters[i], UnwindInfo::cantUnwind());
+            ++i;
         }
 
-        if (param_names.vararg.size()) {
-            loadArgument(param_names.vararg_name, arg_types[i], python_parameters[i], UnwindInfo::cantUnwind());
+        if (param_names.has_vararg_name) {
+            loadArgument(param_names.varArgAsName(), arg_types[i], python_parameters[i], UnwindInfo::cantUnwind());
             i++;
         }
 
-        if (param_names.kwarg.size()) {
+        if (param_names.has_kwarg_name) {
             llvm::Value* passed_dict = python_parameters[i];
             emitter.setNullable(passed_dict, true);
 
@@ -3017,7 +3018,7 @@ public:
             emitter.refConsumed(passed_dict, null_check);
             emitter.refConsumed(created_dict, isnull_terminator);
 
-            loadArgument(param_names.kwarg_name, arg_types[i], phi, UnwindInfo::cantUnwind());
+            loadArgument(param_names.kwArgAsName(), arg_types[i], phi, UnwindInfo::cantUnwind());
             i++;
         }
 

src/codegen/irgen/irgenerator.h

@@ -66,7 +66,7 @@ private:
     llvm::Function* func;
     SourceInfo* source_info;
     std::unique_ptr<PhiAnalysis> phis;
-    ParamNames* param_names;
+    const ParamNames* param_names;
     GCBuilder* gc;
     llvm::MDNode* func_dbg_info;
     RefcountTracker* refcount_tracker;
@@ -84,8 +84,8 @@ private:
 
 public:
     IRGenState(FunctionMetadata* md, CompiledFunction* cf, llvm::Function* func, SourceInfo* source_info,
-               std::unique_ptr<PhiAnalysis> phis, ParamNames* param_names, GCBuilder* gc, llvm::MDNode* func_dbg_info,
-               RefcountTracker* refcount_tracker);
+               std::unique_ptr<PhiAnalysis> phis, const ParamNames* param_names, GCBuilder* gc,
+               llvm::MDNode* func_dbg_info, RefcountTracker* refcount_tracker);
     ~IRGenState();
 
     CFG* getCFG() { return getSourceInfo()->cfg; }
@@ -125,7 +125,7 @@ public:
 
     RefcountTracker* getRefcounts() { return refcount_tracker; }
 
-    ParamNames* getParamNames() { return param_names; }
+    const ParamNames* getParamNames() { return param_names; }
 
     llvm::Value* getPassedClosure();
     llvm::Value* getCreatedClosure();

src/core/cfg.cpp

@@ -2829,14 +2829,9 @@ void VRegInfo::assignVRegs(CFG* cfg, const ParamNames& param_names, ScopeInfo* s
 #endif
 
             if (b == cfg->getStartingBlock()) {
-                for (auto* name : param_names.arg_names) {
+                for (auto* name : param_names.allArgsAsName()) {
                     name->accept(&visitor);
                 }
-                if (param_names.vararg_name)
-                    param_names.vararg_name->accept(&visitor);
-
-                if (param_names.kwarg_name)
-                    param_names.kwarg_name->accept(&visitor);
             }
 
             for (AST_stmt* stmt : b->body) {

src/core/types.h

@@ -219,32 +219,61 @@ static_assert(sizeof(ArgPassSpec) <= sizeof(void*), "ArgPassSpec doesn't fit in
 static_assert(sizeof(ArgPassSpec) == sizeof(uint32_t), "ArgPassSpec::asInt needs to be updated");
 
 struct ParamNames {
-    std::vector<llvm::StringRef> args;
-    llvm::StringRef vararg, kwarg;
+    // the arguments are either an array of char* or AST_Name* depending on if all_args_contains_names is set or not
+    union NameOrStr {
+        NameOrStr(const char* str) : str(str) {}
+        NameOrStr(AST_Name* name) : name(name) {}
 
-    // This members are only set if the InternedStringPool& constructor is used (aka. source is available)!
-    // They are used as an optimization while interpreting because the AST_Names nodes cache important stuff
-    // (InternedString, lookup_type) which would otherwise have to get recomputed all the time.
-    std::vector<AST_Name*> arg_names;
-    AST_Name* vararg_name, *kwarg_name;
+        const char* str;
+        AST_Name* name;
+    };
+    std::vector<NameOrStr> all_args;
 
-    bool takes_param_names;
+    const unsigned char all_args_contains_names : 1;
+    const unsigned char takes_param_names : 1;
+    unsigned char has_vararg_name : 1;
+    unsigned char has_kwarg_name : 1;
 
     explicit ParamNames(AST* ast, InternedStringPool& pool);
-    ParamNames(const std::vector<llvm::StringRef>& args, llvm::StringRef vararg, llvm::StringRef kwarg);
+    ParamNames(const std::vector<const char*>& args, const char* vararg, const char* kwarg);
     static ParamNames empty() { return ParamNames(); }
 
-    int totalParameters() const {
-        return args.size() + (vararg.str().size() == 0 ? 0 : 1) + (kwarg.str().size() == 0 ? 0 : 1);
-    }
+    int numNormalArgs() const { return all_args.size() - has_vararg_name - has_kwarg_name; }
+    int totalParameters() const { return all_args.size(); }
 
     int kwargsIndex() const {
-        assert(kwarg.str().size());
-        return args.size() + (vararg.str().size() == 0 ? 0 : 1);
+        assert(has_kwarg_name);
+        return all_args.size() - 1;
+    }
+
+    llvm::ArrayRef<AST_Name*> argsAsName() const {
+        assert(all_args_contains_names);
+        return llvm::makeArrayRef((AST_Name * const*)all_args.data(), numNormalArgs());
+    }
+
+    llvm::ArrayRef<AST_Name*> allArgsAsName() const {
+        assert(all_args_contains_names);
+        return llvm::makeArrayRef((AST_Name * const*)all_args.data(), all_args.size());
+    }
+
+    std::vector<const char*> allArgsAsStr() const;
+
+    AST_Name* varArgAsName() const {
+        assert(all_args_contains_names);
+        if (has_vararg_name)
+            return all_args[all_args.size() - 1 - has_kwarg_name].name;
+        return NULL;
+    }
+
+    AST_Name* kwArgAsName() const {
+        assert(all_args_contains_names);
+        if (has_kwarg_name)
+            return all_args.back().name;
+        return NULL;
     }
 
 private:
-    ParamNames() : vararg_name(NULL), kwarg_name(NULL), takes_param_names(false) {}
+    ParamNames() : all_args_contains_names(0), takes_param_names(0), has_vararg_name(0), has_kwarg_name(0) {}
 };
 
 // Similar to ArgPassSpec, this struct is how functions specify what their parameter signature is.
@@ -461,11 +490,11 @@ private:
     BoxedCode* code_obj;
 
 public:
-    int num_args;
-    bool takes_varargs, takes_kwargs;
-
     std::unique_ptr<SourceInfo> source; // source can be NULL for functions defined in the C/C++ runtime
-    ParamNames param_names;
+
+    const ParamNames param_names;
+    const bool takes_varargs, takes_kwargs;
+    const int num_args;
 
     FunctionList
         versions; // any compiled versions along with their type parameters; in order from most preferred to least
@@ -518,9 +547,9 @@ public:
     static FunctionMetadata* create(void* f, ConcreteCompilerType* rtn_type, int nargs, bool takes_varargs,
                                     bool takes_kwargs, const ParamNames& param_names = ParamNames::empty(),
                                     ExceptionStyle exception_style = CXX) {
-        assert(!param_names.takes_param_names || nargs == param_names.args.size());
-        assert(takes_varargs || param_names.vararg.str() == "");
-        assert(takes_kwargs || param_names.kwarg.str() == "");
+        assert(!param_names.takes_param_names || nargs == param_names.numNormalArgs());
+        assert(takes_varargs || !param_names.has_vararg_name);
+        assert(takes_kwargs || !param_names.has_kwarg_name);
 
         FunctionMetadata* fmd = new FunctionMetadata(nargs, takes_varargs, takes_kwargs, param_names);
         fmd->addVersion(f, rtn_type, exception_style);

src/runtime/code.cpp

@@ -78,12 +78,8 @@ Box* BoxedCode::varnames(Box* b, void*) noexcept {
         return incref(EmptyTuple);
 
     std::vector<Box*> elts;
-    for (auto sr : param_names.args)
+    for (auto sr : param_names.allArgsAsStr())
         elts.push_back(boxString(sr));
-    if (param_names.vararg.size())
-        elts.push_back(boxString(param_names.vararg));
-    if (param_names.kwarg.size())
-        elts.push_back(boxString(param_names.kwarg));
     auto rtn = BoxedTuple::create(elts.size(), &elts[0]);
     for (auto e : elts)
         Py_DECREF(e);
@@ -95,9 +91,9 @@ Box* BoxedCode::flags(Box* b, void*) noexcept {
     BoxedCode* code = static_cast<BoxedCode*>(b);
 
     int flags = 0;
-    if (code->f->param_names.vararg.size())
+    if (code->f->param_names.has_vararg_name)
         flags |= CO_VARARGS;
-    if (code->f->param_names.kwarg.size())
+    if (code->f->param_names.has_kwarg_name)
         flags |= CO_VARKEYWORDS;
     if (code->f->isGenerator())
         flags |= CO_GENERATOR;

src/runtime/objmodel.cpp

@@ -4066,8 +4066,13 @@ static int placeKeyword(const ParamNames* param_names, llvm::SmallVector<bool, 8
     assert(kw_val);
     assert(kw_name);
 
-    for (int j = 0; j < param_names->args.size(); j++) {
-        if (param_names->args[j] == kw_name->s() && kw_name->size() > 0) {
+    for (int j = 0; j < param_names->numNormalArgs(); j++) {
+        llvm::StringRef s;
+        if (param_names->all_args_contains_names)
+            s = param_names->all_args[j].name->id.s();
+        else
+            s = param_names->all_args[j].str;
+        if (s == kw_name->s() && kw_name->size() > 0) {
             if (params_filled[j]) {
                 raiseExcHelper(TypeError, "%.200s() got multiple values for keyword argument '%s'", func_name_cb(),
                                kw_name->c_str());