From c5aa72acffcc9a4f6c2ea7269d6206fc0c3c1697 Mon Sep 17 00:00:00 2001 From: Kevin Modzelewski Date: Tue, 30 Aug 2016 18:27:06 +0000 Subject: [PATCH 1/2] Turn off implicit-function check in release mode I'm still hoping that we can keep this on in some way, because it continues to find issues. So let's try keeping it on for debug builds. I'm not sure if this is weakening the check enough (there may be other cases like scipy), but it's sounding like we need to have this check turned off in our releases regardless, such as issue #1344. --- from_cpython/Lib/distutils/ccompiler.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/from_cpython/Lib/distutils/ccompiler.py b/from_cpython/Lib/distutils/ccompiler.py index 2825e49fc..42266ff98 100644 --- a/from_cpython/Lib/distutils/ccompiler.py +++ b/from_cpython/Lib/distutils/ccompiler.py @@ -373,7 +373,9 @@ def _get_cc_args(self, pp_opts, debug, before): cc_args[:0] = before if not any ('scipy' in s for s in pp_opts): - cc_args = cc_args + ["-Werror=implicit-function-declaration"] + import sysconfig + if '-DNDEBUG' not in sysconfig.get_config_var('CFLAGS'): + cc_args = cc_args + ["-Werror=implicit-function-declaration"] return cc_args def _fix_compile_args(self, output_dir, macros, include_dirs): From 9e1ed8ab2d9be8221d7a47d1960947b06635991a Mon Sep 17 00:00:00 2001 From: Kevin Modzelewski Date: Tue, 30 Aug 2016 21:49:59 +0000 Subject: [PATCH 2/2] Properly throw ImportErrors on dlopen failure I think it'd be nicer to just abort() like we do here, but cffi is actually testing this behavior (that loading an invalid library causes an ImportError). This is being exposed now because before, the library would fail to compile due to -Werror=implicit-function-declaration and not even get to the loading step. --- src/runtime/import.cpp | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/src/runtime/import.cpp b/src/runtime/import.cpp index e96fb0b11..f89070ae7 100644 --- a/src/runtime/import.cpp +++ b/src/runtime/import.cpp @@ -138,23 +138,16 @@ extern "C" Box* import(int level, Box* from_imports, llvm::StringRef module_name BoxedModule* importCExtension(BoxedString* full_name, const std::string& last_name, const std::string& path) { void* handle = dlopen(path.c_str(), RTLD_NOW); - if (!handle) { - const char* s = dlerror(); - // raiseExcHelper(ImportError, "%s", dlerror()); - fprintf(stderr, "%s\n", s); - exit(1); - } + if (!handle) + raiseExcHelper(ImportError, "%s", dlerror()); assert(handle); std::string initname = "init" + last_name; void (*init)() = (void (*)())dlsym(handle, initname.c_str()); char* error; - if ((error = dlerror()) != NULL) { - // raiseExcHelper(ImportError, "%s", error); - fprintf(stderr, "%s\n", error); - exit(1); - } + if ((error = dlerror()) != NULL) + raiseExcHelper(ImportError, "%s", error); assert(init);