From 6685e56ceddf7c88eb3c39e838a8164941aade05 Mon Sep 17 00:00:00 2001 From: Erich Keane Date: Tue, 4 Oct 2022 12:41:43 -0700 Subject: [PATCH] Disallow dereferencing of void* in C++. as Discussed: https://discourse.llvm.org/t/rfc-can-we-stop-the-extension-to-allow-dereferencing-void-in-c/65708 There is no good reason to allow this when the other compilers all reject this, and it messes with SFINAE/constraint checking. Differential Revision: https://reviews.llvm.org/D135287 --- clang/docs/ReleaseNotes.rst | 12 ++++++++++++ clang/include/clang/Basic/DiagnosticGroups.td | 1 + clang/include/clang/Basic/DiagnosticSemaKinds.td | 7 +++++-- clang/lib/Sema/SemaExpr.cpp | 7 +++++-- .../temp.class/temp.mem.func/p1inst.cpp | 2 +- clang/test/SemaCXX/disallow_void_deref.cpp | 16 ++++++++++++++++ clang/test/SemaCXX/reinterpret-cast.cpp | 10 +++++----- 7 files changed, 45 insertions(+), 10 deletions(-) create mode 100644 clang/test/SemaCXX/disallow_void_deref.cpp diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 513a5eda8080..16e6522a4427 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -117,6 +117,18 @@ code bases. These errors also match MSVC's behavior. +- Clang now diagnoses indirection of ``void *`` in C++ mode as a warning which + defaults to an error. This is compatible with ISO C++, GCC, ICC, and MSVC. This + is also now a SFINAE error so constraint checking and SFINAE checking can be + compatible with other compilers. It is expected that this will be upgraded to + an error-only diagnostic in the next Clang release. + + .. code-block:: c++ + + void func(void *p) { + *p; // Now diagnosed as a warning-as-error. + } + What's New in Clang |release|? ============================== Some of the major new features and improvements to Clang are listed diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index ff88c8acec4c..cddb127cae58 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -961,6 +961,7 @@ def PointerToEnumCast : DiagGroup<"pointer-to-enum-cast", [VoidPointerToEnumCast]>; def PointerToIntCast : DiagGroup<"pointer-to-int-cast", [PointerToEnumCast, VoidPointerToIntCast]>; +def VoidPointerDeref : DiagGroup<"void-ptr-dereference">; def FUseLdPath : DiagGroup<"fuse-ld-path">; diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 8a721d45e78f..d6fbaed126d6 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -6936,8 +6936,11 @@ def err_typecheck_unary_expr : Error< def err_typecheck_indirection_requires_pointer : Error< "indirection requires pointer operand (%0 invalid)">; def ext_typecheck_indirection_through_void_pointer : ExtWarn< - "ISO %select{C|C++}0 does not allow indirection on operand of type %1">, - InGroup>; + "ISO C does not allow indirection on operand of type %0">, + InGroup; +def ext_typecheck_indirection_through_void_pointer_cpp + : ExtWarn<"ISO C++ does not allow indirection on operand of type %0">, + InGroup, DefaultError, SFINAEFailure; def warn_indirection_through_null : Warning< "indirection of non-volatile null pointer will be deleted, not trap">, InGroup; diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index 6d28a4495231..474f86cffd16 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -14536,9 +14536,12 @@ static QualType CheckIndirectionOperand(Sema &S, Expr *Op, ExprValueKind &VK, // [...] the expression to which [the unary * operator] is applied shall // be a pointer to an object type, or a pointer to a function type LangOptions LO = S.getLangOpts(); - if (LO.CPlusPlus || (!(LO.C99 && IsAfterAmp) && !S.isUnevaluatedContext())) + if (LO.CPlusPlus) + S.Diag(OpLoc, diag::ext_typecheck_indirection_through_void_pointer_cpp) + << OpTy << Op->getSourceRange(); + else if (!(LO.C99 && IsAfterAmp) && !S.isUnevaluatedContext()) S.Diag(OpLoc, diag::ext_typecheck_indirection_through_void_pointer) - << LO.CPlusPlus << OpTy << Op->getSourceRange(); + << OpTy << Op->getSourceRange(); } // Dereferences are usually l-values... diff --git a/clang/test/CXX/temp/temp.decls/temp.class/temp.mem.func/p1inst.cpp b/clang/test/CXX/temp/temp.decls/temp.class/temp.mem.func/p1inst.cpp index eb11e3375e5f..6d591457ae14 100644 --- a/clang/test/CXX/temp/temp.decls/temp.class/temp.mem.func/p1inst.cpp +++ b/clang/test/CXX/temp/temp.decls/temp.class/temp.mem.func/p1inst.cpp @@ -8,7 +8,7 @@ struct X0 { template void X0::f(T *t, const U &u) { - *t = u; // expected-warning{{indirection on operand of type 'void *'}} expected-error{{not assignable}} + *t = u; // expected-error{{indirection on operand of type 'void *'}} expected-error{{not assignable}} } void test_f(X0 xfi, X0 xvi, float *fp, void *vp, int i) { diff --git a/clang/test/SemaCXX/disallow_void_deref.cpp b/clang/test/SemaCXX/disallow_void_deref.cpp new file mode 100644 index 000000000000..2981e709525b --- /dev/null +++ b/clang/test/SemaCXX/disallow_void_deref.cpp @@ -0,0 +1,16 @@ +// RUN: %clang_cc1 -fsyntax-only -verify=enabled,sfinae -std=c++20 %s +// RUN: %clang_cc1 -fsyntax-only -verify=sfinae -std=c++20 -Wno-void-ptr-dereference %s + +void f(void* p) { + (void)*p; // enabled-error{{ISO C++ does not allow indirection on operand of type 'void *'}} +} + +template +concept deref = requires (T& t) { + { *t }; // #FAILED_REQ +}; + +static_assert(deref); +// sfinae-error@-1{{static assertion failed}} +// sfinae-note@-2{{because 'void *' does not satisfy 'deref'}} +// sfinae-note@#FAILED_REQ{{because '*t' would be invalid: ISO C++ does not allow indirection on operand of type 'void *'}} diff --git a/clang/test/SemaCXX/reinterpret-cast.cpp b/clang/test/SemaCXX/reinterpret-cast.cpp index 1b84df12129c..ee856485272b 100644 --- a/clang/test/SemaCXX/reinterpret-cast.cpp +++ b/clang/test/SemaCXX/reinterpret-cast.cpp @@ -214,11 +214,11 @@ void dereference_reinterpret_cast() { (void)*reinterpret_cast(v_ptr); // Casting to void pointer - (void)*reinterpret_cast(&a); // expected-warning {{ISO C++ does not allow}} - (void)*reinterpret_cast(&b); // expected-warning {{ISO C++ does not allow}} - (void)*reinterpret_cast(&l); // expected-warning {{ISO C++ does not allow}} - (void)*reinterpret_cast(&d); // expected-warning {{ISO C++ does not allow}} - (void)*reinterpret_cast(&f); // expected-warning {{ISO C++ does not allow}} + (void)*reinterpret_cast(&a); // expected-error {{ISO C++ does not allow}} + (void)*reinterpret_cast(&b); // expected-error {{ISO C++ does not allow}} + (void)*reinterpret_cast(&l); // expected-error {{ISO C++ does not allow}} + (void)*reinterpret_cast(&d); // expected-error {{ISO C++ does not allow}} + (void)*reinterpret_cast(&f); // expected-error {{ISO C++ does not allow}} } void reinterpret_cast_allowlist () {