Skip to content

[clang-linker-wrapper] Add error handling for missing linker path #15862

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 2 commits into from
Oct 25, 2024

Conversation

asudarsa
Copy link
Contributor

In clang-linker-wrapper, we do not explicitly check if --linker-path is not provided.
This PR adds a check to capture this.

Thanks

@asudarsa asudarsa requested review from a team as code owners October 24, 2024 19:10
@@ -411,6 +411,8 @@ Error runLinker(ArrayRef<StringRef> Files, const ArgList &Args) {
// Render the linker arguments and add the newly created image. We add it
// after the output file to ensure it is linked with the correct libraries.
StringRef LinkerPath = Args.getLastArgValue(OPT_linker_path_EQ);
if (LinkerPath.empty())
return createStringError("Host linker is not available");
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this go upstream?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. Happening in parallel.

llvm/llvm-project#113613 (comment)

tests are different, but expect to converge after pulldown.

Thanks

Copy link
Contributor

@sarnex sarnex left a comment

Choose a reason for hiding this comment

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

lgtm assuming the code will be the exact same upstream so we don't have a complicated merge during pulldown

Signed-off-by: Arvind Sudarsanam <[email protected]>
@maarquitos14
Copy link
Contributor

@intel/llvm-gatekeepers I think this is ready to merge.

@sarnex sarnex merged commit 1099a59 into intel:sycl Oct 25, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants