search

sewenew / redis-plus-plus

Redis client written in C++
Apache License 2.0
1.64k stars 351 forks source link

Changes for Nvidia compiler chain #488

Closed ashao closed 1 year ago

ashao commented 1 year ago

Compiling redis++ with the Nvidia compiler chain fails due to warnings thrown during compilation which are converted to errors because of the -Wall compilation flag. Nvidia's compilers are based on the PGI compilers which can be quite pedantic. In this case, a number of blocks of code are seen as unreachable either due to a 'break' immediately following a 'throw' or logic (in the case of reply.h).

The changes in this commit lead to a clean compilation of Redis++. Some of these changes lead to some code that is more verbose than is strictly needed, however are necessary to avoid warnings.

ashao commented 1 year ago

Note this passes RedisCluster and Redis tests:

Testing RedisCluster...
Pass sanity tests
Pass connection commands tests
Pass redlock tests
Pass keys commands tests
Pass string commands tests
Pass list commands tests
Pass hash commands tests
Pass set commands tests
Pass zset commands tests
Pass hyperloglog commands tests
Pass geo commands tests
Pass script commands tests
Pass pubsub tests
Pass pipeline and transaction tests
Pass stream commands tests
Pass all tests
Testing Redis...
Pass sanity tests
Pass connection commands tests
Time to lock and unlock 1000 simultaneous locks with RedLockMutexVessel: 0.0494994 s
Time to lock and unlock 1000 simultaneous locks with RedLockMutex: 0.0453558 s
Time to lock and unlock 1000 simultaneous locks with RedMutex: 0.200129 s
Pass redlock tests
Pass keys commands tests
Pass string commands tests
Pass list commands tests
Pass hash commands tests
Pass set commands tests
Pass zset commands tests
Pass hyperloglog commands tests
Pass geo commands tests
Pass script commands tests
Pass pubsub tests
Pass pipeline and transaction tests
Pass stream commands tests
Pass all tests
sewenew commented 1 year ago

Thanks for your PR! I'll take a review ASAP (when I'm not so busy...)

Regards

ashao commented 1 year ago

Not a problem! I have a workaround for myself, so please take your time when reviewing. Just figured that it might be nice now that Nvidia's compilers are becoming more popular to use.

ashao commented 1 year ago

The other alternative, if you'd prefer a less invasive PR, is that I could modify the CMakeLists.txt to disable the -Wall flag if the Nvidia compiler is detected. I'll leave that up to you

sewenew commented 1 year ago

@ashao Since there's no reply from you, I've made changes to remove those unreachable code branches. You can try the latest code on master branch to check if the new code can pass Nvidia compiler.

I'll close this PR, if you still have problem with it, feel free to open a new one.

Thanks again for your contribution!

Regards