[PATCH] D150875: Make dereferencing a void* a hard-error instead of warn-as-error

[Erich Keane via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGfca4e2add0f8: Make dereferencing a void* a hard-error 
instead of warn-as-error (authored by erichkeane).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150875/new/

https://reviews.llvm.org/D150875

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaExpr.cpp
  clang/test/CXX/temp/temp.decls/temp.class/temp.mem.func/p1inst.cpp
  clang/test/SemaCXX/decl-expr-ambiguity.cpp
  clang/test/SemaCXX/disallow_void_deref.cpp
  clang/test/SemaCXX/reinterpret-cast.cpp

Index: clang/test/SemaCXX/reinterpret-cast.cpp
===
--- clang/test/SemaCXX/reinterpret-cast.cpp
+++ clang/test/SemaCXX/reinterpret-cast.cpp
@@ -214,11 +214,11 @@
   (void)*reinterpret_cast(v_ptr);
 
   // Casting to void pointer
-  (void)*reinterpret_cast(); // expected-error {{ISO C++ does not allow}}
-  (void)*reinterpret_cast(); // expected-error {{ISO C++ does not allow}}
-  (void)*reinterpret_cast(); // expected-error {{ISO C++ does not allow}}
-  (void)*reinterpret_cast(); // expected-error {{ISO C++ does not allow}}
-  (void)*reinterpret_cast(); // expected-error {{ISO C++ does not allow}}
+  (void)*reinterpret_cast(); // expected-error {{indirection not permitted on operand of type 'void *'}}
+  (void)*reinterpret_cast(); // expected-error {{indirection not permitted on operand of type 'void *'}}
+  (void)*reinterpret_cast(); // expected-error {{indirection not permitted on operand of type 'void *'}}
+  (void)*reinterpret_cast(); // expected-error {{indirection not permitted on operand of type 'void *'}}
+  (void)*reinterpret_cast(); // expected-error {{indirection not permitted on operand of type 'void *'}}
 }
 
 void reinterpret_cast_allowlist () {
Index: clang/test/SemaCXX/disallow_void_deref.cpp
===
--- clang/test/SemaCXX/disallow_void_deref.cpp
+++ clang/test/SemaCXX/disallow_void_deref.cpp
@@ -1,8 +1,7 @@
-// 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
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++20 %s
 
 void f(void* p) {
-  (void)*p; // enabled-error{{ISO C++ does not allow indirection on operand of type 'void *'}}
+  (void)*p; // expected-error{{indirection not permitted on operand of type 'void *'}}
 }
 
 template
@@ -11,6 +10,6 @@
 };
 
 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 *'}}
+// expected-error@-1{{static assertion failed}}
+// expected-note@-2{{because 'void *' does not satisfy 'deref'}}
+// expected-note@#FAILED_REQ{{because '*t' would be invalid: indirection not permitted on operand of type 'void *'}}
Index: clang/test/SemaCXX/decl-expr-ambiguity.cpp
===
--- clang/test/SemaCXX/decl-expr-ambiguity.cpp
+++ clang/test/SemaCXX/decl-expr-ambiguity.cpp
@@ -35,7 +35,7 @@
   extern T f3();
   __typeof(*T()) f4(); // expected-warning {{empty parentheses interpreted as a function declaration}} expected-note {{replace parentheses with an initializer}}
   typedef void *V;
-  __typeof(*V()) f5(); // expected-error {{ISO C++ does not allow indirection on operand of type 'V' (aka 'void *')}}
+  __typeof(*V()) f5(); // expected-error {{indirection not permitted on operand of type 'V' (aka 'void *')}}
   T multi1,
 multi2(); // expected-warning {{empty parentheses interpreted as a function declaration}} expected-note {{replace parentheses with an initializer}}
   T(d)[5]; // expected-error {{redefinition of 'd'}}
