Skip to content

Conversation

@Axel-Naumann
Copy link
Member

No description provided.

@phsft-bot
Copy link

Starting build on gcc49/centos7, native/mac1012, gcc49/slc6, gcc62/slc6, native/ubuntu14 with flags -Dvc=OFF -Dimt=ON -Dccache=ON
How to customize builds

#ifndef NULL
#define NULL 0
constexpr const auto R__DEPRECATED(6,12, "Please #include <cstddef> instead of relying on ROOT's NULL")
NULL = nullptr;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe it would be better to either use #define NULL ((void*)0) to be compatible with C, or simply remove it from ROOT altogether and just #include <cstddef> to get it from the standard. I wonder what happens when one uses nullptr to call something from a C library. Won't things break? If not, then just including the standard header is probably the best solution. References: C and C++ versions of NULL.

@Axel-Naumann
Copy link
Member Author

The header will be compiled with C++. And this definition does what C++ is asking for, and allows us to sneak in an attribute.

And btw, it's basically unused because +/- all code is getting NULL from outside this header as RtypesCore.h #includes stddef.h. It's just a (likely irrelevant) fallback.

@vgvassilev
Copy link
Member

Hm, if this is a fallback case why bother to deprecate it?

@amadio
Copy link
Member

amadio commented Jun 20, 2017

I still don't see the advantage in deviating from the standard (i.e., either #include <cstddef> instead of stddef.h or just #define NULL nullptr).

@Axel-Naumann
Copy link
Member Author

I still don't see the advantage in deviating from the standard (i.e., either #include instead of stddef.h or just #define NULL nullptr).

Let's see whether all implementations provide it as prescribed and simply remove it from ROOT (in a new PR). Once that works I'll close this one.

@Axel-Naumann
Copy link
Member Author

Superseded by #668

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants