Merge pull request #1933 from Shaikh-Ubaid/pythoncall_suggestions

certik · web-flow · commit 8838e524f4c3 · 2023-06-17T21:27:56.000-06:00
PythonCall: Add separate CI and rename flag to --link-numpy
diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml
@@ -290,3 +290,46 @@ jobs:
             cd integration_tests
             ./run_tests.py -b llvm
             ./run_tests.py -b c
+
+  cpython_interop:
+    name: Test CPython Interop (@pythoncall)
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v3
+        with:
+          fetch-depth: 0
+
+      - uses: mamba-org/setup-micromamba@v1
+        with:
+          environment-file: ci/environment.yml
+          create-args: >-
+            python=3.10
+            bison=3.4
+
+      - uses: hendrikmuhs/ccache-action@main
+        with:
+          variant: sccache
+          key: ${{ github.job }}-${{ matrix.os }}
+
+      - name: Build Linux
+        shell: bash -l {0}
+        run: |
+            ./build0.sh
+            cmake . -GNinja \
+              -DCMAKE_BUILD_TYPE=Debug \
+              -DWITH_LLVM=yes \
+              -DLFORTRAN_BUILD_ALL=yes \
+              -DWITH_STACKTRACE=no \
+              -DWITH_RUNTIME_STACKTRACE=yes \
+              -DCMAKE_PREFIX_PATH="$CONDA_PREFIX" \
+              -DCMAKE_INSTALL_PREFIX=`pwd`/inst \
+              -DCMAKE_C_COMPILER_LAUNCHER=sccache \
+              -DCMAKE_CXX_COMPILER_LAUNCHER=sccache
+
+            cmake --build . -j16 --target install
+
+      - name: Test Linux
+        shell: bash -l {0}
+        run: |
+            cd integration_tests
+            ./run_tests.py -b cpython_py c_py
diff --git a/integration_tests/CMakeLists.txt b/integration_tests/CMakeLists.txt
@@ -73,11 +73,10 @@ message("LPYTHON_RTLIB_DIR: ${LPYTHON_RTLIB_DIR}")
 message("LPYTHON_RTLIB_LIBRARY: ${LPYTHON_RTLIB_LIBRARY}")
 
 
-macro(RUN_UTIL RUN_FAIL RUN_NAME RUN_FILE_NAME RUN_IMPORT_PATH RUN_LABELS RUN_ENABLE_CPYTHON RUN_EXTRAFILES RUN_EXTRA_ARGS)
+macro(RUN_UTIL RUN_FAIL RUN_NAME RUN_FILE_NAME RUN_LABELS RUN_ENABLE_CPYTHON RUN_EXTRAFILES RUN_EXTRA_ARGS)
     set(fail ${${RUN_FAIL}})
     set(name ${${RUN_NAME}})
     set(file_name ${${RUN_FILE_NAME}})
-    set(import_path ${${RUN_IMPORT_PATH}})
     set(labels ${${RUN_LABELS}})
     set(run_enable_cpython ${${RUN_ENABLE_CPYTHON}})
     set(extra_files ${${RUN_EXTRAFILES}})
