Closed jonnygrant closed 6 months ago
I tried
strict_not_null
but it also didn't give what I need. https://godbolt.org/z/nET6nse31
I tried
strict_not_null
but it also didn't give what I need. https://godbolt.org/z/nET6nse31
That is expected. The difference between strict_not_null
and not_null
is that strict_not_null
has an explicit
constructor whereas not_null
has a constructor that can be called implicitely. And that difference is not relevant for what you want to achieve.
Hi @jonnygrant,
There are some compile-time checks that are offered by GSL not_null, such as disallowing construction directly from the nullptr (i.e., f(nullptr);
), but these are implemented via C++ language features (such as deleting the constructor taking std::nullptr_t
). Otherwise, all checks intentionally occur at runtime.
More comprehensive compile-time checks would be best achieved via a static analysis tool that understands gsl::not_null
. For example MSVC currently has some experimental gsl::not_null support that would raise a compile-time diagnostic on your code example.
Hi @dmitrykobets-msft Many thanks for your reply.
Thank you for the static analysis tool suggestion. It's a good idea. I'm only on GNU/Linux gcc, so I don' think I could compile that unfortunately.
I did patch not_null locally, and now the build stops in an optimized build when it sees my int* is NULL
Example uses the following. It adds a dummy method, just to trigger the warning -Wnotnull
__attribute__ ((nonnull));
https://godbolt.org/z/xd1YeE5cs
Would you consider integrating something similar into the official GSL not_null class?
This might be better using
__attribute__((error("not_null found nullptr")));
https://godbolt.org/z/cMvjqb93T
Could you consider adding this. It works with GCC and Clang.
In your godbolt with error
I changed the definition of test
in main
to int * test = new (std::nothrow) int();
. That code still does not compile because the pointer might be null and the compiler might need to call compile_error_null_check
. So with this proposed solution legitimate code does not compile.
@beinhaerter
I tried your suggestion with GCC and found the same, however with clang, it did compile https://godbolt.org/z/63jYWW794
Same with malloc(8), clang compiled but not gcc. https://godbolt.org/z/hve7hha9x
Maybe the
__attribute__ ((nonnull));
example is better I provided above.
Your __attribute__((nonnull))
change also fails with GCC and int * test = new (std::nothrow) int();
.
Your
__attribute__((nonnull))
change also fails with GCC andint * test = new (std::nothrow) int();
.
May I ask, my idea is to get a not_null build error when a pointer is passed that might be nullptr. So it seems to be working as expected?
You get a nice compile time error when accidentally passing a value that might be a nullptr as a not_null<int*>
In constructor 'constexpr gsl::not_null<T>::not_null(T) [with <template-parameter-2-1> = void; T = int*]',
inlined from 'int main()' at <source>:475:6:
<source>:236:53: error: argument 1 null where non-null expected [-Werror=nonnull]
For such a fatal memory allocation failure by new, I'd put some handler in to log it and abort()
if(nullptr == test)
{
log_error("FATAL new (std::nothrow) int(); returned nullptr\n");
abort();
}
Oddly this attribute nonnull example doesn't work in the else {}, even after checking the pointer is valid. Sharing the link https://godbolt.org/z/7661ffdxK
In your godbolt with
error
I changed the definition oftest
inmain
toint * test = new (std::nothrow) int();
. That code still does not compile because the pointer might be null and the compiler might need to callcompile_error_null_check
. So with this proposed solution legitimate code does not compile.
I updated this error
example after getting the build error for the possible nullptr returned by operator new, so this works well
@jonnygrant thanks for continuing the investigation and iterating on the design.
With your latest changes, this example fails to compile: https://godbolt.org/z/o6oj63Pd1, whereas in the existing GSL implementation it will compile and run fine if the pointer is not null, and runtime error otherwise.
int main()
{
gsl::not_null<int*> test = new (std::nothrow) int();
int a = *test;
(void)a; // suppress unused var warning
}
In fact, even the following code will fail the compile-time check: https://godbolt.org/z/4TG1aP3os
void foo(int* ptr)
{
gsl::not_null<int*> nn = ptr; // compiler can't assert that ptr != nullptr, compile_error_null_check
(void)nn; // suppress unused var warning
}
int main() {}
So the programming model effectively proposed with these changes requires the users of not_null
to manually insert nullchecks:
void foo(int* ptr)
{
if (ptr == nullptr) return;
gsl::not_null<int*> nn = ptr; // compiles fine
(void)nn; // suppress unused var warning
}
int main() {}
If this is intended, then you will probably want to first open a CoreGuidelines issue to discuss this proposal, since it is a non-trivial deviation from the current programming model suggested by not_null
.
@dmitrykobets-msft Many thanks for your reply and the suggestion, I've filed on CppCoreGuidelines now. Let's see what they say.
You're right nullptr checks would need to be inserted in some application code. My idea gives the programmer chance to evaluate where the nullptr came from at build time, and what logic caused it.
Maintainers call: Closed per https://github.com/isocpp/CppCoreGuidelines/issues/2071
Has anyone found an easy way to get not_null to fire off at build time (for those cases that it can see are obvious) ? Would be great if not_null could work at build time. Currently it just calls terminate at runtime.
Pasted the code into Godbolt to demonstrate this example where I pasted GSL not_null : https://godbolt.org/z/oqrTffTYj
I did find one way to get a compile time warning. I call a function with an attribute warning. And I also leave out that function so it fails to link.
:
I can stop the build using this approach below. Unfortunately it only works in an optimized build.