This is another small request for review:

Size of patch: 2/7
Complexity: 1/7

-Brad

-----

Change the return type of is_c_nil from c_int to bool

This commit changes the return type of is_c_nil from c_int to bool. I reviewed the concept with Michael (who write is_c_nil() originally) before implementing. I believe that is_c_nil returns a c_int simply because it was wrapping a C comparison operator that returns an int; making it a bool seems more natural for Chapel's sake and removes a case where we rely on C types in something that an end-user or library writer might want to use.

I implemented this by casting the result to chpl_bool in the C routine that implements c_is_bool(). To do this, I had to move the definition of chpl_bool up in the file.

I added a test to lock in this capability.

The reason I ran into this is that I was writing code of the form:

        if (!is_c_nil(x))

only to find that it wouldn't work due to our lack of support for ! on int types, which led me to be surprised that is_c_nil returned an int. In another patch, I've extended support of ! to include integers; this commit solves the issue in a different way.

I also simplified existing expressions that use is_c_nil() based on this change.


Note for discussion: I think I would've called Chapel's notion of 'NULL' 'c_NULL' rather than 'c_nil', much like we call C's 'double' 'c_double' rather than 'c_real'. Put another way, I worry that a C programmer using Chapel would not find/guess 'c_nil' as quickly as they would 'c_NULL'. Thoughts?
Index: test/studies/filerator/listdir.chpl
===================================================================
--- test/studies/filerator/listdir.chpl	(revision 23710)
+++ test/studies/filerator/listdir.chpl	(working copy)
@@ -21,9 +21,9 @@
   var ent: direntptr;
   //  writeln("***Trying ", path);
   dir = opendir(path:c_string);
-  if (is_c_nil(dir) == 0) {
+  if (!is_c_nil(dir)) {
     ent = readdir(dir);
-    while (is_c_nil(ent) == 0) {
+    while (!is_c_nil(ent)) {
       const filename = ent.d_name();
       if (dotfiles || filename.substring(1) != '.') {
         const fullpath = path + "/" + filename;
Index: test/extern/bradc/externNull.good
===================================================================
--- test/extern/bradc/externNull.good	(revision 0)
+++ test/extern/bradc/externNull.good	(revision 0)
@@ -0,0 +1,2 @@
+true
+false
Index: test/extern/bradc/externNull.chpl
===================================================================
--- test/extern/bradc/externNull.chpl	(revision 0)
+++ test/extern/bradc/externNull.chpl	(revision 0)
@@ -0,0 +1,6 @@
+extern type sPtr;
+extern proc getNullSPtr(): sPtr;
+extern proc getNonNullSPtr(): sPtr;
+
+writeln(is_c_nil(getNullSPtr()));
+writeln(is_c_nil(getNonNullSPtr()));
Index: test/extern/bradc/externNull.h
===================================================================
--- test/extern/bradc/externNull.h	(revision 0)
+++ test/extern/bradc/externNull.h	(revision 0)
@@ -0,0 +1,15 @@
+typedef struct _s {
+  double x;
+} s;
+
+typedef s* sPtr;
+
+s myS;
+
+static inline sPtr getNullSPtr(void) {
+  return NULL;
+}
+
+static inline sPtr getNonNullSPtr(void) {
+  return &myS;
+}
Index: test/extern/bradc/externNull.compopts
===================================================================
--- test/extern/bradc/externNull.compopts	(revision 0)
+++ test/extern/bradc/externNull.compopts	(revision 0)
@@ -0,0 +1 @@
+externNull.h
Index: runtime/include/chpltypes.h
===================================================================
--- runtime/include/chpltypes.h	(revision 23710)
+++ runtime/include/chpltypes.h	(working copy)
@@ -23,8 +23,16 @@
 typedef float c_float;
 typedef double c_double;
 typedef void* c_void_ptr;
+
+// C++ does not support c99 bools
+#ifndef __cplusplus
+typedef _Bool chpl_bool;
+#else
+typedef bool chpl_bool;
+#endif
+
 #define c_nil NULL
-static inline c_int is_c_nil(void* x) { return x==NULL; }
+static inline c_int is_c_nil(void* x) { return (chpl_bool)(x==NULL); }
 static inline void* c_pointer_return(void* x) { return x; }
 
 typedef enum {
@@ -148,13 +156,6 @@
 #define UINT64( i) ((uint64_t)(UINT64_C(i)))
 
 
-// C++ does not support c99 bools
-#ifndef __cplusplus
-typedef _Bool chpl_bool;
-#else
-typedef bool chpl_bool;
-#endif
-
 typedef int8_t chpl_bool8;
 typedef int16_t chpl_bool16;
 typedef int32_t chpl_bool32;
Index: modules/standard/IO.chpl
===================================================================
--- modules/standard/IO.chpl	(revision 23710)
+++ modules/standard/IO.chpl	(working copy)
@@ -531,7 +531,7 @@
 }
 
 proc file.check() {
-  if(1 == is_c_nil(_file_internal)) {
+  if(is_c_nil(_file_internal)) {
     halt("Operation attempted on an invalid file");
   }
 }
Index: modules/standard/SysBasic.chpl
===================================================================
--- modules/standard/SysBasic.chpl	(revision 23710)
+++ modules/standard/SysBasic.chpl	(working copy)
@@ -38,7 +38,7 @@
 // To generate legal C prototypes, we have to manually instantiate this
 // prototype for each pointer type that might be associated with 'x'.
 pragma "no prototype"
-extern proc is_c_nil(x):c_int;
+extern proc is_c_nil(x):bool;
 
 // local C pointer class used in C integration.
 // Similar to _ddata from ChapelBase, but differs 
------------------------------------------------------------------------------
Open source business process management suite built on Java and Eclipse
Turn processes into business applications with Bonita BPM Community Edition
Quickly connect people, data, and systems into organized workflows
Winner of BOSSIE, CODIE, OW2 and Gartner awards
http://p.sf.net/sfu/Bonitasoft
_______________________________________________
Chapel-developers mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/chapel-developers

Reply via email to