Skip to content

Commit

Permalink
fix #428: ensure SWIG API raises exception
Browse files Browse the repository at this point in the history
  • Loading branch information
biojppm committed Apr 28, 2024
1 parent 5f66101 commit 93d76be
Show file tree
Hide file tree
Showing 8 changed files with 70 additions and 15 deletions.
7 changes: 6 additions & 1 deletion api/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ c4_log("found swig ${SWIG_VERSION}: ${SWIG_EXECUTABLE}")
# https://cmake.org/cmake/help/v3.13/module/UseSWIG.html
include(UseSWIG)

if(NOT RYML_DEFAULT_CALLBACK_USES_EXCEPTIONS)
message(FATAL_ERROR "API requires exceptions")
endif()

set(RYML_API_DIR ${CMAKE_CURRENT_LIST_DIR})
set(RYML_SWIG_SRC ${RYML_API_DIR}/ryml.i)

Expand Down Expand Up @@ -99,7 +103,8 @@ if(RYML_BUILD_API_PYTHON)
if(WIN32)
target_compile_definitions(${t} PUBLIC __WIN32__)
endif()
target_compile_definitions(${t} PUBLIC ${RYML_SWIG_ARCH_DEFINES})
c4_get_transitive_property(ryml COMPILE_DEFINITIONS ryml_defs)
target_compile_definitions(${t} PUBLIC ${RYML_SWIG_ARCH_DEFINES} ${ryml_defs})

