apache / datasketches-cpp

Core C++ Sketch Library
https://datasketches.apache.org
Apache License 2.0
225 stars 71 forks source link

Reduce likelihood of macro collision #338

Closed jbapple closed 1 year ago

jbapple commented 1 year ago

. . . by removing one macro and prefacing the other with the logical module it's associated with.

coveralls commented 1 year ago

Pull Request Test Coverage Report for Build 4029456874


Totals Coverage Status
Change from base Build 4027761243: 0.0%
Covered Lines: 2328
Relevant Lines: 2480

💛 - Coveralls
jmalkin commented 1 year ago

I feel like it might be worth doing this for the other macros, too?

Alternatively, would it make more sense to make this a namedspaced c++ thing?

jbapple commented 1 year ago

I feel like it might be worth doing this for the other macros, too?

Done.

Alternatively, would it make more sense to make this a namedspaced c++ thing?

ROTL64 and CSA (new in this commit) could be replaced by inline functions, which could be namespaced. The others cannot.

jmalkin commented 1 year ago

I was thinking that we could alternatively restructure as a proper C++ class, but this certainly works for now and does improve naming safety. Thank you!