fix some codegen bugs: NIM_BOOL, NIM_STATIC_ASSERT, --passc:-std=... (etc) (#13798)

* fix cgen bool D20200328T203812
* --passc:std=c++17 (etc) now works instead of silently ignored
* document caveats for NIM_NIL

compiler/extccomp.nim

@@ -612,6 +612,11 @@ proc getCompileCFileCmd*(conf: ConfigRef; cfile: Cfile,
   # We produce files like module.nim.cpp, so the absolute Nim filename is not
   # cfile.name but `cfile.cname.changeFileExt("")`:
   var options = cFileSpecificOptions(conf, cfile.nimname, cfile.cname.changeFileExt("").string)
+  if useCpp(conf, cfile.cname):
+    # needs to be prepended so that --passc:-std=c++17 can override default.
+    # we could avoid allocation by making cFileSpecificOptions inplace
+    options = CC[c].cppXsupport & ' ' & options
+
   var exe = getConfigVar(conf, c, ".exe")
   if exe.len == 0: exe = getCompilerExe(conf, c, cfile.cname)
 
@@ -620,9 +625,6 @@ proc getCompileCFileCmd*(conf: ConfigRef; cfile: Cfile,
       ospNeedsPIC in platform.OS[conf.target.targetOS].props:
     options.add(' ' & CC[c].pic)
 
-  if useCpp(conf, cfile.cname):
-    options.add(' ' & CC[c].cppXsupport)
-
   var compilePattern: string
   # compute include paths:
   var includeCmd = CC[c].includeCmd & quoteShell(conf.libpath)

lib/nimbase.h

@@ -262,6 +262,21 @@ __AVR__
 #include <limits.h>
 #include <stddef.h>
 
+// define NIM_STATIC_ASSERT
+// example use case: CT sizeof for importc types verification
+// where we have {.completeStruct.} (or lack of {.incompleteStruct.})
+#if (defined(__STDC_VERSION__) && __STDC_VERSION__ >= 201112L)
+#define NIM_STATIC_ASSERT(x, msg) _Static_assert((x), msg)
+#elif defined(__cplusplus)
+#define NIM_STATIC_ASSERT(x, msg) static_assert((x), msg)
+#else
+#define NIM_STATIC_ASSERT(x, msg) typedef int NIM_STATIC_ASSERT_AUX[(x) ? 1 : -1];
+// On failure, your C compiler will say something like:
+//   "error: 'NIM_STATIC_ASSERT_AUX' declared as an array with a negative size"
+// we could use a better fallback to also show line number, using:
+// http://www.pixelbeat.org/programming/gcc/static_assert.html
+#endif
+
 /* C99 compiler? */
 #if (defined(__STDC_VERSION__) && (__STDC_VERSION__ >= 199901))
 #  define HAVE_STDINT_H
@@ -288,34 +303,40 @@ __AVR__
 namespace USE_NIM_NAMESPACE {
 #endif
 
+// preexisting check, seems paranoid, maybe remove
+#if defined(NIM_TRUE) || defined(NIM_FALSE) || defined(NIM_BOOL)
+#error "nim reserved preprocessor macros clash"
+#endif
+
 /* bool types (C++ has it): */
 #ifdef __cplusplus
-#  ifndef NIM_TRUE
+#define NIM_BOOL bool
-#    define NIM_TRUE true
+#elif (defined(__STDC_VERSION__) && __STDC_VERSION__ >= 199901)
-#  endif
+// see #13798: to avoid conflicts for code emitting `#include <stdbool.h>`
-#  ifndef NIM_FALSE
+#define NIM_BOOL _Bool
-#    define NIM_FALSE false
+#else
-#  endif
+typedef unsigned char NIM_BOOL; // best effort
-#  define NIM_BOOL bool
+#endif
+
+NIM_STATIC_ASSERT(sizeof(NIM_BOOL) == 1, ""); // check whether really needed
+
+#define NIM_TRUE true
+#define NIM_FALSE false
+
+#ifdef __cplusplus
 #  if __cplusplus >= 201103L
 #    /* nullptr is more type safe (less implicit conversions than 0) */
 #    define NIM_NIL nullptr
 #  else
-#    /* consider using NULL if comment below for NIM_NIL doesn't apply to C++ */
+#    // both `((void*)0)` and `NULL` would cause codegen to emit
+#    // error: assigning to 'Foo *' from incompatible type 'void *'
+#    // but codegen could be fixed if need. See also potential caveat regarding
+#    // NULL.
+#    // However, `0` causes other issues, see #13798
 #    define NIM_NIL 0
 #  endif
 #else
-#  ifdef bool
+#  include <stdbool.h>
-#    define NIM_BOOL bool
-#  else
-  typedef unsigned char NIM_BOOL;
-#  endif
-#  ifndef NIM_TRUE
-#    define NIM_TRUE ((NIM_BOOL) 1)
-#  endif
-#  ifndef NIM_FALSE
-#    define NIM_FALSE ((NIM_BOOL) 0)
-#  endif
 #  define NIM_NIL ((void*)0) /* C's NULL is fucked up in some C compilers, so
                               the generated code does not rely on it anymore */
 #endif
@@ -516,10 +537,8 @@ static inline void GCGuard (void *ptr) { asm volatile ("" :: "X" (ptr)); }
 #  define GC_GUARD
 #endif
 
-/* Test to see if Nim and the C compiler agree on the size of a pointer.
+// Test to see if Nim and the C compiler agree on the size of a pointer.
-   On disagreement, your C compiler will say something like:
+NIM_STATIC_ASSERT(sizeof(NI) == sizeof(void*) && NIM_INTBITS == sizeof(NI)*8, "");
-   "error: 'Nim_and_C_compiler_disagree_on_target_architecture' declared as an array with a negative size" */
-typedef int Nim_and_C_compiler_disagree_on_target_architecture[sizeof(NI) == sizeof(void*) && NIM_INTBITS == sizeof(NI)*8 ? 1 : -1];
 
 #ifdef USE_NIM_NAMESPACE
 }

tests/ccgbugs/mstatic_assert.nim

@@ -0,0 +1,6 @@
+{.emit:"""
+NIM_STATIC_ASSERT(sizeof(bool) == 1, "");
+#warning "foo2"
+NIM_STATIC_ASSERT(sizeof(bool) == 2, "");
+#warning "foo3"
+""".}

tests/ccgbugs/tcodegenbug_bool.nim

@@ -0,0 +1,11 @@
+discard """
+"""
+
+# issue #13798
+{.emit:"""
+#include <stdbool.h>
+void fun(bool a){}
+""".}
+
+proc fun(a: bool) {.importc.}
+fun(true)

tests/trunner.nim

@@ -0,0 +1,46 @@
+discard """
+  joinable: false
+"""
+
+## tests that don't quite fit the mold and are easier to handle via `execCmdEx`
+## A few others could be added to here to simplify code.
+
+import std/[strformat,os,osproc,strutils]
+
+proc runCmd(file, options = ""): auto =
+  let mode = if existsEnv("NIM_COMPILE_TO_CPP"): "cpp" else: "c"
+  const nim = getCurrentCompilerExe()
+  const testsDir = currentSourcePath().parentDir
+  let fileabs = testsDir / file.unixToNativePath
+  doAssert fileabs.existsFile, fileabs
+  let cmd = fmt"{nim} {mode} {options} --hints:off {fileabs}"
+  result = execCmdEx(cmd)
+  when false: # uncomment if you need to debug
+    echo result[0]
+    echo result[1]
+
+proc testCodegenStaticAssert() =
+  let (output, exitCode) = runCmd("ccgbugs/mstatic_assert.nim")
+  doAssert "sizeof(bool) == 2" in output
+  doAssert exitCode != 0
+
+proc testCTFFI() =
+  let (output, exitCode) = runCmd("vm/mevalffi.nim", "--experimental:compiletimeFFI")
+  let expected = """
+hello world stderr
+hi stderr
+foo
+foo:100
+foo:101
+foo:102:103
+foo:102:103:104
+foo:0.03:asdf:103:105
+ret={s1:foobar s2:foobar age:25 pi:3.14}
+"""
+  doAssert output == expected, output
+  doAssert exitCode == 0
+
+when defined(nimHasLibFFIEnabled):
+  testCTFFI()
+else: # don't run twice the same test
+  testCodegenStaticAssert()

tests/vm/tevalffi.nim

@@ -1,28 +0,0 @@
-discard """
-  joinable: false
-"""
-
-import std/[strformat,os,osproc]
-
-proc main() =
-  const nim = getCurrentCompilerExe()
-  const file = currentSourcePath().parentDir / "mevalffi.nim"
-  # strangely, --hint:cc:off was needed
-  let cmd = fmt"{nim} c -f --experimental:compiletimeFFI --hints:off --hint:cc:off {file}"
-  let (output, exitCode) = execCmdEx(cmd)
-  let expected = """
-hello world stderr
-hi stderr
-foo
-foo:100
-foo:101
-foo:102:103
-foo:102:103:104
-foo:0.03:asdf:103:105
-ret={s1:foobar s2:foobar age:25 pi:3.14}
-"""
-  doAssert output == expected, output
-  doAssert exitCode == 0
-
-when defined(nimHasLibFFIEnabled):
-  main()