# Install the SWIG .so/.dll file
install(
Expand Down
23 changes: 11 additions & 12 deletions api/python/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ endif
# How to invoke python
PYTHON := python
# How to invoke pytest
PYTEST := $(PYTHON) -m pytest -vvv
PYTEST := $(PYTHON) -m pytest -vvv -s

ACTIVATE=[[ -e $(ACTIVATE_SCRIPT) ]] && source $(ACTIVATE_SCRIPT);

Expand All @@ -40,10 +40,11 @@ venv:
# Setup requirements.
${ACTIVATE} pip install -v -r requirements.txt
${ACTIVATE} pip install -v -e ../..
@${ACTIVATE} $(PYTHON) -c "from ryml.version import version as v; print('Installed version:', v)"
${ACTIVATE} $(PYTHON) -c "from ryml.version import version as v; print('Installed version:', v)"

.PHONY: build-sdist
build-sdist: | $(ACTIVATE_SCRIPT)
${ACTIVATE} (cd ../..; pip show build)
${ACTIVATE} (cd ../..; $(PYTHON) -m build --sdist --outdir $(PWD)/dist)


Expand All @@ -55,30 +56,27 @@ build-wheel: | $(ACTIVATE_SCRIPT)
${ACTIVATE} pip wheel -v dist/*.tar.gz --wheel-dir $(PWD)/dist

.PHONY: build
build:
rm -rf build dist
$(MAKE) build-sdist
$(MAKE) build-wheel
build: build-sdist build-wheel

# PYPI_TEST = --repository-url https://test.pypi.org/legacy/
PYPI_TEST = --repository testpypi

.PHONY: upload-test
upload-test: | $(ACTIVATE_SCRIPT)
make clean
make build-sdist
$(MAKE) clean
$(MAKE) build-sdist
${ACTIVATE} twine upload ${PYPI_TEST} dist/*

.PHONY: upload
upload: | $(ACTIVATE_SCRIPT)
make clean
make build-sdist
$(MAKE) clean
$(MAKE) build-sdist
${ACTIVATE} twine upload --verbose dist/*

.PHONY: check
check: | $(ACTIVATE_SCRIPT)
make clean
make build-wheel
$(MAKE) clean
$(MAKE) build-wheel
${ACTIVATE} twine check dist/*.whl

.PHONY: install
Expand All @@ -87,6 +85,7 @@ install: | $(ACTIVATE_SCRIPT)

.PHONY: test
test: | $(ACTIVATE_SCRIPT)
${ACTIVATE} pip install -v -e ../..
${ACTIVATE} $(PYTEST) tests

.PHONY: version
Expand Down
2 changes: 2 additions & 0 deletions api/python/requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,5 @@ ninja
pyyaml
prettytable
pytest
build
twine
24 changes: 24 additions & 0 deletions api/python/tests/test_parse.py
Original file line number Diff line number Diff line change
Expand Up @@ -481,6 +481,30 @@ def test44_emit_json_short_buf(self):



# -----------------------------------------------------------------------------
# -----------------------------------------------------------------------------
# -----------------------------------------------------------------------------

class TestParseFailure(unittest.TestCase):

yaml = "[:HELLO: b}"

def setUp(self):
self.src_as_str = str(__class__.yaml)
self.src_as_bytearray = bytearray(__class__.yaml, "utf8")

def test_in_arena(self):
self.assertNotEqual(self.src_as_str, "")
with self.assertRaises(RuntimeError):
tree = ryml.parse_in_arena(self.src_as_str)

def test_in_place(self):
self.assertNotEqual(self.src_as_bytearray, "")
with self.assertRaises(RuntimeError):
tree = ryml.parse_in_place(self.src_as_bytearray)



# -----------------------------------------------------------------------------
# -----------------------------------------------------------------------------
# -----------------------------------------------------------------------------
Expand Down
25 changes: 23 additions & 2 deletions api/ryml.i
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@

%module ryml


//-----------------------------------------------------------------------------
// this block will be pasted verbatim in the generated C++ source file

Expand All @@ -23,8 +22,8 @@ using csubstr = c4::csubstr;

%}

//-----------------------------------------------------------------------------

//-----------------------------------------------------------------------------

%apply (const char *STRING, size_t LENGTH) { (const char *str, size_t len) };
%apply (char *STRING, size_t LENGTH) { (char *str, size_t len) };
Expand Down Expand Up @@ -91,6 +90,8 @@ using csubstr = c4::csubstr;
%typemap(typecheck) c4::csubstr = const char *;


//-----------------------------------------------------------------------------

%typemap(out) c4::csubstr {
#if defined(SWIGPYTHON)
if($1.str == nullptr) {
Expand All @@ -111,6 +112,26 @@ using csubstr = c4::csubstr;
};


//-----------------------------------------------------------------------------

// Language independent exception handler.
// KEEP THIS BEFORE THE FOLLOWING FUNCTIONS!
// see https://stackoverflow.com/a/61621747
%include <std_string.i>
%include <exception.i>
%exception {
try {
$action
} catch(std::exception &e) {
SWIG_exception(SWIG_RuntimeError, e.what());
} catch(...) {
SWIG_exception(SWIG_UnknownError, "unknown error");
}
}


//-----------------------------------------------------------------------------

%inline %{

void parse_csubstr(c4::csubstr s, c4::yml::Tree *t)
Expand Down
1 change: 1 addition & 0 deletions doc/sphinx_other_languages.rst
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ using `emscripten`:
-D RYML_BUILD_TESTS=ON \
-D RYML_BUILD_BENCHMARKS=OFF \
-D RYML_TEST_SUITE=OFF \
-D RYML_DEFAULT_CALLBACK_USES_EXCEPTIONS=ON \
-D CMAKE_BUILD_TYPE=Release \
-D CMAKE_CXX_FLAGS='-s DISABLE_EXCEPTION_CATCHING=0'
cmake --build build/emscripten --target ryml-test-run -j
Expand Down
2 changes: 2 additions & 0 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ def get_environment_cmake_flags():
cmake_component='python',
cmake_configure_options=get_environment_cmake_flags() + [
"-DRYML_BUILD_API:BOOL=ON",
"-DRYML_DEFAULT_CALLBACKS:BOOL=ON",
"-DRYML_DEFAULT_CALLBACK_USES_EXCEPTIONS:BOOL=ON",
# Force cmake to use the Python interpreter we are currently
# using to run setup.py
"-DPython3_EXECUTABLE:FILEPATH=" + sys.executable,
Expand Down
1 change: 1 addition & 0 deletions src/c4/yml/common.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
# endif
#endif // RYML_NO_DEFAULT_CALLBACKS


namespace c4 {
namespace yml {

Expand Down

0 comments on commit 93d76be

Please sign in to comment.