Skip to content
New issue

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

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

Already on GitHub? # to your account

Fixed a crash and other updates to bring hyde up to date with llvm 13 #72

Merged
merged 3 commits into from
Mar 14, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
2 changes: 1 addition & 1 deletion matchers/utilities.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ std::string GetSignature(const ASTContext* n,
return signature.str();
}

if (!isTrailing) {
if (!isTrailing && functionT) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixes a runtime crash we were hitting.

if (functionT->isConst()) signature << " const";
if (functionT->isVolatile()) signature << " volatile";
if (functionT->isRestrict()) signature << " restrict";
Expand Down
112 changes: 86 additions & 26 deletions sources/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,12 @@ static cl::extrahelp HydeHelp(

/**************************************************************************************************/

bool IsVerbose() {
return ToolDiagnostic == ToolDiagnosticVerbose || ToolDiagnostic == ToolDiagnosticVeryVerbose;
}

/**************************************************************************************************/

std::pair<filesystem::path, hyde::json> load_hyde_config(
filesystem::path src_file) try {
bool found{false};
Expand Down Expand Up @@ -257,6 +263,14 @@ std::pair<filesystem::path, hyde::json> load_hyde_config(
directory_walk(src_file);
}

if (IsVerbose()) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added some diagnostics in the verbose case.

if (found) {
std::cout << "INFO: hyde-config file: " << hyde_config_path.string() << '\n';
} else {
std::cout << "INFO: hyde-config file: not found\n";
}
}

return found ?
std::make_pair(hyde_config_path.parent_path(),
hyde::json::parse(filesystem::ifstream(hyde_config_path))) :
Expand All @@ -267,7 +281,12 @@ std::pair<filesystem::path, hyde::json> load_hyde_config(

/**************************************************************************************************/

std::vector<std::string> integrate_hyde_config(int argc, const char** argv) {
struct command_line_args {
std::vector<std::string> _hyde;
std::vector<std::string> _clang;
};

command_line_args integrate_hyde_config(int argc, const char** argv) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I split the command line arguments into two sets: those used for hyde, and those to be forwarded to the clang driver. This makes things a little bit cleaner below as the tool is setting up both itself and clang-tooling.

auto cmdline_first = &argv[1];
auto cmdline_last = &argv[argc];
auto cmdline_mid = std::find_if(cmdline_first, cmdline_last,
Expand Down Expand Up @@ -305,6 +324,8 @@ std::vector<std::string> integrate_hyde_config(int argc, const char** argv) {
std::tie(config_dir, config) =
load_hyde_config(cli_hyde_flags.empty() ? "" : cli_hyde_flags.back());

hyde_flags.push_back(argv[0]);

if (exists(config_dir)) current_path(config_dir);

if (config.count("clang_flags")) {
Expand Down Expand Up @@ -333,46 +354,52 @@ std::vector<std::string> integrate_hyde_config(int argc, const char** argv) {
hyde_flags.insert(hyde_flags.end(), cli_hyde_flags.begin(), cli_hyde_flags.end());
clang_flags.insert(clang_flags.end(), cli_clang_flags.begin(), cli_clang_flags.end());

std::vector<std::string> result;
hyde_flags.push_back("--");

result.emplace_back(argv[0]);
command_line_args result;

result.insert(result.end(), hyde_flags.begin(), hyde_flags.end());

result.emplace_back("--");

if (!clang_flags.empty()) {
// it'd be nice if we could move these into place.
result.insert(result.end(), clang_flags.begin(), clang_flags.end());
}
result._hyde = std::move(hyde_flags);
result._clang = std::move(clang_flags);

return result;
}

/**************************************************************************************************/

std::vector<std::string> source_paths(int argc, const char** argv) {
return CommonOptionsParser(argc, argv, MyToolCategory).getSourcePathList();
namespace {

/**************************************************************************************************/

CommonOptionsParser MakeOptionsParser(int argc, const char** argv) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The interface to CommonOptionsParser has changed since clang 9 - this updates to the newer interface.

auto MaybeOptionsParser = CommonOptionsParser::create(argc, argv, MyToolCategory);
if (!MaybeOptionsParser) {
throw MaybeOptionsParser.takeError();
}
return std::move(*MaybeOptionsParser);
}

/**************************************************************************************************/

bool IsVerbose() {
return ToolDiagnostic == ToolDiagnosticVerbose || ToolDiagnostic == ToolDiagnosticVeryVerbose;
} // namespace

/**************************************************************************************************/

std::vector<std::string> source_paths(int argc, const char** argv) {
return MakeOptionsParser(argc, argv).getSourcePathList();
}

/**************************************************************************************************/

int main(int argc, const char** argv) try {
auto sources = source_paths(argc, argv);
std::vector<std::string> args = integrate_hyde_config(argc, argv);
int new_argc = static_cast<int>(args.size());
std::vector<const char*> new_argv(args.size(), nullptr);
command_line_args args = integrate_hyde_config(argc, argv);
int new_argc = static_cast<int>(args._hyde.size());
std::vector<const char*> new_argv(args._hyde.size(), nullptr);

std::transform(args.begin(), args.end(), new_argv.begin(),
std::transform(args._hyde.begin(), args._hyde.end(), new_argv.begin(),
[](const auto& arg) { return arg.c_str(); });

CommonOptionsParser OptionsParser(new_argc, &new_argv[0], MyToolCategory);
CommonOptionsParser OptionsParser(MakeOptionsParser(new_argc, &new_argv[0]));

if (UseSystemClang) {
AutoResourceDirectory = true;
Expand All @@ -385,7 +412,12 @@ int main(int argc, const char** argv) try {

if (IsVerbose()) {
std::cout << "INFO: Args:\n";
for (const auto& arg : args) {
std::cout << "INFO: (hyde)\n";
for (const auto& arg : args._hyde) {
std::cout << "INFO: " << arg << '\n';
}
std::cout << "INFO: (clang)\n";
for (const auto& arg : args._clang) {
std::cout << "INFO: " << arg << '\n';
}
std::cout << "INFO: Working directory: " << filesystem::current_path().string()
Expand All @@ -399,7 +431,6 @@ int main(int argc, const char** argv) try {
s.insert(i);
}
sourcePaths.assign(s.begin(), s.end());
ClangTool Tool(OptionsParser.getCompilations(), sourcePaths);
MatchFinder Finder;
hyde::processing_options options{sourcePaths, ToolAccessFilter, NamespaceBlacklist, ProcessClassMethods};

Expand All @@ -423,6 +454,11 @@ int main(int argc, const char** argv) try {

clang::tooling::CommandLineArguments arguments;

// start by appending the command line clang args.
for (auto& arg : args._clang) {
arguments.emplace_back(std::move(arg));
}

if (ToolDiagnostic == ToolDiagnosticVeryVerbose) {
arguments.emplace_back("-v");
}
Expand Down Expand Up @@ -491,17 +527,35 @@ int main(int argc, const char** argv) try {
arguments.emplace_back("-resource-dir=" + resource_dir.string());
}

//
// Specify the hyde preprocessor macro
//
arguments.emplace_back("-DADOBE_TOOL_HYDE=1");

//
// Spin up the tool and run it.
//

ClangTool Tool(OptionsParser.getCompilations(), sourcePaths);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a tweak to how the clang-specific command line arguments are sent into the driver.

We have a major problem here, where the driver will try to helpfully auto-detect the type of file being compiled by the driver, and this setting cannot be overridden. For example, in Photoshop we have a number of header files that end in .h, even though they contain C++ code within them. The driver will auto detect them as a C header file, and thus throw errors when c++ flags are passed (e.g., -std=c++14). Even trying to force the compilation type to C++ via -x c++ doesn't work, as it's later overridden in the driver command line arguments by the auto-detect function. (You can see this happening if you run hyde in very verbose mode.)


Tool.appendArgumentsAdjuster([](const CommandLineArguments& arguments, StringRef Filename){
return arguments;
});

Tool.appendArgumentsAdjuster(OptionsParser.getArgumentsAdjuster());

Tool.appendArgumentsAdjuster(
getInsertArgumentAdjuster(arguments, clang::tooling::ArgumentInsertPosition::END));

Tool.appendArgumentsAdjuster([](const CommandLineArguments& arguments, StringRef Filename){
return arguments;
});

if (Tool.run(newFrontendActionFactory(&Finder).get()))
throw std::runtime_error("compilation failed.");

//
// Take the results of the tool and process them.
//

hyde::json paths = hyde::json::object();
paths["src_root"] = YamlSrcDir;
paths["src_path"] = sourcePaths[0]; // Hmm... including multiple sources
Expand Down Expand Up @@ -543,10 +597,16 @@ int main(int argc, const char** argv) try {
}
}
} catch (const std::exception& error) {
std::cerr << "Error: " << error.what() << '\n';
std::cerr << "Fatal error: " << error.what() << '\n';
return EXIT_FAILURE;
} catch (const Error& error) {
std::string description;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added an exception output for cases when llvm::Error is thrown by the engine (e.g., when failing to construct the command options parser).

raw_string_ostream stream(description);
stream << error;
std::cerr << "Fatal error: " << stream.str() << '\n';
return EXIT_FAILURE;
} catch (...) {
std::cerr << "Error: unknown\n";
std::cerr << "Fatal error: unknown\n";
return EXIT_FAILURE;
}

Expand Down