[PATCH] D60139: [clang-tidy] Add bugprone-placement-new-target-type-mismatch check

[Dennis Luxen via Phabricator via cfe-commits]

DennisL updated this revision to Diff 194505.
DennisL marked an inline comment as done.
DennisL added a comment.

Changed the following

- addressed reviewer feedback
- fetch the placement parameter as written
- add further test cases as requested
- stylistic changes
- add nothrow parameter support
- ignore matches with unresolved template parameters
- add examples of bad and good code


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

https://reviews.llvm.org/D60139

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/PlacementNewTargetTypeMismatch.cpp
  clang-tidy/bugprone/PlacementNewTargetTypeMismatch.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-placement-new-target-type-mismatch.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-placement-new-target-type-mismatch.cpp

Index: test/clang-tidy/bugprone-placement-new-target-type-mismatch.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-placement-new-target-type-mismatch.cpp
@@ -0,0 +1,118 @@
+// RUN: %check_clang_tidy %s bugprone-placement-new-target-type-mismatch %t
+
+// definitions
+	
+namespace std {
+struct nothrow_t { explicit nothrow_t() = default; } nothrow;
+template T* addressof(T& arg) noexcept;
+template< class T > struct remove_reference  {typedef T type;};
+template< class T > struct remove_reference  {typedef T type;};
+template< class T > struct remove_reference {typedef T type;};
+template< class T >
+T&& forward( typename std::remove_reference::type& t ) noexcept;
+} // namespace std
+
+using size_type = unsigned long;
+void *operator new(size_type, void *);
+void *operator new[](size_type, void *);
+void* operator new(size_type size, const std::nothrow_t&) noexcept;
+void* operator new(size_type size, const std::nothrow_t&) noexcept;
+void* operator new[](size_type size, const std::nothrow_t&) noexcept;
+
+struct Foo {
+  int a;
+  int b;
+  int c;
+  int d;
+};
+
+template
+T& getT() {
+  static T f;
+  return f;
+}
+
+// instances emitting warnings
+
+void f1() {
+  struct Dummy {
+int a;
+int b;
+  };
+  int *ptr = new int;
+  new (ptr) Dummy;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: placement new parameter and allocated type mismatch [bugprone-placement-new-target-type-mismatch]
+}
+
+void f2() {
+  int * ptr = new int;
+  new (ptr) Foo;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: placement new parameter and allocated type mismatch [bugprone-placement-new-target-type-mismatch]
+}
+
+void f3() {
+  char *ptr = new char[17*sizeof(char)];
+  new (ptr) float[13];
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: placement new parameter and allocated type mismatch [bugprone-placement-new-target-type-mismatch]
+}
+
+void f4() {
+  new (std::addressof(getT())) Foo;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: placement new parameter and allocated type mismatch [bugprone-placement-new-target-type-mismatch]
+}
+
+void f5() {
+  char *ptr = new char[17*sizeof(char)];
+  new (ptr) float{13.f};
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: placement new parameter and allocated type mismatch [bugprone-placement-new-target-type-mismatch]
+}
+
+void f6() {
+  char array[17*sizeof(char)];
+  new (array) float{13.f};
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: placement new parameter and allocated type mismatch [bugprone-placement-new-target-type-mismatch]
+}
+
+// instances not emitting a warning
+
+void g1() {
+  Foo * ptr = new Foo;
+  new (ptr) Foo;
+}
+
+void g2() {
+  char *ptr = new char[17*sizeof(char)];
+  new ((float *)ptr) float{13.f};
+}
+
+void g3() {
+  char array[17*sizeof(char)];
+  new (array) char('A');
+}
+
+void g4() {
+  new ((void *)std::addressof(getT())) Foo;
+}
+
+union
+{
+  char * buffer;
+} Union;
+
+template 
+void g5(U &&... V) {
+  new (static_cast(Union.buffer)) T(std::forward(V)...);
+}
+
+template 
+void g6(U &&... V) {
+  new (std::nothrow) T(std::forward(V)...);
+}
+
+template  void g7(U &&... Us) {
+  new (static_cast(Union.buffer)) T(std::forward(Us)...);
+}
+
+template  void g8(U &&... Us) {
+  new (Union.buffer) T(std::forward(Us)...);
+}
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -55,6 +55,7 @@
bugprone-move-forwarding-reference
bugprone-multiple-statement-macro
bugprone-parent-virtual-call
+   bugprone-placement-new-target-type-mismatch
bugprone-sizeof-container
bugprone-sizeof-expression
bugprone-string-constructor
Index: docs/clang-tidy/checks/bugprone-placement-new-target-type-mismatch.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/bugprone-placement-new-target-type-mismatch.rst
@@ -0,0 +1,23 @@
+.. title:: clang-tidy - misc-placement-new-target-type-mismatch
+

[PATCH] D60139: [clang-tidy] Add bugprone-placement-new-target-type-mismatch check

[Dennis Luxen via Phabricator via cfe-commits]

DennisL marked 12 inline comments as done.
DennisL added a comment.

In D60139#1460233 , @JonasToth wrote:

> Hey Dennis,
>
> my 2cents on the check. I think it is very good to have! Did you check coding 
> guidelines if they say something to this issue? (e.g. cppcoreguidelines, 
> hicpp, cert) As we have modules for them it would be great to make aliases to 
> this check if they demand this to be checked.


Thanks for the great suggestions. Updated the diff according to the feedback. 
Also checked with cppcoreguidelines, hicpp as well as cert. Only cert has a 
related, yet different rule 

 stating that calls to placement new shall be provided with properly aligned 
pointers. I'd say this should be a distinct check. Happy to work on it after 
this one.




