mattreecebentley / plf_colony

An unordered C++ data container providing fast iteration/insertion/erasure while maintaining pointer/iterator validity to non-erased elements regardless of insertions/erasures. Provides higher-performance than std:: library containers for high-modification scenarios with unordered data.
https://plflib.org/colony.htm
zlib License
398 stars 33 forks source link

Modernize NULL -> nullptr? #24

Closed 0x8000-0000 closed 5 years ago

0x8000-0000 commented 5 years ago

Is plf::colony used on in environments that don't have nullptr? Can all "NULL" references be replaced with "nullptr"? Or at least do like fmtlib and use version/feature testing to select nullptr on compilers that support it?

I'm working on a prototype and turned all warnings and clang-tidy and cppcheck to full blast, and I keep getting quite a few of these:

3 external/plf_colony/plf_colony.h|1302 col 23 warning| error: zero as null pointer constant [-Werror,-Wzero-as-null-pointer-constant]
4 || if (next_group == NULL)
5 || ^~~~
6 || nullptr

Thank you!

mattreecebentley commented 5 years ago

Hi florin- I'm not keen to do that as null is guaranteed to be zero in c++, which enables a few optimizations. Nullptr, as far as I'm aware is not. Those warnings you're getting are strange, possibly faulty, given that they're correctly interpreting null as literal zero And this is valid.

On Saturday, 4 May 2019, Florin Iucha notifications@github.com wrote:

Is plf::colony used on in environments that don't have nullptr? Can all "NULL" references be replaced with "nullptr"? Or at least do like fmtlib and use version/feature testing to select nullptr on compilers that support it?

I'm working on a prototype and turned all warnings and clang-tidy and cppcheck to full blast, and I keep getting quite a few of these:

3 external/plf_colony/plf_colony.h|1302 col 23 warning| error: zero as null pointer constant [-Werror,-Wzero-as-null-pointer-constant] 4 || if (next_group == NULL) 5 || ^~~~ 6 || nullptr

Thank you!

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/mattreecebentley/plf_colony/issues/24, or mute the thread https://github.com/notifications/unsubscribe-auth/ABE4FITSAUOCY2SII3OBH4DPTWC5RANCNFSM4HKY3IPQ .

0x8000-0000 commented 5 years ago

Hi Matt,

I would be really surprised if nullptr and NULL are different in their binary value. If they weren't, there would be no incremental migration path for legacy code.

Tried this on Godbolt:

#include <cstdlib>

int squareNullptr(int* num) {
    if (num == nullptr)
    {
        return 0;
    }
    else
    {
        return 5 * (*num);
    }
}

int squareNULL(int* num) {
    if (num == NULL)
    {
        return 0;
    }
    else
    {
        return 5 * (*num);
    }
}

int square0(int* num) {
    if (! num)
    {
        return 0;
    }
    else
    {
        return 5 * (*num);
    }
}

https://godbolt.org/z/FZwA8T

They all compile to the same code on GCC9, Clang8, MSVC19.20 for x86-64 and ARM. We can explicitly see the constant 0 being loaded.

Do you know of a specific architecture/compiler where nullptr is not 0. I know the standard guarantees nullptr to be implicitly convertible to null pointer value. And I most code that I've seen assumes "memset(ptr, 0, size);" will achieve the same thing we now call "zero initialization" :)

0x8000-0000 commented 5 years ago

I'm trying to parse Jonathan Wakely's answer on stackoverflow: https://stackoverflow.com/questions/33656742/c11-backwards-compatibility-conversion-of-null-integer-constant-to-pointer/33656869#

A null pointer constant is an integer literal (2.14.2) with value zero or a prvalue of type std::nullptr_t.

This seems to leave open the possibility of 0 and std::nullptr_t being different, but in that case I still don't get why a compiler, in optimizing mode, won't make the substitution on its own, if it is worth it.

mattreecebentley commented 5 years ago

It's not the compiler that's doing the optimising, it's me. At any rate, there's no rational reason to change it. The warnings you're getting are questionable, as they're simply expanding the null macro without taking into account the nature of null, from my perspective.

On Sunday, 5 May 2019, Florin Iucha notifications@github.com wrote:

I'm trying to parse Jonathan Wakely's answer on stackoverflow: https://stackoverflow.com/questions/33656742/c11-backwards-compatibility- conversion-of-null-integer-constant-to-pointer/33656869#

A null pointer constant is an integer literal (2.14.2) with value zero or a prvalue of type std::nullptr_t.

This seems to leave open the possibility of 0 and std::nullptr_t being different, but in that case I still don't get why a compiler, in optimizing mode, won't make the substitution on its own, if it is worth it.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/mattreecebentley/plf_colony/issues/24#issuecomment-489384630, or mute the thread https://github.com/notifications/unsubscribe-auth/ABE4FIXJPOOAJGLOVSPXBJLPTZFU5ANCNFSM4HKY3IPQ .

mattreecebentley commented 5 years ago

ps. I get what you're saying about implementation, but you can't do the optimisation I'm doing without NULL guaranteed as zero. line 1062

0x8000-0000 commented 5 years ago

I understand you have made a decision, but I will leave you with this parting thoughts.

You can check via static_assert that nullptr, NULL and 0 have the same bit representation.

Also, whether you need to "hide" a special value inside a pointer, it does not matter whether that value is NULL or nullptr, you have to check for it before dereferencing the pointer.

mattreecebentley commented 5 years ago

None of that's relevant to what I've said - it's just a fact that nullptr may not be zero.