search

ETLCPP / etl

Embedded Template Library
https://www.etlcpp.com
MIT License
2.17k stars 387 forks source link

Add an etl::nullptr_t type to <etl/nullptr.h> #924

Closed tigran2008 closed 2 months ago

tigran2008 commented 2 months ago

Well, the title is self-explanatory. This is a tiny change anyway, and the code is basically what's proposed in #922.

Edit: As of this moment, I changed the pre-C++11 code from an enum into a custom class

semanticdiff-com[bot] commented 2 months ago

Review changes with SemanticDiff.

tigran2008 commented 2 months ago

I'm so sorry for the mistakes, I'm a total beginner to C++ and I'm trying my best :/

tigran2008 commented 2 months ago

After I'm done, I promise I'll squash the commits 😅

jwellbelove commented 2 months ago

You can run the unit tests locally if you want to check before you push, though it does take some time to do all C++11/14/17/20 variants.

tigran2008 commented 2 months ago

Do you think this implementation of etl::nullptr_t is acceptable? It instantiates a _nullptr instance beforehand, and it occupies one byte of storage, unlike the previous one, which didn't occupy any (without usage). I could also do #define ETL_NULLPTR (etl::nullptr()) but I thought it wouldn't be good to instantiate a new object everywhere a nullptr is used..

Also, if you do think it's an acceptable solution, do you think I should add a (void)_nullptr; // silence unused warning? I didn't get an unused variable warning on my computer (gcc version 13.3.1 20240522 (Red Hat 13.3.1-1) (GCC) on Fedora 39) but I just noticed that it warns on https://cpp.sh

tigran2008 commented 2 months ago

Do you think I should also delete etl::addressof(ETL_NULLPTR)?

tigran2008 commented 2 months ago

Well, I'll be waiting for a comment from you before squashing the commits

jwellbelove commented 2 months ago

I'll have a look tomorrow.