Comment at: clang-tidy/bugprone/PlacementNewTargetTypeMismatch.cpp:42
+
+  assert((Cast->getSubExpr()->getType()->isPointerType() ||
+ Cast->getSubExpr()->getType()->isArrayType()) &&

JonasToth wrote:
> Is this universally true? What about the nothrow-overload, would that 
> interfere?
Thanks, rewrote that check.


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

https://reviews.llvm.org/D60139



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

[PATCH] D60139: [clang-tidy] Add bugprone-placement-new-target-type-mismatch check

[Dennis Luxen via Phabricator via cfe-commits]

DennisL updated this revision to Diff 194283.
DennisL added a comment.

- handle array to ptr decay
- updated error msg
- additional tests for arry to ptr decay


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

https://reviews.llvm.org/D60139

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/PlacementNewTargetTypeMismatch.cpp
  clang-tidy/bugprone/PlacementNewTargetTypeMismatch.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-placement-new-target-type-mismatch.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-placement-new-target-type-mismatch.cpp

Index: test/clang-tidy/bugprone-placement-new-target-type-mismatch.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-placement-new-target-type-mismatch.cpp
@@ -0,0 +1,86 @@
+// RUN: %check_clang_tidy %s bugprone-placement-new-target-type-mismatch %t
+
+// definitions
+
+using size_type = unsigned long;
+void *operator new(size_type, void *);
+void *operator new[](size_type, void *);
+
+namespace std {
+template T* addressof(T& arg) noexcept;
+} // namespace std
+
+struct Foo {
+  int a;
+  int b;
+  int c;
+  int d;
+};
+
+template
+T& getT() {
+  static T f;
+  return f;
+}
+
+// instances emitting warnings
+
+void f1() {
+  struct Dummy {
+int a;
+int b;
+  };
+  int *ptr = new int;
+  new (ptr) Dummy;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: placement new parameter and allocated type mismatch [bugprone-placement-new-target-type-mismatch]
+}
+
+void f2() {
+  int * ptr = new int;
+  new (ptr) Foo;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: placement new parameter and allocated type mismatch [bugprone-placement-new-target-type-mismatch]
+}
+
+void f3() {
+  char *ptr = new char[17*sizeof(char)];
+  new (ptr) float[13];
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: placement new parameter and allocated type mismatch [bugprone-placement-new-target-type-mismatch]
+}
+
+void f4() {
+  new (std::addressof(getT())) Foo;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: placement new parameter and allocated type mismatch [bugprone-placement-new-target-type-mismatch]
+}
+
+void f5() {
+  char *ptr = new char[17*sizeof(char)];
+  new (ptr) float{13.f};
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: placement new parameter and allocated type mismatch [bugprone-placement-new-target-type-mismatch]
+}
+
+void f6() {
+  new ((void *)std::addressof(getT())) Foo;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: placement new parameter and allocated type mismatch [bugprone-placement-new-target-type-mismatch]
+}
+
+void f7() {
+  char array[17*sizeof(char)];
+  new (array) float{13.f};
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: placement new parameter and allocated type mismatch [bugprone-placement-new-target-type-mismatch]
+}
+// instances not emitting a warning
+
+void g1() {
+  Foo * ptr = new Foo;
+  new (ptr) Foo;
+}
+
+void g2() {
+  char *ptr = new char[17*sizeof(char)];
+  new ((float *)ptr) float{13.f};
+}
+
+void g3() {
+  char array[17*sizeof(char)];
+  new (array) char('A');
+}
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -55,6 +55,7 @@
bugprone-move-forwarding-reference
bugprone-multiple-statement-macro
bugprone-parent-virtual-call
+   bugprone-placement-new-target-type-mismatch
bugprone-sizeof-container
bugprone-sizeof-expression
bugprone-string-constructor
Index: docs/clang-tidy/checks/bugprone-placement-new-target-type-mismatch.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/bugprone-placement-new-target-type-mismatch.rst
@@ -0,0 +1,8 @@
+.. title:: clang-tidy - misc-placement-new-target-type-mismatch
+
+misc-placement-new-target-type-mismatch
+===
+
+Finds placement-new calls where the pointer type of the adress mismatches the
+type of the created value.
+
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -130,6 +130,12 @@
   ` now supports `OverrideSpelling`
   and `FinalSpelling` options.
 
+- New :doc:`bugprone-placement-new-target-type-mismatch
+  ` check.
+
+  Finds placement-new calls where the pointer type of the adress mismatches the
+  type of the created value.
+
 - New :doc:`openmp-exception-escape
   ` check.
 
Index: clang-tidy/bugprone/PlacementNewTargetTypeMismatch.h
===
--- /dev/null
+++ clang-tidy/bugprone/PlacementNewTargetTypeMismatch.h
@@ -0,0 +1,35 @@
+//===--- PlacementNewTargetTypeMismatch.h - clang-tidy --*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See 

[PATCH] D60139: [clang-tidy] Add bugprone-placement-new-target-type-mismatch check

[Dennis Luxen via Phabricator via cfe-commits]

DennisL updated this revision to Diff 194137.

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

https://reviews.llvm.org/D60139

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/PlacementNewTargetTypeMismatch.cpp
  clang-tidy/bugprone/PlacementNewTargetTypeMismatch.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-placement-new-target-type-mismatch.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-placement-new-target-type-mismatch.cpp

Index: test/clang-tidy/bugprone-placement-new-target-type-mismatch.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-placement-new-target-type-mismatch.cpp
@@ -0,0 +1,76 @@
+// RUN: %check_clang_tidy %s bugprone-placement-new-target-type-mismatch %t
+
+// definitions
+
+using size_type = unsigned long;
+void *operator new(size_type, void *);
+void *operator new[](size_type, void *);
+
+namespace std {
+template T* addressof(T& arg) noexcept;
+} // namespace std
+
+struct Foo {
+  int a;
+  int b;
+  int c;
+  int d;
+};
+
+template
+T& getT() {
+  static T f;
+  return f;
+}
+
+// instances emitting warnings
+
+void f1() {
+  struct Dummy {
+int a;
+int b;
+  };
+  int *ptr = new int;
+  new (ptr) Dummy;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: placement new of incompatible types [bugprone-placement-new-target-type-mismatch]
+}
+
+void f2() {
+  int * ptr = new int;
+  new (ptr) Foo;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: placement new of incompatible types [bugprone-placement-new-target-type-mismatch]
+}
+
+void f3() {
+  char *ptr = new char[17*sizeof(char)];
+  new (ptr) float[13];
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: placement new of incompatible types [bugprone-placement-new-target-type-mismatch]
+}
+
+void f4() {
+  new (std::addressof(getT())) Foo;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: placement new of incompatible types [bugprone-placement-new-target-type-mismatch]
+}
+
+void f5() {
+  char *ptr = new char[17*sizeof(char)];
+  new (ptr) float{13.f};
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: placement new of incompatible types [bugprone-placement-new-target-type-mismatch]
+}
+
+void f6() {
+  new ((void *)std::addressof(getT())) Foo;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: placement new of incompatible types [bugprone-placement-new-target-type-mismatch]
+}
+
+// instances not emitting a warning
+
+void f7() {
+  Foo * ptr = new Foo;
+  new (ptr) Foo;
+}
+
+void f8() {
+  char *ptr = new char[17*sizeof(char)];
+  new((float *)ptr) float{13.f};
+}
\ No newline at end of file
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -55,6 +55,7 @@
bugprone-move-forwarding-reference
bugprone-multiple-statement-macro
bugprone-parent-virtual-call
+   bugprone-placement-new-target-type-mismatch
bugprone-sizeof-container
bugprone-sizeof-expression
bugprone-string-constructor
Index: docs/clang-tidy/checks/bugprone-placement-new-target-type-mismatch.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/bugprone-placement-new-target-type-mismatch.rst
@@ -0,0 +1,8 @@
+.. title:: clang-tidy - misc-placement-new-target-type-mismatch
+
+misc-placement-new-target-type-mismatch
+===
+
+Finds placement-new calls where the pointer type of the adress mismatches the
+type of the created value.
+
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -130,6 +130,12 @@
   ` now supports `OverrideSpelling`
   and `FinalSpelling` options.
 
+- New :doc:`bugprone-placement-new-target-type-mismatch
+  ` check.
+
+  Finds placement-new calls where the pointer type of the adress mismatches the
+  type of the created value.
+
 - New :doc:`openmp-exception-escape
   ` check.
 
Index: clang-tidy/bugprone/PlacementNewTargetTypeMismatch.h
===
--- /dev/null
+++ clang-tidy/bugprone/PlacementNewTargetTypeMismatch.h
@@ -0,0 +1,35 @@
+//===--- PlacementNewTargetTypeMismatch.h - clang-tidy --*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_PLACEMENTNEWTARGETTYPEMISMATCHCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_PLACEMENTNEWTARGETTYPEMISMATCHCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+/// Finds 

[PATCH] D60139: [clang-tidy] Add bugprone-placement-new-target-type-mismatch check

[Dennis Luxen via Phabricator via cfe-commits]

DennisL updated this revision to Diff 194136.
DennisL retitled this revision from "[clang-tidy] Add 
misc-placement-new-target-type-mismatch check" to "[clang-tidy] Add 
bugprone-placement-new-target-type-mismatch check".
DennisL added a comment.

Removed debug output


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

https://reviews.llvm.org/D60139

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/PlacementNewTargetTypeMismatch.cpp
  clang-tidy/bugprone/PlacementNewTargetTypeMismatch.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-placement-new-target-type-mismatch.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-placement-new-target-type-mismatch.cpp

Index: test/clang-tidy/bugprone-placement-new-target-type-mismatch.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-placement-new-target-type-mismatch.cpp
@@ -0,0 +1,76 @@
+// RUN: %check_clang_tidy %s bugprone-placement-new-target-type-mismatch %t
+
+// definitions
+
+using size_type = unsigned long;
+void *operator new(size_type, void *);
+void *operator new[](size_type, void *);
+
+namespace std {
+template T* addressof(T& arg) noexcept;
+} // namespace std
+
+struct Foo {
+  int a;
+  int b;
+  int c;
+  int d;
+};
+
+template
+T& getT() {
+  static T f;
+  return f;
+}
+
+// instances emitting warnings
+
+void f1() {
+  struct Dummy {
+int a;
+int b;
+  };
+  int *ptr = new int;
+  new (ptr) Dummy;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: placement new of incompatible types [bugprone-placement-new-target-type-mismatch]
+}
+
+void f2() {
+  int * ptr = new int;
+  new (ptr) Foo;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: placement new of incompatible types [bugprone-placement-new-target-type-mismatch]
+}
+
+void f3() {
+  char *ptr = new char[17*sizeof(char)];
+  new (ptr) float[13];
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: placement new of incompatible types [bugprone-placement-new-target-type-mismatch]
+}
+
+void f4() {
+  new (std::addressof(getT())) Foo;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: placement new of incompatible types [bugprone-placement-new-target-type-mismatch]
+}
+
+void f5() {
+  char *ptr = new char[17*sizeof(char)];
+  new (ptr) float{13.f};
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: placement new of incompatible types [bugprone-placement-new-target-type-mismatch]
+}
+
+void f6() {
+  new ((void *)std::addressof(getT())) Foo;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: placement new of incompatible types [bugprone-placement-new-target-type-mismatch]
+}
+
+// instances not emitting a warning
+
+void f7() {
+  Foo * ptr = new Foo;
+  new (ptr) Foo;
+}
+
+void f8() {
+  char *ptr = new char[17*sizeof(char)];
+  new((float *)ptr) float{13.f};
+}
\ No newline at end of file
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -55,6 +55,7 @@
bugprone-move-forwarding-reference
bugprone-multiple-statement-macro
bugprone-parent-virtual-call
+   bugprone-placement-new-target-type-mismatch
bugprone-sizeof-container
bugprone-sizeof-expression
bugprone-string-constructor
Index: docs/clang-tidy/checks/bugprone-placement-new-target-type-mismatch.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/bugprone-placement-new-target-type-mismatch.rst
@@ -0,0 +1,8 @@
+.. title:: clang-tidy - misc-placement-new-target-type-mismatch
+
+misc-placement-new-target-type-mismatch
+===
+
+Finds placement-new calls where the pointer type of the adress mismatches the
+type of the created value.
+
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -130,6 +130,12 @@
   ` now supports `OverrideSpelling`
   and `FinalSpelling` options.
 
+- New :doc:`bugprone-placement-new-target-type-mismatch
+  ` check.
+
+  Finds placement-new calls where the pointer type of the adress mismatches the
+  type of the created value.
+
 - New :doc:`openmp-exception-escape
   ` check.
 
Index: clang-tidy/bugprone/PlacementNewTargetTypeMismatch.h
===
--- /dev/null
+++ clang-tidy/bugprone/PlacementNewTargetTypeMismatch.h
@@ -0,0 +1,35 @@
+//===--- PlacementNewTargetTypeMismatch.h - clang-tidy --*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef 

[PATCH] D60139: [clang-tidy] Add misc-placement-new-target-type-mismatch check

[Dennis Luxen via Phabricator via cfe-commits]

DennisL updated this revision to Diff 194135.
DennisL added a comment.

The following has been updated:

- moved check to bugprone instead of misc
- more tests
- rewritten check logic


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

https://reviews.llvm.org/D60139

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/PlacementNewTargetTypeMismatch.cpp
  clang-tidy/bugprone/PlacementNewTargetTypeMismatch.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-placement-new-target-type-mismatch.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-placement-new-target-type-mismatch.cpp

Index: test/clang-tidy/bugprone-placement-new-target-type-mismatch.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-placement-new-target-type-mismatch.cpp
@@ -0,0 +1,76 @@
+// RUN: %check_clang_tidy %s bugprone-placement-new-target-type-mismatch %t
+
+// definitions
+
+using size_type = unsigned long;
+void *operator new(size_type, void *);
+void *operator new[](size_type, void *);
+
+namespace std {
+template T* addressof(T& arg) noexcept;
+} // namespace std
+
+struct Foo {
+  int a;
+  int b;
+  int c;
+  int d;
+};
+
+template
+T& getT() {
+  static T f;
+  return f;
+}
+
+// instances emitting warnings
+
+void f1() {
+  struct Dummy {
+int a;
+int b;
+  };
+  int *ptr = new int;
+  new (ptr) Dummy;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: placement new of incompatible types [bugprone-placement-new-target-type-mismatch]
+}
+
+void f2() {
+  int * ptr = new int;
+  new (ptr) Foo;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: placement new of incompatible types [bugprone-placement-new-target-type-mismatch]
+}
+
+void f3() {
+  char *ptr = new char[17*sizeof(char)];
+  new (ptr) float[13];
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: placement new of incompatible types [bugprone-placement-new-target-type-mismatch]
+}
+
+void f4() {
+  new (std::addressof(getT())) Foo;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: placement new of incompatible types [bugprone-placement-new-target-type-mismatch]
+}
+
+void f5() {
+  char *ptr = new char[17*sizeof(char)];
+  new (ptr) float{13.f};
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: placement new of incompatible types [bugprone-placement-new-target-type-mismatch]
+}
+
+void f6() {
+  new ((void *)std::addressof(getT())) Foo;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: placement new of incompatible types [bugprone-placement-new-target-type-mismatch]
+}
+
+// instances not emitting a warning
+
+void f7() {
+  Foo * ptr = new Foo;
+  new (ptr) Foo;
+}
+
+void f8() {
+  char *ptr = new char[17*sizeof(char)];
+  new((float *)ptr) float{13.f};
+}
\ No newline at end of file
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -55,6 +55,7 @@
bugprone-move-forwarding-reference
bugprone-multiple-statement-macro
bugprone-parent-virtual-call
+   bugprone-placement-new-target-type-mismatch
bugprone-sizeof-container
bugprone-sizeof-expression
bugprone-string-constructor
Index: docs/clang-tidy/checks/bugprone-placement-new-target-type-mismatch.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/bugprone-placement-new-target-type-mismatch.rst
@@ -0,0 +1,8 @@
+.. title:: clang-tidy - bugprone-placement-new-target-type-mismatch
+
+bugprone-placement-new-target-type-mismatch
+===
+
+Finds placement-new calls where the pointer type of the adress mismatches the
+type of the created value.
+
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -130,6 +130,12 @@
   ` now supports `OverrideSpelling`
   and `FinalSpelling` options.
 
+- New :doc:`bugprone-placement-new-target-type-mismatch
+  ` check.
+
+  Finds placement-new calls where the pointer type of the adress mismatches the
+  type of the created value.
+
 - New :doc:`openmp-exception-escape
   ` check.
 
Index: clang-tidy/bugprone/PlacementNewTargetTypeMismatch.h
===
--- /dev/null
+++ clang-tidy/bugprone/PlacementNewTargetTypeMismatch.h
@@ -0,0 +1,35 @@
+//===--- PlacementNewTargetTypeMismatch.h - clang-tidy --*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_PLACEMENTNEWTARGETTYPEMISMATCHCHECK_H
+#define 

[PATCH] D60139: [clang-tidy] Add misc-placement-new-target-type-mismatch check

[Dennis Luxen via Phabricator via cfe-commits]

DennisL added a comment.

In D60139#1456123 , @alexfh wrote:

> Looks like this check would fit better into the `bugprone` module.


Excellent suggestion. Thanks. Will follow up with an updated Diff.


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

https://reviews.llvm.org/D60139



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

[PATCH] D60139: [clang-tidy] Add misc-placement-new-target-type-mismatch check

[Dennis Luxen via Phabricator via cfe-commits]

DennisL updated this revision to Diff 193839.
DennisL marked an inline comment as done.
DennisL added a comment.

Sync ReleaseNotes.rst with docs


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

https://reviews.llvm.org/D60139

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/MiscTidyModule.cpp
  clang-tidy/misc/PlacementNewTargetTypeMismatch.cpp
  clang-tidy/misc/PlacementNewTargetTypeMismatch.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-placement-new-target-type-mismatch.rst
  test/clang-tidy/misc-placement-new-target-type-mismatch.cpp

Index: test/clang-tidy/misc-placement-new-target-type-mismatch.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-placement-new-target-type-mismatch.cpp
@@ -0,0 +1,82 @@
+// RUN: %check_clang_tidy %s misc-placement-new-target-type-mismatch %t
+
+// definitions
+
+using size_type = unsigned long;
+void *operator new(size_type, void *);
+void *operator new[](size_type, void *);
+
+namespace std {
+template T* addressof(T& arg) noexcept;
+} // namespace std
+
+struct Foo {
+  int a;
+  int b;
+  int c;
+  int d;
+};
+
+template
+T& getT() {
+  static T f;
+  return f;
+}
+
+// instances emitting warnings
+
+void f1() {
+  struct Dummy {
+int a;
+int b;
+  };
+  int *ptr = new int;
+  new (ptr) Dummy;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: placement new of incompatible types [misc-placement-new-target-type-mismatch]
+  delete ptr;
+}
+
+void f2() {
+  int * ptr = new int;
+  new (ptr) Foo;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: placement new of incompatible types [misc-placement-new-target-type-mismatch]
+  delete ptr;
+}
+
+void f3() {
+  char *ptr = new char[17*sizeof(char)];
+  new(ptr) float[13];
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: placement new of incompatible types [misc-placement-new-target-type-mismatch]
+  delete[] ptr;
+}
+
+void f4() {
+  new (std::addressof(getT())) Foo;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: placement new of incompatible types [misc-placement-new-target-type-mismatch]
+}
+
+void f5() {
+  char *ptr = new char[17*sizeof(char)];
+  new(ptr) float{13.f};
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: placement new of incompatible types [misc-placement-new-target-type-mismatch]
+  delete[] ptr;
+}
+
+// instances not emitting a warning
+
+void f6() {
+  Foo * ptr = new Foo;
+  new (ptr) Foo;
+  delete ptr;
+}
+
+void f7() {
+  new ((void *)std::addressof(getT())) Foo;
+}
+
+void f8() {
+  char *ptr = new char[17*sizeof(char)];
+  new((float *)ptr) float{13.f};
+
+  delete[] ptr;
+}
\ No newline at end of file
Index: docs/clang-tidy/checks/misc-placement-new-target-type-mismatch.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/misc-placement-new-target-type-mismatch.rst
@@ -0,0 +1,8 @@
+.. title:: clang-tidy - misc-placement-new-target-type-mismatch
+
+misc-placement-new-target-type-mismatch
+===
+
+Finds placement-new calls where the pointer type of the adress mismatches the
+type of the created value.
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -184,6 +184,7 @@
misc-new-delete-overloads
misc-non-copyable-objects
misc-non-private-member-variables-in-classes
+   misc-placement-new-target-type-mismatch
misc-redundant-expression
misc-static-assert
misc-throw-by-value-catch-by-reference
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -130,6 +130,12 @@
   ` now supports `OverrideSpelling`
   and `FinalSpelling` options.
 
+- New :doc:`misc-placement-new-target-type-mismatch
+  ` check.
+
+  Finds placement-new calls where the pointer type of the adress mismatches the
+  type of the created value.
+
 - New :doc:`openmp-exception-escape
   ` check.
 
Index: clang-tidy/misc/PlacementNewTargetTypeMismatch.h
===
--- /dev/null
+++ clang-tidy/misc/PlacementNewTargetTypeMismatch.h
@@ -0,0 +1,36 @@
+//===--- PlacementNewTargetTypeMismatch.h - clang-tidy -*- C++
+//-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_PLACEMENTNEWTARGETTYPEMISMATCHCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_PLACEMENTNEWTARGETTYPEMISMATCHCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// Finds placement-new calls 

[PATCH] D60139: [clang-tidy] Add misc-placement-new-target-type-mismatch check

[Dennis Luxen via Phabricator via cfe-commits]

DennisL updated this revision to Diff 193715.
DennisL marked an inline comment as done.
DennisL retitled this revision from "[clang-tidy] Add 
misc-placement-new-target-size check" to "[clang-tidy] Add 
misc-placement-new-target-type-mismatch check".
DennisL edited the summary of this revision.
DennisL added a comment.

Addressed reviewer feedback and also simplified the implementation, renamed the 
test to be more concise, and added more test cases.


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

https://reviews.llvm.org/D60139

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/MiscTidyModule.cpp
  clang-tidy/misc/PlacementNewTargetTypeMismatch.cpp
  clang-tidy/misc/PlacementNewTargetTypeMismatch.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-placement-new-target-type-mismatch.rst
  test/clang-tidy/misc-placement-new-target-type-mismatch.cpp

Index: test/clang-tidy/misc-placement-new-target-type-mismatch.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-placement-new-target-type-mismatch.cpp
@@ -0,0 +1,82 @@
+// RUN: %check_clang_tidy %s misc-placement-new-target-type-mismatch %t
+
+// definitions
+
+using size_type = unsigned long;
+void *operator new(size_type, void *);
+void *operator new[](size_type, void *);
+
+namespace std {
+template T* addressof(T& arg) noexcept;
+} // namespace std
+
+struct Foo {
+  int a;
+  int b;
+  int c;
+  int d;
+};
+
+template
+T& getT() {
+  static T f;
+  return f;
+}
+
+// instances emitting warnings
+
+void f1() {
+  struct Dummy {
+int a;
+int b;
+  };
+  int *ptr = new int;
+  new (ptr) Dummy;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: placement new of incompatible types [misc-placement-new-target-type-mismatch]
+  delete ptr;
+}
+
+void f2() {
+  int * ptr = new int;
+  new (ptr) Foo;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: placement new of incompatible types [misc-placement-new-target-type-mismatch]
+  delete ptr;
+}
+
+void f3() {
+  char *ptr = new char[17*sizeof(char)];
+  new(ptr) float[13];
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: placement new of incompatible types [misc-placement-new-target-type-mismatch]
+  delete[] ptr;
+}
+
+void f4() {
+  new (std::addressof(getT())) Foo;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: placement new of incompatible types [misc-placement-new-target-type-mismatch]
+}
+
+void f5() {
+  char *ptr = new char[17*sizeof(char)];
+  new(ptr) float{13.f};
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: placement new of incompatible types [misc-placement-new-target-type-mismatch]
+  delete[] ptr;
+}
+
+// instances not emitting a warning
+
+void f6() {
+  Foo * ptr = new Foo;
+  new (ptr) Foo;
+  delete ptr;
+}
+
+void f7() {
+  new ((void *)std::addressof(getT())) Foo;
+}
+
+void f8() {
+  char *ptr = new char[17*sizeof(char)];
+  new((float *)ptr) float{13.f};
+
+  delete[] ptr;
+}
\ No newline at end of file
Index: docs/clang-tidy/checks/misc-placement-new-target-type-mismatch.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/misc-placement-new-target-type-mismatch.rst
@@ -0,0 +1,8 @@
+.. title:: clang-tidy - misc-placement-new-target-type-mismatch
+
+misc-placement-new-target-type-mismatch
+===
+
+Finds placement-new calls where the pointer type of the adress mismatches the
+type of the created value.
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -184,6 +184,7 @@
misc-new-delete-overloads
misc-non-copyable-objects
misc-non-private-member-variables-in-classes
+   misc-placement-new-target-type-mismatch
misc-redundant-expression
misc-static-assert
misc-throw-by-value-catch-by-reference
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -130,6 +130,13 @@
   ` now supports `OverrideSpelling`
   and `FinalSpelling` options.
 
+- New :doc:`misc-placement-new-target-type-mismatch
+  ` check.
+
+  Finds placement-new calls where the size of the pointee type of the placement 
+  parameter is smaller than the size of the constructed type and the pointer is
+  implicitly cast to ``void *``.
+
 - New :doc:`openmp-exception-escape
   ` check.
 
Index: clang-tidy/misc/PlacementNewTargetTypeMismatch.h
===
--- /dev/null
+++ clang-tidy/misc/PlacementNewTargetTypeMismatch.h
@@ -0,0 +1,36 @@
+//===--- PlacementNewTargetTypeMismatch.h - clang-tidy -*- C++
+//-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH 

[PATCH] D60139: [clang-tidy] Add misc-placement-new-target-size check

[Dennis Luxen via Phabricator via cfe-commits]

DennisL updated this revision to Diff 193471.
DennisL added a comment.

Simplify logic


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

https://reviews.llvm.org/D60139

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/MiscTidyModule.cpp
  clang-tidy/misc/PlacementNewTargetSizeCheck.cpp
  clang-tidy/misc/PlacementNewTargetSizeCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-placement-new-target-size.rst
  test/clang-tidy/misc-placement-new-target-size.cpp

Index: test/clang-tidy/misc-placement-new-target-size.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-placement-new-target-size.cpp
@@ -0,0 +1,15 @@
+// RUN: %check_clang_tidy %s misc-placement-new-target-size %t
+
+using size_type = unsigned long;
+void *operator new(size_type, void *);
+
+void f() {
+  struct Dummy {
+int a;
+int b;
+  };
+  int *ptr = new int;
+  new (ptr) Dummy;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: placement new has insufficient target size [misc-placement-new-target-size]
+  delete ptr;
+}
Index: docs/clang-tidy/checks/misc-placement-new-target-size.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/misc-placement-new-target-size.rst
@@ -0,0 +1,10 @@
+.. title:: clang-tidy - misc-placement-new-target-size
+
+misc-placement-new-target-size
+==
+
+
+Finds placement-new calls where the size of the pointee type of the placement 
+parameter is smaller than the size of the constructed type and the pointer is
+implicitly cast to ``void *``.
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -184,6 +184,7 @@
misc-new-delete-overloads
misc-non-copyable-objects
misc-non-private-member-variables-in-classes
+   misc-placement-new-target-size
misc-redundant-expression
misc-static-assert
misc-throw-by-value-catch-by-reference
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -130,6 +130,13 @@
   ` now supports `OverrideSpelling`
   and `FinalSpelling` options.
 
+- New :doc:`misc-placement-new-target-size
+  ` check.
+
+  Finds placement-new calls where the size of the pointee type of the placement 
+  parameter is smaller than the size of the constructed type and the pointer is
+  implicitly cast to ``void *``.
+
 - New :doc:`openmp-exception-escape
   ` check.
 
Index: clang-tidy/misc/PlacementNewTargetSizeCheck.h
===
--- /dev/null
+++ clang-tidy/misc/PlacementNewTargetSizeCheck.h
@@ -0,0 +1,37 @@
+//===--- PlacementNewTargetSizeCheck.h - clang-tidy -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_PLACEMENTNEWTARGETSIZECHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_PLACEMENTNEWTARGETSIZECHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// This checker finds placement new statements that construct objects into
+/// allocated space where the pointer to that allocated space is of a type
+/// smaller than the constructed object and the pointer is implicitly cast to
+/// void *.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-placement-new-target-size.html
+class PlacementNewTargetSizeCheck : public ClangTidyCheck {
+public:
+  PlacementNewTargetSizeCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult ) override;
+};
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_PLACEMENTNEWTARGETSIZECHECK_H
Index: clang-tidy/misc/PlacementNewTargetSizeCheck.cpp
===
--- /dev/null
+++ clang-tidy/misc/PlacementNewTargetSizeCheck.cpp
@@ -0,0 +1,78 @@
+//===--- PlacementNewTargetSizeCheck.cpp - clang-tidy -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "PlacementNewTargetSizeCheck.h"
+#include 

[PATCH] D60139: [clang-tidy] Add misc-placement-new-target-size check

[Dennis Luxen via Phabricator via cfe-commits]

DennisL updated this revision to Diff 193454.
DennisL added a comment.

Remove debug output


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

https://reviews.llvm.org/D60139

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/MiscTidyModule.cpp
  clang-tidy/misc/PlacementNewTargetSizeCheck.cpp
  clang-tidy/misc/PlacementNewTargetSizeCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-placement-new-target-size.rst
  test/clang-tidy/misc-placement-new-target-size.cpp

Index: test/clang-tidy/misc-placement-new-target-size.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-placement-new-target-size.cpp
@@ -0,0 +1,15 @@
+// RUN: %check_clang_tidy %s misc-placement-new-target-size %t
+
+using size_type = unsigned long;
+void *operator new(size_type, void *);
+
+void f() {
+  struct Dummy {
+int a;
+int b;
+  };
+  int *ptr = new int;
+  new (ptr) Dummy;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: placement new has insufficient target size [misc-placement-new-target-size]
+  delete ptr;
+}
Index: docs/clang-tidy/checks/misc-placement-new-target-size.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/misc-placement-new-target-size.rst
@@ -0,0 +1,10 @@
+.. title:: clang-tidy - misc-placement-new-target-size
+
+misc-placement-new-target-size
+==
+
+
+Finds placement-new calls where the size of the pointee type of the placement 
+parameter is smaller than the size of the constructed type and the pointer is
+implicitly cast to ``void *``.
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -184,6 +184,7 @@
misc-new-delete-overloads
misc-non-copyable-objects
misc-non-private-member-variables-in-classes
+   misc-placement-new-target-size
misc-redundant-expression
misc-static-assert
misc-throw-by-value-catch-by-reference
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -130,6 +130,13 @@
   ` now supports `OverrideSpelling`
   and `FinalSpelling` options.
 
+- New :doc:`misc-placement-new-target-size
+  ` check.
+
+  Finds placement-new calls where the size of the pointee type of the placement 
+  parameter is smaller than the size of the constructed type and the pointer is
+  implicitly cast to ``void *``.
+
 - New :doc:`openmp-exception-escape
   ` check.
 
Index: clang-tidy/misc/PlacementNewTargetSizeCheck.h
===
--- /dev/null
+++ clang-tidy/misc/PlacementNewTargetSizeCheck.h
@@ -0,0 +1,37 @@
+//===--- PlacementNewTargetSizeCheck.h - clang-tidy -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_PLACEMENTNEWTARGETSIZECHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_PLACEMENTNEWTARGETSIZECHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// This checker finds placement new statements that construct objects into
+/// allocated space where the pointer to that allocated space is of a type
+/// smaller than the constructed object and the pointer is implicitly cast to
+/// void *.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-placement-new-target-size.html
+class PlacementNewTargetSizeCheck : public ClangTidyCheck {
+public:
+  PlacementNewTargetSizeCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult ) override;
+};
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_PLACEMENTNEWTARGETSIZECHECK_H
Index: clang-tidy/misc/PlacementNewTargetSizeCheck.cpp
===
--- /dev/null
+++ clang-tidy/misc/PlacementNewTargetSizeCheck.cpp
@@ -0,0 +1,78 @@
+//===--- PlacementNewTargetSizeCheck.cpp - clang-tidy -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "PlacementNewTargetSizeCheck.h"
+#include 