@@ -89,19 +88,11 @@ macro(RUN_UTIL RUN_FAIL RUN_NAME RUN_FILE_NAME RUN_IMPORT_PATH RUN_LABELS RUN_EN
 
     if (${KIND} IN_LIST labels)
         if (KIND STREQUAL "llvm")
-            if (import_path)
-                add_custom_command(
-                    OUTPUT ${name}.o
-                    COMMAND ${LPYTHON} -c -I ${CMAKE_CURRENT_SOURCE_DIR}/${import_path} ${extra_args} ${CMAKE_CURRENT_SOURCE_DIR}/${file_name}.py -o ${name}.o
-                    DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/${file_name}.py
-                    VERBATIM)
-            else ()
-                add_custom_command(
-                    OUTPUT ${name}.o
-                    COMMAND ${LPYTHON} -c ${extra_args} ${CMAKE_CURRENT_SOURCE_DIR}/${file_name}.py -o ${name}.o
-                    DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/${file_name}.py
-                    VERBATIM)
-            endif()
+            add_custom_command(
+                OUTPUT ${name}.o
+                COMMAND ${LPYTHON} -c ${extra_args} ${CMAKE_CURRENT_SOURCE_DIR}/${file_name}.py -o ${name}.o
+                DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/${file_name}.py
+                VERBATIM)
             add_executable(${name} ${name}.o ${extra_files})
             set_target_properties(${name} PROPERTIES LINKER_LANGUAGE C)
             target_link_libraries(${name} lpython_rtlib)
@@ -113,30 +104,34 @@ macro(RUN_UTIL RUN_FAIL RUN_NAME RUN_FILE_NAME RUN_IMPORT_PATH RUN_LABELS RUN_EN
                 set_tests_properties(${name} PROPERTIES WILL_FAIL TRUE)
             endif()
         elseif(KIND STREQUAL "c")
-            if (import_path)
-                add_custom_command(
-                    OUTPUT ${name}.c
-                    COMMAND ${LPYTHON} ${extra_args} -I ${CMAKE_CURRENT_SOURCE_DIR}/${import_path} --show-c ${CMAKE_CURRENT_SOURCE_DIR}/${file_name}.py > ${name}.c
-                    DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/${file_name}.py
-                    VERBATIM)
-            else ()
-                add_custom_command(
-                    OUTPUT ${name}.c
-                    COMMAND ${LPYTHON} ${extra_args} --show-c ${CMAKE_CURRENT_SOURCE_DIR}/${file_name}.py > ${name}.c
-                    DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/${file_name}.py
-                    VERBATIM)
-            endif()
-
+            add_custom_command(
+                OUTPUT ${name}.c
+                COMMAND ${LPYTHON} ${extra_args} --show-c ${CMAKE_CURRENT_SOURCE_DIR}/${file_name}.py > ${name}.c
+                DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/${file_name}.py
+                VERBATIM)
             add_executable(${name} ${name}.c ${extra_files})
             target_include_directories(${name} PRIVATE ${CMAKE_SOURCE_DIR})
             set_target_properties(${name} PROPERTIES LINKER_LANGUAGE C)
             target_link_libraries(${name} lpython_rtlib)
-            if (run_enable_cpython)
-                target_include_directories(${name} PRIVATE ${NUMPY_INCLUDE_DIR})
-                target_link_libraries(${name} Python::Python)
-                if (extra_files)
-                    file(COPY ${CMAKE_CURRENT_SOURCE_DIR}/${extra_files} DESTINATION ${CMAKE_CURRENT_BINARY_DIR})
-                endif()
+            add_test(${name} ${CMAKE_CURRENT_BINARY_DIR}/${name})
+            if (labels)
+                set_tests_properties(${name} PROPERTIES LABELS "${labels}")
+            endif()
+            if (${fail})
+                set_tests_properties(${name} PROPERTIES WILL_FAIL TRUE)
+            endif()
+        elseif(KIND STREQUAL "c_py")
+            add_custom_command(
+                OUTPUT ${name}.c
+                COMMAND ${LPYTHON} ${extra_args} --show-c ${CMAKE_CURRENT_SOURCE_DIR}/${file_name}.py > ${name}.c
+                DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/${file_name}.py
+                VERBATIM)
+            add_executable(${name} ${name}.c ${extra_files})
+            target_include_directories(${name} PRIVATE ${CMAKE_SOURCE_DIR} ${NUMPY_INCLUDE_DIR})
+            set_target_properties(${name} PROPERTIES LINKER_LANGUAGE C)
+            target_link_libraries(${name} lpython_rtlib Python::Python)
+            if (extra_files)
+                file(COPY ${CMAKE_CURRENT_SOURCE_DIR}/${extra_files} DESTINATION ${CMAKE_CURRENT_BINARY_DIR})
             endif()
             add_test(${name} ${CMAKE_CURRENT_BINARY_DIR}/${name})
             if (labels)
@@ -145,7 +140,7 @@ macro(RUN_UTIL RUN_FAIL RUN_NAME RUN_FILE_NAME RUN_IMPORT_PATH RUN_LABELS RUN_EN
             if (${fail})
                 set_tests_properties(${name} PROPERTIES WILL_FAIL TRUE)
             endif()
-        elseif(KIND STREQUAL "cpython")
+        elseif((KIND STREQUAL "cpython") OR (KIND STREQUAL "cpython_py"))
             # CPython test
             if (extra_files)
                 set(PY_MOD "${name}_mod")
@@ -243,7 +238,7 @@ macro(RUN_UTIL RUN_FAIL RUN_NAME RUN_FILE_NAME RUN_IMPORT_PATH RUN_LABELS RUN_EN
 endmacro(RUN_UTIL)
 
 macro(RUN)
-    set(options FAIL NOFAST ENABLE_CPYTHON ENABLE_CNUMPY)
+    set(options FAIL NOFAST ENABLE_CPYTHON LINK_NUMPY)
     set(oneValueArgs NAME IMPORT_PATH)
     set(multiValueArgs LABELS EXTRAFILES)
     cmake_parse_arguments(RUN "${options}" "${oneValueArgs}"
@@ -252,21 +247,28 @@ macro(RUN)
     set(RUN_EXTRA_ARGS "")
     set(RUN_FILE_NAME ${RUN_NAME})
 
+    if (RUN_LINK_NUMPY)
+        set(RUN_EXTRA_ARGS ${RUN_EXTRA_ARGS} --link-numpy)
+        set(RUN_ENABLE_CPYTHON TRUE)
+    endif()
+
     if (RUN_ENABLE_CPYTHON)
         set(RUN_EXTRA_ARGS ${RUN_EXTRA_ARGS} --enable-cpython)
     endif()
 
-    if (RUN_ENABLE_CNUMPY)
-        set(RUN_EXTRA_ARGS ${RUN_EXTRA_ARGS} --enable-cnumpy)
+    if (RUN_IMPORT_PATH)
+        # Only one import path supported for now
+        # Later add support for multiple import paths by looping over and appending to extra args
+        set(RUN_EXTRA_ARGS ${RUN_EXTRA_ARGS} -I${CMAKE_CURRENT_SOURCE_DIR}/${RUN_IMPORT_PATH})
     endif()
 
-    RUN_UTIL(RUN_FAIL RUN_NAME RUN_FILE_NAME RUN_IMPORT_PATH RUN_LABELS RUN_ENABLE_CPYTHON RUN_EXTRAFILES RUN_EXTRA_ARGS)
+    RUN_UTIL(RUN_FAIL RUN_NAME RUN_FILE_NAME RUN_LABELS RUN_ENABLE_CPYTHON RUN_EXTRAFILES RUN_EXTRA_ARGS)
 
     if ((NOT DISABLE_FAST) AND (NOT RUN_NOFAST))
         set(RUN_EXTRA_ARGS ${RUN_EXTRA_ARGS} --fast)
         set(RUN_NAME "${RUN_NAME}_FAST")
-        list(REMOVE_ITEM RUN_LABELS cpython) # remove cpython from --fast test
-        RUN_UTIL(RUN_FAIL RUN_NAME RUN_FILE_NAME RUN_IMPORT_PATH RUN_LABELS RUN_ENABLE_CPYTHON RUN_EXTRAFILES RUN_EXTRA_ARGS)
+        list(REMOVE_ITEM RUN_LABELS cpython cpython_py) # remove cpython, cpython_py from --fast test
+        RUN_UTIL(RUN_FAIL RUN_NAME RUN_FILE_NAME RUN_LABELS RUN_ENABLE_CPYTHON RUN_EXTRAFILES RUN_EXTRA_ARGS)
     endif()
 endmacro(RUN)
 
@@ -280,43 +282,31 @@ macro(COMPILE)
     cmake_parse_arguments(COMPILE "${options}" "${oneValueArgs}"
                           "${multiValueArgs}" ${ARGN} )
     set(name ${COMPILE_NAME})
-    set(import_path ${COMPILE_IMPORT_PATH})
     if (NOT name)
         message(FATAL_ERROR "Must specify the NAME argument")
     endif()
 
+    set(extra_args "")
+    if (COMPILE_IMPORT_PATH)
+        # Only one import path supported for now
+        # Later add support for multiple import paths by looping over and appending to extra args
+        set(extra_args ${extra_args} -I${CMAKE_CURRENT_SOURCE_DIR}/${COMPILE_IMPORT_PATH})
+    endif()
+
     if (${KIND} IN_LIST COMPILE_LABELS)
         if (KIND STREQUAL "llvm")
-            if (import_path)
-                add_custom_command(
-                    OUTPUT ${name}.o
-                    COMMAND ${LPYTHON} --disable-main -c -I ${CMAKE_CURRENT_SOURCE_DIR}/${import_path} ${CMAKE_CURRENT_SOURCE_DIR}/${name}.py -o ${name}.o
-                    DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/${name}.py
-                    VERBATIM)
-            else ()
-                add_custom_command(
-                    OUTPUT ${name}.o
-                    COMMAND ${LPYTHON} --disable-main -c ${CMAKE_CURRENT_SOURCE_DIR}/${name}.py -o ${name}.o
-                    DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/${name}.py
-                    VERBATIM)
-            endif()
-
+            add_custom_command(
+                OUTPUT ${name}.o
+                COMMAND ${LPYTHON} ${extra_args} --disable-main -c ${CMAKE_CURRENT_SOURCE_DIR}/${name}.py -o ${name}.o
+                DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/${name}.py
+                VERBATIM)
             add_library(${name} OBJECT ${name}.o)
         elseif(KIND STREQUAL "c")
-            if (import_path)
-                add_custom_command(
-                    OUTPUT ${name}.c
-                    COMMAND ${LPYTHON} --disable-main -I ${CMAKE_CURRENT_SOURCE_DIR}/${import_path} --show-c ${CMAKE_CURRENT_SOURCE_DIR}/${name}.py > ${name}.c
-                    DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/${name}.py
-                    VERBATIM)
-            else ()
-                add_custom_command(
-                    OUTPUT ${name}.c
-                    COMMAND ${LPYTHON} --disable-main --show-c ${CMAKE_CURRENT_SOURCE_DIR}/${name}.py > ${name}.c
-                    DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/${name}.py
-                    VERBATIM)
-            endif()
-
+            add_custom_command(
+                OUTPUT ${name}.c
+                COMMAND ${LPYTHON} ${extra_args} --disable-main --show-c ${CMAKE_CURRENT_SOURCE_DIR}/${name}.py > ${name}.c
+                DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/${name}.py
+                VERBATIM)
             add_library(${name} OBJECT ${name}.c)
             target_link_libraries(${name} lpython_rtlib)
         elseif(KIND STREQUAL "cpython")
@@ -515,8 +505,8 @@ RUN(NAME bindc_05   LABELS llvm c
         EXTRAFILES bindc_05b.c)
 RUN(NAME bindc_06   LABELS llvm c
         EXTRAFILES bindc_06b.c)
-RUN(NAME bindpy_01           LABELS cpython c ENABLE_CPYTHON NOFAST EXTRAFILES bindpy_01_module.py)
-RUN(NAME bindpy_02           LABELS cpython c ENABLE_CPYTHON ENABLE_CNUMPY EXTRAFILES bindpy_02_module.py)
+RUN(NAME bindpy_01           LABELS cpython_py c_py ENABLE_CPYTHON NOFAST EXTRAFILES bindpy_01_module.py)
+RUN(NAME bindpy_02           LABELS cpython_py c_py LINK_NUMPY EXTRAFILES bindpy_02_module.py)
 RUN(NAME test_generics_01    LABELS cpython llvm c NOFAST)
 RUN(NAME test_cmath          LABELS cpython llvm c NOFAST)
 RUN(NAME test_complex_01        LABELS cpython llvm c wasm wasm_x64)
diff --git a/integration_tests/run_tests.py b/integration_tests/run_tests.py
@@ -6,7 +6,7 @@
 
 # Initialization
 DEFAULT_THREADS_TO_USE = 8 # default no of threads is 8
-SUPPORTED_BACKENDS = ['llvm', 'c', 'wasm', 'cpython', 'x86', 'wasm_x86', 'wasm_x64']
+SUPPORTED_BACKENDS = ['llvm', 'c', 'wasm', 'cpython', 'x86', 'wasm_x86', 'wasm_x64', 'c_py', 'cpython_py']
 BASE_DIR = os.path.dirname(os.path.realpath(__file__))
 LPYTHON_PATH = f"{BASE_DIR}/../src/bin"
 
diff --git a/src/bin/lpython.cpp b/src/bin/lpython.cpp
@@ -1249,7 +1249,7 @@ int link_executable(const std::vector<std::string> &infiles,
         if (compiler_options.enable_cpython) {
             std::string py_version = "3.10";
             std::string py_flags = R"(-I $CONDA_PREFIX/include/python)" + py_version + R"( -L$CONDA_PREFIX/lib -Wl,-rpath -Wl,$CONDA_PREFIX/lib -lpython)" + py_version + R"()";
-            if (compiler_options.enable_cnumpy) {
+            if (compiler_options.link_numpy) {
                 py_flags += R"( -I$CONDA_PREFIX/lib/python)" + py_version + R"(/site-packages/numpy/core/include)";
             }
             cmd += " " + py_flags;
@@ -1565,7 +1565,7 @@ int main(int argc, char *argv[])
         app.add_flag("--get-rtl-dir", print_rtl_dir, "Print the path to the runtime library file");
         app.add_flag("--verbose", compiler_options.verbose, "Print debugging statements");
         app.add_flag("--enable-cpython", compiler_options.enable_cpython, "Enable CPython runtime");
-        app.add_flag("--enable-cnumpy", compiler_options.enable_cnumpy, "Enable C-Numpy runtime");
+        app.add_flag("--link-numpy", compiler_options.link_numpy, "Enable NumPy runtime (implies --enable-cpython)");
 
         // LSP specific options
         app.add_flag("--show-errors", show_errors, "Show errors when LSP is running in the background");
@@ -1613,6 +1613,10 @@ int main(int argc, char *argv[])
             compiler_options.enable_bounds_checking = true;
         }
 
+        if (compiler_options.link_numpy) {
+            compiler_options.enable_cpython = true;
+        }
+
         if (arg_version) {
             std::string version = LFORTRAN_VERSION;
             std::cout << "LPython version: " << version << std::endl;
diff --git a/src/libasr/codegen/asr_to_c.cpp b/src/libasr/codegen/asr_to_c.cpp
@@ -934,7 +934,7 @@ R"(
             body += "\n";
         }
 
-        if (compiler_options.enable_cnumpy) {
+        if (compiler_options.link_numpy) {
             user_defines.insert("NPY_NO_DEPRECATED_API NPY_1_7_API_VERSION");
             headers.insert("numpy/arrayobject.h");
             body +=
diff --git a/src/libasr/utils.h b/src/libasr/utils.h
@@ -59,7 +59,7 @@ struct CompilerOptions {
     bool verbose = false;
     bool pass_cumulative = false;
     bool enable_cpython = false;
-    bool enable_cnumpy = false;
+    bool link_numpy = false;
     std::vector<std::string> import_paths;
     Platform platform;
 

Original file line number	Diff line number	Diff line change
`@@ -934,7 +934,7 @@ R"(`
`934`	`934`	`body += "\n";`
`935`	`935`	`}`
`936`	`936`
`937`		`- if (compiler_options.enable_cnumpy) {`
	`937`	`+ if (compiler_options.link_numpy) {`
`938`	`938`	`user_defines.insert("NPY_NO_DEPRECATED_API NPY_1_7_API_VERSION");`
`939`	`939`	`headers.insert("numpy/arrayobject.h");`
`940`	`940`	`body +=`