Skip to content

Filter functions, extern vars, defines and unused types from included headers #113

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

Merged
merged 8 commits into from
Jul 10, 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
6 changes: 4 additions & 2 deletions bindgen/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,6 @@ add_executable(bindgen
defines/DefineFinderActionFactory.h
TypeTranslator.h
TypeTranslator.cpp
HeaderManager.h
HeaderManager.cpp
CycleDetection.h
Utils.h
ir/IR.h
Expand Down Expand Up @@ -72,6 +70,10 @@ add_executable(bindgen
ir/types/FunctionPointerType.h
ir/types/ArrayType.cpp
ir/types/ArrayType.h
ir/Location.h
ir/Location.cpp
ir/LocationManager.h
ir/LocationManager.cpp
)

if (STATIC_LINKING)
Expand Down
32 changes: 0 additions & 32 deletions bindgen/HeaderManager.cpp

This file was deleted.

15 changes: 0 additions & 15 deletions bindgen/HeaderManager.h

This file was deleted.

42 changes: 13 additions & 29 deletions bindgen/Main.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#include "defines/DefineFinderActionFactory.h"
#include "ir/LocationManager.h"
#include "visitor/ScalaFrontendActionFactory.h"
#include <clang/Tooling/CommonOptionsParser.h>

Expand All @@ -11,19 +12,7 @@ int main(int argc, const char *argv[]) {

llvm::cl::opt<std::string> LibName("name", llvm::cl::cat(Category),
llvm::cl::desc("Library name"));
llvm::cl::opt<std::string> StdHeaders(
"std-headers", llvm::cl::cat(Category),
llvm::cl::desc("Path to a file with the list of headers for which "
"bindings\n"
"will not be generated. "
"The list contains header files names\n"
"and package names that contain bindings for these "
"headers.\nExample:\n"
"math.h=scala.scalanative.native.math\n"
"stdlib.h=scala.scalanative.native.stdlib"));
llvm::cl::opt<bool> PrintHeadersLocation(
"location", llvm::cl::cat(Category),
llvm::cl::desc("Print list of parsed headers"));

llvm::cl::opt<std::string> ExcludePrefix(
"exclude-prefix", llvm::cl::cat(Category),
llvm::cl::desc("Functions and unused typedefs will be removed if their "
Expand All @@ -41,6 +30,12 @@ int main(int argc, const char *argv[]) {
clang::tooling::ClangTool Tool(op.getCompilations(),
op.getSourcePathList());

if (op.getSourcePathList().size() != 1) {
llvm::errs() << "Error: Only one file may be processed at a time.\n";
llvm::errs().flush();
return -1;
}

auto libName = LibName.getValue();
if (libName.empty()) {
llvm::errs()
Expand All @@ -64,14 +59,10 @@ int main(int argc, const char *argv[]) {
objectName = "nativeLib";
}

auto stdhead = StdHeaders.getValue();
if (!stdhead.empty()) {
headerMan.LoadConfig(stdhead);
}

locations.clear();
char *resolved = realpath(op.getSourcePathList()[0].c_str(), nullptr);
LocationManager locationManager(resolved);

IR ir(libName, linkName, objectName, Package.getValue());
IR ir(libName, linkName, objectName, Package.getValue(), locationManager);

DefineFinderActionFactory defineFinderActionFactory(ir);
int result = Tool.run(&defineFinderActionFactory);
Expand All @@ -82,15 +73,8 @@ int main(int argc, const char *argv[]) {
ScalaFrontendActionFactory actionFactory(ir);
result = Tool.run(&actionFactory);

auto printLoc = PrintHeadersLocation.getValue();
if (printLoc) {
for (const auto &location : locations) {
llvm::outs() << location.c_str();
}
} else {
ir.generate(ExcludePrefix.getValue());
llvm::outs() << ir;
}
ir.generate(ExcludePrefix.getValue());
llvm::outs() << ir;
llvm::outs().flush();
return result;
}
2 changes: 1 addition & 1 deletion bindgen/TypeTranslator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ TypeTranslator::translateStructOrUnionOrEnum(const clang::QualType &qtpe) {
/* type is not yet defined.
* TypeDef with nullptr will be created.
* nullptr will be replaced by actual type when the type is declared. */
typeDef = ir.addTypeDef(nameWithoutSpace, nullptr);
typeDef = ir.addTypeDef(nameWithoutSpace, nullptr, nullptr);
Copy link
Member

Choose a reason for hiding this comment

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

Eventually passing a location here would allow us to provide a better diagnostic if an opaque type is used in a problematic way.

Copy link
Member Author

Choose a reason for hiding this comment

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

It is a good idea but we should not use location field for this.
Maybe create an optional field typeDeclarationLocation

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 started doing it but it required passing Location instance to methods of TypeTranslator and it does not look good to me, so I decided to leave it for now.
I hope that after fixing #106 we will not see the error message often (except the case when one wants to generated bindings for broken header).

return typeDef;
}

Expand Down
7 changes: 7 additions & 0 deletions bindgen/defines/DefineFinder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ DefineFinder::DefineFinder(IR &ir, const clang::CompilerInstance &compiler,
void DefineFinder::MacroDefined(const clang::Token &macroNameTok,
const clang::MacroDirective *md) {
clang::SourceManager &sm = compiler.getSourceManager();
if (!sm.isInMainFile(md->getLocation())) {
/* include defines only from the original header */
return;
}
if (sm.isWrittenInMainFile(macroNameTok.getLocation()) && md->isDefined() &&
!md->getMacroInfo()->isFunctionLike()) {
/* save defines only from the given header.
Expand Down Expand Up @@ -100,6 +104,9 @@ 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;
}
Copy link
Member

Choose a reason for hiding this comment

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

Having a test for this would be very valuable.

if (sm.isWrittenInMainFile(macroNameTok.getLocation()) &&
md.getMacroInfo() && !md.getMacroInfo()->isFunctionLike()) {
std::string macroName = macroNameTok.getIdentifierInfo()->getName();
Expand Down
16 changes: 12 additions & 4 deletions bindgen/ir/Enum.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,16 @@ std::string Enumerator::getName() { return name; }
int64_t Enumerator::getValue() { return value; }

Enum::Enum(std::string name, std::string type,
std::vector<Enumerator> enumerators)
std::vector<Enumerator> enumerators,
std::shared_ptr<Location> location)
: PrimitiveType(std::move(type)), name(std::move(name)),
enumerators(std::move(enumerators)) {}
enumerators(std::move(enumerators)), location(std::move(location)) {}

bool Enum::isAnonymous() const { return name.empty(); }

std::shared_ptr<TypeDef> Enum::generateTypeDef() {
assert(!isAnonymous());
return std::make_shared<TypeDef>("enum_" + name, shared_from_this());
return std::make_shared<TypeDef>(getTypeAlias(), shared_from_this(),
nullptr);
}

llvm::raw_ostream &operator<<(llvm::raw_ostream &s, const Enum &e) {
Expand Down Expand Up @@ -49,3 +50,10 @@ llvm::raw_ostream &operator<<(llvm::raw_ostream &s, const Enum &e) {
}

std::string Enum::getName() const { return name; }

std::string Enum::getTypeAlias() const {
assert(!isAnonymous());
return "enum_" + name;
}

std::shared_ptr<Location> Enum::getLocation() const { return location; }
8 changes: 7 additions & 1 deletion bindgen/ir/Enum.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ class Enumerator {
class Enum : public PrimitiveType, public std::enable_shared_from_this<Enum> {
public:
Enum(std::string name, std::string type,
std::vector<Enumerator> enumerators);
std::vector<Enumerator> enumerators,
std::shared_ptr<Location> location);

bool isAnonymous() const;

Expand All @@ -31,9 +32,14 @@ class Enum : public PrimitiveType, public std::enable_shared_from_this<Enum> {

friend llvm::raw_ostream &operator<<(llvm::raw_ostream &s, const Enum &e);

std::string getTypeAlias() const;

std::shared_ptr<Location> getLocation() const;

private:
std::string name; // might be empty
std::vector<Enumerator> enumerators;
std::shared_ptr<Location> location;
};

#endif // SCALA_NATIVE_BINDGEN_ENUM_H
Loading