Represent enums as enums, not int, if possible

Use enum types instead of int for the enum-valued parameters and
function return values, this is more type-safe and clear for the users
of the library.

Change cpp_enum unit test to use C++ to check that C++ enum wrappers
can at least be compiled, but still use C API in it.

Note that enum whose underlying type is bigger than int still don't
work, but this is no different from what it was before, so just document
this limitation but don't do anything else about it for now.

This commit is best viewed ignoring whitespace-only changes.

Doc/Manual/C.html

@@ -664,6 +664,7 @@ C++ wrappers try to provide a similar API to the original C++ API being wrapped,
 Other ones are due to things that could be supported but haven't been implemented yet:
 <ul>
   <li>Only single, not multiple, inheritance is currently supported.</li>
+  <li>Only enums using <tt>int</tt> (or smaller type) as underlying type are supported.</li>
 </ul>
 </p>
 

Examples/test-suite/c/cpp_enum_runme.cxx

@@ -4,8 +4,7 @@
 
 int main(int argc, const char *argv[]) {
 
-	// We don't have "enum SOME_ENUM"
+	enum cpp_enum_SOME_ENUM e = cpp_enum_ENUM_ONE, *p;
-	int e = cpp_enum_ENUM_ONE, *p;
 
 	// check the constructor's default value
 	cpp_enum_StructWithEnums *s = cpp_enum_StructWithEnums_new();

Lib/c/c.swg

@@ -122,8 +122,12 @@ same_macro_all_primitive_types_but_void(cref_as_value,ctype);
 %typemap(ctype) SWIGTYPE &    "$*resolved_type*"
 %typemap(ctype) SWIGTYPE [ANY] "$resolved_type*"
 %typemap(ctype) SWIGTYPE * [ANY] "$resolved_type**"
-%typemap(ctype) enum SWIGTYPE "int"
+
-%typemap(ctype) enum SWIGTYPE &, enum SWIGTYPE * "int *"
+// enums
+%typemap(ctype) enum SWIGTYPE       "$resolved_type"
+%typemap(ctype) enum SWIGTYPE *     "$resolved_type*"
+%typemap(ctype) enum SWIGTYPE &     "$*resolved_type*"
+%typemap(ctype) enum SWIGTYPE [ANY] "$resolved_type*"
 
 %typemap(ctype, fragment="stdbool_inc") bool, const bool, const bool & "bool"
 %typemap(ctype, fragment="stdbool_inc") bool *, const bool *, bool & "bool *"

Source/Modules/c.cxx

@@ -823,14 +823,16 @@ private:
       Type_Ptr,
       Type_Ref,
       Type_Obj,
+      Type_Enm,
       Type_Max
     } typeKind = Type_Max;
 
-    // These correspond to the typemaps for SWIGTYPE*, SWIGTYPE& and SWIGTYPE, respectively, defined in c.swg.
+    // These correspond to the typemaps for SWIGTYPE*, SWIGTYPE&, SWIGTYPE and enum SWIGTYPE, respectively, defined in c.swg.
     static const char* typemaps[Type_Max] = {
       "$resolved_type*",
       "$*resolved_type*",
       "$&resolved_type*",
+      "$resolved_type",
     };
 
     for (int i = 0; i < Type_Max; ++i) {
@@ -853,10 +855,87 @@ private:
       return;
     }
 
+    // The logic here is somewhat messy because we use the same "$resolved_type*" typemap for pointers/references to both enums and classes, but we actually
+    // need to do quite different things for them. It could probably be simplified by changing the typemaps to be distinct, but this would require also updating
+    // the code for C wrappers generation in substituteResolvedTypeSpecialVariable().
     scoped_dohptr resolved_type(SwigType_typedef_resolve_all(type));
-    scoped_dohptr stripped_type(SwigType_strip_qualifiers(resolved_type));
+    scoped_dohptr base_resolved_type(SwigType_base(resolved_type));
 
     scoped_dohptr typestr;
