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

2019-04-11 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D60139#1461269 , @DennisL wrote:

> 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.


What is the rationale for separating those checks? It sort of feels like there 
is some overlap between alignment, size, and buffer type.

I'm not certain I agree with the way this check operates -- what problem is it 
trying to solve? For instance, this code would trigger on this check, but is 
perfectly valid code:

  static_assert(sizeof(int) == sizeof(float));
  static_assert(alignof(int) == alignof(float));
  int buffer;
  float *fp = new () float;

The situations I can think of where the type mismatch is an issue would be for 
types with nontrivial special members, so there are times when this check makes 
sense, but perhaps it's too restrictive currently, but you may have a different 
problem in mind?




Comment at: clang-tidy/bugprone/PlacementNewTargetTypeMismatch.cpp:20
+namespace {
+AST_MATCHER(Expr, isPlacementNewExpr) {
+  const auto *NewExpr = dyn_cast();

This should match on `CXXNewExpr` instead of `Expr`, then you won't need the 
`dyn_cast` below.



Comment at: clang-tidy/bugprone/PlacementNewTargetTypeMismatch.cpp:29
+  Finder->addMatcher(
+  expr(isPlacementNewExpr(), unless(hasDescendant(unresolvedLookupExpr(
+  .bind("NewExpr"),

This should probably use `cxxNewExpr()` instead of `expr()` so you match on 
something more specific.



Comment at: clang-tidy/bugprone/PlacementNewTargetTypeMismatch.cpp:38
+  assert(NewExpr && "Matched node bound by 'NewExpr' shoud be a 'CXXNewExpr'");
+  assert(NewExpr->getNumPlacementArgs() != 0 && "");
+

This assert message looks a bit incomplete. ;-)



Comment at: clang-tidy/bugprone/PlacementNewTargetTypeMismatch.cpp:43
+  assert(PlacementExpr != nullptr && "PlacementExpr should not be null");
+  const CastExpr *Cast = dyn_cast(PlacementExpr);
+  if (Cast == nullptr)

You can use `const auto *` here.



Comment at: test/clang-tidy/bugprone-placement-new-target-type-mismatch.cpp:56
+  new (ptr) float[13];
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: placement new parameter and 
allocated type mismatch [bugprone-placement-new-target-type-mismatch]
+}

While this code definitely has a bug from the buffer overflow, I don't think 
the diagnostic is valuable here. This is a very common pattern and I suspect 
this will yield a lot of false positives, unless the check starts taking 
alignment and buffer size into consideration.


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

2019-04-10 Thread 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

2019-04-10 Thread 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

2019-04-09 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments.



Comment at: clang-tidy/bugprone/PlacementNewTargetTypeMismatch.cpp:38
+  const CastExpr *Cast = dyn_cast(PlacementExpr);
+  if (nullptr == Cast) {
+return;

JonasToth wrote:
> braces, above the comment does not follow the correct-sentence style
I would put nullptr as second argument for == operator.


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

2019-04-09 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment.

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.




Comment at: clang-tidy/bugprone/PlacementNewTargetTypeMismatch.cpp:21
+void PlacementNewTargetTypeMismatch::registerMatchers(MatchFinder *Finder) {
+  // We only want the records that call 'new' with an adress parameter
+  Finder->addMatcher(cxxNewExpr(hasDescendant(castExpr())).bind("NewExpr"), 
this);

Please write full sentences with punctuation in comments.



Comment at: clang-tidy/bugprone/PlacementNewTargetTypeMismatch.cpp:22
+  // We only want the records that call 'new' with an adress parameter
+  Finder->addMatcher(cxxNewExpr(hasDescendant(castExpr())).bind("NewExpr"), 
this);
+}

placement-new? please make a new matcher (locally) to match that, because its 
is more specific.
You can use other checks as example, grep for "AST_MATCHER"



Comment at: clang-tidy/bugprone/PlacementNewTargetTypeMismatch.cpp:30
+
+  if (0 == NewExpr->getNumPlacementArgs()) {
+return;

Please ellide braces.



Comment at: clang-tidy/bugprone/PlacementNewTargetTypeMismatch.cpp:38
+  const CastExpr *Cast = dyn_cast(PlacementExpr);
+  if (nullptr == Cast) {
+return;

braces, above the comment does not follow the correct-sentence style



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

Is this universally true? What about the nothrow-overload, would that interfere?



Comment at: clang-tidy/bugprone/PlacementNewTargetTypeMismatch.cpp:45
+ "Cast of placement parameter requires a pointer or an array type");
+  const QualType  =
+  
Cast->getSubExpr()->getType()->getPointeeOrArrayElementType()->getCanonicalTypeInternal();

`QualType` is usually used a value-type, not const-ref. Please follow that for 
consistency.



Comment at: clang-tidy/bugprone/PlacementNewTargetTypeMismatch.cpp:50
+
+  if (PlacementParameterType != AllocatedType) {
+diag(PlacementExpr->getExprLoc(), "placement new parameter and allocated 
type mismatch");

braces.



Comment at: 
docs/clang-tidy/checks/bugprone-placement-new-target-type-mismatch.rst:7
+Finds placement-new calls where the pointer type of the adress mismatches the
+type of the created value.
+

Please add examples to demonstrate good and bad code for the user.



Comment at: test/clang-tidy/bugprone-placement-new-target-type-mismatch.cpp:5
+
+using size_type = unsigned long;
+void *operator new(size_type, void *);

Please add a test for the nothrow overload.



Comment at: test/clang-tidy/bugprone-placement-new-target-type-mismatch.cpp:86
+  new (array) char('A');
+}

Please add test, where everything is hidden behind templates and only the 
type-substitution leads to the error.
A test including macros helps as well, to show they do not interfere (as should 
be the case with this 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

2019-04-09 Thread 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

2019-04-08 Thread 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

2019-04-08 Thread 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