lebedev.ri added inline comments.
================
Comment at: clang-tidy/readability/ContainerSizeEmptyCheck.cpp:34
return;
const auto ValidContainer = qualType(hasUnqualifiedDesugaredType(
----------------
```
auto ContainerLenghtFuncNames = anyOf(hasName("size"), hasName("length"));
```
pick better naem
================
Comment at: clang-tidy/readability/ContainerSizeEmptyCheck.cpp:40
isConst(), parameterCountIs(0), isPublic(),
- hasName("size"),
+ anyOf(hasName("size"), hasName("length")),
returns(qualType(isInteger(), unless(booleanType()))))
----------------
s/`anyOf(hasName("size"), hasName("length")),`/`ContainerLenghtFuncNames,`/
================
Comment at: clang-tidy/readability/ContainerSizeEmptyCheck.cpp:40-65
+ anyOf(hasName("size"), hasName("length")),
returns(qualType(isInteger(), unless(booleanType()))))
.bind("size")),
has(cxxMethodDecl(isConst(), parameterCountIs(0), isPublic(),
hasName("empty"), returns(booleanType()))
.bind("empty")))
.bind("container")))))));
----------------
lebedev.ri wrote:
> s/`anyOf(hasName("size"), hasName("length")),`/`ContainerLenghtFuncNames,`/
We want `anyOf()` in both cases, with the same list of suspects.
If it is not a `anyOf()` in the second case, then naturally it won't work
(can't have more than one name)
If it is not a `anyOf()` in the first case, then it will e.g. work if all of
them are present.
So we just want to ensure that we do the same thing in both cases.
================
Comment at: clang-tidy/readability/ContainerSizeEmptyCheck.cpp:65
hasType(references(ValidContainer))))),
- callee(cxxMethodDecl(hasName("size"))), WrongUse,
+ callee(cxxMethodDecl(anyOf(hasName("size"),
hasName("length")))), WrongUse,
unless(hasAncestor(cxxMethodDecl(
----------------
lebedev.ri wrote:
> This line looks too long, clang-format might be too intrusive, so at least
> ```
> callee(cxxMethodDecl(anyOf(hasName("size"),
> hasName("length")))),
> WrongUse,
>
> ```
s/`callee(cxxMethodDecl(anyOf(hasName("size"), hasName("length")))),
WrongUse,`/`callee(cxxMethodDecl(ContainerLenghtFuncNames)), WrongUse,`/
================
Comment at: test/clang-tidy/readability-container-size-empty.cpp:19-20
basic_string<T> operator+(const basic_string<T>& other) const;
unsigned long size() const;
+ unsigned long length() const;
bool empty() const;
----------------
Quolyk wrote:
> lebedev.ri wrote:
> > Does it still work if only one of these exists?
> It works indeed, do you suggest adding test case for this?
Hmm, actually, i think there is an easier solution, see other inline comment.
Repository:
rCTE Clang Tools Extra
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D56644/new/
https://reviews.llvm.org/D56644
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
