[PATCH] D128372: [Clang-Tidy] Empty Check

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

In D128372#3991516 , @MyDeveloperDay 
wrote:

> Ironically as an aside... looking back at the review because of the discource 
> article.
>
> The whole reason why I added the [[nodiscard]] checker to clang-tidy 4 years 
> ago was to cover exactly this use case...and many others like it where users 
> have a function which returns a boolean and takes no arguments are pointer or 
> reference (empty() included)
>
> https://reviews.llvm.org/D55433
>
> Shouldn't people be using this checker to add nodiscard and let the compiler 
> to the heavy lifting. Was there a use case as to why that solution doesn't 
> solve this?

That check is for library authors: this one is for library users. There are a 
non-insignificant number of cases where an unused call to `empty` pops up, 
hence the desire for this check.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

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

[PATCH] D128372: [Clang-Tidy] Empty Check

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

Please take a loon on https://github.com/llvm/llvm-project/issues/59487.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

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

[PATCH] D128372: [Clang-Tidy] Empty Check

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

Ironically as an aside... looking back at the review because of the discource 
article.

The whole reason why I added the [[nodiscard]] checker to clang-tidy 4 years 
ago was to cover exactly this use case...and many others like it where users 
have a function which returns a boolean and takes no arguments are pointer or 
reference (empty() included)

https://reviews.llvm.org/D55433

Shouldn't people be using this checker to add nodiscard and let the compiler to 
the heavy lifting. Was there a use case as to why that solution doesn't solve 
this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

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

[PATCH] D128372: [Clang-Tidy] Empty Check

[Christopher Di Bella via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rGec3f8feddf81: [Clang-Tidy] Empty Check (authored by 
abrahamcd, committed by cjdb).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone/standalone-empty.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp
@@ -0,0 +1,578 @@
+// RUN: %check_clang_tidy %s bugprone-standalone-empty %t
+
+namespace std {
+template 
+struct vector {
+  bool empty();
+};
+
+template 
+struct vector_with_clear {
+  bool empty();
+  void clear();
+};
+
+template 
+struct vector_with_void_empty {
+  void empty();
+  void clear();
+};
+
+template 
+struct vector_with_int_empty {
+  int empty();
+  void clear();
+};
+
+template 
+struct vector_with_clear_args {
+  bool empty();
+  void clear(int i);
+};
+
+template 
+struct vector_with_clear_variable {
+  bool empty();
+  int clear;
+};
+
+template 
+bool empty(T &&);
+
+} // namespace std
+
+namespace absl {
+struct string {
+  bool empty();
+};
+
+struct string_with_clear {
+  bool empty();
+  void clear();
+};
+
+struct string_with_void_empty {
+  void empty();
+  void clear();
+};
+
+struct string_with_int_empty {
+  int empty();
+  void clear();
+};
+
+struct string_with_clear_args {
+  bool empty();
+  void clear(int i);
+};
+
+struct string_with_clear_variable {
+  bool empty();
+  int clear;
+};
+
+template 
+bool empty(T &&);
+} // namespace absl
+
+namespace test {
+template 
+void empty(T &&);
+} // namespace test
+
+namespace base {
+template 
+struct base_vector {
+void clear();
+};
+
+template 
+struct base_vector_clear_with_args {
+void clear(int i);
+};
+
+template 
+struct base_vector_clear_variable {
+int clear;
+};
+
+struct base_vector_non_dependent {
+void clear();
+};
+
+template 
+struct vector : base_vector {
+bool empty();
+};
+
+template 
+struct vector_clear_with_args : base_vector_clear_with_args {
+bool empty();
+};
+
+template 
+struct vector_clear_variable : base_vector_clear_variable {
+bool empty();
+};
+
+template 
+struct vector_non_dependent : base_vector_non_dependent {
+bool empty();
+};
+
+template 
+bool empty(T &&);
+
+} // namespace base
+
+namespace qualifiers {
+template 
+struct vector_with_const_clear {
+  bool empty() const;
+  void clear() const;
+};
+
+template 
+struct vector_with_const_empty {
+  bool empty() const;
+  void clear();
+};
+
+template 
+struct vector_with_volatile_clear {
+  bool empty() volatile;
+  void clear() volatile;
+};
+
+template 
+struct vector_with_volatile_empty {
+  bool empty() volatile;
+  void clear();
+};
+
+template 
+bool empty(T &&);
+} // namespace qualifiers
+
+
+void test_member_empty() {
+  {
+std::vector v;
+v.empty();
+// CHECK-MESSAGES: :[[#@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+std::vector_with_void_empty v;
+v.empty();
+// no-warning
+  }
+
+  {
+std::vector_with_clear v;
+v.empty();
+// CHECK-MESSAGES: :[[#@LINE-1]]:5: warning: ignoring the result of 'empty()'; did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}
+  }
+
+  {
+std::vector_with_int_empty v;
+v.empty();
+// CHECK-MESSAGES: :[[#@LINE-1]]:5: warning: ignoring the result of 'empty()'; did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}
+  }
+
+  {
+std::vector_with_clear_args v;
+v.empty();
+// CHECK-MESSAGES: :[[#@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+std::vector_with_clear_variable v;
+v.empty();
+// CHECK-MESSAGES: :[[#@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+absl::string s;
+s.empty();
+// CHECK-MESSAGES: :[[#@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+absl::string_with_void_empty s;
+s.empty();
+// no-warning
+  }
+
+  {
+absl::string_with_clear s;
+s.empty();
+// CHECK-MESSAGES: :[[#@LINE-1]]:5: warning: ignoring the result of 'empty()'; did you mean 'clear()'? 

[PATCH] D128372: [Clang-Tidy] Empty Check

[Denis Nikitin via Phabricator via cfe-commits]

denik added a comment.

In D128372#3965162 , @MaskRay wrote:

> I just took a glance and the code looks reasonable.

@MaskRay thanks for the review! I have updated the patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

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

[PATCH] D128372: [Clang-Tidy] Empty Check

[Denis Nikitin via Phabricator via cfe-commits]

denik updated this revision to Diff 479684.
denik added a comment.

Replaced deprecated [[@LINE-1]]


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone/standalone-empty.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp
@@ -0,0 +1,578 @@
+// RUN: %check_clang_tidy %s bugprone-standalone-empty %t
+
+namespace std {
+template 
+struct vector {
+  bool empty();
+};
+
+template 
+struct vector_with_clear {
+  bool empty();
+  void clear();
+};
+
+template 
+struct vector_with_void_empty {
+  void empty();
+  void clear();
+};
+
+template 
+struct vector_with_int_empty {
+  int empty();
+  void clear();
+};
+
+template 
+struct vector_with_clear_args {
+  bool empty();
+  void clear(int i);
+};
+
+template 
+struct vector_with_clear_variable {
+  bool empty();
+  int clear;
+};
+
+template 
+bool empty(T &&);
+
+} // namespace std
+
+namespace absl {
+struct string {
+  bool empty();
+};
+
+struct string_with_clear {
+  bool empty();
+  void clear();
+};
+
+struct string_with_void_empty {
+  void empty();
+  void clear();
+};
+
+struct string_with_int_empty {
+  int empty();
+  void clear();
+};
+
+struct string_with_clear_args {
+  bool empty();
+  void clear(int i);
+};
+
+struct string_with_clear_variable {
+  bool empty();
+  int clear;
+};
+
+template 
+bool empty(T &&);
+} // namespace absl
+
+namespace test {
+template 
+void empty(T &&);
+} // namespace test
+
+namespace base {
+template 
+struct base_vector {
+void clear();
+};
+
+template 
+struct base_vector_clear_with_args {
+void clear(int i);
+};
+
+template 
+struct base_vector_clear_variable {
+int clear;
+};
+
+struct base_vector_non_dependent {
+void clear();
+};
+
+template 
+struct vector : base_vector {
+bool empty();
+};
+
+template 
+struct vector_clear_with_args : base_vector_clear_with_args {
+bool empty();
+};
+
+template 
+struct vector_clear_variable : base_vector_clear_variable {
+bool empty();
+};
+
+template 
+struct vector_non_dependent : base_vector_non_dependent {
+bool empty();
+};
+
+template 
+bool empty(T &&);
+
+} // namespace base
+
+namespace qualifiers {
+template 
+struct vector_with_const_clear {
+  bool empty() const;
+  void clear() const;
+};
+
+template 
+struct vector_with_const_empty {
+  bool empty() const;
+  void clear();
+};
+
+template 
+struct vector_with_volatile_clear {
+  bool empty() volatile;
+  void clear() volatile;
+};
+
+template 
+struct vector_with_volatile_empty {
+  bool empty() volatile;
+  void clear();
+};
+
+template 
+bool empty(T &&);
+} // namespace qualifiers
+
+
+void test_member_empty() {
+  {
+std::vector v;
+v.empty();
+// CHECK-MESSAGES: :[[#@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+std::vector_with_void_empty v;
+v.empty();
+// no-warning
+  }
+
+  {
+std::vector_with_clear v;
+v.empty();
+// CHECK-MESSAGES: :[[#@LINE-1]]:5: warning: ignoring the result of 'empty()'; did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}
+  }
+
+  {
+std::vector_with_int_empty v;
+v.empty();
+// CHECK-MESSAGES: :[[#@LINE-1]]:5: warning: ignoring the result of 'empty()'; did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}
+  }
+
+  {
+std::vector_with_clear_args v;
+v.empty();
+// CHECK-MESSAGES: :[[#@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+std::vector_with_clear_variable v;
+v.empty();
+// CHECK-MESSAGES: :[[#@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+absl::string s;
+s.empty();
+// CHECK-MESSAGES: :[[#@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+absl::string_with_void_empty s;
+s.empty();
+// no-warning
+  }
+
+  {
+absl::string_with_clear s;
+s.empty();
+// CHECK-MESSAGES: :[[#@LINE-1]]:5: warning: ignoring the result of 'empty()'; did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  s.clear();{{$}}
+  }
+
+  {
+absl::string_with_int_empty s;
+s.empty();
+// CHECK-MESSAGES: 

[PATCH] D128372: [Clang-Tidy] Empty Check

[Denis Nikitin via Phabricator via cfe-commits]

denik updated this revision to Diff 479677.
denik marked 2 inline comments as done.
denik added a comment.

Format changes.

Removed blank lines and fixed test types.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone/standalone-empty.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp
@@ -0,0 +1,578 @@
+// RUN: %check_clang_tidy %s bugprone-standalone-empty %t
+
+namespace std {
+template 
+struct vector {
+  bool empty();
+};
+
+template 
+struct vector_with_clear {
+  bool empty();
+  void clear();
+};
+
+template 
+struct vector_with_void_empty {
+  void empty();
+  void clear();
+};
+
+template 
+struct vector_with_int_empty {
+  int empty();
+  void clear();
+};
+
+template 
+struct vector_with_clear_args {
+  bool empty();
+  void clear(int i);
+};
+
+template 
+struct vector_with_clear_variable {
+  bool empty();
+  int clear;
+};
+
+template 
+bool empty(T &&);
+
+} // namespace std
+
+namespace absl {
+struct string {
+  bool empty();
+};
+
+struct string_with_clear {
+  bool empty();
+  void clear();
+};
+
+struct string_with_void_empty {
+  void empty();
+  void clear();
+};
+
+struct string_with_int_empty {
+  int empty();
+  void clear();
+};
+
+struct string_with_clear_args {
+  bool empty();
+  void clear(int i);
+};
+
+struct string_with_clear_variable {
+  bool empty();
+  int clear;
+};
+
+template 
+bool empty(T &&);
+} // namespace absl
+
+namespace test {
+template 
+void empty(T &&);
+} // namespace test
+
+namespace base {
+template 
+struct base_vector {
+void clear();
+};
+
+template 
+struct base_vector_clear_with_args {
+void clear(int i);
+};
+
+template 
+struct base_vector_clear_variable {
+int clear;
+};
+
+struct base_vector_non_dependent {
+void clear();
+};
+
+template 
+struct vector : base_vector {
+bool empty();
+};
+
+template 
+struct vector_clear_with_args : base_vector_clear_with_args {
+bool empty();
+};
+
+template 
+struct vector_clear_variable : base_vector_clear_variable {
+bool empty();
+};
+
+template 
+struct vector_non_dependent : base_vector_non_dependent {
+bool empty();
+};
+
+template 
+bool empty(T &&);
+
+} // namespace base
+
+namespace qualifiers {
+template 
+struct vector_with_const_clear {
+  bool empty() const;
+  void clear() const;
+};
+
+template 
+struct vector_with_const_empty {
+  bool empty() const;
+  void clear();
+};
+
+template 
+struct vector_with_volatile_clear {
+  bool empty() volatile;
+  void clear() volatile;
+};
+
+template 
+struct vector_with_volatile_empty {
+  bool empty() volatile;
+  void clear();
+};
+
+template 
+bool empty(T &&);
+} // namespace qualifiers
+
+
+void test_member_empty() {
+  {
+std::vector v;
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+std::vector_with_void_empty v;
+v.empty();
+// no-warning
+  }
+
+  {
+std::vector_with_clear v;
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()'; did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}
+  }
+
+  {
+std::vector_with_int_empty v;
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()'; did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}
+  }
+
+  {
+std::vector_with_clear_args v;
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+std::vector_with_clear_variable v;
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+absl::string s;
+s.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+absl::string_with_void_empty s;
+s.empty();
+// no-warning
+  }
+
+  {
+absl::string_with_clear s;
+s.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()'; did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  s.clear();{{$}}
+  }
+
+  {
+

[PATCH] D128372: [Clang-Tidy] Empty Check

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

I just took a glance and the code looks reasonable.




Comment at: clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp:131
+bool HasClear = !Candidates.empty();
+
+if (HasClear) {

delete blank line



Comment at: clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp:160
+const Expr *Arg = NonMemberCall->getArg(0);
+
+CXXRecordDecl *ArgRecordDecl = Arg->getType()->getAsCXXRecordDecl();

delete blank line



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp:161
+std::vector v;
+
+v.empty();

unneeded blank line

ditto below

They just make tests less compact without good values.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp:177
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 
'empty()'; did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}

`[[@LINE-1]]` is deprecated lit syntax. Use `[[#@LINE-1]]`. Don't use the 
prevailing style in clang-tidy tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

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

[PATCH] D128372: [Clang-Tidy] Empty Check

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added a comment.

In D128372#3957982 , @cjdb wrote:

> @njames93 if you don't have any further concerns, I am going to merge this on 
> Friday afternoon, as it will have been four months since there has been a 
> maintainer's input.

@cjdb I have raised a concern about the lack of reviewer feedback in clang-tidy 
here:
https://discourse.llvm.org/t/rfc-improve-code-review-process-for-clang-tidy/66740

If more people express their concerns perhaps it can be escalated to the point 
where action is taken?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

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

[PATCH] D128372: [Clang-Tidy] Empty Check

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

@njames93 if you don't have any further concerns, I am going to merge this on 
Friday afternoon, as it will have been four months since there has been a 
maintainer's input.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

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

[PATCH] D128372: [Clang-Tidy] Empty Check

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

In D128372#3883214 , @denik wrote:

> @Eugene.Zelenko , could you please stamp the patch if you don't have any 
> other concerns?

Please wait for developers approval. I look only for trivial issues.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

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

[PATCH] D128372: [Clang-Tidy] Empty Check

[Denis Nikitin via Phabricator via cfe-commits]

denik added a comment.

@Eugene.Zelenko , could you please stamp the patch if you don't have any other 
concerns?

@njames93 , if you don't  have spare cycles could you please suggest other 
reviewers?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

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

[PATCH] D128372: [Clang-Tidy] Empty Check

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

@LegalizeAdulthood @njames93 is there anything actionable that's left for this 
patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

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

[PATCH] D128372: [Clang-Tidy] Empty Check

[Denis Nikitin via Phabricator via cfe-commits]

denik added a comment.

@LegalizeAdulthood, @njames93, friendly ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

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

[PATCH] D128372: [Clang-Tidy] Empty Check

[Denis Nikitin via Phabricator via cfe-commits]

denik added a comment.

@LegalizeAdulthood, @njames93, is there anything else we should address in this 
change?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

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

[PATCH] D128372: [Clang-Tidy] Empty Check

[Denis Nikitin via Phabricator via cfe-commits]

denik added a comment.

Any updates from the reviewers?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

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

[PATCH] D128372: [Clang-Tidy] Empty Check

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

Drive-by comments: this patch may need a review from common reviewers like 
@njames93.

I assume that @LegalizeAdulthood's comment (about the file hierarchy since 
there was a big reorganization few months ago) has been addressed. If you are 
concerned with unable to contact @LegalizeAdulthood, I think folks like 
@njames93 can decive moving forward the patch despite the "Request Changes" 
state. That said, it'll always be good to give @LegalizeAdulthood some time 
when the patch is in a ready-to-submit state.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

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

[PATCH] D128372: [Clang-Tidy] Empty Check

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

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

[PATCH] D128372: [Clang-Tidy] Empty Check

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

This patch has been open for weeks without maintainer input, despite repeated 
requests, and @abrahamcd finishes his internship this Friday. We would very 
much appreciate direction on whether or not D128372 
 is ready to be merged.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

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

[PATCH] D128372: [Clang-Tidy] Empty Check

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd updated this revision to Diff 454635.
abrahamcd marked 2 inline comments as done.
abrahamcd added a comment.

Formatting fixes and synchronization of documentation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone/standalone-empty.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp
@@ -0,0 +1,629 @@
+// RUN: %check_clang_tidy %s bugprone-standalone-empty %t
+
+namespace std {
+template 
+struct vector {
+  bool empty();
+};
+
+template 
+struct vector_with_clear {
+  bool empty();
+  void clear();
+};
+
+template 
+struct vector_with_void_empty {
+  void empty();
+  void clear();
+};
+
+template 
+struct vector_with_int_empty {
+  int empty();
+  void clear();
+};
+
+template 
+struct vector_with_clear_args {
+  bool empty();
+  void clear(int i);
+};
+
+template 
+struct vector_with_clear_variable {
+  bool empty();
+  int clear;
+};
+
+template 
+bool empty(T &&);
+
+} // namespace std
+
+namespace absl {
+struct string {
+  bool empty();
+};
+
+struct string_with_clear {
+  bool empty();
+  void clear();
+};
+
+struct string_with_void_empty {
+  void empty();
+  void clear();
+};
+
+struct string_with_int_empty {
+  int empty();
+  void clear();
+};
+
+struct string_with_clear_args {
+  bool empty();
+  void clear(int i);
+};
+
+struct string_with_clear_variable {
+  bool empty();
+  int clear;
+};
+
+template 
+bool empty(T &&);
+} // namespace absl
+
+namespace test {
+template 
+void empty(T &&);
+} // namespace test
+
+namespace base {
+template 
+struct base_vector {
+void clear();
+};
+
+template 
+struct base_vector_clear_with_args {
+void clear(int i);
+};
+
+template 
+struct base_vector_clear_variable {
+int clear;
+};
+
+struct base_vector_non_dependent {
+void clear();
+};
+
+template 
+struct vector : base_vector {
+bool empty();
+};
+
+template 
+struct vector_clear_with_args : base_vector_clear_with_args {
+bool empty();
+};
+
+template 
+struct vector_clear_variable : base_vector_clear_variable {
+bool empty();
+};
+
+template 
+struct vector_non_dependent : base_vector_non_dependent {
+bool empty();
+};
+
+template 
+bool empty(T &&);
+
+} // namespace base
+
+namespace qualifiers {
+template 
+struct vector_with_const_clear {
+  bool empty() const;
+  void clear() const;
+};
+
+template 
+struct vector_with_const_empty {
+  bool empty() const;
+  void clear();
+};
+
+template 
+struct vector_with_volatile_clear {
+  bool empty() volatile;
+  void clear() volatile;
+};
+
+template 
+struct vector_with_volatile_empty {
+  bool empty() volatile;
+  void clear();
+};
+
+template 
+bool empty(T &&);
+} // namespace qualifiers
+
+
+int test_member_empty() {
+  {
+std::vector v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+std::vector_with_void_empty v;
+
+v.empty();
+// no-warning
+  }
+
+  {
+std::vector_with_clear v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()'; did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}
+  }
+
+  {
+std::vector_with_int_empty v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()'; did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}
+  }
+
+  {
+std::vector_with_clear_args v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+std::vector_with_clear_variable v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+absl::string s;
+
+s.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+absl::string_with_void_empty s;
+
+s.empty();
+// no-warning
+  }
+
+  {
+absl::string_with_clear s;
+
+s.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()'; did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  s.clear();{{$}}
+  }
+

[PATCH] D128372: [Clang-Tidy] Empty Check

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:127
+
+  Warns when `empty()` is used on a range and the result is ignored. Suggests 
`clear()` as an alternative
+  if it is an existing member function.

Please synchronize with first statement in documentation.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp:629
+  }
+
+

Excessive newline.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

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

[PATCH] D128372: [Clang-Tidy] Empty Check

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

@njames93, @LegalizeAdulthood, are there any further concerns regarding this CL?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

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

[PATCH] D128372: [Clang-Tidy] Empty Check

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd marked 2 inline comments as done.
abrahamcd added a comment.

Hi, checking in on this again, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

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

[PATCH] D128372: [Clang-Tidy] Empty Check

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd updated this revision to Diff 451212.
abrahamcd added a comment.

Minor formatting/naming fixes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone/standalone-empty.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp
@@ -0,0 +1,631 @@
+// RUN: %check_clang_tidy %s bugprone-standalone-empty %t
+
+namespace std {
+template 
+struct vector {
+  bool empty();
+};
+
+template 
+struct vector_with_clear {
+  bool empty();
+  void clear();
+};
+
+template 
+struct vector_with_void_empty {
+  void empty();
+  void clear();
+};
+
+template 
+struct vector_with_int_empty {
+  int empty();
+  void clear();
+};
+
+template 
+struct vector_with_clear_args {
+  bool empty();
+  void clear(int i);
+};
+
+template 
+struct vector_with_clear_variable {
+  bool empty();
+  int clear;
+};
+
+template 
+bool empty(T &&);
+
+} // namespace std
+
+namespace absl {
+struct string {
+  bool empty();
+};
+
+struct string_with_clear {
+  bool empty();
+  void clear();
+};
+
+struct string_with_void_empty {
+  void empty();
+  void clear();
+};
+
+struct string_with_int_empty {
+  int empty();
+  void clear();
+};
+
+struct string_with_clear_args {
+  bool empty();
+  void clear(int i);
+};
+
+struct string_with_clear_variable {
+  bool empty();
+  int clear;
+};
+
+template 
+bool empty(T &&);
+} // namespace absl
+
+namespace test {
+template 
+void empty(T &&);
+} // namespace test
+
+namespace base {
+template 
+struct base_vector {
+void clear();
+};
+
+template 
+struct base_vector_clear_with_args {
+void clear(int i);
+};
+
+template 
+struct base_vector_clear_variable {
+int clear;
+};
+
+struct base_vector_non_dependent {
+void clear();
+};
+
+template 
+struct vector : base_vector {
+bool empty();
+};
+
+template 
+struct vector_clear_with_args : base_vector_clear_with_args {
+bool empty();
+};
+
+template 
+struct vector_clear_variable : base_vector_clear_variable {
+bool empty();
+};
+
+template 
+struct vector_non_dependent : base_vector_non_dependent {
+bool empty();
+};
+
+template 
+bool empty(T &&);
+
+} // namespace base
+
+namespace qualifiers {
+template 
+struct vector_with_const_clear {
+  bool empty() const;
+  void clear() const;
+};
+
+template 
+struct vector_with_const_empty {
+  bool empty() const;
+  void clear();
+};
+
+template 
+struct vector_with_volatile_clear {
+  bool empty() volatile;
+  void clear() volatile;
+};
+
+template 
+struct vector_with_volatile_empty {
+  bool empty() volatile;
+  void clear();
+};
+
+template 
+bool empty(T &&);
+} // namespace qualifiers
+
+
+int test_member_empty() {
+  {
+std::vector v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+std::vector_with_void_empty v;
+
+v.empty();
+// no-warning
+  }
+
+  {
+std::vector_with_clear v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()'; did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}
+  }
+
+  {
+std::vector_with_int_empty v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()'; did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}
+  }
+
+  {
+std::vector_with_clear_args v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+std::vector_with_clear_variable v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+absl::string s;
+
+s.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+absl::string_with_void_empty s;
+
+s.empty();
+// no-warning
+  }
+
+  {
+absl::string_with_clear s;
+
+s.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()'; did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  s.clear();{{$}}
+  }
+
+  {
+absl::string_with_int_empty s;
+
+s.empty();
+// 

[PATCH] D128372: [Clang-Tidy] Empty Check

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd updated this revision to Diff 451209.
abrahamcd marked 5 inline comments as done.
abrahamcd added a comment.

Adds check for the compatibility of qualifiers on the clear method and the 
object it is called on.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone/standalone-empty.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp
@@ -0,0 +1,632 @@
+// RUN: %check_clang_tidy %s bugprone-standalone-empty %t
+
+namespace std {
+template 
+struct vector {
+  bool empty();
+};
+
+template 
+struct vector_with_clear {
+  bool empty();
+  void clear();
+};
+
+template 
+struct vector_with_void_empty {
+  void empty();
+  void clear();
+};
+
+template 
+struct vector_with_int_empty {
+  int empty();
+  void clear();
+};
+
+template 
+struct vector_with_clear_args {
+  bool empty();
+  void clear(int i);
+};
+
+template 
+struct vector_with_clear_variable {
+  bool empty();
+  int clear;
+};
+
+template 
+bool empty(T &&);
+
+} // namespace std
+
+namespace absl {
+struct string {
+  bool empty();
+};
+
+struct string_with_clear {
+  bool empty();
+  void clear();
+};
+
+struct string_with_void_empty {
+  void empty();
+  void clear();
+};
+
+struct string_with_int_empty {
+  int empty();
+  void clear();
+};
+
+struct string_with_clear_args {
+  bool empty();
+  void clear(int i);
+};
+
+struct string_with_clear_variable {
+  bool empty();
+  int clear;
+};
+
+template 
+bool empty(T &&);
+} // namespace absl
+
+namespace test {
+template 
+void empty(T &&);
+} // namespace test
+
+namespace base {
+template 
+struct base_vector {
+void clear();
+};
+
+template 
+struct base_vector_clear_with_args {
+void clear(int i);
+};
+
+template 
+struct base_vector_clear_variable {
+int clear;
+};
+
+struct base_vector_non_dependent {
+void clear();
+};
+
+template 
+struct vector : base_vector {
+bool empty();
+};
+
+template 
+struct vector_clear_with_args : base_vector_clear_with_args {
+bool empty();
+};
+
+template 
+struct vector_clear_variable : base_vector_clear_variable {
+bool empty();
+};
+
+template 
+struct vector_non_dependent : base_vector_non_dependent {
+bool empty();
+};
+
+template 
+bool empty(T &&);
+
+} // namespace base
+
+
+
+int test_member_empty() {
+  {
+std::vector v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+std::vector_with_void_empty v;
+
+v.empty();
+// no-warning
+  }
+
+  {
+std::vector_with_clear v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()'; did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}
+  }
+
+  {
+std::vector_with_int_empty v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()'; did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}
+  }
+
+  {
+std::vector_with_clear_args v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+std::vector_with_clear_variable v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+absl::string s;
+
+s.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+absl::string_with_void_empty s;
+
+s.empty();
+// no-warning
+  }
+
+  {
+absl::string_with_clear s;
+
+s.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()'; did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  s.clear();{{$}}
+  }
+
+  {
+absl::string_with_int_empty s;
+
+s.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()'; did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  s.clear();{{$}}
+  }
+
+  {
+absl::string_with_clear_args s;
+
+s.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  

[PATCH] D128372: [Clang-Tidy] Empty Check

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp:177-178
+  diag(NonMemberLoc, "ignoring the result of '%0', did you mean 
'clear()'?")
+  << llvm::dyn_cast(NonMemberCall->getCalleeDecl())
+ ->getQualifiedNameAsString()
+  << FixItHint::CreateReplacement(ReplacementRange, ReplacementText);

abrahamcd wrote:
> njames93 wrote:
> > Diagnostics can accept args as `const NamedDecl *`, so you can omit the 
> > call to `getQualifiedNameAsString()`.
> > The use of `dyn_cast` here is suspicious. If the CalleeDecl isn't a 
> > `NamedDecl`, this would assert when you try and call 
> > `getQualifiedNameAsString`, but equally I can't see any case why the 
> > CalleeDecl wouldn't be a `NamedDecl`. @rsmith Can you think of any 
> > situation where this could happen?
> Seems that without `getQualifiedNameAsString()` I get longer less-readable 
> versions of the name, e.g. `'empty &>'` instead of just 
> `'std::empty'`. Do you think the extra information is helpful here?
I'm partial to `'std::empty'` because that's presumably what the user will 
type, and so it'll be easier for them to understand. `'empty 
&>'` isn't useful because we almost always defer to type deduction.



Comment at: clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp:57-72
+  using ast_matchers::callee;
+  using ast_matchers::callExpr;
+  using ast_matchers::cxxMemberCallExpr;
+  using ast_matchers::cxxMethodDecl;
+  using ast_matchers::expr;
+  using ast_matchers::functionDecl;
+  using ast_matchers::hasName;

njames93 wrote:
> These using declarations are annoying, the common convention in tidy checks 
> is to just use a global `using namespace ast_matchers` at the top of the file.
Adding them just after the namespaces are opened would be good.

I've had issues with refactoring llvm-project files in the past that have 
global //using-directives//, so I'd prefer it if we stuck with the 
//using-declarations// please. If they're a part of the preamble, they become 
additional lines that will be scrolled over.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp:120-121
+llvm::find_if(Methods, [](const CXXMethodDecl *F) {
+  return F->getDeclName().getAsString() == "clear" &&
+ F->getMinRequiredArguments() == 0;
+});

njames93 wrote:
> We also need to check qualifiers. If there exists a clear method but its 
> unavailable due to it not being const when our member is const. The fix would 
> result in a compile error.
Good catch. We probably shouldn't be suggesting `clear()` on a const object 
anyway.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

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

[PATCH] D128372: [Clang-Tidy] Empty Check

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp:177-178
+  diag(NonMemberLoc, "ignoring the result of '%0', did you mean 
'clear()'?")
+  << llvm::dyn_cast(NonMemberCall->getCalleeDecl())
+ ->getQualifiedNameAsString()
+  << FixItHint::CreateReplacement(ReplacementRange, ReplacementText);

njames93 wrote:
> Diagnostics can accept args as `const NamedDecl *`, so you can omit the call 
> to `getQualifiedNameAsString()`.
> The use of `dyn_cast` here is suspicious. If the CalleeDecl isn't a 
> `NamedDecl`, this would assert when you try and call 
> `getQualifiedNameAsString`, but equally I can't see any case why the 
> CalleeDecl wouldn't be a `NamedDecl`. @rsmith Can you think of any situation 
> where this could happen?
Seems that without `getQualifiedNameAsString()` I get longer less-readable 
versions of the name, e.g. `'empty &>'` instead of just 
`'std::empty'`. Do you think the extra information is helpful here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

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

[PATCH] D128372: [Clang-Tidy] Empty Check

[Nathan James via Phabricator via cfe-commits]

njames93 added inline comments.



Comment at: clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp:124
+  return isa(ND) &&
+ llvm::dyn_cast(ND)->getMinRequiredArguments() 
==
+ 0;

`dyn_cast` isn't needed here as we have already checked we are dealing with a 
`CXXMethodDecl`.



Comment at: clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp:132
+  diag(MemberLoc,
+   "ignoring the result of 'empty()', did you mean 'clear()'? ")
+  << FixItHint::CreateReplacement(ReplacementRange, "clear");

The 'did you mean' diagnostics in clang all use a semicolon `;` instead of a 
comma between the message and suggestion.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp:177-178
+  diag(NonMemberLoc, "ignoring the result of '%0', did you mean 
'clear()'?")
+  << llvm::dyn_cast(NonMemberCall->getCalleeDecl())
+ ->getQualifiedNameAsString()
+  << FixItHint::CreateReplacement(ReplacementRange, ReplacementText);

Diagnostics can accept args as `const NamedDecl *`, so you can omit the call to 
`getQualifiedNameAsString()`.
The use of `dyn_cast` here is suspicious. If the CalleeDecl isn't a 
`NamedDecl`, this would assert when you try and call 
`getQualifiedNameAsString`, but equally I can't see any case why the CalleeDecl 
wouldn't be a `NamedDecl`. @rsmith Can you think of any situation where this 
could happen?



Comment at: clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp:57-72
+  using ast_matchers::callee;
+  using ast_matchers::callExpr;
+  using ast_matchers::cxxMemberCallExpr;
+  using ast_matchers::cxxMethodDecl;
+  using ast_matchers::expr;
+  using ast_matchers::functionDecl;
+  using ast_matchers::hasName;

These using declarations are annoying, the common convention in tidy checks is 
to just use a global `using namespace ast_matchers` at the top of the file.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp:120-121
+llvm::find_if(Methods, [](const CXXMethodDecl *F) {
+  return F->getDeclName().getAsString() == "clear" &&
+ F->getMinRequiredArguments() == 0;
+});

We also need to check qualifiers. If there exists a clear method but its 
unavailable due to it not being const when our member is const. The fix would 
result in a compile error.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

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

[PATCH] D128372: [Clang-Tidy] Empty Check

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd updated this revision to Diff 450430.
abrahamcd added a comment.

Adds non-dependent base class test case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone/standalone-empty.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp
@@ -0,0 +1,508 @@
+// RUN: %check_clang_tidy %s bugprone-standalone-empty %t
+
+namespace std {
+template 
+struct vector {
+  bool empty();
+};
+
+template 
+struct vector_with_clear {
+  bool empty();
+  void clear();
+};
+
+template 
+struct vector_with_void_empty {
+  void empty();
+  void clear();
+};
+
+template 
+struct vector_with_int_empty {
+  int empty();
+  void clear();
+};
+
+template 
+struct vector_with_clear_args {
+  bool empty();
+  void clear(int i);
+};
+
+template 
+struct vector_with_clear_variable {
+  bool empty();
+  int clear;
+};
+
+template 
+bool empty(T &&);
+
+} // namespace std
+
+namespace absl {
+struct string {
+  bool empty();
+};
+
+struct string_with_clear {
+  bool empty();
+  void clear();
+};
+
+struct string_with_void_empty {
+  void empty();
+  void clear();
+};
+
+struct string_with_int_empty {
+  int empty();
+  void clear();
+};
+
+struct string_with_clear_args {
+  bool empty();
+  void clear(int i);
+};
+
+struct string_with_clear_variable {
+  bool empty();
+  int clear;
+};
+
+template 
+bool empty(T &&);
+} // namespace absl
+
+namespace test {
+template 
+void empty(T &&);
+} // namespace test
+
+namespace base {
+template 
+struct base_vector {
+void clear();
+};
+
+template 
+struct base_vector_clear_with_args {
+void clear(int i);
+};
+
+template 
+struct base_vector_clear_variable {
+int clear;
+};
+
+struct base_vector_non_dependent {
+void clear();
+};
+
+template 
+struct vector : base_vector {
+bool empty();
+};
+
+template 
+struct vector_clear_with_args : base_vector_clear_with_args {
+bool empty();
+};
+
+template 
+struct vector_clear_variable : base_vector_clear_variable {
+bool empty();
+};
+
+template 
+struct vector_non_dependent : base_vector_non_dependent {
+bool empty();
+};
+
+template 
+bool empty(T &&);
+
+}
+
+int test_member_empty() {
+  {
+std::vector v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+std::vector_with_void_empty v;
+
+v.empty();
+// no-warning
+  }
+
+  {
+std::vector_with_clear v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}
+  }
+
+  {
+std::vector_with_int_empty v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}
+  }
+
+  {
+std::vector_with_clear_args v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+std::vector_with_clear_variable v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+absl::string s;
+
+s.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+absl::string_with_void_empty s;
+
+s.empty();
+// no-warning
+  }
+
+  {
+absl::string_with_clear s;
+
+s.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  s.clear();{{$}}
+  }
+
+  {
+absl::string_with_int_empty s;
+
+s.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  s.clear();{{$}}
+  }
+
+  {
+absl::string_with_clear_args s;
+
+s.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+absl::string_with_clear_variable s;
+
+s.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the 

[PATCH] D128372: [Clang-Tidy] Empty Check

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd updated this revision to Diff 450407.
abrahamcd added a comment.

Checks for `clear()` in base classes as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone/standalone-empty.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp
@@ -0,0 +1,483 @@
+// RUN: %check_clang_tidy %s bugprone-standalone-empty %t
+
+namespace std {
+template 
+struct vector {
+  bool empty();
+};
+
+template 
+struct vector_with_clear {
+  bool empty();
+  void clear();
+};
+
+template 
+struct vector_with_void_empty {
+  void empty();
+  void clear();
+};
+
+template 
+struct vector_with_int_empty {
+  int empty();
+  void clear();
+};
+
+template 
+struct vector_with_clear_args {
+  bool empty();
+  void clear(int i);
+};
+
+template 
+struct vector_with_clear_variable {
+  bool empty();
+  int clear;
+};
+
+template 
+bool empty(T &&);
+
+} // namespace std
+
+namespace absl {
+struct string {
+  bool empty();
+};
+
+struct string_with_clear {
+  bool empty();
+  void clear();
+};
+
+struct string_with_void_empty {
+  void empty();
+  void clear();
+};
+
+struct string_with_int_empty {
+  int empty();
+  void clear();
+};
+
+struct string_with_clear_args {
+  bool empty();
+  void clear(int i);
+};
+
+struct string_with_clear_variable {
+  bool empty();
+  int clear;
+};
+
+template 
+bool empty(T &&);
+} // namespace absl
+
+namespace test {
+template 
+void empty(T &&);
+} // namespace test
+
+namespace base {
+template 
+struct base_vector {
+void clear();
+};
+
+template 
+struct base_vector_clear_with_args {
+void clear(int i);
+};
+
+template 
+struct base_vector_clear_variable {
+int clear;
+};
+
+template 
+struct vector : base_vector {
+bool empty();
+};
+
+template 
+struct vector_clear_with_args : base_vector_clear_with_args {
+bool empty();
+};
+
+template 
+struct vector_clear_variable : base_vector_clear_variable {
+bool empty();
+};
+
+template 
+bool empty(T &&);
+
+}
+
+int test_member_empty() {
+  {
+std::vector v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+std::vector_with_void_empty v;
+
+v.empty();
+// no-warning
+  }
+
+  {
+std::vector_with_clear v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}
+  }
+
+  {
+std::vector_with_int_empty v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}
+  }
+
+  {
+std::vector_with_clear_args v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+std::vector_with_clear_variable v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+absl::string s;
+
+s.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+absl::string_with_void_empty s;
+
+s.empty();
+// no-warning
+  }
+
+  {
+absl::string_with_clear s;
+
+s.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  s.clear();{{$}}
+  }
+
+  {
+absl::string_with_int_empty s;
+
+s.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  s.clear();{{$}}
+  }
+
+  {
+absl::string_with_clear_args s;
+
+s.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+absl::string_with_clear_variable s;
+
+s.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+}
+
+int test_qualified_empty() {
+  {
+absl::string_with_clear v;
+
+std::empty(v);
+

[PATCH] D128372: [Clang-Tidy] Empty Check

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

rsmith added inline comments.



Comment at: clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp:23
+#include "llvm/ADT/SmallVector.h"
+#include 
+





Comment at: clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp:149
+  return;
+CXXRecordDecl::method_range Methods = ArgRecordDecl->methods();
+DeclContext::specific_decl_iterator Clear =

This only looks for direct members, not ones found in a base class. Eg, this 
will not find `clear()` in `llvm::SmallVector`. If you have a `Sema` 
object here, you could do a real name lookup; otherwise, you can use 
`CXXRecordDecl::lookupInBases` to do the lookup. You should be able to find 
some examples by searching for existing calls to that function.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

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

[PATCH] D128372: [Clang-Tidy] Empty Check

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd added a comment.

Hi all, I just wanted to check in on this again and see if any more feedback or 
progress could be made. Thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

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

[PATCH] D128372: [Clang-Tidy] Empty Check

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd added a comment.

Gentle ping!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

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

[PATCH] D128372: [Clang-Tidy] Empty Check

[Denis Nikitin via Phabricator via cfe-commits]

denik accepted this revision.
denik added a comment.

Thanks Abraham!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

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

[PATCH] D128372: [Clang-Tidy] Empty Check

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb accepted this revision.
cjdb added a comment.

Thanks for working on this! I'll merge this on your behalf once there's 
maintainer approval.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

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

[PATCH] D128372: [Clang-Tidy] Empty Check

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd updated this revision to Diff 443056.
abrahamcd marked 3 inline comments as done.
abrahamcd added a comment.

Adds argument 0 test case and refactoring based on feedback.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone/standalone-empty.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp
@@ -0,0 +1,321 @@
+// RUN: %check_clang_tidy %s bugprone-standalone-empty %t
+
+namespace std {
+template 
+struct vector {
+  bool empty();
+};
+
+template 
+struct vector_with_clear {
+  bool empty();
+  void clear();
+};
+
+template 
+struct vector_with_void_empty {
+  void empty();
+  void clear();
+};
+
+template 
+struct vector_with_int_empty {
+  int empty();
+  void clear();
+};
+
+template 
+bool empty(T &&);
+
+} // namespace std
+
+namespace absl {
+struct string {
+  bool empty();
+};
+
+struct string_with_clear {
+  bool empty();
+  void clear();
+};
+
+struct string_with_void_empty {
+  void empty();
+  void clear();
+};
+
+struct string_with_int_empty {
+  int empty();
+  void clear();
+};
+
+template 
+bool empty(T &&);
+} // namespace absl
+
+namespace test {
+template 
+void empty(T &&);
+} // namespace test
+
+int test_member_empty() {
+  {
+std::vector v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+std::vector_with_void_empty v;
+
+v.empty();
+// no-warning
+  }
+
+  {
+std::vector_with_clear v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}
+  }
+
+  {
+std::vector_with_int_empty v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}
+  }
+
+  {
+absl::string s;
+
+s.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+absl::string_with_void_empty s;
+
+s.empty();
+// no-warning
+  }
+
+  {
+absl::string_with_clear s;
+
+s.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  s.clear();{{$}}
+  }
+
+  {
+absl::string_with_int_empty s;
+
+s.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  s.clear();{{$}}
+  }
+}
+
+int test_qualified_empty() {
+  {
+absl::string_with_clear v;
+
+std::empty(v);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'std::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}
+
+absl::empty(v);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'absl::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}
+
+test::empty(v);
+// no-warning
+  }
+
+  {
+absl::string s;
+
+std::empty(s);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
+  }
+
+  {
+std::empty(0);
+// no-warning
+
+absl::empty(nullptr);
+// no-warning
+  }
+}
+
+int test_unqualified_empty() {
+  {
+std::vector v;
+
+empty(v);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
+  }
+
+  {
+std::vector_with_void_empty v;
+
+empty(v);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'std::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}
+  }
+
+  {
+std::vector_with_clear v;
+
+empty(v);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'std::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}
+  }
+
+  {
+std::vector_with_int_empty v;
+
+empty(v);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 

[PATCH] D128372: [Clang-Tidy] Empty Check

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added inline comments.



Comment at: clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp:128
+  << FixItHint::CreateReplacement(ReplacementRange, "clear");
+} else {
+  diag(MemberLoc, "ignoring the result of 'empty()'");

Let's eliminate some of these else-clauses by using early exits.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

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

[PATCH] D128372: [Clang-Tidy] Empty Check

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp:146
+CXXRecordDecl *ArgRecordDecl = Arg->getType()->getAsCXXRecordDecl();
+if (ArgRecordDecl == NULL)
+  return;

`nullptr`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

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

[PATCH] D128372: [Clang-Tidy] Empty Check

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd updated this revision to Diff 443017.
abrahamcd edited the summary of this revision.
abrahamcd added a comment.

Adds functionality for only warning on the case of unused return value
of the call to `empty()` and removes using-directive.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone/standalone-empty.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp
@@ -0,0 +1,313 @@
+// RUN: %check_clang_tidy %s bugprone-standalone-empty %t
+
+namespace std {
+template 
+struct vector {
+  bool empty();
+};
+
+template 
+struct vector_with_clear {
+  bool empty();
+  void clear();
+};
+
+template 
+struct vector_with_void_empty {
+  void empty();
+  void clear();
+};
+
+template 
+struct vector_with_int_empty {
+  int empty();
+  void clear();
+};
+
+template 
+bool empty(T &&);
+
+} // namespace std
+
+namespace absl {
+struct string {
+  bool empty();
+};
+
+struct string_with_clear {
+  bool empty();
+  void clear();
+};
+
+struct string_with_void_empty {
+  void empty();
+  void clear();
+};
+
+struct string_with_int_empty {
+  int empty();
+  void clear();
+};
+
+template 
+bool empty(T &&);
+} // namespace absl
+
+namespace test {
+template 
+void empty(T &&);
+} // namespace test
+
+int test_member_empty() {
+  {
+std::vector v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+std::vector_with_void_empty v;
+
+v.empty();
+// no-warning
+  }
+
+  {
+std::vector_with_clear v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}
+  }
+
+  {
+std::vector_with_int_empty v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}
+  }
+
+  {
+absl::string s;
+
+s.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+absl::string_with_void_empty s;
+
+s.empty();
+// no-warning
+  }
+
+  {
+absl::string_with_clear s;
+
+s.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  s.clear();{{$}}
+  }
+
+  {
+absl::string_with_int_empty s;
+
+s.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  s.clear();{{$}}
+  }
+}
+
+int test_qualified_empty() {
+  {
+absl::string_with_clear v;
+
+std::empty(v);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'std::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}
+
+absl::empty(v);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'absl::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}
+
+test::empty(v);
+// no-warning
+  }
+
+  {
+absl::string s;
+
+std::empty(s);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
+  }
+}
+
+int test_unqualified_empty() {
+  {
+std::vector v;
+
+empty(v);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
+  }
+
+  {
+std::vector_with_void_empty v;
+
+empty(v);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'std::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}
+  }
+
+  {
+std::vector_with_clear v;
+
+empty(v);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'std::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}
+  }
+
+  {
+std::vector_with_int_empty v;
+
+empty(v);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'std::empty', 

[PATCH] D128372: [Clang-Tidy] Empty Check

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd planned changes to this revision.
abrahamcd added a comment.

Stuck on determining unused return value, uploaded current state for @denik and 
@cjdb to take a look.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

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

[PATCH] D128372: [Clang-Tidy] Empty Check

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd updated this revision to Diff 440742.
abrahamcd added a comment.

Progress Update

Hit a roadblock with determining unused return value,
uploading current state for further work.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone/standalone-empty.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp
@@ -0,0 +1,244 @@
+// RUN: %check_clang_tidy %s bugprone-standalone-empty %t
+
+namespace std {
+template 
+struct vector {
+  bool empty();
+};
+
+template 
+struct vector_with_clear {
+  bool empty();
+  void clear();
+};
+
+template 
+struct vector_with_void_empty {
+  void empty();
+  void clear();
+};
+
+template 
+struct vector_with_int_empty {
+  int empty();
+  void clear();
+};
+
+template 
+bool empty(T &&);
+
+} // namespace std
+
+namespace absl {
+struct string {
+  bool empty();
+};
+
+struct string_with_clear {
+  bool empty();
+  void clear();
+};
+
+struct string_with_void_empty {
+  void empty();
+  void clear();
+};
+
+struct string_with_int_empty {
+  int empty();
+  void clear();
+};
+
+template 
+bool empty(T &&);
+} // namespace absl
+
+namespace test {
+template 
+void empty(T &&);
+} // namespace test
+
+int test_member_empty() {
+  {
+std::vector v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+
+bool test = v.empty();
+//(void) v.empty();
+// no-warning
+// NOTE: This test not currently passing.
+  }
+
+  {
+std::vector_with_void_empty v;
+
+v.empty();
+// no-warning
+  }
+
+  {
+std::vector_with_clear v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}
+  }
+
+  {
+std::vector_with_int_empty v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}
+  }
+
+  {
+absl::string s;
+
+s.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+absl::string_with_void_empty s;
+
+s.empty();
+// no-warning
+  }
+
+  {
+absl::string_with_clear s;
+
+s.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  s.clear();{{$}}
+  }
+
+  {
+absl::string_with_int_empty s;
+
+s.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  s.clear();{{$}}
+  }
+}
+
+int test_qualified_empty() {
+  {
+absl::string_with_clear v;
+
+std::empty(v);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'std::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}
+
+absl::empty(v);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'absl::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}
+
+test::empty(v);
+// no-warning
+  }
+
+  {
+absl::string s;
+
+std::empty(s);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
+  }
+}
+
+int test_unqualified_empty() {
+  {
+std::vector v;
+
+empty(v);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
+  }
+
+  {
+std::vector_with_void_empty v;
+
+empty(v);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'std::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}
+  }
+
+  {
+std::vector_with_clear v;
+
+empty(v);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'std::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}
+  }
+
+  {
+std::vector_with_int_empty v;
+
+empty(v);
+// CHECK-MESSAGES: 

[PATCH] D128372: [Clang-Tidy] Empty Check

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added inline comments.



Comment at: clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp:76
+
+auto Methods = Arg->getType()->getAsCXXRecordDecl()->methods();
+auto Clear = llvm::find_if(Methods, [](const CXXMethodDecl *F) {

denik wrote:
> Check `getAsCXXRecordDecl()` for null and add a test case.
We should also have a test to ensure `empty(0)` doesn't segfault.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

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

[PATCH] D128372: [Clang-Tidy] Empty Check

[Denis Nikitin via Phabricator via cfe-commits]

denik added a comment.

Thanks for adding a check! Please check my comments.




Comment at: clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp:76
+
+auto Methods = Arg->getType()->getAsCXXRecordDecl()->methods();
+auto Clear = llvm::find_if(Methods, [](const CXXMethodDecl *F) {

Check `getAsCXXRecordDecl()` for null and add a test case.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp:61
+
+int test_member_empty() {
+  std::vector s;

I think you also need `no-warning` test cases when the empty() result is used.  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

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

[PATCH] D128372: [Clang-Tidy] Empty Check

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd updated this revision to Diff 440346.
abrahamcd marked 7 inline comments as done.
abrahamcd added a comment.

Addressed review edits and clarity feedback.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone/standalone-empty.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp
@@ -0,0 +1,202 @@
+// RUN: %check_clang_tidy %s bugprone-standalone-empty %t
+
+namespace std {
+template 
+struct vector {
+  bool empty();
+};
+
+template 
+struct vector_with_clear {
+  bool empty();
+  void clear();
+};
+
+template 
+struct vector_with_void_empty {
+  void empty();
+  void clear();
+};
+
+template 
+struct vector_with_int_empty {
+  int empty();
+  void clear();
+};
+
+template 
+bool empty(T &&);
+
+} // namespace std
+
+namespace absl {
+struct string {
+  bool empty();
+};
+
+struct string_with_clear {
+  bool empty();
+  void clear();
+};
+
+struct string_with_void_empty {
+  void empty();
+  void clear();
+};
+
+struct string_with_int_empty {
+  int empty();
+  void clear();
+};
+
+template 
+bool empty(T &&);
+} // namespace absl
+
+namespace test {
+template 
+void empty(T &&);
+} // namespace test
+
+int test_member_empty() {
+  std::vector s;
+
+  s.empty();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+
+  std::vector_with_void_empty m;
+
+  m.empty();
+  // no-warning
+
+  std::vector_with_clear v;
+
+  v.empty();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}v.clear();{{$}}
+
+  std::vector_with_int_empty w;
+
+  w.empty();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}w.clear();{{$}}
+
+  absl::string x;
+
+  x.empty();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+
+  absl::string_with_void_empty y;
+
+  y.empty();
+  // no-warning
+
+  absl::string_with_clear z;
+
+  z.empty();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}z.clear();{{$}}
+
+  absl::string_with_int_empty a;
+
+  a.empty();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}a.clear();{{$}}
+}
+
+int test_qualified_empty() {
+  absl::string_with_clear v;
+
+  std::empty(v);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'std::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}v.clear();{{$}}
+
+  absl::empty(v);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'absl::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}v.clear();{{$}}
+
+  test::empty(v);
+  // no-warning
+
+  absl::string w;
+
+  std::empty(w);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
+
+}
+
+int test_unqualified_empty() {
+  std::vector v;
+
+  empty(v);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
+
+  std::vector_with_void_empty w;
+
+  empty(w);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'std::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}w.clear();{{$}}
+
+  std::vector_with_clear x;
+
+  empty(x);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'std::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}x.clear();{{$}}
+
+  std::vector_with_int_empty y;
+
+  empty(y);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'std::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}y.clear();{{$}}
+
+  absl::string z;
+
+  empty(z);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'absl::empty' [bugprone-standalone-empty]
+
+  absl::string_with_void_empty a;
+
+  empty(a);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 

[PATCH] D128372: [Clang-Tidy] Empty Check

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd added inline comments.



Comment at: clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp:51-57
+auto Methods = MemberCall->getRecordDecl()->methods();
+auto Clear = llvm::find_if(Methods, [](const CXXMethodDecl *F) {
+  return F->getDeclName().getAsString() == "clear" &&
+ F->getMinRequiredArguments() == 0;
+});
+
+if (Clear != Methods.end()) {

cjdb wrote:
> Note to other reviewers: I suggested this because we couldn't work out a 
> better way to identify if .clear() exists (Abraham can best expand on the 
> issues regarding how a matcher wasn't working).
Yes, I wrote a matcher for the declaration of a .clear()  member function and 
another matcher for a call to .empty(), but it seemed like the check() function 
runs multiple times to match the different matchers. It would find .clear() on 
one run of check() and find .empty() on another, and I couldn't find a way to 
see if .clear() had already been found on a previous run of check().


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

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

[PATCH] D128372: [Clang-Tidy] Empty Check

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd updated this revision to Diff 440286.
abrahamcd added a comment.

Removed old test file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone/standalone-empty.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp
@@ -0,0 +1,202 @@
+// RUN: %check_clang_tidy %s bugprone-standalone-empty %t
+
+namespace std {
+template 
+struct vector {
+  bool empty();
+};
+
+template 
+struct vector_with_clear {
+  bool empty();
+  void clear();
+};
+
+template 
+struct vector_with_void_empty {
+  void empty();
+  void clear();
+};
+
+template 
+struct vector_with_int_empty {
+  int empty();
+  void clear();
+};
+
+template 
+bool empty(T &&);
+
+} // namespace std
+
+namespace absl {
+struct string {
+  bool empty();
+};
+
+struct string_with_clear {
+  bool empty();
+  void clear();
+};
+
+struct string_with_void_empty {
+  void empty();
+  void clear();
+};
+
+struct string_with_int_empty {
+  int empty();
+  void clear();
+};
+
+template 
+bool empty(T &&);
+} // namespace absl
+
+namespace test {
+template 
+void empty(T &&);
+} // namespace test
+
+int test_member_empty() {
+  std::vector s;
+
+  s.empty();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+
+  std::vector_with_void_empty m;
+
+  m.empty();
+  // no-warning
+
+  std::vector_with_clear v;
+
+  v.empty();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}v.clear();{{$}}
+
+  std::vector_with_int_empty w;
+
+  w.empty();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}w.clear();{{$}}
+
+  absl::string x;
+
+  x.empty();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+
+  absl::string_with_void_empty y;
+
+  y.empty();
+  // no-warning
+
+  absl::string_with_clear z;
+
+  z.empty();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}z.clear();{{$}}
+
+  absl::string_with_int_empty a;
+
+  a.empty();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}a.clear();{{$}}
+}
+
+int test_qualified_empty() {
+  absl::string_with_clear v;
+
+  std::empty(v);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'std::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}v.clear();{{$}}
+
+  absl::empty(v);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'absl::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}v.clear();{{$}}
+
+  test::empty(v);
+  // no-warning
+
+  absl::string w;
+
+  std::empty(w);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
+
+}
+
+int test_unqualified_empty() {
+  std::vector v;
+
+  empty(v);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
+
+  std::vector_with_void_empty w;
+
+  empty(w);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'std::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}w.clear();{{$}}
+
+  std::vector_with_clear x;
+
+  empty(x);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'std::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}x.clear();{{$}}
+
+  std::vector_with_int_empty y;
+
+  empty(y);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'std::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}y.clear();{{$}}
+
+  absl::string z;
+
+  empty(z);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'absl::empty' [bugprone-standalone-empty]
+
+  absl::string_with_void_empty a;
+
+  empty(a);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'absl::empty', did you mean 'clear()'? 

[PATCH] D128372: [Clang-Tidy] Empty Check

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood requested changes to this revision.
LegalizeAdulthood added inline comments.
This revision now requires changes to proceed.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-standalone-empty.cpp:1
+// RUN: %check_clang_tidy %s bugprone-standalone-empty %t
+

This file should be removed from the changelist


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

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

[PATCH] D128372: [Clang-Tidy] Empty Check

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp:55
+
+auto Methods = MemberCall->getRecordDecl()->methods();
+auto Clear = llvm::find_if(Methods, [](const CXXMethodDecl *F) {

Please don't use auto unless type is explicitly stated in same statement or 
iterator.



Comment at: clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp:76
+
+auto Methods = Arg->getType()->getAsCXXRecordDecl()->methods();
+auto Clear = llvm::find_if(Methods, [](const CXXMethodDecl *F) {

Please don't use auto unless type is explicitly stated in same statement or 
iterator.



Comment at: clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.h:22
+/// For the user-facing documentation see:
+/// 
http://clang.llvm.org/extra/clang-tidy/checks/bugprone-standalone-empty.html
+class StandaloneEmptyCheck : public ClangTidyCheck {

Please fix URL.



Comment at: clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.h:29
+  void check(const ast_matchers::MatchFinder::MatchResult ) override;
+};
+

Please define `isLanguageVersionSupported()`.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/bugprone/standalone-empty.rst:5
+=
+
+

Excessive newline.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/bugprone/standalone-empty.rst:9
+
+The ``empty()`` method on several common ranges returns a boolean indicating
+whether or not the range is empty, but is often mistakenly interpreted as

Boolean.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

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

[PATCH] D128372: [Clang-Tidy] Empty Check

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd updated this revision to Diff 439938.
abrahamcd marked 7 inline comments as done.
abrahamcd retitled this revision from "Clang-Tidy Empty Check" to "[Clang-Tidy] 
Empty Check".
abrahamcd added a comment.
Herald added a subscriber: xazax.hun.

Added functionality to check if member function `clear()` exists before
suggesting it as a fix, including updated tests to reflect that. Fixed
path change and rebase errors from the initial revision.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone/standalone-empty.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone-standalone-empty.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp
@@ -0,0 +1,202 @@
+// RUN: %check_clang_tidy %s bugprone-standalone-empty %t
+
+namespace std {
+template 
+struct vector {
+  bool empty();
+};
+
+template 
+struct vector_with_clear {
+  bool empty();
+  void clear();
+};
+
+template 
+struct vector_with_void_empty {
+  void empty();
+  void clear();
+};
+
+template 
+struct vector_with_int_empty {
+  int empty();
+  void clear();
+};
+
+template 
+bool empty(T &&);
+
+} // namespace std
+
+namespace absl {
+struct string {
+  bool empty();
+};
+
+struct string_with_clear {
+  bool empty();
+  void clear();
+};
+
+struct string_with_void_empty {
+  void empty();
+  void clear();
+};
+
+struct string_with_int_empty {
+  int empty();
+  void clear();
+};
+
+template 
+bool empty(T &&);
+} // namespace absl
+
+namespace test {
+template 
+void empty(T &&);
+} // namespace test
+
+int test_member_empty() {
+  std::vector s;
+
+  s.empty();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+
+  std::vector_with_void_empty m;
+
+  m.empty();
+  // no-warning
+
+  std::vector_with_clear v;
+
+  v.empty();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}v.clear();{{$}}
+
+  std::vector_with_int_empty w;
+
+  w.empty();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}w.clear();{{$}}
+
+  absl::string x;
+
+  x.empty();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+
+  absl::string_with_void_empty y;
+
+  y.empty();
+  // no-warning
+
+  absl::string_with_clear z;
+
+  z.empty();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}z.clear();{{$}}
+
+  absl::string_with_int_empty a;
+
+  a.empty();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}a.clear();{{$}}
+}
+
+int test_qualified_empty() {
+  absl::string_with_clear v;
+
+  std::empty(v);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'std::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}v.clear();{{$}}
+
+  absl::empty(v);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'absl::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}v.clear();{{$}}
+
+  test::empty(v);
+  // no-warning
+
+  absl::string w;
+
+  std::empty(w);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
+
+}
+
+int test_unqualified_empty() {
+  std::vector v;
+
+  empty(v);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
+
+  std::vector_with_void_empty w;
+
+  empty(w);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'std::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}w.clear();{{$}}
+
+  std::vector_with_clear x;
+
+  empty(x);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'std::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}x.clear();{{$}}
+
+  std::vector_with_int_empty y;
+
+  empty(y);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the 

[PATCH] D128372: Clang-Tidy Empty Check

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood requested changes to this revision.
LegalizeAdulthood added a comment.
This revision now requires changes to proceed.

Clang-Tidy tests and docs have moved to module subdirectories.

Please rebase this onto `main:HEAD` and:

- fold your changes into the appropriate subdirs, stripping the module prefix 
from new files.
- make the target `check-clang-extra` to validate your tests
- make the target `docs-clang-tools-html` to validate any docs changes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

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

[PATCH] D128372: Clang-Tidy Empty Check

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

Tests locations were changed recently too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

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

[PATCH] D128372: Clang-Tidy Empty Check

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

Please add documentation for check.




Comment at: clang-tools-extra/docs/ReleaseNotes.rst:121
+- New :doc:`bugprone-standalone-empty
+  ` check.
+

Path to documentation was changed recently.



Comment at: clang-tools-extra/docs/clang-tidy/checks/list.rst:39
 
-   `abseil-cleanup-ctad `_, "Yes"
-   `abseil-duration-addition `_, "Yes"
-   `abseil-duration-comparison `_, "Yes"
-   `abseil-duration-conversion-cast `_, 
"Yes"
-   `abseil-duration-division `_, "Yes"
-   `abseil-duration-factory-float `_, "Yes"
-   `abseil-duration-factory-scale `_, "Yes"
-   `abseil-duration-subtraction `_, "Yes"
-   `abseil-duration-unnecessary-conversion 
`_, "Yes"
-   `abseil-faster-strsplit-delimiter 
`_, "Yes"
-   `abseil-no-internal-dependencies `_,
-   `abseil-no-namespace `_,
-   `abseil-redundant-strcat-calls `_, "Yes"
-   `abseil-str-cat-append `_, "Yes"
-   `abseil-string-find-startswith `_, "Yes"
-   `abseil-string-find-str-contains `_, 
"Yes"
-   `abseil-time-comparison `_, "Yes"
-   `abseil-time-subtraction `_, "Yes"
-   `abseil-upgrade-duration-conversions 
`_, "Yes"
-   `altera-id-dependent-backward-branch 
`_,
-   `altera-kernel-name-restriction `_,
-   `altera-single-work-item-barrier `_,
-   `altera-struct-pack-align `_, "Yes"
-   `altera-unroll-loops `_,
-   `android-cloexec-accept `_, "Yes"
-   `android-cloexec-accept4 `_, "Yes"
-   `android-cloexec-creat `_, "Yes"
-   `android-cloexec-dup `_, "Yes"
-   `android-cloexec-epoll-create `_, "Yes"
-   `android-cloexec-epoll-create1 `_, "Yes"
-   `android-cloexec-fopen `_, "Yes"
-   `android-cloexec-inotify-init `_, "Yes"
-   `android-cloexec-inotify-init1 `_, "Yes"
-   `android-cloexec-memfd-create `_, "Yes"
-   `android-cloexec-open `_, "Yes"
-   `android-cloexec-pipe `_, "Yes"
-   `android-cloexec-pipe2 `_, "Yes"
-   `android-cloexec-socket `_, "Yes"
-   `android-comparison-in-temp-failure-retry 
`_,
-   `boost-use-to-string `_, "Yes"
-   `bugprone-argument-comment `_, "Yes"
-   `bugprone-assert-side-effect `_,
-   `bugprone-bad-signal-to-kill-thread 
`_,
-   `bugprone-bool-pointer-implicit-conversion 
`_, "Yes"
-   `bugprone-branch-clone `_,
-   `bugprone-copy-constructor-init `_, 
"Yes"
-   `bugprone-dangling-handle `_,
-   `bugprone-dynamic-static-initializers 
`_,
-   `bugprone-easily-swappable-parameters 
`_,
-   `bugprone-exception-escape `_,
-   `bugprone-fold-init-type `_,
-   `bugprone-forward-declaration-namespace 
`_,
-   `bugprone-forwarding-reference-overload 
`_,
-   `bugprone-implicit-widening-of-multiplication-result 
`_, "Yes"
-   `bugprone-inaccurate-erase `_, "Yes"
-   `bugprone-incorrect-roundings `_,
-   `bugprone-infinite-loop `_,
-   `bugprone-integer-division `_,
-   `bugprone-lambda-function-name `_,
-   `bugprone-macro-parentheses `_, "Yes"
-   `bugprone-macro-repeated-side-effects 
`_,
-   `bugprone-misplaced-operator-in-strlen-in-alloc 
`_, "Yes"
-   `bugprone-misplaced-pointer-arithmetic-in-alloc 
`_, "Yes"
-   `bugprone-misplaced-widening-cast `_,
-   `bugprone-move-forwarding-reference 
`_, "Yes"
-   `bugprone-multiple-statement-macro 
`_,
-   `bugprone-no-escape `_,
-   `bugprone-not-null-terminated-result 
`_, "Yes"
-   `bugprone-parent-virtual-call `_, "Yes"
-   `bugprone-posix-return `_, "Yes"
-   `bugprone-redundant-branch-condition 
`_, "Yes"
-   `bugprone-reserved-identifier `_, "Yes"
-   `bugprone-shared-ptr-array-mismatch 
`_, "Yes"
-   `bugprone-signal-handler `_,
-   `bugprone-signed-char-misuse `_,
-   `bugprone-sizeof-container `_,
-   `bugprone-sizeof-expression `_,
-   `bugprone-spuriously-wake-up-functions 
`_,
-   `bugprone-string-constructor `_, "Yes"
-   `bugprone-string-integer-assignment 
`_, "Yes"
-   `bugprone-string-literal-with-embedded-nul 
`_,
-   `bugprone-stringview-nullptr `_, "Yes"
-   `bugprone-suspicious-enum-usage `_,
-   `bugprone-suspicious-include `_,
-   `bugprone-suspicious-memory-comparison 
`_,
-   `bugprone-suspicious-memset-usage 
`_, "Yes"
-   `bugprone-suspicious-missing-comma 
`_,
-   `bugprone-suspicious-semicolon `_, "Yes"
-   `bugprone-suspicious-string-compare 
`_, "Yes"
-   `bugprone-swapped-arguments `_, "Yes"
-   `bugprone-terminating-continue `_, "Yes"
-   `bugprone-throw-keyword-missing `_,
-   `bugprone-too-small-loop-variable `_,
-   `bugprone-unchecked-optional-access 
`_,
-   `bugprone-undefined-memory-manipulation 
`_,
-   `bugprone-undelegated-constructor `_,
-   `bugprone-unhandled-exception-at-new 
`_,
-   `bugprone-unhandled-self-assignment 
`_,
-   `bugprone-unused-raii `_, "Yes"
-   `bugprone-unused-return-value `_,
-   `bugprone-use-after-move `_,
-   `bugprone-virtual-near-miss `_, "Yes"
-   `cert-dcl21-cpp `_, "Yes"
-   `cert-dcl50-cpp `_,
-   `cert-dcl58-cpp `_,
-   `cert-env33-c `_,
-   `cert-err33-c `_,
-   `cert-err34-c `_,
-   `cert-err52-cpp `_,
-   `cert-err58-cpp `_,
-   `cert-err60-cpp `_,
-   `cert-flp30-c `_,
-   `cert-mem57-cpp `_,
-   `cert-msc50-cpp `_,
-   

[PATCH] D128372: Clang-Tidy Empty Check

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added inline comments.



Comment at: clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp:61
+   "ignoring the result of 'empty()', did you mean 'clear()'? ");
+  Builder << FixItHint::CreateReplacement(ReplacementRange, "clear");
+}

I wonder if we should add a fixit hint that suggests `x = T{};` in the event 
`x.clear()` isn't available (e.g. `std::stack`).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

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

[PATCH] D128372: Clang-Tidy Empty Check

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

In D128372#3602881 , @tschuett wrote:

> Just for reference:
> https://reviews.llvm.org/D128267

Ack. I still think this CL is useful, given that not every library will have 
`[[nodiscard]]`, and because it can suggest appropriate alternatives.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

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

[PATCH] D128372: Clang-Tidy Empty Check

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

Just for reference:
https://reviews.llvm.org/D128267


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

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

[PATCH] D128372: Clang-Tidy Empty Check

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

A great start, thanks!




Comment at: clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp:51-57
+auto Methods = MemberCall->getRecordDecl()->methods();
+auto Clear = llvm::find_if(Methods, [](const CXXMethodDecl *F) {
+  return F->getDeclName().getAsString() == "clear" &&
+ F->getMinRequiredArguments() == 0;
+});
+
+if (Clear != Methods.end()) {

Note to other reviewers: I suggested this because we couldn't work out a better 
way to identify if .clear() exists (Abraham can best expand on the issues 
regarding how a matcher wasn't working).



Comment at: clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp:75-78
+diag(NonMemberLoc, "ignoring the result of '%0'")
+<< llvm::dyn_cast(NonMemberCall->getCalleeDecl())
+   ->getQualifiedNameAsString()
+<< FixItHint::CreateReplacement(ReplacementRange, ReplacementText);

It looks as though this is unconditionally suggesting clear (even if it doesn't 
exist), and doesn't suggest it in the message.



Comment at: clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.h:18
+
+/// FIXME: Write a short description.
+///

Please fix this :)



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:123
+
+  Warns when `empty()` is used on a container and the result is ignored.
+

* s/container/range
* This should probably mention it suggests `clear()` if it's a member function.



Comment at: clang-tools-extra/docs/clang-tidy/checks/list.rst:39
 
-   `abseil-cleanup-ctad `_, "Yes"
-   `abseil-duration-addition `_, "Yes"
-   `abseil-duration-comparison `_, "Yes"
-   `abseil-duration-conversion-cast `_, 
"Yes"
-   `abseil-duration-division `_, "Yes"
-   `abseil-duration-factory-float `_, "Yes"
-   `abseil-duration-factory-scale `_, "Yes"
-   `abseil-duration-subtraction `_, "Yes"
-   `abseil-duration-unnecessary-conversion 
`_, "Yes"
-   `abseil-faster-strsplit-delimiter 
`_, "Yes"
-   `abseil-no-internal-dependencies `_,
-   `abseil-no-namespace `_,
-   `abseil-redundant-strcat-calls `_, "Yes"
-   `abseil-str-cat-append `_, "Yes"
-   `abseil-string-find-startswith `_, "Yes"
-   `abseil-string-find-str-contains `_, 
"Yes"
-   `abseil-time-comparison `_, "Yes"
-   `abseil-time-subtraction `_, "Yes"
-   `abseil-upgrade-duration-conversions 
`_, "Yes"
-   `altera-id-dependent-backward-branch 
`_,
-   `altera-kernel-name-restriction `_,
-   `altera-single-work-item-barrier `_,
-   `altera-struct-pack-align `_, "Yes"
-   `altera-unroll-loops `_,
-   `android-cloexec-accept `_, "Yes"
-   `android-cloexec-accept4 `_, "Yes"
-   `android-cloexec-creat `_, "Yes"
-   `android-cloexec-dup `_, "Yes"
-   `android-cloexec-epoll-create `_, "Yes"
-   `android-cloexec-epoll-create1 `_, "Yes"
-   `android-cloexec-fopen `_, "Yes"
-   `android-cloexec-inotify-init `_, "Yes"
-   `android-cloexec-inotify-init1 `_, "Yes"
-   `android-cloexec-memfd-create `_, "Yes"
-   `android-cloexec-open `_, "Yes"
-   `android-cloexec-pipe `_, "Yes"
-   `android-cloexec-pipe2 `_, "Yes"
-   `android-cloexec-socket `_, "Yes"
-   `android-comparison-in-temp-failure-retry 
`_,
-   `boost-use-to-string `_, "Yes"
-   `bugprone-argument-comment `_, "Yes"
-   `bugprone-assert-side-effect `_,
-   `bugprone-bad-signal-to-kill-thread 
`_,
-   `bugprone-bool-pointer-implicit-conversion 
`_, "Yes"
-   `bugprone-branch-clone `_,
-   `bugprone-copy-constructor-init `_, 
"Yes"
-   `bugprone-dangling-handle `_,
-   `bugprone-dynamic-static-initializers 
`_,
-   `bugprone-easily-swappable-parameters 
`_,
-   `bugprone-exception-escape `_,
-   `bugprone-fold-init-type `_,
-   `bugprone-forward-declaration-namespace 
`_,
-   `bugprone-forwarding-reference-overload 
`_,
-   `bugprone-implicit-widening-of-multiplication-result 
`_, "Yes"
-   `bugprone-inaccurate-erase `_, "Yes"
-   `bugprone-incorrect-roundings `_,
-   `bugprone-infinite-loop `_,
-   `bugprone-integer-division `_,
-   `bugprone-lambda-function-name `_,
-   `bugprone-macro-parentheses `_, "Yes"
-   `bugprone-macro-repeated-side-effects 
`_,
-   `bugprone-misplaced-operator-in-strlen-in-alloc 
`_, "Yes"
-   `bugprone-misplaced-pointer-arithmetic-in-alloc 
`_, "Yes"
-   `bugprone-misplaced-widening-cast `_,
-   `bugprone-move-forwarding-reference 
`_, "Yes"
-   `bugprone-multiple-statement-macro 
`_,
-   `bugprone-no-escape `_,
-   `bugprone-not-null-terminated-result 
`_, "Yes"
-   `bugprone-parent-virtual-call `_, "Yes"
-   `bugprone-posix-return `_, "Yes"
-   `bugprone-redundant-branch-condition 
`_, "Yes"
-   `bugprone-reserved-identifier `_, "Yes"
-   `bugprone-shared-ptr-array-mismatch 
`_, "Yes"
-   `bugprone-signal-handler `_,
-   `bugprone-signed-char-misuse `_,
-   `bugprone-sizeof-container `_,
-   `bugprone-sizeof-expression `_,
-   `bugprone-spuriously-wake-up-functions 
`_,
-   

[PATCH] D128372: Clang-Tidy Empty Check

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd created this revision.
Herald added subscribers: carlosgalvezp, abrachet, phosek, mgorny.
Herald added a project: All.
abrahamcd requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: cfe-commits, sstefan1.
Herald added a project: clang-tools-extra.

Adds a clang-tidy check for the incorrect use of `empty()` on a
container when the result of the call is ignored.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D128372

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone-standalone-empty.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-standalone-empty.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-standalone-empty.cpp
@@ -0,0 +1,79 @@
+// RUN: %check_clang_tidy %s bugprone-standalone-empty %t
+
+namespace std {
+
+template 
+struct vector {
+  bool empty();
+  void clear();
+};
+
+template 
+bool empty(T &&);
+
+} // namespace std
+
+namespace absl {
+template 
+struct vector {
+  bool empty();
+  void clear();
+};
+
+template 
+bool empty(T &&);
+} // namespace absl
+
+int test_member_empty() {
+  std::vector v;
+
+  v.empty();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}v.clear();{{$}}
+
+  absl::vector w;
+
+  w.empty();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}w.clear();{{$}}
+}
+
+int test_qualified_empty() {
+  absl::vector v;
+
+  std::empty(v);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}v.clear();{{$}}
+
+  absl::empty(v);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'absl::empty' [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}v.clear();{{$}}
+}
+
+int test_unqualified_empty() {
+  std::vector v;
+
+  empty(v);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}v.clear();{{$}}
+
+  absl::vector w;
+
+  empty(w);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'absl::empty' [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}w.clear();{{$}}
+
+  {
+using std::empty;
+empty(v);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}
+  }
+
+  {
+using absl::empty;
+empty(w);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'absl::empty' [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  w.clear();{{$}}
+  }
+}
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -36,328 +36,329 @@
 .. csv-table::
:header: "Name", "Offers fixes"
 
-   `abseil-cleanup-ctad `_, "Yes"
-   `abseil-duration-addition `_, "Yes"
-   `abseil-duration-comparison `_, "Yes"
-   `abseil-duration-conversion-cast `_, "Yes"
-   `abseil-duration-division `_, "Yes"
-   `abseil-duration-factory-float `_, "Yes"
-   `abseil-duration-factory-scale `_, "Yes"
-   `abseil-duration-subtraction `_, "Yes"
-   `abseil-duration-unnecessary-conversion `_, "Yes"
-   `abseil-faster-strsplit-delimiter `_, "Yes"
-   `abseil-no-internal-dependencies `_,
-   `abseil-no-namespace `_,
-   `abseil-redundant-strcat-calls `_, "Yes"
-   `abseil-str-cat-append `_, "Yes"
-   `abseil-string-find-startswith `_, "Yes"
-   `abseil-string-find-str-contains `_, "Yes"
-   `abseil-time-comparison `_, "Yes"
-   `abseil-time-subtraction `_, "Yes"
-   `abseil-upgrade-duration-conversions `_, "Yes"
-   `altera-id-dependent-backward-branch `_,
-   `altera-kernel-name-restriction `_,
-   `altera-single-work-item-barrier `_,
-   `altera-struct-pack-align `_, "Yes"
-   `altera-unroll-loops `_,
-   `android-cloexec-accept `_, "Yes"
-   `android-cloexec-accept4 `_, "Yes"
-   `android-cloexec-creat `_, "Yes"
-   `android-cloexec-dup `_, "Yes"
-   `android-cloexec-epoll-create `_, "Yes"
-   `android-cloexec-epoll-create1 `_, "Yes"
-   `android-cloexec-fopen `_, "Yes"
-   `android-cloexec-inotify-init `_, "Yes"
-   `android-cloexec-inotify-init1 `_, "Yes"
-   `android-cloexec-memfd-create `_, "Yes"
-   `android-cloexec-open `_, "Yes"
-   `android-cloexec-pipe `_, "Yes"
-