refactor: fix macos build and cleanup cmake#221
Merged
Conversation
|
Merging to
|
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
65059b0 to
34a052a
Compare
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
34a052a to
52b3c7d
Compare
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR fixes the macOS build and cleans up various CMake configurations and code formatting issues. Key changes include conversion specifier updates for printing 64‐bit values, modifications to CMake linking and visibility settings, and renaming/reordering of several cache eviction function prototypes.
Reviewed Changes
Copilot reviewed 33 out of 97 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/test_dataStructure.c | Updated printf format specifier for obj_id printing |
| test/CMakeLists.txt | Changed linking dependency from common modules to profiler/core libraries |
| scripts/* | Improved cross‐platform support for processor count and dependency installation |
| libCacheSim/utils/mystr.* | Updated conversion function to use snprintf with explicit buffer length |
| libCacheSim/trace* | Updated printf format specifiers and linking options |
| libCacheSim/profiler/, mrcProfiler/ | Adjusted printf conversion specifiers to correctly cast 64‐bit values |
| libCacheSim/include/libCacheSim/reader.h | Changed printing of line_buf to safely handle NULL pointers |
| libCacheSim/include/libCacheSim/macro.h | Simplified macros by removing redundant FMT parameters |
| libCacheSim/include/libCacheSim/evictionAlgo.h | Reordered and renamed several cache eviction function prototypes |
| libCacheSim/dataStructure/* | Updated usage of std::move in splay tree implementations and fixed include guard spelling |
| libCacheSim/cache/* | Updated printf conversion specifiers; adjusted CMake link options for improved dependency management |
| CMakeLists.txt | Restructured module linkage and added visibility settings for symbol exports |
Comments suppressed due to low confidence (2)
libCacheSim/include/libCacheSim/evictionAlgo.h:53
- The function prototypes in this file have been significantly reordered and renamed. Ensure that the updated API design is consistently documented and that all consumers of these interfaces are updated accordingly.
cache_t *CAR_init(const common_cache_params_t ccache_params,
libCacheSim/dataStructure/minimalIncrementCBF.h:1
- The include guard identifier has been corrected from '_MINIMAL_INCREMENR_CBF_H' to '_MINIMAL_INCREMENT_CBF_H'. Verify that this change does not conflict with any external dependencies.
#ifndef _MINIMAL_INCREMENT_CBF_H
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR fixes the build on macOS. During the fix, a few more cmake changes are introduced. Notably, we previously used
export_dynamiclinked flag, which is not available on MacOS, now we changed the symbols to default visibility.