[PATCH] D60139: [clang-tidy] Add misc-placement-new-target-size check

[Dennis Luxen via Phabricator via cfe-commits]

DennisL updated this revision to Diff 193449.
DennisL marked 13 inline comments as done.
DennisL added a comment.

Updated patch to address reviewer feedback


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

https://reviews.llvm.org/D60139

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/MiscTidyModule.cpp
  clang-tidy/misc/PlacementNewTargetSizeCheck.cpp
  clang-tidy/misc/PlacementNewTargetSizeCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-placement-new-target-size.rst
  test/clang-tidy/misc-placement-new-target-size.cpp

Index: test/clang-tidy/misc-placement-new-target-size.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-placement-new-target-size.cpp
@@ -0,0 +1,15 @@
+// RUN: %check_clang_tidy %s misc-placement-new-target-size %t
+
+using size_type = unsigned long;
+void *operator new(size_type, void *);
+
+void f() {
+  struct Dummy {
+int a;
+int b;
+  };
+  int *ptr = new int;
+  new (ptr) Dummy;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: placement new has insufficient target size [misc-placement-new-target-size]
+  delete ptr;
+}
Index: docs/clang-tidy/checks/misc-placement-new-target-size.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/misc-placement-new-target-size.rst
@@ -0,0 +1,10 @@
+.. title:: clang-tidy - misc-placement-new-target-size
+
+misc-placement-new-target-size
+==
+
+
+Finds placement-new calls where the size of the pointee type of the placement 
+parameter is smaller than the size of the constructed type and the pointer is
+implicitly cast to ``void *``.
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -184,6 +184,7 @@
misc-new-delete-overloads
misc-non-copyable-objects
misc-non-private-member-variables-in-classes
+   misc-placement-new-target-size
misc-redundant-expression
misc-static-assert
misc-throw-by-value-catch-by-reference
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -130,6 +130,13 @@
   ` now supports `OverrideSpelling`
   and `FinalSpelling` options.
 
+- New :doc:`misc-placement-new-target-size
+  ` check.
+
+  Finds placement-new calls where the size of the pointee type of the placement 
+  parameter is smaller than the size of the constructed type and the pointer is
+  implicitly cast to ``void *``.
+
 - New :doc:`openmp-exception-escape
   ` check.
 
Index: clang-tidy/misc/PlacementNewTargetSizeCheck.h
===
--- /dev/null
+++ clang-tidy/misc/PlacementNewTargetSizeCheck.h
@@ -0,0 +1,37 @@
+//===--- PlacementNewTargetSizeCheck.h - clang-tidy -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_PLACEMENTNEWTARGETSIZECHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_PLACEMENTNEWTARGETSIZECHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// This checker finds placement new statements that construct objects into
+/// allocated space where the pointer to that allocated space is of a type
+/// smaller than the constructed object and the pointer is implicitly cast to
+/// void *.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-placement-new-target-size.html
+class PlacementNewTargetSizeCheck : public ClangTidyCheck {
+public:
+  PlacementNewTargetSizeCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult ) override;
+};
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_PLACEMENTNEWTARGETSIZECHECK_H
Index: clang-tidy/misc/PlacementNewTargetSizeCheck.cpp
===
--- /dev/null
+++ clang-tidy/misc/PlacementNewTargetSizeCheck.cpp
@@ -0,0 +1,79 @@
+//===--- PlacementNewTargetSizeCheck.cpp - clang-tidy -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//

[PATCH] D60139: Add clang-tidy/checks/misc-placement-new-target-size check

[Dennis Luxen via Phabricator via cfe-commits]

DennisL created this revision.
Herald added subscribers: cfe-commits, mgorny.
Herald added a project: clang.

Finds placement-new calls where the size of the pointee type of the placement 
parameter is smaller than the size of the constructed type and the pointer is 
implicitly cast to ``void *``


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D60139

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/MiscTidyModule.cpp
  clang-tidy/misc/PlacementNewTargetSizeCheck.cpp
  clang-tidy/misc/PlacementNewTargetSizeCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-placement-new-target-size.rst
  test/clang-tidy/misc-placement-new-target-size.cpp

Index: test/clang-tidy/misc-placement-new-target-size.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-placement-new-target-size.cpp
@@ -0,0 +1,14 @@
+// RUN: %check_clang_tidy %s misc-placement-new-target-size %t
+
+void f() {
+  struct Dummy {
+int a;
+int b;
+  };
+  int * ptr = new int;
+  new (ptr) Dummy;
+  delete ptr;
+
+}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'f' is insufficiently awesome [misc-placement-new-target-size]
+
Index: docs/clang-tidy/checks/misc-placement-new-target-size.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/misc-placement-new-target-size.rst
@@ -0,0 +1,10 @@
+.. title:: clang-tidy - misc-placement-new-target-size
+
+misc-placement-new-target-size
+==
+
+
+Finds placement-new calls where the size of the pointee type of the placement 
+parameter is smaller than the size of the constructed type and the pointer is
+implicitly cast to ``void *``.
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -184,6 +184,7 @@
misc-new-delete-overloads
misc-non-copyable-objects
misc-non-private-member-variables-in-classes
+   misc-placement-new-target-size
misc-redundant-expression
misc-static-assert
misc-throw-by-value-catch-by-reference
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -130,6 +130,9 @@
   ` now supports `OverrideSpelling`
   and `FinalSpelling` options.
 
