On Wed, 1 Nov 2023 07:32:23 GMT, Thomas Stuefe <stu...@openjdk.org> wrote:
>> src/hotspot/share/nmt/memMapPrinter.cpp line 79: >> >>> 77: const void* const min = MAX2(from1, from2); >>> 78: const void* const max = MIN2(to1, to2); >>> 79: return min < max; >> >> I had to rewrite it as: >> >> `return MAX2(from1, from2) < MIN2(to1, to2);` >> >> to make sure I understand it, or better yet, why not have it as a macros? >> >> `#define RANGE_INTERSECT(min, max) (return MAX2(from1, from2) < MIN2(to1, >> to2))` >> >> MAX2 and MIN2 are macros already and we're not doing anything fancy here. > > I'll do the former, that is clearer I agree, but leave the latter out (I > assume with the macro you mean add it to globalDefenitions.hpp). > > I fear that a lot of bikeshedding and general discussions would start, and to > do it properly needs a bit more time. Because we really would benefit from > having a nice templatized utility classes for ranges. Like MemRegion, but > that one is tied to HeapWord* I think. `MAX2` and `MIN2` are not macros, they're `constexpr` free functions, and that's what a `RANGE_INTERSECT` free function should be also. Unexpected because of their names, I know. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/16301#discussion_r1378579302
