Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix opaque types usages #114

Merged
merged 1 commit into from
Jul 22, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions bindgen/Utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
3 changes: 0 additions & 3 deletions bindgen/defines/DefineFinder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -108,9 +108,6 @@ void DefineFinder::MacroUndefined(const clang::Token &macroNameTok,
return;
}
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();
Expand Down
6 changes: 4 additions & 2 deletions bindgen/ir/Function.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand Down
70 changes: 52 additions & 18 deletions bindgen/ir/IR.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -102,40 +102,61 @@ llvm::raw_ostream &operator<<(llvm::raw_ostream &s, const IR &ir) {

std::string objectName = handleReservedWords(ir.objectName);

if (!ir.libObjEmpty()) {
bool isLibObjectEmpty = ir.libObjEmpty();

if (!isLibObjectEmpty) {
if (!ir.linkName.empty()) {
s << "@native.link(\"" << ir.linkName << "\")\n";
}

s << "@native.extern\n"
<< "object " << objectName << " {\n";
}

for (const auto &typeDef : ir.typeDefs) {
if (ir.shouldOutput(typeDef)) {
s << *typeDef;
}
for (const auto &typeDef : ir.typeDefs) {
if (ir.shouldOutput(typeDef)) {
s << *typeDef;
} else if (isAliasForOpaqueType(typeDef.get()) &&
ir.inMainFile(*typeDef)) {
llvm::errs() << "Warning: type alias " + typeDef->getName()
<< " is skipped because it is an unused alias for "
"incomplete type."
<< "\n";
llvm::errs().flush();
}
}

for (const auto &variable : ir.variables) {
for (const auto &variable : ir.variables) {
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) {
for (const auto &varDefine : ir.varDefines) {
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 {
llvm::errs()
<< "Warning: Function " << func->getName()
<< " is skipped because Scala Native does not support "
"passing structs and arrays by value.\n";
llvm::errs().flush();
}
for (const auto &func : ir.functions) {
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;
}
}

if (!isLibObjectEmpty) {
s << "}\n\n";
}

Expand Down Expand Up @@ -449,5 +470,18 @@ 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;
}
if (!inMainFile(*type)) {
/* remove unused types from included files */
return false;
}
auto *typeDef = dynamic_cast<TypeDef *>(type.get());
if (typeDef) {
/* unused typedefs from main file are printed only if they are not
* aliases for an opaque type. */
return !isAliasForOpaqueType(typeDef);
}
return true;
}
10 changes: 7 additions & 3 deletions bindgen/ir/IR.h
Original file line number Diff line number Diff line change
Expand Up @@ -149,9 +149,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;
Expand Down
13 changes: 6 additions & 7 deletions bindgen/ir/TypeDef.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest to remove the comment completely, else please change it to say: opaque type.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used incomplete because it is the term that clang uses in error messages.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay makes sense then.

}
s << " type " + handleReservedWords(typeDef.name) + " = " +
typeDef.getType()->str() + "\n";
s << "\n";
return s;
}

Expand Down
4 changes: 4 additions & 0 deletions bindgen/ir/VarDefine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
2 changes: 2 additions & 0 deletions bindgen/ir/VarDefine.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};
Expand Down
7 changes: 6 additions & 1 deletion bindgen/ir/Variable.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#include "Variable.h"
#include "../Utils.h"

Variable::Variable(const std::string &name, std::shared_ptr<Type> type)
: TypeAndName(name, type) {}
Expand All @@ -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());
}
2 changes: 2 additions & 0 deletions bindgen/ir/Variable.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
36 changes: 36 additions & 0 deletions tests/samples/OpaqueTypes.h
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
#include "include/OpaqueTypes.h"

typedef struct points points;

struct point;
Expand All @@ -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
26 changes: 26 additions & 0 deletions tests/samples/OpaqueTypes.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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._
Expand All @@ -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
}
}
7 changes: 7 additions & 0 deletions tests/samples/include/OpaqueTypes.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
struct s;

extern struct s externVar; // removed. No warning printed

typedef struct undefinedIncludedStruct undefinedIncludedStruct;

void useUndefinedIncludedStruct(undefinedIncludedStruct);
Original file line number Diff line number Diff line change
Expand Up @@ -58,5 +58,26 @@ class BindgenReportingSpec extends FunSpec {
|Warning: Function returnUnion is skipped because Scala Native does not support passing structs and arrays by value.""".stripMargin
)
}

it("Skips variable with opaque type") {
val bindings =
bindgen(input = """struct undefinedStruct;
|extern struct undefinedStruct removedExtern;
|#define removedExternAlias removedExtern
|""".stripMargin)
assert(
bindings.errs == """Error: Variable removedExtern is skipped because it has incomplete type.
|Error: Variable alias removedExternAlias is skipped because it has incomplete type.""".stripMargin)

}

it("Skips unused alias for opaque type") {
val bindings =
bindgen(input = """union undefinedUnion;
|typedef union undefinedUnion aliasForUndefinedUnion;
|""".stripMargin)
assert(
bindings.errs == "Warning: type alias aliasForUndefinedUnion is skipped because it is an unused alias for incomplete type.")
}
}
}