Index: clang/test/CXX/temp/temp.decls/temp.class/temp.mem.func/p1inst.cpp
===
--- clang/test/CXX/temp/temp.decls/temp.class/temp.mem.func/p1inst.cpp
+++ clang/test/CXX/temp/temp.decls/temp.class/temp.mem.func/p1inst.cpp
@@ -8,7 +8,7 @@
 
 template
 void X0::f(T *t, const U ) {
-  *t = u; // expected-error{{indirection on operand of type 'void *'}} expected-error{{not assignable}}
+  *t = u; // expected-error{{indirection not permitted on operand of type 'void *'}} expected-error{{not assignable}}
 }
 
 void test_f(X0 xfi, X0 xvi, float *fp, void *vp, int i) {
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -14957,7 +14957,7 @@
 //   be a pointer to an object type, or a pointer to a function type
 LangOptions LO = S.getLangOpts();
 if (LO.CPlusPlus)
-  S.Diag(OpLoc, 

[PATCH] D150875: Make dereferencing a void* a hard-error instead of warn-as-error

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

Fantastic! Presuming precommit CI comes back green this time, LGTM.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150875/new/

https://reviews.llvm.org/D150875

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D150875: Make dereferencing a void* a hard-error instead of warn-as-error

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

I did the llvm-test-suite build, and have a handful of linker issues (because 
I've not built libclang, and don't have some newer GCC library for some float16 
commands), but everythign builds at least.  If necessary, I can try to make 
sure it all builds, but I hope/suspect this is sufficient.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150875/new/

https://reviews.llvm.org/D150875

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D150875: Make dereferencing a void* a hard-error instead of warn-as-error

[Erich Keane via Phabricator via cfe-commits]

erichkeane updated this revision to Diff 524667.
erichkeane added a comment.

Fix tests + Aaron's formatting request.  Guess I forgot to upload this last 
night!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150875/new/

https://reviews.llvm.org/D150875

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaExpr.cpp
  clang/test/CXX/temp/temp.decls/temp.class/temp.mem.func/p1inst.cpp
  clang/test/SemaCXX/decl-expr-ambiguity.cpp
  clang/test/SemaCXX/disallow_void_deref.cpp
  clang/test/SemaCXX/reinterpret-cast.cpp

Index: clang/test/SemaCXX/reinterpret-cast.cpp
===
--- clang/test/SemaCXX/reinterpret-cast.cpp
+++ clang/test/SemaCXX/reinterpret-cast.cpp
@@ -214,11 +214,11 @@
   (void)*reinterpret_cast(v_ptr);
 
   // Casting to void pointer
-  (void)*reinterpret_cast(); // expected-error {{ISO C++ does not allow}}
-  (void)*reinterpret_cast(); // expected-error {{ISO C++ does not allow}}
-  (void)*reinterpret_cast(); // expected-error {{ISO C++ does not allow}}
-  (void)*reinterpret_cast(); // expected-error {{ISO C++ does not allow}}
-  (void)*reinterpret_cast(); // expected-error {{ISO C++ does not allow}}
+  (void)*reinterpret_cast(); // expected-error {{indirection not permitted on operand of type 'void *'}}
+  (void)*reinterpret_cast(); // expected-error {{indirection not permitted on operand of type 'void *'}}
+  (void)*reinterpret_cast(); // expected-error {{indirection not permitted on operand of type 'void *'}}
+  (void)*reinterpret_cast(); // expected-error {{indirection not permitted on operand of type 'void *'}}
+  (void)*reinterpret_cast(); // expected-error {{indirection not permitted on operand of type 'void *'}}
 }
 
 void reinterpret_cast_allowlist () {
Index: clang/test/SemaCXX/disallow_void_deref.cpp
===
--- clang/test/SemaCXX/disallow_void_deref.cpp
+++ clang/test/SemaCXX/disallow_void_deref.cpp
@@ -1,8 +1,7 @@
-// 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
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++20 %s
 
 void f(void* p) {
-  (void)*p; // enabled-error{{ISO C++ does not allow indirection on operand of type 'void *'}}
+  (void)*p; // expected-error{{indirection not permitted on operand of type 'void *'}}
 }
 
 template
@@ -11,6 +10,6 @@
 };
 
 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 *'}}
+// expected-error@-1{{static assertion failed}}
+// expected-note@-2{{because 'void *' does not satisfy 'deref'}}
+// expected-note@#FAILED_REQ{{because '*t' would be invalid: indirection not permitted on operand of type 'void *'}}
Index: clang/test/SemaCXX/decl-expr-ambiguity.cpp
===
--- clang/test/SemaCXX/decl-expr-ambiguity.cpp
+++ clang/test/SemaCXX/decl-expr-ambiguity.cpp
@@ -35,7 +35,7 @@
   extern T f3();
   __typeof(*T()) f4(); // expected-warning {{empty parentheses interpreted as a function declaration}} expected-note {{replace parentheses with an initializer}}
   typedef void *V;
-  __typeof(*V()) f5(); // expected-error {{ISO C++ does not allow indirection on operand of type 'V' (aka 'void *')}}
+  __typeof(*V()) f5(); // expected-error {{indirection not permitted on operand of type 'V' (aka 'void *')}}
   T multi1,
 multi2(); // expected-warning {{empty parentheses interpreted as a function declaration}} expected-note {{replace parentheses with an initializer}}
   T(d)[5]; // expected-error {{redefinition of 'd'}}
Index: clang/test/CXX/temp/temp.decls/temp.class/temp.mem.func/p1inst.cpp
===
--- clang/test/CXX/temp/temp.decls/temp.class/temp.mem.func/p1inst.cpp
+++ clang/test/CXX/temp/temp.decls/temp.class/temp.mem.func/p1inst.cpp
@@ -8,7 +8,7 @@
 
 template
 void X0::f(T *t, const U ) {
-  *t = u; // expected-error{{indirection on operand of type 'void *'}} expected-error{{not assignable}}
+  *t = u; // expected-error{{indirection not permitted on operand of type 'void *'}} expected-error{{not assignable}}
 }
 
 void test_f(X0 xfi, X0 xvi, float *fp, void *vp, int i) {
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -14957,7 +14957,7 @@
 //   be a pointer to an object type, or a pointer to a function type
 LangOptions LO = S.getLangOpts();
 if (LO.CPlusPlus)
-  S.Diag(OpLoc, diag::ext_typecheck_indirection_through_void_pointer_cpp)
+  S.Diag(OpLoc, diag::err_typecheck_indirection_through_void_pointer_cpp)

[PATCH] D150875: Make dereferencing a void* a hard-error instead of warn-as-error

[Erich Keane via Phabricator via cfe-commits]

erichkeane marked an inline comment as done.
erichkeane added a comment.

In D150875#4362434 , @aaron.ballman 
wrote:

> Precommit CI found various failures that need to be addressed. Also, have you 
> tested to see what breaks in llvm-test-suite?

Woops!  I must have forgotten to re-make my diff before submitting.  I fixed 
those in the patch earlier.

I have not tested this against the llvm-test-suite as I'm not thrilled having 
to figure out how to build that again :)  But I guess I should make a companion 
patch ahead of time.  Last time however I don't recall us needing to add the 
flag for the warning however.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150875/new/

https://reviews.llvm.org/D150875

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D150875: Make dereferencing a void* a hard-error instead of warn-as-error

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Precommit CI found various failures that need to be addressed. Also, have you 
tested to see what breaks in llvm-test-suite?




Comment at: clang/docs/ReleaseNotes.rst:58
   Clang will only search for std::coroutine_traits for coroutines then.
+- Clang no longer allows dereferencing of a ``void*`` as an extension. Clang 16
+  converted this to a default-error as ``-Wvoid-ptr-dereference``, as well as a




CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150875/new/

https://reviews.llvm.org/D150875

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D150875: Make dereferencing a void* a hard-error instead of warn-as-error

[Erich Keane via Phabricator via cfe-commits]

erichkeane updated this revision to Diff 523748.
erichkeane added a comment.

Update release note per @MaskRay , change error message per @rsmith


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150875/new/

https://reviews.llvm.org/D150875

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaExpr.cpp
  clang/test/SemaCXX/disallow_void_deref.cpp


Index: clang/test/SemaCXX/disallow_void_deref.cpp
===
--- clang/test/SemaCXX/disallow_void_deref.cpp
+++ clang/test/SemaCXX/disallow_void_deref.cpp
@@ -1,8 +1,7 @@
-// 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
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++20 %s
 
 void f(void* p) {
-  (void)*p; // enabled-error{{ISO C++ does not allow indirection on operand of 
type 'void *'}}
+  (void)*p; // expected-error{{ISO C++ does not allow indirection on operand 
of type 'void *'}}
 }
 
 template
