Replace opaque type with CStruct0

kornilova203 · kornilova203 · commit a5c00aa353e7 · 2018-07-18T01:09:41.000+08:00
Add comment line to incomplete types
diff --git a/bindgen/Utils.h b/bindgen/Utils.h
@@ -126,4 +126,20 @@ template <typename T> static inline bool isAliasForType(Type *type) {
     return false;
 }
 
+/**
+ * @return true if typedef references opaque type directly or through a
+ * chain of typedefs.
+ */
+static inline bool isAliasForOpaqueType(const Type *type) {
+    assert(type);
+    auto *typeDef = dynamic_cast<const TypeDef *>(type);
+    if (typeDef) {
+        if (!typeDef->getType()) {
+            return true;
+        }
+        return isAliasForOpaqueType(typeDef->getType().get());
+    }
+    return false;
+}
+
 #endif // UTILS_H
diff --git a/bindgen/defines/DefineFinder.cpp b/bindgen/defines/DefineFinder.cpp
@@ -104,9 +104,6 @@ void DefineFinder::MacroUndefined(const clang::Token &macroNameTok,
                                   const clang::MacroDefinition &md,
                                   const clang::MacroDirective *undef) {
     clang::SourceManager &sm = compiler.getSourceManager();
-    if (!sm.isInMainFile(undef->getLocation())) {
-        return;
-    }
     if (sm.isWrittenInMainFile(macroNameTok.getLocation()) &&
         md.getMacroInfo() && !md.getMacroInfo()->isFunctionLike()) {
         std::string macroName = macroNameTok.getIdentifierInfo()->getName();
diff --git a/bindgen/ir/Function.cpp b/bindgen/ir/Function.cpp
@@ -72,12 +72,14 @@ bool Function::isLegalScalaNativeFunction() const {
     /* Return type and parameters types cannot be array types because array type
      * in this case is always represented as a pointer to element type */
     if (isAliasForType<Struct>(retType.get()) ||
-        isAliasForType<Union>(retType.get())) {
+        isAliasForType<Union>(retType.get()) ||
+        isAliasForOpaqueType(retType.get())) {
         return false;
     }
     for (const auto &parameter : parameters) {
         if (isAliasForType<Struct>(parameter->getType().get()) ||
-            isAliasForType<Union>(parameter->getType().get())) {
+            isAliasForType<Union>(parameter->getType().get()) ||
+            isAliasForOpaqueType(parameter->getType().get())) {
             return false;
         }
     }
diff --git a/bindgen/ir/IR.cpp b/bindgen/ir/IR.cpp
@@ -1,5 +1,6 @@
 #include "IR.h"
 #include "../Utils.h"
+#include "types/PointerType.h"
 
 IR::IR(std::string libName, std::string linkName, std::string objectName,
        std::string packageName, const LocationManager &locationManager)
@@ -111,26 +112,49 @@ llvm::raw_ostream &operator<<(llvm::raw_ostream &s, const IR &ir) {
         for (const auto &typeDef : ir.typeDefs) {
             if (ir.shouldOutput(typeDef)) {
                 s << *typeDef;
+                if (typeDef->getType()) {
+                    auto *structOrUnion =
+                        dynamic_cast<StructOrUnion *>(typeDef->getType().get());
+                    if (structOrUnion &&
+                        structOrUnion->hasIllegalUsageOfOpaqueType()) {
+                        llvm::errs()
+                            << "Error: record " << structOrUnion->getName()
+                            << " has field of incomplete type. Declarations "
+                               "that use this type may not work properly.\n";
+                        llvm::errs().flush();
+                    }
+                }
             }
         }
 
         for (const auto &variable : ir.variables) {
-            s << *variable;
+            if (!variable->hasIllegalUsageOfOpaqueType()) {
+                s << *variable;
+            } else {
+                llvm::errs() << "Error: Variable " << variable->getName()
+                             << " is skipped because it has incomplete type.\n";
+            }
         }
 
         for (const auto &varDefine : ir.varDefines) {
-            s << *varDefine;
+            if (!varDefine->hasIllegalUsageOfOpaqueType()) {
+                s << *varDefine;
+            } else {
+                llvm::errs() << "Error: Variable alias " << varDefine->getName()
+                             << " is skipped because it has incomplete type\n";
+                llvm::errs().flush();
+            }
         }
 
         for (const auto &func : ir.functions) {
-            if (func->isLegalScalaNativeFunction()) {
-                s << *func;
-            } else {
+            if (!func->isLegalScalaNativeFunction()) {
                 llvm::errs()
                     << "Warning: Function " << func->getName()
                     << " is skipped because Scala Native does not support "
                        "passing structs and arrays by value.\n";
                 llvm::errs().flush();
+            } else {
+                s << *func;
             }
         }
 
@@ -167,13 +191,15 @@ llvm::raw_ostream &operator<<(llvm::raw_ostream &s, const IR &ir) {
         s << "object " << ir.libName << "Helpers {\n";
 
         for (const auto &st : ir.structs) {
-            if (ir.shouldOutput(st) && st->hasHelperMethods()) {
+            if (ir.shouldOutput(st) && st->hasHelperMethods() &&
+                !st->hasIllegalUsageOfOpaqueType()) {
                 s << "\n" << st->generateHelperClass();
             }
         }
 
         for (const auto &u : ir.unions) {
-            if (ir.shouldOutput(u) && u->hasHelperMethods()) {
+            if (ir.shouldOutput(u) && u->hasHelperMethods() &&
+                !u->hasIllegalUsageOfOpaqueType()) {
                 s << "\n" << u->generateHelperClass();
             }
         }
@@ -447,5 +473,14 @@ bool IR::hasOutputtedDeclaration(
 
 template <typename T>
 bool IR::shouldOutput(const std::shared_ptr<T> &type) const {
-    return inMainFile(*type) || isTypeUsed(type, true);
+    if (isTypeUsed(type, true)) {
+        return true;
+    }
+    auto *typeDef = dynamic_cast<TypeDef *>(type.get());
+    if (typeDef) {
+        /* unused typedefs from main file are not printed if they are aliases
+         * for opaque types */
+        return !isAliasForOpaqueType(typeDef) && inMainFile(*typeDef);
+    }
+    return inMainFile(*type);
 }
diff --git a/bindgen/ir/IR.h b/bindgen/ir/IR.h
@@ -148,9 +148,13 @@ class IR {
     template <typename T> bool inMainFile(const T &type) const;
 
     /**
-     * @tparam T Type subclass
-     * @return true if type is in main file or it is used by declaration from
-     *         main file.
+     * @tparam T Enum, Struct, Union or TypeDef
+     * @return true if the type will be printed.
+     *         Following types are not printed:
+     *         - Unused types from included headers
+     *         - Unused typedefs from main header if they reference an opaque
+     *           type (if such typedef is used then true is returned but error
+     *           message is printed when bindings are generated)
      */
     template <typename T>
     bool shouldOutput(const std::shared_ptr<T> &type) const;
diff --git a/bindgen/ir/Struct.cpp b/bindgen/ir/Struct.cpp
@@ -49,6 +49,15 @@ std::shared_ptr<Location> StructOrUnion::getLocation() const {
 
 bool StructOrUnion::hasHelperMethods() const { return !fields.empty(); }
 
+bool StructOrUnion::hasIllegalUsageOfOpaqueType() const {
+    for (const auto &field : fields) {
+        if (isAliasForOpaqueType(field->getType().get())) {
+            return true;
+        }
+    }
+    return false;
+}
+
 Struct::Struct(std::string name, std::vector<std::shared_ptr<Field>> fields,
                uint64_t typeSize, std::shared_ptr<Location> location)
     : StructOrUnion(std::move(name), std::move(fields), std::move(location)),
diff --git a/bindgen/ir/Struct.h b/bindgen/ir/Struct.h
@@ -43,6 +43,12 @@ class StructOrUnion {
 
     bool hasHelperMethods() const;
 
+    /**
+     * @return true if the record contains field of opaque type or an array
+     *         of elements of opaque type.
+     */
+    bool hasIllegalUsageOfOpaqueType() const;
+
   protected:
     std::string name;
     std::vector<std::shared_ptr<Field>> fields;
diff --git a/bindgen/ir/TypeDef.cpp b/bindgen/ir/TypeDef.cpp
@@ -9,14 +9,13 @@ TypeDef::TypeDef(std::string name, std::shared_ptr<Type> type,
       location(std::move(location)) {}
 
 llvm::raw_ostream &operator<<(llvm::raw_ostream &s, const TypeDef &typeDef) {
-    if (!typeDef.getType()) {
-        llvm::errs() << "Error: type definition for " << typeDef.getName()
-                     << " was not found.\n";
-        llvm::errs().flush();
-        return s;
+    s << "  type " << handleReservedWords(typeDef.name) << " = ";
+    if (typeDef.type) {
+        s << typeDef.getType()->str();
+    } else {
+        s << "native.CStruct0 // incomplete type";
     }
-    s << "  type " + handleReservedWords(typeDef.name) + " = " +
-             typeDef.getType()->str() + "\n";
+    s << "\n";
     return s;
 }
 
diff --git a/bindgen/ir/VarDefine.cpp b/bindgen/ir/VarDefine.cpp
@@ -10,3 +10,7 @@ llvm::raw_ostream &operator<<(llvm::raw_ostream &s,
       << varDefine.variable->getType()->str() << " = native.extern\n";
     return s;
 }
+
+bool VarDefine::hasIllegalUsageOfOpaqueType() const {
+    return variable->hasIllegalUsageOfOpaqueType();
+}
diff --git a/bindgen/ir/VarDefine.h b/bindgen/ir/VarDefine.h
@@ -14,6 +14,8 @@ class VarDefine : public Define {
     friend llvm::raw_ostream &operator<<(llvm::raw_ostream &s,
                                          const VarDefine &varDefine);
 
+    bool hasIllegalUsageOfOpaqueType() const;
+
   private:
     std::shared_ptr<Variable> variable;
 };
diff --git a/bindgen/ir/Variable.cpp b/bindgen/ir/Variable.cpp
@@ -1,4 +1,5 @@
 #include "Variable.h"
+#include "../Utils.h"
 
 Variable::Variable(const std::string &name, std::shared_ptr<Type> type)
     : TypeAndName(name, type) {}
@@ -7,4 +8,8 @@ llvm::raw_ostream &operator<<(llvm::raw_ostream &s, const Variable &variable) {
     s << "  val " << variable.getName() << ": " << variable.getType()->str()
       << " = native.extern\n";
     return s;
-}
+}
+
+bool Variable::hasIllegalUsageOfOpaqueType() const {
+    return isAliasForOpaqueType(type.get());
+}
diff --git a/bindgen/ir/Variable.h b/bindgen/ir/Variable.h
@@ -10,6 +10,8 @@ class Variable : public TypeAndName {
 
     friend llvm::raw_ostream &operator<<(llvm::raw_ostream &s,
                                          const Variable &variable);
+
+    bool hasIllegalUsageOfOpaqueType() const;
 };
 
 #endif // SCALA_NATIVE_BINDGEN_VARIABLE_H
diff --git a/tests/samples/OpaqueTypes.h b/tests/samples/OpaqueTypes.h
@@ -1,3 +1,5 @@
+#include "include/OpaqueTypes.h"
+
 typedef struct points points;
 
 struct point;
@@ -21,3 +23,37 @@ struct point {
     int x;
     int y;
 };
+
+struct undefinedStruct;
+
+void usePointerToUndefinedStruct(struct undefinedStruct *);
+
+struct structWithPointerToUndefinedStruct {
+    struct undefinedStruct *field;
+};
+
+union unionWithPointerToUndefinedStruct {
+    struct undefinedStruct *field;
+};
+
+typedef union undefinedUnion undefinedUnion;
+
+typedef undefinedUnion *aliasToPointerOfUndefinedUnion;
+
+aliasToPointerOfUndefinedUnion *fun();
+
+typedef struct undefinedStruct aliasForUndefinedStruct; // okay
+
+aliasForUndefinedStruct *returnPointerToAliasOfUndefinedStruct();
+
+void usePointerToUndefinedIncludedStruct(undefinedIncludedStruct *);
+
+typedef aliasToPointerOfUndefinedUnion (
+    *functionPointerWithPointerToOpaqueType)(struct undefinedStruct **);
+
+void useUndefinedStruct(
+    struct undefinedStruct); // removed. Error message is printed
+
+extern struct undefinedStruct removedExtern; // removed
+
+#define removedExternAlias removedExtern // removed
diff --git a/tests/samples/OpaqueTypes.scala b/tests/samples/OpaqueTypes.scala
@@ -6,13 +6,27 @@ import scala.scalanative.native._
 @native.link("bindgentests")
 @native.extern
 object OpaqueTypes {
+  type struct_undefinedIncludedStruct = native.CStruct0 // incomplete type
+  type undefinedIncludedStruct = struct_undefinedIncludedStruct
   type struct_points = native.CStruct2[native.Ptr[struct_point], native.Ptr[struct_point]]
   type points = struct_points
   type struct_point = native.CStruct2[native.CInt, native.CInt]
   type union_u = native.CArray[Byte, native.Nat._4]
   type u = union_u
+  type struct_undefinedStruct = native.CStruct0 // incomplete type
+  type struct_structWithPointerToUndefinedStruct = native.CStruct1[native.Ptr[struct_undefinedStruct]]
+  type union_unionWithPointerToUndefinedStruct = native.CArray[Byte, native.Nat._8]
+  type union_undefinedUnion = native.CStruct0 // incomplete type
+  type undefinedUnion = union_undefinedUnion
+  type aliasToPointerOfUndefinedUnion = native.Ptr[undefinedUnion]
+  type aliasForUndefinedStruct = struct_undefinedStruct
+  type functionPointerWithPointerToOpaqueType = native.CFunctionPtr1[native.Ptr[native.Ptr[struct_undefinedStruct]], native.Ptr[undefinedUnion]]
   def move(point: native.Ptr[struct_point], x: native.CInt, y: native.CInt): native.Ptr[struct_point] = native.extern
   def processPoints(p: native.Ptr[points]): native.Ptr[union_u] = native.extern
+  def usePointerToUndefinedStruct(anonymous0: native.Ptr[struct_undefinedStruct]): Unit = native.extern
+  def fun(): native.Ptr[native.Ptr[undefinedUnion]] = native.extern
+  def returnPointerToAliasOfUndefinedStruct(): native.Ptr[aliasForUndefinedStruct] = native.extern
+  def usePointerToUndefinedIncludedStruct(anonymous0: native.Ptr[undefinedIncludedStruct]): Unit = native.extern
 }
 
 import OpaqueTypes._
@@ -37,10 +51,22 @@ object OpaqueTypesHelpers {
 
   def struct_point()(implicit z: native.Zone): native.Ptr[struct_point] = native.alloc[struct_point]
 
+  implicit class struct_structWithPointerToUndefinedStruct_ops(val p: native.Ptr[struct_structWithPointerToUndefinedStruct]) extends AnyVal {
+    def field: native.Ptr[struct_undefinedStruct] = !p._1
+    def field_=(value: native.Ptr[struct_undefinedStruct]): Unit = !p._1 = value
+  }
+
+  def struct_structWithPointerToUndefinedStruct()(implicit z: native.Zone): native.Ptr[struct_structWithPointerToUndefinedStruct] = native.alloc[struct_structWithPointerToUndefinedStruct]
+
   implicit class union_u_pos(val p: native.Ptr[union_u]) extends AnyVal {
     def i: native.Ptr[native.CInt] = p.cast[native.Ptr[native.CInt]]
     def i_=(value: native.CInt): Unit = !p.cast[native.Ptr[native.CInt]] = value
     def f: native.Ptr[native.CFloat] = p.cast[native.Ptr[native.CFloat]]
     def f_=(value: native.CFloat): Unit = !p.cast[native.Ptr[native.CFloat]] = value
   }
+
+  implicit class union_unionWithPointerToUndefinedStruct_pos(val p: native.Ptr[union_unionWithPointerToUndefinedStruct]) extends AnyVal {
+    def field: native.Ptr[native.Ptr[struct_undefinedStruct]] = p.cast[native.Ptr[native.Ptr[struct_undefinedStruct]]]
+    def field_=(value: native.Ptr[struct_undefinedStruct]): Unit = !p.cast[native.Ptr[native.Ptr[struct_undefinedStruct]]] = value
+  }
 }
diff --git a/tests/samples/include/OpaqueTypes.h b/tests/samples/include/OpaqueTypes.h
@@ -0,0 +1,7 @@
+struct s;
+
+extern struct s externVar; // removed. No warning printed
+
+typedef struct undefinedIncludedStruct undefinedIncludedStruct;
+
+void useUndefinedIncludedStruct(undefinedIncludedStruct);
diff --git a/tests/src/test/scala/org/scalanative/bindgen/BindgenSpec.scala b/tests/src/test/scala/org/scalanative/bindgen/BindgenSpec.scala
diff --git a/tools/src/main/scala/org/scalanative/bindgen/Bindgen.scala b/tools/src/main/scala/org/scalanative/bindgen/Bindgen.scala
diff --git a/tools/src/main/scala/org/scalanative/bindgen/Bindings.scala b/tools/src/main/scala/org/scalanative/bindgen/Bindings.scala

Original file line number	Diff line number	Diff line change
`@@ -72,12 +72,14 @@ bool Function::isLegalScalaNativeFunction() const {`
`72`	`72`	`/* Return type and parameters types cannot be array types because array type`
`73`	`73`	`* in this case is always represented as a pointer to element type */`
`74`	`74`	`if (isAliasForType<Struct>(retType.get()) \|\|`
`75`		`- isAliasForType<Union>(retType.get())) {`
	`75`	`+ isAliasForType<Union>(retType.get()) \|\|`
	`76`	`+ isAliasForOpaqueType(retType.get())) {`
`76`	`77`	`return false;`
`77`	`78`	`}`
`78`	`79`	`for (const auto &parameter : parameters) {`
`79`	`80`	`if (isAliasForType<Struct>(parameter->getType().get()) \|\|`
`80`		`- isAliasForType<Union>(parameter->getType().get())) {`
	`81`	`+ isAliasForType<Union>(parameter->getType().get()) \|\|`
	`82`	`+ isAliasForOpaqueType(parameter->getType().get())) {`
`81`	`83`	`return false;`
`82`	`84`	`}`
`83`	`85`	`}`
Original file line number	Diff line number	Diff line change
`@@ -10,3 +10,7 @@ llvm::raw_ostream &operator<<(llvm::raw_ostream &s,`
`10`	`10`	`<< varDefine.variable->getType()->str() << " = native.extern\n";`
`11`	`11`	`return s;`
`12`	`12`	`}`
	`13`	`+`
	`14`	`+bool VarDefine::hasIllegalUsageOfOpaqueType() const {`
	`15`	`+ return variable->hasIllegalUsageOfOpaqueType();`
	`16`	`+}`