Skip to content

Conversation

anutosh491
Copy link
Collaborator

Description

CppInterOp used to use target_link_options before #483 went in. Not sure why this change was made. set_target_properties comes under the old style cmake and target_link_options and target_compile_options falls under mordern cmake which cmake promotes to be used with target_link_libraries.

This PR reverts back to what we were originally using. add_llvm_library internally make use of target_link_libraries and we should use target_link_options that goes with it .

Type of change

Please tick all options which are relevant.

  • Bug fix
  • New feature
  • Requires documentation updates

Testing

Please describe the test(s) that you added and ran to verify your changes.

Checklist

  • I have read the contribution guide recently

@anutosh491
Copy link
Collaborator Author

Was requested here #483 (comment) originally.

@mcbarton
Copy link
Collaborator

Copy link

codecov bot commented Mar 17, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 72.71%. Comparing base (ca6055f) to head (191fc62).
Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #524   +/-   ##
=======================================
  Coverage   72.71%   72.71%           
=======================================
  Files           9        9           
  Lines        3618     3618           
=======================================
  Hits         2631     2631           
  Misses        987      987           
🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@anutosh491 anutosh491 force-pushed the target_link_options branch from d2f6f70 to 191fc62 Compare March 17, 2025 11:00
@mcbarton mcbarton merged commit 5450a6a into compiler-research:main Mar 17, 2025
71 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.

2 participants