@@ -11,6 +10,6 @@
 };
 
 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 *'}}
+// expected-error@-1{{static assertion failed}}
+// expected-note@-2{{because 'void *' does not satisfy 'deref'}}
+// expected-note@#FAILED_REQ{{because '*t' would be invalid: ISO C++ does not 
allow indirection on operand of type 'void *'}}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -14957,7 +14957,7 @@
 //   be a pointer to an object type, or a pointer to a function type
 LangOptions LO = S.getLangOpts();
 if (LO.CPlusPlus)
-  S.Diag(OpLoc, diag::ext_typecheck_indirection_through_void_pointer_cpp)
+  S.Diag(OpLoc, diag::err_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)
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6981,9 +6981,8 @@
 def ext_typecheck_indirection_through_void_pointer : ExtWarn<
   "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 err_typecheck_indirection_through_void_pointer_cpp
+: Error<"indirection not permitted on operand of type %0">;
 def warn_indirection_through_null : Warning<
   "indirection of non-volatile null pointer will be deleted, not trap">,
   InGroup;
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -55,6 +55,10 @@
 -
 - Clang won't search for coroutine_traits in std::experimental namespace any 
more.
   Clang will only search for std::coroutine_traits for coroutines then.
+- Clang no longer allows dereferencing of a ``void*`` as an extension. Clang 16
+  converted this to a default-error as ``-Wvoid-ptr-dereference``, as well as a
+  SFINAE error. This flag is still valid however, as it disables the equivalent
+  warning in C.
 
 ABI Changes in This Version
 ---


Index: clang/test/SemaCXX/disallow_void_deref.cpp
===
--- clang/test/SemaCXX/disallow_void_deref.cpp
+++ clang/test/SemaCXX/disallow_void_deref.cpp
@@ -1,8 +1,7 @@
-// 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
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++20 %s
 
 void f(void* p) {
-  (void)*p; // enabled-error{{ISO C++ does not allow indirection on operand of type 'void *'}}
+  (void)*p; // expected-error{{ISO C++ does not allow indirection on operand of type 'void *'}}
 }
 
 template
@@ -11,6 +10,6 @@
 };
 
 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 *'}}
+// expected-error@-1{{static assertion failed}}
+// expected-note@-2{{because 'void *' does not satisfy 'deref'}}
+// expected-note@#FAILED_REQ{{because '*t' would be invalid: ISO C++ does not allow indirection on operand of 

[PATCH] D150875: Make dereferencing a void* a hard-error instead of warn-as-error

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:59
+- Clang no longer allows dereferencing of a ``void*`` as an extension. Clang 16
+  converted this to a warning-as-default-error as well as a SFINAE error.
 

MaskRay wrote:
> Mention `-Wvoid-ptr-dereference`?
> 
> Clang 16 converted this to a default-error `-Wvoid-ptr-dereference` as well 
> as a SFINAE error.
> 
> Does this patch intentionally keep `-Wvoid-ptr-dereference` (which is a no-op 
> now)?
Good suggestions!  

Yes, Clang keeps `Wvoid-ptr-dereference` because it is still a valid flag for C.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150875/new/

https://reviews.llvm.org/D150875

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D150875: Make dereferencing a void* a hard-error instead of warn-as-error

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:59
+- Clang no longer allows dereferencing of a ``void*`` as an extension. Clang 16
+  converted this to a warning-as-default-error as well as a SFINAE error.
 

Mention `-Wvoid-ptr-dereference`?

Clang 16 converted this to a default-error `-Wvoid-ptr-dereference` as well as 
a SFINAE error.

Does this patch intentionally keep `-Wvoid-ptr-dereference` (which is a no-op 
now)?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150875/new/

https://reviews.llvm.org/D150875

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D150875: Make dereferencing a void* a hard-error instead of warn-as-error

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D150875#4353658 , @erichkeane 
wrote:

