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

bkietz · 2024-07-19T19:23:07Z

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.

The text was updated successfully, but these errors were encountered:

paleolimbot · 2024-07-19T19:40:48Z

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

arrow-nanoarrow/python/bootstrap.py

Lines 24 to 170 in fcf3a80

    
           # 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).

WillAyd · 2024-11-14T20:04:19Z

Not sure if there's an appetite for the work in #669 but it looks like the autopxd2 library might avoid this issue

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

bkietz commented Jul 19, 2024

paleolimbot commented Jul 19, 2024

WillAyd commented Nov 14, 2024

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

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

Comments

bkietz commented Jul 19, 2024

paleolimbot commented Jul 19, 2024

WillAyd commented Nov 14, 2024