Skip to content

force TBB linker#15

Merged
TexLeeV merged 2 commits intomainfrom
cachy-os-link-failures
Mar 28, 2026
Merged

force TBB linker#15
TexLeeV merged 2 commits intomainfrom
cachy-os-link-failures

Conversation

@TexLeeV
Copy link
Copy Markdown
Owner

@TexLeeV TexLeeV commented Mar 27, 2026

Description

Brief description of what this PR changes and why.

Fixes #(issue number, if applicable)

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New learning module (adds new exercises/tests)
  • Enhancement (improves existing module or documentation)
  • Documentation update
  • Build system/infrastructure change

Changes Made

Testing

How have you tested these changes?

  • All tests pass locally (ctest --preset gcc --verbose)
  • Built specific targets successfully
  • Manually verified instrumentation output (if applicable)
  • Tested on multiple compilers (if applicable): GCC ___, Clang ___

Test Output (if applicable)

Paste relevant test output here

Code Quality Checklist

  • Code follows project style guidelines (see CONTRIBUTING.md)
  • No trailing whitespace on any line
  • Files end with exactly one newline
  • Constructor initializer lists use leading comma format
  • No syntax violations or compiler warnings

Documentation

  • Updated main README.md and docs/LEARNING_PATH.md (if adding or changing module registration)
  • Q/A/R patterns are correct and preserved (if applicable)
  • No additional README files created in subdirectories
  • No large summary sections added to code files

Socratic Pattern (if applicable)

For changes involving Q/A/R content:

  • Questions are specific and answerable by running code
  • Questions focus on observable behavior
  • Responses validate or correct with technical precision
  • Used instrumentation (EventLog) to make behavior visible

Additional Notes

Any additional context, screenshots, or information that reviewers should know.

License Agreement

  • I confirm that my contributions are my original work and agree to license them under the MIT License

Copilot AI review requested due to automatic review settings March 27, 2026 22:42
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates the STL learning module’s CMake configuration to ensure the algorithms test target links against Intel oneTBB.

Changes:

  • Add a CMake find_package(TBB REQUIRED) dependency in learning_stl.
  • Link test_algorithms against TBB::tbb via the existing add_learning_test(...) helper.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +3 to +4
find_package(TBB REQUIRED)

Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

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

find_package(TBB REQUIRED) makes TBB a hard configure-time dependency for the entire project because learning_stl is always added via add_subdirectory(...) (top-level CMakeLists.txt:40). That will break configuration on environments where TBB isn’t installed, even though only test_algorithms links it. Consider (a) moving find_package(TBB ...) to the top-level alongside other dependencies and/or (b) making TBB optional and only adding/linking test_algorithms when TBB is found (with a clear message when it’s skipped).

Copilot uses AI. Check for mistakes.
Comment on lines +3 to +7
find_package(TBB REQUIRED)

add_learning_test(test_container_internals tests/test_container_internals.cpp instrumentation)
add_learning_test(test_iterators tests/test_iterators.cpp instrumentation)
add_learning_test(test_algorithms tests/test_algorithms.cpp instrumentation)
add_learning_test(test_algorithms tests/test_algorithms.cpp instrumentation TBB::tbb)
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

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

The PR description is still the default template (no filled-in rationale, linked issue, or testing notes). Since this change introduces a new external dependency (TBB) and changes link behavior, please update the PR description to explain the motivating linker failure/use-case and how it was validated (e.g., which compiler/libstdc++/CI environment).

Copilot uses AI. Check for mistakes.
@TexLeeV TexLeeV merged commit c9976f1 into main Mar 28, 2026
2 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