> Ok, so I was curious why this was an extension.  This is the original patch: 
> https://github.com/llvm/llvm-project/commit/80877c228d019e9cdca6295f3197867cc86e1720
>  where @rsmith mentions there is no good reason for this limitation, then 
> mentions a core issue having been filed.  I found CWG1923 
> (https://open-std.org/JTC1/SC22/WG21/docs/cwg_closed.html#1923), which is 
> said issue.  The resolution is "Needs an EWG Paper", which seems to be the 
> end of the line, as I see no such paper?

Because the Core issue ended up in extension territory and no extension paper 
came out, and because we don't know there's evidence of a significant user 
community, I think it's reasonable to remove this extension (it doesn't match 
our usual requirements for adding extensions). However, if @rsmith is planning 
to write such a paper (or if there is a paper and we just haven't spotted it), 
then that's a different story.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150875/new/

https://reviews.llvm.org/D150875

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D150875: Make dereferencing a void* a hard-error instead of warn-as-error

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Ok, so I was curious why this was an extension.  This is the original patch: 
https://github.com/llvm/llvm-project/commit/80877c228d019e9cdca6295f3197867cc86e1720
 where @rsmith mentions there is no good reason for this limitation, then 
mentions a core issue having been filed.  I found CWG1923 
(https://open-std.org/JTC1/SC22/WG21/docs/cwg_closed.html#1923), which is said 
issue.  The resolution is "Needs an EWG Paper", which seems to be the end of 
the line, as I see no such paper?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150875/new/

https://reviews.llvm.org/D150875

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D150875: Make dereferencing a void* a hard-error instead of warn-as-error

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.



In D150875#4353535 , @rsmith wrote:

> In D150875#4353384 , @erichkeane 
> wrote:
>
>> We are the only of the major compilers with this "extension" (I hesitate to 
>> call it that, as I'm not sure this FITS in the 'extension's permitted by 
>> standard)
>
> I'm not objecting to removing this extension, but... do you have reason to 
> doubt that it's conforming, or just a lack of confidence that it is? (If the 
> `SFINAEFailure` change wasn't enough, then it's not clear to me why this 
> change would be. We use a `SFINAEFailure` diagnostic for other extensions, 
> and if that's not sufficient for conformance then we probably have a lot of 
> conformance gaps of this kind.)

No, just lack of confidence.  I guess if it is supposed to be ill-formed, we 
can permit it (except in SFINAE?), right?  The intent here is mostly just for 
'cleanup' as we see it.




Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:6985
+def err_typecheck_indirection_through_void_pointer_cpp
+: Error<"ISO C++ does not allow indirection on operand of type %0">;
 def warn_indirection_through_null : Warning<

rsmith wrote:
> We normally only use this "ISO C++ does not allow" phrasing for extensions 
> (with the implication being that ISO C++ doesn't allow it, but Clang does). 
> Can you rephrase the diagnostic too, to remove those unnecessary words?
I definitely can.  Though, I wonder if we can just use the 
err_typecheck_indirection_requires_pointer above instead and remove this 
diagnostic?  We might need to change that to `indirection requires non-void 
pointer operand (%0 invalid)`.  WDYT?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150875/new/

https://reviews.llvm.org/D150875

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D150875: Make dereferencing a void* a hard-error instead of warn-as-error

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

In D150875#4353384 , @erichkeane 
wrote:

> We are the only of the major compilers with this "extension" (I hesitate to 
> call it that, as I'm not sure this FITS in the 'extension's permitted by 
> standard)

I'm not objecting to removing this extension, but... do you have reason to 
doubt that it's conforming, or just a lack of confidence that it is? (If the 
`SFINAEFailure` change wasn't enough, then it's not clear to me why this change 
would be. We use a `SFINAEFailure` diagnostic for other extensions, and if 
that's not sufficient for conformance then we probably have a lot of 
conformance gaps of this kind.)




Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:6985
+def err_typecheck_indirection_through_void_pointer_cpp
+: Error<"ISO C++ does not allow indirection on operand of type %0">;
 def warn_indirection_through_null : Warning<

We normally only use this "ISO C++ does not allow" phrasing for extensions 
(with the implication being that ISO C++ doesn't allow it, but Clang does). Can 
you rephrase the diagnostic too, to remove those unnecessary words?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150875/new/

https://reviews.llvm.org/D150875

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D150875: Make dereferencing a void* a hard-error instead of warn-as-error

[Jessica Clarke via Phabricator via cfe-commits]

jrtc27 added a comment.

In D150875#4353484 , @erichkeane 
wrote:

> In D150875#4353467 , @jrtc27 wrote:
>
>> We heavily rely on this extension in CheriBSD via `__typeof__((*(p))) * 
>> __capability` as we want to be able to take any pointer, including to an 
>> array or function that needs to undergo decay to be an actual pointer, and 
>> turn it into a `__capability`-qualified one. Presumably you're saying we 
>> should instead use `__typeof__((0, (p))) __capability` as an uglier 
>> alternative way to force decay?
>
> Do you do that in C++, or just C?  Note that this does NOT change the 
> behavior in C.  In C++ I'd probably just suggest using `std::decay`.

In practice just C (kernel-only macro), though ideally the macro would still 
work in C++ (at least on CheriBSD; C++-based OSes will differ, but can of 
course do their own thing).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150875/new/

https://reviews.llvm.org/D150875

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D150875: Make dereferencing a void* a hard-error instead of warn-as-error

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D150875#4353467 , @jrtc27 wrote:

> We heavily rely on this extension in CheriBSD via `__typeof__((*(p))) * 
> __capability` as we want to be able to take any pointer, including to an 
> array or function that needs to undergo decay to be an actual pointer, and 
> turn it into a `__capability`-qualified one. Presumably you're saying we 
> should instead use `__typeof__((0, (p))) __capability` as an uglier 
> alternative way to force decay?

Do you do that in C++, or just C?  Note that this does NOT change the behavior 
in C.  In C++ I'd probably just suggest using `std::decay`.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150875/new/

https://reviews.llvm.org/D150875

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D150875: Make dereferencing a void* a hard-error instead of warn-as-error

[Jessica Clarke via Phabricator via cfe-commits]

jrtc27 added a comment.

We heavily rely on this extension in CheriBSD via `__typeof__((*(p))) * 
__capability` as we want to be able to take any pointer, including to an array 
or function that needs to undergo decay to be an actual pointer, and turn it 
into a `__capability`-qualified one. Presumably you're saying we should instead 
use `__typeof((0, (p)))__ __capability` as an uglier alternative way to force 
decay?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150875/new/

https://reviews.llvm.org/D150875

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D150875: Make dereferencing a void* a hard-error instead of warn-as-error

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D150875#4353435 , @rnk wrote:

> In D150875#4353398 , @aaron.ballman 
> wrote:
>
>> Note, there was an RFC: 
>> https://discourse.llvm.org/t/rfc-can-we-stop-the-extension-to-allow-dereferencing-void-in-c/65708
>
> Thanks, that addresses my concern. Please link to it from the commit message, 
> it will help anyone bisecting the behavior change have context.

Will do!  I've put it in my local git, and will put it in phab as well.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150875/new/

https://reviews.llvm.org/D150875

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D150875: Make dereferencing a void* a hard-error instead of warn-as-error

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

In D150875#4353398 , @aaron.ballman 
wrote:

> Note, there was an RFC: 
> https://discourse.llvm.org/t/rfc-can-we-stop-the-extension-to-allow-dereferencing-void-in-c/65708

Thanks, that addresses my concern. Please link to it from the commit message, 
it will help anyone bisecting the behavior change have context.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150875/new/

https://reviews.llvm.org/D150875

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D150875: Make dereferencing a void* a hard-error instead of warn-as-error

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D150875#4353358 , @rnk wrote:

> Can you expand on the motivation for removing this extension? This doesn't 
> seem to save a lot of code complexity, and it increases migration costs for 
> our users. I'd like to have some motivation for putting them through the 
> trouble of migrating.

Note, there was an RFC: 
https://discourse.llvm.org/t/rfc-can-we-stop-the-extension-to-allow-dereferencing-void-in-c/65708


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150875/new/

https://reviews.llvm.org/D150875

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D150875: Make dereferencing a void* a hard-error instead of warn-as-error

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D150875#4353358 , @rnk wrote:

> Can you expand on the motivation for removing this extension? This doesn't 
> seem to save a lot of code complexity, and it increases migration costs for 
> our users. I'd like to have some motivation for putting them through the 
> trouble of migrating.

The motivation is essentially the same motivation for making this a SFINAE 
error: We are the only of the major compilers with this "extension" (I hesitate 
to call it that, as I'm not sure this FITS in the 'extension's permitted by 
standard), and we gave warning last release in the release notes that we'd be 
doing this.  You're right there is no additional code complexity, but there IS 
standards compliance issues, as well as "do what everyone else does".

I WILL say the SFINAE change last release was more significant as a motivation, 
since it had the ability to change the overload set.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150875/new/

https://reviews.llvm.org/D150875

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D150875: Make dereferencing a void* a hard-error instead of warn-as-error

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D150875#4353209 , @rupprecht wrote:

> I found one use here: 
> https://github.com/ericniebler/range-v3/blob/48dc2eb666c07e6afc8ec5edf7db9a5c6cde7a56/include/range/v3/functional/invoke.hpp#L51

Huh, that is an interesting use!  I notice that he enables that for both GCC 
AND Clang, but probably doesn't notice because he disables Wpragma as well.  So 
this change would change the legality of his program (potentially) for Clang 
(though, we ALREADY would have broken any SFINAE/concepts uses because of the 
SFINAE change in 16), but only to be what GCC gives him?  I think we can be ok 
with that?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150875/new/

https://reviews.llvm.org/D150875

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D150875: Make dereferencing a void* a hard-error instead of warn-as-error

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

Can you expand on the motivation for removing this extension? This doesn't seem 
to save a lot of code complexity, and it increases migration costs for our 
users. I'd like to have some motivation for putting them through the trouble of 
migrating.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150875/new/

https://reviews.llvm.org/D150875

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D150875: Make dereferencing a void* a hard-error instead of warn-as-error

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added a comment.

LGTM


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150875/new/

https://reviews.llvm.org/D150875

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D150875: Make dereferencing a void* a hard-error instead of warn-as-error

[Jordan Rupprecht via Phabricator via cfe-commits]

rupprecht added a comment.

I found one use here: 
https://github.com/ericniebler/range-v3/blob/48dc2eb666c07e6afc8ec5edf7db9a5c6cde7a56/include/range/v3/functional/invoke.hpp#L51


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150875/new/

https://reviews.llvm.org/D150875

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D150875: Make dereferencing a void* a hard-error instead of warn-as-error

[Erich Keane via Phabricator via cfe-commits]

erichkeane created this revision.
erichkeane added reviewers: clang-vendors, clang-language-wg.
Herald added a project: All.
erichkeane requested review of this revision.

Clang 16 changed to consider dereferencing a void* to be a
warning-as-error, plus made this an error in SFINAE contexts, since this
resulted in incorrect template instantiation.  When doing so, the Clang
16 documentation was updated to reflect that this was likely to change
again to a non-disablable error in the next version.

As there has been no response to changing from a warning to an error, I
believe this is a non-controversial change.

This patch changes this to be an Error, consistent with the standard and
other compilers.


https://reviews.llvm.org/D150875

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaExpr.cpp
  clang/test/SemaCXX/disallow_void_deref.cpp


Index: clang/test/SemaCXX/disallow_void_deref.cpp
===
--- clang/test/SemaCXX/disallow_void_deref.cpp
+++ clang/test/SemaCXX/disallow_void_deref.cpp
@@ -1,8 +1,7 @@
-// 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
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++20 %s
 
 void f(void* p) {
-  (void)*p; // enabled-error{{ISO C++ does not allow indirection on operand of 
type 'void *'}}
+  (void)*p; // expected-error{{ISO C++ does not allow indirection on operand 
of type 'void *'}}
 }
 
 template
