diff --git a/src/cpyrt/CPPMethod.h b/src/cpyrt/CPPMethod.h index 4fdd792d..74ec31b2 100644 --- a/src/cpyrt/CPPMethod.h +++ b/src/cpyrt/CPPMethod.h @@ -74,6 +74,8 @@ class CPPMethod : public PyCallable { PyObject* Call(CPPInstance*& self, cpyrt_PyArgs_t args, size_t nargsf, PyObject* kwds, CallContext* ctxt = nullptr) override; + interop::TCppMethod_t GetMethod() const { return fMethod; } + protected: virtual bool ProcessArgs(PyCallArgs& args); @@ -83,7 +85,6 @@ class CPPMethod : public PyCallable { CallContext* ctxt = nullptr); PyObject* Execute(void* self, ptrdiff_t offset, CallContext* ctxt = nullptr); - interop::TCppMethod_t GetMethod() { return fMethod; } interop::TCppScope_t GetScope() { return fScope; } Executor* GetExecutor() { return fExecutor; } std::string GetSignatureString(bool show_formalargs = true); diff --git a/src/cpyrt/TemplateProxy.cxx b/src/cpyrt/TemplateProxy.cxx index d826c00f..1a48a224 100644 --- a/src/cpyrt/TemplateProxy.cxx +++ b/src/cpyrt/TemplateProxy.cxx @@ -283,9 +283,23 @@ PyObject* TemplateProxy::Instantiate(const std::string& fname, // Case 3/4: pre-existing method that was either not found b/c the full // templated name was constructed in this call or it failed as overload else if (bIsCppOL) { - // TODO: see above, since the call hasn't happened yet, this overload may - // already exist and fail again. - ((CPPOverload*)pyol)->AdoptMethod(meth); // takes ownership + // The lookup is cached before the call, so when the call then fails + // (e.g. an implicit conversion attempt that is not allowed to nest), the + // next call instantiates the same method again. Adopting it again would + // grow the overload set with duplicates and force the whole set to be + // ranked anew on every call. + bool known = false; + for (PyCallable* pc : ((CPPOverload*)pyol)->fMethodInfo->fMethods) { + auto* existing = dynamic_cast(pc); + if (existing && existing->GetMethod() == cppmeth) { + known = true; + break; + } + } + if (known) + delete meth; + else + ((CPPOverload*)pyol)->AdoptMethod(meth); // takes ownership } // Case 5: must be a template proxy, meaning that current template name is diff --git a/src/interop/interop_wrapper.cxx b/src/interop/interop_wrapper.cxx index d26697c8..62faa9d8 100644 --- a/src/interop/interop_wrapper.cxx +++ b/src/interop/interop_wrapper.cxx @@ -524,6 +524,36 @@ bool interop::AppendTypesSlow(const std::string& name, std::lock_guard Lock(InterOpMutex); + // Every lookup below declares code through the interpreter, which parses, + // commits a transaction and leaves a new declaration in the AST, so each one + // costs more than the last. The same names come back constantly (overload + // ranking resolves every argument type of every candidate on each call that + // is not memoized), so remember what a name resolved to. A name that fails + // may still resolve once more declarations are available, so only successes + // are cached. + using SlowTypesKey = std::pair; + static std::map> s_cache; + const SlowTypesKey cache_key{name, parent.data}; + // the integral values are strings whose ownership passes to the caller + // (which frees them), so the cache keeps and hands out copies of its own + auto copy_of = [](const Cpp::TemplateArgInfo& info) { + return Cpp::TemplateArgInfo(info.m_Type, info.m_IntegralValue + ? strdup(info.m_IntegralValue) + : nullptr); + }; + if (auto cached = s_cache.find(cache_key); cached != s_cache.end()) { + for (const Cpp::TemplateArgInfo& info : cached->second) + types.push_back(copy_of(info)); + return false; + } + const size_t n_types_before = types.size(); + auto remember = [&] { + std::vector resolved; + for (size_t i = n_types_before; i < types.size(); ++i) + resolved.push_back(copy_of(types[i])); + s_cache.emplace(cache_key, std::move(resolved)); + }; + // We might have an entire expression such as int, double. static unsigned long long struct_count = 0; std::string code = @@ -554,7 +584,10 @@ bool interop::AppendTypesSlow(const std::string& name, TCppScope_t instance_class = Cpp::GetScopeFromType(varN); size_t oldSize = types.size(); Cpp::GetClassTemplateInstantiationArgs(instance_class, types); - return oldSize == types.size(); + if (oldSize == types.size()) + return true; + remember(); + return false; } } @@ -593,6 +626,7 @@ bool interop::AppendTypesSlow(const std::string& name, strdup(std::to_string(Cpp::GetEnumConstantValue(scope)).c_str()); types.emplace_back(type.data, integral_value); } + remember(); return false; } diff --git a/test/test_regression.py b/test/test_regression.py index 2a2bda4a..4a24033c 100644 --- a/test/test_regression.py +++ b/test/test_regression.py @@ -1727,3 +1727,51 @@ def test56_enum_arg_typedefed_underlying_type(self): assert ns.pick(ns.S.A) == 1 assert ns.pick(ns.S.B) == 5 + + def test57_repeated_enum_constant_template_argument(self): + """A template argument spelled as an enum constant resolves repeatedly""" + + import cppjit + + cppjit.cppdef("""\ + namespace RepeatedEnumArg { + enum E { A = 1, B = 2 }; + template struct T1 { static constexpr int v = (int)e; }; + template struct T2 { static constexpr int v = (int)e; }; + }""") + + ns = cppjit.gbl.RepeatedEnumArg + + # the resolved argument is memoized by name; its integral value is a + # string the first instantiation frees, so the second needs its own + assert ns.T1["RepeatedEnumArg::E::A"].v == 1 + assert ns.T2["RepeatedEnumArg::E::A"].v == 1 + assert ns.T2["RepeatedEnumArg::E::B"].v == 2 + + def test58_repeated_implicit_tuple_conversion(self): + """Repeated implicit tuple-to-pair conversion does not grow the overload set""" + + import cppjit + + cppjit.cppdef("""\ + #include + namespace RepeatedImplicitTuple { + int first(std::pair&& p) { return p.first; } + }""") + + ns = cppjit.gbl.RepeatedImplicitTuple + pair = cppjit.gbl.std.pair["int", "int"] + + def n_ctor_overloads(): + # the instantiated converting constructor is cached on the class + # under the constructor's own name + ol = pair.__dict__.get("pair") + return len(ol.__doc__.split("\n")) if ol is not None else 0 + + assert ns.first((1, 2)) == 1 + n = n_ctor_overloads() + assert n > 0 + + for i in range(20): + assert ns.first((i, 2)) == i + assert n_ctor_overloads() == n