+- New :doc:`misc-placement-new-target-size
+  ` check.
+
 - New :doc:`openmp-exception-escape
   ` check.
 
Index: clang-tidy/misc/PlacementNewTargetSizeCheck.h
===
--- /dev/null
+++ clang-tidy/misc/PlacementNewTargetSizeCheck.h
@@ -0,0 +1,36 @@
+//===--- PlacementNewTargetSizeCheck.h - clang-tidy -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_PLACEMENTNEWTARGETSIZECHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_PLACEMENTNEWTARGETSIZECHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// This checker finds placement new statements that construct objects into allocated
+/// space where the pointer to that allocated space is of a type smaller than the
+/// constructed object and the pointer is implicitly cast to void *.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-placement-new-target-size.html
+class PlacementNewTargetSizeCheck : public ClangTidyCheck {
+public:
+  PlacementNewTargetSizeCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult ) override;
+};
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_PLACEMENTNEWTARGETSIZECHECK_H
Index: clang-tidy/misc/PlacementNewTargetSizeCheck.cpp
===
--- /dev/null
+++ clang-tidy/misc/PlacementNewTargetSizeCheck.cpp
@@ -0,0 +1,78 @@
+//===--- PlacementNewTargetSizeCheck.cpp - clang-tidy -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include 
+
+#include "PlacementNewTargetSizeCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/RecordLayout.h"
+#include 