@@ -11,6 +10,6 @@
 };
 
 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 *'}}
+// expected-error@-1{{static assertion failed}}
+// expected-note@-2{{because 'void *' does not satisfy 'deref'}}
+// expected-note@#FAILED_REQ{{because '*t' would be invalid: ISO C++ does not 
allow indirection on operand of type 'void *'}}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -14957,7 +14957,7 @@
 //   be a pointer to an object type, or a pointer to a function type
 LangOptions LO = S.getLangOpts();
 if (LO.CPlusPlus)
-  S.Diag(OpLoc, diag::ext_typecheck_indirection_through_void_pointer_cpp)
+  S.Diag(OpLoc, diag::err_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)
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6981,9 +6981,8 @@
 def ext_typecheck_indirection_through_void_pointer : ExtWarn<
   "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 err_typecheck_indirection_through_void_pointer_cpp
+: Error<"ISO C++ does not allow indirection on operand of type %0">;
 def warn_indirection_through_null : Warning<
   "indirection of non-volatile null pointer will be deleted, not trap">,
   InGroup;
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -55,6 +55,8 @@
 -
 - Clang won't search for coroutine_traits in std::experimental namespace any 
more.
   Clang will only search for std::coroutine_traits for coroutines then.
+- Clang no longer allows dereferencing of a ``void*`` as an extension. Clang 16
+  converted this to a warning-as-default-error as well as a SFINAE error.
 
 ABI Changes in This Version
 ---


Index: clang/test/SemaCXX/disallow_void_deref.cpp
===
--- clang/test/SemaCXX/disallow_void_deref.cpp
+++ clang/test/SemaCXX/disallow_void_deref.cpp
@@ -1,8 +1,7 @@
-// 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
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++20 %s
 
 void f(void* p) {
-  (void)*p; // enabled-error{{ISO C++ does not allow indirection on operand of type 'void *'}}
+  (void)*p; // expected-error{{ISO C++ does not allow indirection on operand of type 'void *'}}
 }
 
 template
@@ -11,6 +10,6 @@
 };
 
 static_assert(deref);
-// sfinae-error@-1{{static assertion failed}}
-//