Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 61 additions & 2 deletions src/interop/interop_wrapper.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,9 @@ static inline bool is_integral(std::string& s) {
s = "1";
return true;
}
return !s.empty() && std::find_if(s.begin(), s.end(), [](unsigned char c) {
// allow a leading minus (negative literal)
auto begin = s.begin() + (s.size() > 1 && s[0] == '-' ? 1 : 0);
return !s.empty() && std::find_if(begin, s.end(), [](unsigned char c) {
return !std::isdigit(c);
}) == s.end();
}
Expand Down Expand Up @@ -469,6 +471,22 @@ static bool is_identifier(std::string_view s) {
std::all_of(s.begin() + 1, s.end(), is_valid_body);
};

// A template argument carried by name needs a CppInterOp that resolves the
// name; such a CppInterOp exports SupportsNamedTemplateArguments.
static bool supportsNamedTemplateArgs() {
#ifdef _WIN32
return false; // no dlsym; enable once the pin guarantees the capability
#else
static const bool Supported = [] {
// CppInterOp is dlopen'ed RTLD_LOCAL; its exports need its own handle.
void* handle = dlopen(cppinterop_paths().Library.c_str(),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems a weird way to check if a feature is there or not. Can’t we rely on the version hash, for example?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The probe guards a window, not a permanent difference. cppjit pins CppInterOp by commit sha. The current pin (9802d619) predates #1074, so Cpp::GetNamed does not resolve a named constant yet. Without the guard, the by-name tests fail on the pinned build.

I looked at the version hash. Cpp::GetVersion() reads the VERSION file, which is the same before and after #1074. Cpp::GetBuildInfo() has no git sha, and a sha has no order, so a check cannot express "at least this commit".

So I think the pin is already the version check. My proposal: drop the probe and hold this PR until #1074 merges. Then I rebase, move the pin to a commit that carries #1074, and the by-name tests run unconditionally. This matches #63, which waited on #1101 the same way. The removal is ready locally.

Tell me if you prefer a different order.

RTLD_LOCAL | RTLD_NOW | RTLD_NOLOAD);
return handle && dlsym(handle, "cppinterop_SupportsNamedTemplateArguments");
}();
return Supported;
#endif
}

// returns true if no new type was added.
bool interop::AppendTypesSlow(const std::string& name,
std::vector<Cpp::TemplateArgInfo>& types,
Expand Down Expand Up @@ -503,6 +521,32 @@ bool interop::AppendTypesSlow(const std::string& name,
// outside the query scope, e.g. `typedef Foo Bar;` at TU consulted
// from a method on Foo).
if (is_identifier(name)) {
// true/false are identifier-shaped value literals.
if (name == "true" || name == "false") {
types.emplace_back(Cpp::GetType("bool").data,
strdup(name == "true" ? "1" : "0"));
return false;
}
if (supportsNamedTemplateArgs()) {
TCppScope_t named = parent ? Cpp::GetNamed(name, parent) : nullptr;
if (!named)
named = Cpp::GetNamed(name);
// The identifier may name a non-type entity (constexpr variable, enum
// constant); pass its qualified name so Sema gets an expression, not the
// entity's type.
if (named && (Cpp::IsVariable(named) || Cpp::IsEnumConstant(named))) {
types.emplace_back(
Cpp::GetTypeFromScope(named).data,
strdup(Cpp::GetQualifiedCompleteName(named).c_str()));
return false;
}
// Template name (template-template arg): no type; carried by name.
if (named && Cpp::IsTemplate(named)) {
types.emplace_back(
nullptr, strdup(Cpp::GetQualifiedCompleteName(named).c_str()));
return false;
}
}
TCppType_t type = parent ? Cpp::GetType(name, parent) : nullptr;
if (!type)
type = Cpp::GetType(name);
Expand Down Expand Up @@ -572,16 +616,31 @@ bool interop::AppendTypesSlow(const std::string& name,
}

if (!type) {
// Qualified template name (template-template arg).
if (supportsNamedTemplateArgs()) {
if (TCppScope_t named = GetEnumFromCompleteName(i)) {
if (Cpp::IsTemplate(named)) {
types.emplace_back(
nullptr, strdup(Cpp::GetQualifiedCompleteName(named).c_str()));
continue;
}
}
}
types.clear();
return true;
}

if (is_integral(i))
integral_value = strdup(i.c_str());
if (TCppScope_t scope = GetEnumFromCompleteName(i))
if (TCppScope_t scope = GetEnumFromCompleteName(i)) {
if (Cpp::IsEnumConstant(scope))
integral_value =
strdup(std::to_string(Cpp::GetEnumConstantValue(scope)).c_str());
// A variable is a non-type argument; pass its name (see the identifier
// path).
else if (supportsNamedTemplateArgs() && Cpp::IsVariable(scope))
integral_value = strdup(Cpp::GetQualifiedCompleteName(scope).c_str());
}
types.emplace_back(type.data, integral_value);
}
return false;
Expand Down
22 changes: 22 additions & 0 deletions test/support.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from __future__ import print_function

import ctypes
import os
import subprocess
import sys
Expand Down Expand Up @@ -142,3 +143,24 @@ def _jit_resolves_std_filesystem():

CAN_JIT_STD_FILESYSTEM = _jit_resolves_std_filesystem()
IS_VALGRIND = True if os.getenv("IS_VALGRIND") else False


def _has_named_template_args():
"""Whether a template argument may name a constant.

Look for the same CppInterOp export that cppjit gates on. A rejected
instantiation leaves interpreter state that changes later tests.
"""

libname = {
"win32": "clangCppInterOp.dll",
"darwin": "libclangCppInterOp.dylib",
}.get(sys.platform, "libclangCppInterOp.so")
lib = os.path.join(os.path.dirname(cppjit.__file__), "interop", "lib", libname)
try:
return hasattr(ctypes.CDLL(lib), "cppinterop_SupportsNamedTemplateArguments")
except OSError:
return False


HAS_NAMED_TEMPLATE_ARGS = _has_named_template_args()
52 changes: 52 additions & 0 deletions test/test_templates.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import py
from pytest import mark, raises
from support import (
HAS_NAMED_TEMPLATE_ARGS,
IS_CLANG_REPL,
IS_CLING,
IS_LINUX_ARM,
Expand Down Expand Up @@ -84,6 +85,57 @@ def test02_non_type_template_args(self):
assert cppjit.gbl.nt_templ_args[1]() == 1
assert cppjit.gbl.nt_templ_args[256]() == 256

# negative literals are values, not types
assert cppjit.gbl.nt_templ_args[-1]() == -1
assert cppjit.gbl.nt_templ_args[-256]() == -256

# true/false are identifier-shaped value literals
cppjit.cppdef("template<bool b> bool nt_templ_bool() { return b; };")
assert cppjit.gbl.nt_templ_bool["true"]() is True
assert cppjit.gbl.nt_templ_bool["false"]() is False

@mark.skipif(
not HAS_NAMED_TEMPLATE_ARGS,
reason="needs a CppInterOp that resolves named template arguments",
)
def test02a_named_template_args(self):
"""Use of template names and named constants as template arguments"""

import cppjit

cppjit.cppdef("""\
template <typename T> struct NtPlain {};
constexpr int kNtThree = 3;
enum NtEnum { kNtFour = 4 };
namespace ntarg {
template <typename T> using Alias = NtPlain<T>;
namespace inner { template <typename T> struct Nested {}; }
template <template <typename> typename TT> struct TakesTmpl {};
template <int N> struct TakesInt {};
constexpr int kFive = 5;
}""")

gbl = cppjit.gbl

# a template name, unqualified and qualified, plus an alias template
assert (
gbl.ntarg.TakesTmpl["NtPlain"].__cpp_name__ == "ntarg::TakesTmpl<NtPlain>"
)
assert (
gbl.ntarg.TakesTmpl["ntarg::Alias"].__cpp_name__
== "ntarg::TakesTmpl<ntarg::Alias>"
)
assert (
gbl.ntarg.TakesTmpl["ntarg::inner::Nested"].__cpp_name__
== "ntarg::TakesTmpl<ntarg::inner::Nested>"
)

# a named constant is an expression, not an integer literal: unqualified
# and qualified constexpr variables, and an enum constant
assert gbl.ntarg.TakesInt["kNtThree"].__cpp_name__ == "ntarg::TakesInt<3>"
assert gbl.ntarg.TakesInt["ntarg::kFive"].__cpp_name__ == "ntarg::TakesInt<5>"
assert gbl.ntarg.TakesInt["kNtFour"].__cpp_name__ == "ntarg::TakesInt<4>"

def test03_templated_function(self):
"""Templated global and static functions lookup and calls"""

Expand Down
Loading