[PATCH] D37677: [libc++] implement future synchronization using atomic_flag

[Dennis Luxen via Phabricator via cfe-commits]

dennis.luxen planned changes to this revision.
dennis.luxen added a comment.

In https://reviews.llvm.org/D37677#868851, @EricWF wrote:

> I agree with the general consensus that we should only make this change if 
> it's significantly faster, and only after we have a test that demonstrates 
> this.
>
> Unfortunately I don't recall exactly why I wrote that TODO in the first 
> place, but I'm sure I meant changing `__state_`, and not the lock. I suspect 
> it had to do with http://llvm.org/PR24692 .


I talked to Marshall during CPPCon and he mentioned that the remark to use 
atomic was related to using `std::call_once`. I will rework this patch and use 
the defines from `__config` to mark it as an ABI breaking change.


https://reviews.llvm.org/D37677



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

[PATCH] D37677: [libc++] implement future synchronization using atomic_flag

[Dennis Luxen via Phabricator via cfe-commits]

dennis.luxen added a comment.

In https://reviews.llvm.org/D37677#866362, @bcraig wrote:

> Is there a benchmark where this demonstrates some performance improvement?  I 
> fear that the switch to condition_variable_any will swamp any performance 
> gains from the switch to a spin lock.
>
> Also, the spin lock is being held during allocating operations (the exception 
> throws and at_thread_exit code).  That's a little long to be holding a spin 
> lock.


