-
Notifications
You must be signed in to change notification settings - Fork 80
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
Findthrust.cmake: Fix the bug in regular replacement. #408
Conversation
Fix bug stotko#407: The original regular replacement does not consider the possibility that THRUST_VERSION may be followed by comments.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #408 +/- ##
=======================================
Coverage 97.33% 97.33%
=======================================
Files 31 31
Lines 2512 2512
=======================================
Hits 2445 2445
Misses 67 67 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the fix!
Why does |
This regex statement does not match comments, but does match the current version numbers. |
The original code was like this before being modified: # Findthrust.cmake
...
file(STRINGS "${THRUST_INCLUDE_DIR}/thrust/version.h"
THRUST_VERSION_STRING
REGEX "#define THRUST_VERSION[ \t]+([0-9x]+)")
string(REGEX REPLACE "#define THRUST_VERSION[ \t]+" "" THRUST_VERSION_STRING ${THRUST_VERSION_STRING})
... // thrust/version.h
...
#define THRUST_VERSION 200400 // macro expansion with ## requires this to be a single value
... Why is |
Because CMake regular matching uses one line as the minimum judgment unit. When the content in this line matches the regular expression, this line will be insert into You can do the following experiment # CMakeLists.txt
file(STRINGS "./temp.txt"
TEST_STRING
REGEX "e"
)
message(STATUS "TEST_STRING: ${TEST_STRING}") temp.txt
#define THRUST_VERSION 200400
Hello world You will see the output |
Fix bug #407: The original regular replacement does not consider the possibility that THRUST_VERSION may be followed by comments.