llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
29.24k stars 12.07k forks source link

[clang] False Positive for [-Wsentinel] when mixing C and C++ #72290

Open Febbe opened 1 year ago

Febbe commented 1 year ago

When using a C API from C++, clang-cl warns because of missing sentinels, even when NULL is used as sentinel: clang-cl : 17.0.1 ucrt: 10.0.22000.0 visula studio build tools 2022: 17.7.4 msvc: 14.37.32822 (v143)

[build] [157/358  40% :: 18.645] Building CXX object src\core\CMakeFiles\xournalpp-core.dir\gui\MainWindow.cpp.obj
[build] C:\xxx\MainWindow.cpp(59,111): warning: missing sentinel in function call [-Wsentinel]
[build]    59 |     g_object_set(appSettings, "gtk-application-prefer-dark-theme", control->getSettings()->isDarkTheme(), NULL);
[build]       |                                                                                                               ^
[build]       |                                                                                                               , nullptr
[build] C:\vcpkg\installed\x64-windows\include\glib-2.0\gobject/gobject.h(470,10): note: function has been explicitly marked sentinel here
[build]   470 | void        g_object_set                      (gpointer        object,
[build]       |             ^
[build] 1 warning generate
tbaederr commented 1 year ago

I'm having a hard time reproducing this - can you came up with a standalone test case?

tbaederr commented 1 year ago

Okay, I was able to do it.

glib used to define NULL as (0L) in c++:

#ifndef NULL
#  ifdef __cplusplus
#  define NULL        (0L)
#  else /* !__cplusplus */
#  define NULL        ((void*) 0)
#  endif /* !__cplusplus */
#endif

but doesn't do that anymore for c++11:

#ifndef NULL
#  if G_CXX_STD_CHECK_VERSION (11)
#    define NULL (nullptr)
#  elif defined (G_CXX_STD_VERSION)
#    define NULL (0L)
#  else
#    define NULL ((void*) 0)
#  endif /* G_CXX_STD_CHECK_VERSION (11) */

See https://gitlab.gnome.org/GNOME/glib/-/blob/main/glib/gmacros.h?ref_type=heads#L914-922

And ASTContext::isSentinelNullExpr() returns false for a non-pointer typed 0.

Could you check what your glib version defines NULL as?

Febbe commented 1 year ago

Could you check what your glib version defines NULL as?

~~Glib does not define it, since it is defined by libc: In LLVM/lib/clang/17/include/stddef.h it is defined as 0:~~ ~~

#if defined(__need_NULL)
#undef NULL
#ifdef __cplusplus
#  if !defined(__MINGW32__) && !defined(_MSC_VER)
#    define NULL __null
#  else
#    define NULL 0
#  endif
#else
#  define NULL ((void*)0)
#endif
#ifdef __cplusplus
#if defined(_MSC_EXTENSIONS) && defined(_NATIVE_NULLPTR_SUPPORTED)
namespace std { typedef decltype(nullptr) nullptr_t; }
using ::std::nullptr_t;
#endif
#endif
#undef __need_NULL
#endif /* defined(__need_NULL) */

~~ clangd shows me that, but it should be defined in a MSVC STL header.

Febbe commented 1 year ago

In corecrt.h from C:\Program Files (x86)\Windows Kits\10\Include\10.0.22000.0\ucrt

#ifndef NULL
    #ifdef __cplusplus
        #define NULL 0
    #else
        #define NULL ((void *)0)
    #endif
#endif

It is also defined as 0 literal.

Also in C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Tools\MSVC\14.37.32822\include\vcruntime.h:

#ifndef NULL
    #ifdef __cplusplus
        #define NULL 0
    #else
        #define NULL ((void *)0)
    #endif
#endif

This check should not warn, when the varargs have a 0/0L literal as sentinel.