stotko / stdgpu

stdgpu: Efficient STL-like Data Structures on the GPU
https://stotko.github.io/stdgpu/
Apache License 2.0
1.15k stars 81 forks source link

Findthrust.cmake: Fix the bug in regular replacement. #408

Closed HWZen closed 6 months ago

HWZen commented 6 months ago

Fix bug #407: The original regular replacement does not consider the possibility that THRUST_VERSION may be followed by comments.

codecov[bot] commented 6 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 97.33%. Comparing base (71a5aef) to head (f0febd7).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #408 +/- ## ======================================= Coverage 97.33% 97.33% ======================================= Files 31 31 Lines 2512 2512 ======================================= Hits 2445 2445 Misses 67 67 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

dengchenlong commented 3 months ago

Why does REGEX "#define THRUST_VERSION[ \t]+([0-9x]+)") match the comments that follow?

HWZen commented 3 months ago

Why does REGEX "#define THRUST_VERSION[ \t]+([0-9x]+)") match the comments that follow?

This regex statement does not match comments, but does match the current version numbers.

dengchenlong commented 3 months ago

Why does REGEX "#define THRUST_VERSION[ \t]+([0-9x]+)") match the comments that follow?

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 THRUST_VERSION_STRING equal to "#define THRUST_VERSION 200400 // macro expansion with ## requires this to be a single value" instead of "#define THRUST_VERSION 200400" before string(REGEX REPLACE ...)?

HWZen commented 3 months ago

Why does REGEX "#define THRUST_VERSION[ \t]+([0-9x]+)") match the comments that follow?

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 THRUST_VERSION_STRING equal to "#define THRUST_VERSION 200400 // macro expansion with ## requires this to be a single value" instead of "#define THRUST_VERSION 200400" before string(REGEX REPLACE ...)?

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 THRUST_VERSION_STRING

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 -- TEST_STRING: #define THRUST_VERSION 200400;Hello world