Thanks @bcraig, I will devise a benchmark and report back with its numbers for 
X86_64.


https://reviews.llvm.org/D37677



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

[PATCH] D37677: [libc++] implement future synchronization using atomic_flag

[Dennis Luxen via Phabricator via cfe-commits]

dennis.luxen created this revision.

This task is listed in TODO.txt. The implementation swaps mutex against a 
spinlock based on atomic_flag. The spin lock itself is implemented as a nested 
class in a protected context of the associated state.


https://reviews.llvm.org/D37677

Files:
  include/future
  src/future.cpp

Index: src/future.cpp
===
--- src/future.cpp
+++ src/future.cpp
@@ -91,7 +91,7 @@
 void
 __assoc_sub_state::set_value()
 {
-unique_lock __lk(__mut_);
+unique_lock<__spin_lock> __lk(__mut_);
 #ifndef _LIBCPP_NO_EXCEPTIONS
 if (__has_value())
 throw future_error(make_error_code(future_errc::promise_already_satisfied));
@@ -103,7 +103,7 @@
 void
 __assoc_sub_state::set_value_at_thread_exit()
 {
-unique_lock __lk(__mut_);
+unique_lock<__spin_lock> __lk(__mut_);
 #ifndef _LIBCPP_NO_EXCEPTIONS
 if (__has_value())
 throw future_error(make_error_code(future_errc::promise_already_satisfied));
@@ -115,7 +115,7 @@
 void
 __assoc_sub_state::set_exception(exception_ptr __p)
 {
-unique_lock __lk(__mut_);
+unique_lock<__spin_lock> __lk(__mut_);
 #ifndef _LIBCPP_NO_EXCEPTIONS
 if (__has_value())
 throw future_error(make_error_code(future_errc::promise_already_satisfied));
@@ -128,7 +128,7 @@
 void
 __assoc_sub_state::set_exception_at_thread_exit(exception_ptr __p)
 {
-unique_lock __lk(__mut_);
+unique_lock<__spin_lock> __lk(__mut_);
 #ifndef _LIBCPP_NO_EXCEPTIONS
 if (__has_value())
 throw future_error(make_error_code(future_errc::promise_already_satisfied));
@@ -140,29 +140,29 @@
 void
 __assoc_sub_state::__make_ready()
 {
-unique_lock __lk(__mut_);
+unique_lock<__spin_lock> __lk(__mut_);
 __state_ |= ready;
 __cv_.notify_all();
 }
 
 void
 __assoc_sub_state::copy()
 {
-unique_lock __lk(__mut_);
+unique_lock<__spin_lock> __lk(__mut_);
 __sub_wait(__lk);
 if (__exception_ != nullptr)
 rethrow_exception(__exception_);
 }
 
 void
 __assoc_sub_state::wait()
 {
-unique_lock __lk(__mut_);
+unique_lock<__spin_lock> __lk(__mut_);
 __sub_wait(__lk);
 }
 
 void
-__assoc_sub_state::__sub_wait(unique_lock& __lk)
+__assoc_sub_state::__sub_wait(unique_lock<__spin_lock>& __lk)
 {
 if (!__is_ready())
 {
Index: include/future
===
--- include/future
+++ include/future
@@ -367,7 +367,8 @@
 #include 
 #include 
 #include 
-#include 
+#include 
+#include 
 #include 
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
@@ -533,12 +534,24 @@
 {
 protected:
 exception_ptr __exception_;
-mutable mutex __mut_;
-mutable condition_variable __cv_;
+
+mutable class __spin_lock
+{
+atomic_flag __locked_ = ATOMIC_FLAG_INIT ;
+public:
+void lock() {
+while (__locked_.test_and_set(memory_order_acquire)) { ; }
+}
+void unlock() {
+__locked_.clear(memory_order_release);
+}
+} __mut_;
+
+mutable condition_variable_any __cv_;
 unsigned __state_;
 
 virtual void __on_zero_shared() _NOEXCEPT;
-void __sub_wait(unique_lock& __lk);
+void __sub_wait(unique_lock<__spin_lock>& __lk);
 public:
 enum
 {
@@ -558,7 +571,7 @@
 _LIBCPP_INLINE_VISIBILITY
 void __set_future_attached()
 {
-lock_guard __lk(__mut_);
+lock_guard<__spin_lock> __lk(__mut_);
 __state_ |= __future_attached;
 }
 _LIBCPP_INLINE_VISIBILITY
@@ -596,7 +609,7 @@
 future_status
 __assoc_sub_state::wait_until(const chrono::time_point<_Clock, _Duration>& __abs_time) const
 {
-unique_lock __lk(__mut_);
+unique_lock<__spin_lock> __lk(__mut_);
 if (__state_ & deferred)
 return future_status::deferred;
 while (!(__state_ & ready) && _Clock::now() < __abs_time)
@@ -663,7 +676,7 @@
 __assoc_state<_Rp>::set_value(_Arg& __arg)
 #endif
 {
-unique_lock __lk(this->__mut_);
+unique_lock<__spin_lock> __lk(this->__mut_);
 if (this->__has_value())
 __throw_future_error(future_errc::promise_already_satisfied);
 ::new(&__value_) _Rp(_VSTD::forward<_Arg>(__arg));
@@ -680,7 +693,7 @@
 __assoc_state<_Rp>::set_value_at_thread_exit(_Arg& __arg)
 #endif
 {
-unique_lock __lk(this->__mut_);
+unique_lock<__spin_lock> __lk(this->__mut_);
 if (this->__has_value())
 __throw_future_error(future_errc::promise_already_satisfied);
 ::new(&__value_) _Rp(_VSTD::forward<_Arg>(__arg));
@@ -692,7 +705,7 @@
 _Rp
 __assoc_state<_Rp>::move()
 {
-unique_lock __lk(this->__mut_);
+unique_lock<__spin_lock> __lk(this->__mut_);
 this->__sub_wait(__lk);
 if (this->__exception_ != nullptr)
 rethrow_exception(this->__exception_);
@@ -703,7 +716,7 @@
 typename add_lvalue_reference<_Rp>::type
 __assoc_state<_Rp>::copy()
 {
-unique_lock __lk(this->__mut_);
+unique_lock<__spin_lock> __lk(this->__mut_);