Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

When generating .pxd, warn for public but unexported functions #563

Open
bkietz opened this issue Jul 19, 2024 · 1 comment
Open

When generating .pxd, warn for public but unexported functions #563

bkietz opened this issue Jul 19, 2024 · 1 comment

Comments

@bkietz
Copy link
Member

bkietz commented Jul 19, 2024

It's possible to add a function to inline_array.h or other header included by nanoarrow.h which is nevertheless not included in generated .pxd. The error is then simply "no matching function" when compiling the .pxd, which is not informative. It'd be better if the pxd could recognize function definitions or declarations which aren't translatable into a cdef, then warn if those aren't on an exclusion list.

@paleolimbot
Copy link
Member

That's a great point! The .pxd generator is really helpful but pretty wild:

# Generate the nanoarrow_c.pxd file used by the Cython extensions
class PxdGenerator:
def __init__(self):
self._define_regexes()
def generate_pxd(self, file_in, file_out):
file_in_name = pathlib.Path(file_in).name
# Read the header
content = None
with open(file_in, "r") as input:
content = input.read()
# Strip comments
content = self.re_comment.sub("", content)
# Replace NANOARROW_MAX_FIXED_BUFFERS with its value
content = self._preprocess_content(content)
# Find typedefs, types, and function definitions
typedefs = self._find_typedefs(content)
types = self._find_types(content)
func_defs = self._find_func_defs(content)
# Make corresponding cython definitions
typedefs_cython = [self._typdef_to_cython(t, " ") for t in typedefs]
types_cython = [self._type_to_cython(t, " ") for t in types]
func_defs_cython = [self._func_def_to_cython(d, " ") for d in func_defs]
# Unindent the header
header = self.re_newline_plus_indent.sub("\n", self._pxd_header())
# Write nanoarrow_c.pxd
with open(file_out, "wb") as output:
output.write(header.encode("UTF-8"))
output.write(
f'\ncdef extern from "{file_in_name}" nogil:\n'.encode("UTF-8")
)
# A few things we add in manually
self._write_defs(output)
for type in types_cython:
output.write(type.encode("UTF-8"))
output.write(b"\n\n")
for typedef in typedefs_cython:
output.write(typedef.encode("UTF-8"))
output.write(b"\n")
output.write(b"\n")
for func_def in func_defs_cython:
output.write(func_def.encode("UTF-8"))
output.write(b"\n")
def _preprocess_content(self, content):
return content
def _write_defs(self, output):
pass
def _define_regexes(self):
self.re_comment = re.compile(r"\s*//[^\n]*")
self.re_typedef = re.compile(r"typedef(?P<typedef>[^;]+)")
self.re_type = re.compile(
r"(?P<type>struct|union|enum) (?P<name>Arrow[^ ]+) {(?P<body>[^}]*)}"
)
self.re_func_def = re.compile(
r"\n(static inline )?(?P<const>const )?(struct |enum )?"
r"(?P<return_type>[A-Za-z0-9_*]+) "
r"(?P<name>Arrow[A-Za-z0-9]+)\((?P<args>[^\)]*)\);"
)
self.re_tagged_type = re.compile(
r"(?P<type>struct|union|enum) (?P<name>Arrow[A-Za-z]+)"
)
self.re_struct_delim = re.compile(r";\s*")
self.re_enum_delim = re.compile(r",\s*")
self.re_whitespace = re.compile(r"\s+")
self.re_newline_plus_indent = re.compile(r"\n +")
def _strip_comments(self, content):
return self.re_comment.sub("", content)
def _find_typedefs(self, content):
return [m.groupdict() for m in self.re_typedef.finditer(content)]
def _find_types(self, content):
return [m.groupdict() for m in self.re_type.finditer(content)]
def _find_func_defs(self, content):
return [m.groupdict() for m in self.re_func_def.finditer(content)]
def _typdef_to_cython(self, t, indent=""):
typedef = t["typedef"]
typedef = self.re_tagged_type.sub(r"\2", typedef)
return f"{indent}ctypedef {typedef}"
def _type_to_cython(self, t, indent=""):
type = t["type"]
name = t["name"]
body = self.re_tagged_type.sub(r"\2", t["body"].strip())
if type == "enum":
items = [item for item in self.re_enum_delim.split(body) if item]
else:
items = [item for item in self.re_struct_delim.split(body) if item]
cython_body = f"\n{indent} ".join([""] + items)
return f"{indent}{type} {name}:{cython_body}"
def _func_def_to_cython(self, d, indent=""):
return_type = d["return_type"].strip()
if d["const"]:
return_type = "const " + return_type
name = d["name"]
args = re.sub(r"\s+", " ", d["args"].strip())
args = self.re_tagged_type.sub(r"\2", args)
# Cython doesn't do (void)
if args == "void":
args = ""
return f"{indent}{return_type} {name}({args})"
def _pxd_header(self):
return """
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
# cython: language_level = 3
"""

The functions that are included in the .pxd match a very narrow regex that only includes declarations (not definitions). I forget exactly why this happened but I think allowing it to end with something other than ); caused some problems (it's tricky because functions definitions can span multiple lines and contain nested parentheses).

When the .pxd generator was written we had more constraints than we do now: we used to allow downloading a pre-concatenated nanoarrow.h/c to support installing from github when CMake wasn't installed, but now we have nightly builds and the concatenator is written in Python. We could now probably use pycparser to find declarations or definitions (pure Python is preferable because Windows).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants