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

Conversation

fosterbrereton
Copy link
Contributor

Hyde was originally built against version 9 of clang tooling. Now that we're at version 13, I figure it's time to update things.

I'll go through the PR and add comments as necessary to detail the changes.

@fosterbrereton fosterbrereton requested a review from jaredwy June 10, 2022 20:55
@@ -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.

sources/main.cpp Outdated
@@ -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.

sources/main.cpp Outdated
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.

sources/main.cpp Outdated

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

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.

sources/main.cpp Outdated
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).

sources/main.cpp Outdated
// 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.)

@fosterbrereton fosterbrereton merged commit 214b967 into master Mar 14, 2023
@fosterbrereton fosterbrereton deleted the fosterbrereton/llvm13-updates branch March 14, 2023 22:24
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants