This revision was automatically updated to reflect the committed changes.
Closed by commit rL281307: [clang-tidy] Extend readability-container-size-empty
to arbitrary class with… (authored by omtcyfz).
Changed prior to commit:
https://reviews.llvm.org/D24349?vs=71119=71120#toc
Repository:
omtcyfz updated this revision to Diff 71119.
omtcyfz marked an inline comment as done.
omtcyfz added a comment.
Combine two `returns` matchers.
https://reviews.llvm.org/D24349
Files:
clang-tidy/readability/ContainerSizeEmptyCheck.cpp
alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.
LG
Comment at: clang-tidy/readability/ContainerSizeEmptyCheck.cpp:35
@@ +34,3 @@
+ hasName("size"), returns(isInteger()),
+
omtcyfz updated this revision to Diff 70989.
omtcyfz added a comment.
Messed up with the last diff; fix that + clang-format the check.
https://reviews.llvm.org/D24349
Files:
clang-tidy/readability/ContainerSizeEmptyCheck.cpp
docs/clang-tidy/checks/readability-container-size-empty.rst
omtcyfz updated this revision to Diff 70986.
omtcyfz marked 4 inline comments as done.
omtcyfz added a comment.
Address another round of comments.
https://reviews.llvm.org/D24349
Files:
clang-tidy/readability/ContainerSizeEmptyCheck.cpp
alexfh requested changes to this revision.
alexfh added a comment.
This revision now requires changes to proceed.
In https://reviews.llvm.org/D24349#538102, @aaron.ballman wrote:
> In https://reviews.llvm.org/D24349#538098, @alexfh wrote:
>
> > Thank you for the patch!
> >
> > In
omtcyfz updated this revision to Diff 70822.
omtcyfz added a comment.
Allow inheritance for `size()` and `empty()`.
https://reviews.llvm.org/D24349
Files:
clang-tidy/readability/ContainerSizeEmptyCheck.cpp
docs/clang-tidy/checks/readability-container-size-empty.rst
omtcyfz removed rL LLVM as the repository for this revision.
omtcyfz updated this revision to Diff 70818.
omtcyfz added a comment.
Restricted `size()` and `empty()` functions a little bit more.
https://reviews.llvm.org/D24349
Files:
clang-tidy/readability/ContainerSizeEmptyCheck.cpp
aaron.ballman added a comment.
In https://reviews.llvm.org/D24349#538098, @alexfh wrote:
> Thank you for the patch!
>
> In https://reviews.llvm.org/D24349#537500, @aaron.ballman wrote:
>
> > In https://reviews.llvm.org/D24349#537350, @Eugene.Zelenko wrote:
> >
> > > Probably check should have
alexfh added a comment.
Thank you for the patch!
In https://reviews.llvm.org/D24349#537500, @aaron.ballman wrote:
> In https://reviews.llvm.org/D24349#537350, @Eugene.Zelenko wrote:
>
> > Probably check should have options to extend list of containers and also to
> > assume all classes with
Prazek added inline comments.
Comment at: clang-tidy/readability/ContainerSizeEmptyCheck.cpp:34
@@ +33,3 @@
+ has(functionDecl(
+ isPublic(), hasName("size"), returns(isInteger()),
+ unless(anyOf(returns(isAnyCharacter()), returns(booleanType()),
omtcyfz added a comment.
In https://reviews.llvm.org/D24349#537624, @aaron.ballman wrote:
> I think that's reasonable, depending on whether we find false positives with
> the warning as well (I have a slight concern about `size()` and `empty()`
> being unrelated operations on a non-container
aaron.ballman added a comment.
In https://reviews.llvm.org/D24349#537595, @xazax.hun wrote:
> In https://reviews.llvm.org/D24349#537594, @omtcyfz wrote:
>
> > In https://reviews.llvm.org/D24349#537589, @Eugene.Zelenko wrote:
> >
> > > If size() and empty() change object's state, it may be not
xazax.hun added a comment.
In https://reviews.llvm.org/D24349#537594, @omtcyfz wrote:
> In https://reviews.llvm.org/D24349#537589, @Eugene.Zelenko wrote:
>
> > If size() and empty() change object's state, it may be not equivalent
> > replacement.
>
>
> True. But my point is that they are not
omtcyfz added a comment.
In https://reviews.llvm.org/D24349#537589, @Eugene.Zelenko wrote:
> If size() and empty() change object's state, it may be not equivalent
> replacement.
True. But my point is that they are not required to do that if they're just not
marked `const`.
Repository:
rL
Eugene.Zelenko added a comment.
If size() and empty() change object's state, it may be not equivalent
replacement.
Repository:
rL LLVM
https://reviews.llvm.org/D24349
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
omtcyfz marked an inline comment as done.
omtcyfz added a comment.
In https://reviews.llvm.org/D24349#537350, @Eugene.Zelenko wrote:
> Probably check should have options to extend list of containers and also to
> assume all classes with integer type size() const and bool empty() const as
>
aaron.ballman added a comment.
In https://reviews.llvm.org/D24349#537549, @Eugene.Zelenko wrote:
> Should we also check for absence of parameters in size() and empty() as well
> as const?
I think that would be reasonable.
https://reviews.llvm.org/D24349
Eugene.Zelenko added a comment.
Should we also check for absence of parameters in size() and empty() as well as
const?
https://reviews.llvm.org/D24349
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
aaron.ballman added a comment.
In https://reviews.llvm.org/D24349#537350, @Eugene.Zelenko wrote:
> Probably check should have options to extend list of containers and also to
> assume all classes with integer type size() const and bool empty() const as
> containers. It may be not trivial to
Eugene.Zelenko added a comment.
Probably check should have options to extend list of containers and also to
assume all classes with integer type size() const and bool empty() const as
containers. It may be not trivial to find out all custom containers and last
option will be helpful to
omtcyfz marked an inline comment as done.
Comment at: clang-tidy/readability/ContainerSizeEmptyCheck.cpp:33
@@ +32,3 @@
+ const auto validContainer = namedDecl(
+ has(functionDecl(
+ isPublic(), hasName("size"), returns(isInteger()),
Thank you!
omtcyfz updated this revision to Diff 70717.
omtcyfz added a comment.
Blacklist `enum` and `bool` return types for `size()`.
https://reviews.llvm.org/D24349
Files:
clang-tidy/readability/ContainerSizeEmptyCheck.cpp
test/clang-tidy/readability-container-size-empty.cpp
Index:
aaron.ballman added a subscriber: aaron.ballman.
aaron.ballman added a reviewer: aaron.ballman.
Comment at: clang-tidy/readability/ContainerSizeEmptyCheck.cpp:33
@@ +32,3 @@
+ const auto validContainer = namedDecl(
+ has(functionDecl(isPublic(), hasName("size"),
omtcyfz created this revision.
omtcyfz added reviewers: alexfh, Eugene.Zelenko, klimek.
omtcyfz added a subscriber: cfe-commits.
Implementing [[ https://llvm.org/bugs/show_bug.cgi?id=26823 | feature request
]].
This patch extends readability-container-size-empty check allowing it to
produce
25 matches
Mail list logo