+    if (SwigType_isenum(base_resolved_type)) {
+      String* enumname = NULL;
+      if (Node* const enum_node = Language::instance()->enumLookup(base_resolved_type)) {
+	// This is the name of the enum in C wrappers, it should be already set by getEnumName().
+	enumname = Getattr(enum_node, "enumname");
+
+	if (enumname) {
+	  String* const enum_symname = Getattr(enum_node, "sym:name");
+
+	  if (Checkattr(enum_node, "ismember", "1")) {
+	    Node* const parent_class = parentNode(enum_node);
+	    typestr = NewStringf("%s::%s", Getattr(parent_class, "sym:name"), enum_symname);
+	  } else {
+	    typestr = Copy(enum_symname);
+	  }
+	}
+      }
+
+      if (!enumname) {
+	// Unknown enums are mapped to int and no casts are necessary in this case.
+	typestr = NewString("int");
+      }
+
+      if (SwigType_ispointer(type))
+	Append(typestr, " *");
+      else if (SwigType_isreference(type))
+	Append(typestr, " &");
+
+      scoped_dohptr rtype_cast(enumname ? NewStringf("(%s)", typestr.get()) : NewStringEmpty());
+
+      switch (typeKind) {
+	case Type_Ptr:
+	  if (rtype_desc) {
+	    Append(rtype_desc->wrap_start(), rtype_cast);
+	  }
+
+	  if (ptype_desc) {
+	    if (enumname)
+	      Printv(ptype_desc->wrap_start(), "(", enumname, "*)", NIL);
+	  }
+	  break;
+
+	case Type_Ref:
+	  if (rtype_desc) {
+	    Printv(rtype_desc->wrap_start(), rtype_cast.get(), "*(", NIL);
+	    Append(rtype_desc->wrap_end(), ")");
+	  }
+
+	  if (ptype_desc) {
+	    if (enumname)
+	      Printv(ptype_desc->wrap_start(), "(", enumname, "*)", NIL);
+	    Append(ptype_desc->wrap_start(), "&");
+	  }
+	  break;
+
+	case Type_Enm:
+	  if (rtype_desc) {
+	    Append(rtype_desc->wrap_start(), rtype_cast);
+	  }
+
+	  if (ptype_desc) {
+	    if (enumname)
+	      Printv(ptype_desc->wrap_start(), "(", enumname, ")", NIL);
+	  }
+	  break;
+
+	case Type_Obj:
+	case Type_Max:
+	  // Unreachable, but keep here to avoid -Wswitch warnings.
+	  assert(0);
+      }
+    } else {
+      scoped_dohptr stripped_type(SwigType_strip_qualifiers(resolved_type));
+
       String* classname;
       if (Node* const class_node = Language::instance()->classLookup(stripped_type)) {
 	typestr = SwigType_str(type, 0);
@@ -945,10 +1024,12 @@ private:
 	  }
 	  break;
 
+	case Type_Enm:
 	case Type_Max:
 	  // Unreachable, but keep here to avoid -Wswitch warnings.
 	  assert(0);
       }
+    }
 
     Replaceall(s, typemaps[typeKind], typestr);
   }
@@ -1156,6 +1237,11 @@ public:
   String *getEnumName(Node *n) {
     String *enumname = Getattr(n, "enumname");
     if (!enumname) {
+      // We can't use forward-declared enums because we can't define them for C wrappers (we could forward declare them in C++ if their underlying type,
+      // available as "inherit" node attribute, is specified, but not in C), so we have no choice but to use "int" for them.
+      if (Checkattr(n, "sym:weak", "1"))
+	return NULL;
+
       String *symname = Getattr(n, "sym:name");
       if (symname) {
 	// Add in class scope when referencing enum if not a global enum
@@ -1187,6 +1273,27 @@ public:
    * ----------------------------------------------------------------------------- */
 
   void substituteResolvedTypeSpecialVariable(SwigType *classnametype, String *tm, const char *classnamespecialvariable) {
+    scoped_dohptr btype(SwigType_base(classnametype));
+    if (SwigType_isenum(btype)) {
+      Node* const enum_node = enumLookup(btype);
+      String* const enumname = enum_node ? getEnumName(enum_node) : NULL;
+
+      // We use the enum name in the wrapper declaration if it's available, as this makes it more type safe, but we always use just int for the function
+      // definition because we don't have the enum declaration in scope there. This obviously only actually works if the actual enum underlying type is int (or
+      // smaller).
+      maybe_owned_dohptr c_enumname;
+      if (current_output == output_wrapper_decl && enumname) {
+	// We need to add "enum" iff this is not already a typedef for the enum.
+	if (Checkattr(enum_node, "allows_typedef", "1"))
+	  c_enumname.assign_non_owned(enumname);
+	else
+	  c_enumname.assign_owned(NewStringf("enum %s", enumname));
+      } else {
+	c_enumname.assign_owned(NewString("int"));
+      }
+
+      Replaceall(tm, classnamespecialvariable, c_enumname);
+    } else {
       if (!CPlusPlus) {
 	// Just use the original C type when not using C++, we know that this type can be used in the wrappers.
 	Clear(tm);
@@ -1196,15 +1303,6 @@ public:
 	return;
       }
 
-    scoped_dohptr btype(SwigType_base(classnametype));
-    if (SwigType_isenum(classnametype)) {
-      Node* const enum_node = enumLookup(btype);
-      String* const enumname = enum_node ? getEnumName(enum_node) : NULL;
-      if (enumname)
-	Replaceall(tm, classnamespecialvariable, enumname);
-      else
-	Replaceall(tm, classnamespecialvariable, NewStringf("int"));
-    } else {
       String* typestr = NIL;
       if (current_output == output_wrapper_def || Cmp(btype, "SwigObj") == 0) {
 	// Special case, just leave it unchanged.