[clang-tidy] false positive and invalid fix for readability-container-size-empty with regex

[sadan4]

given the following code

#include <regex>
void foo() {
    std::smatch match;
    if (match.length());
}

clang-tidy will suggest to replace match.length() with !match.empty(); however, this is invalid because for successful matches of zero length, match.length() == 0 and match.empty() == false.

godbolt sample demonstrating clang-tidy's incorrect diagnostic https://godbolt.org/z/qnzc4Pcrc

godbolt sample demonstrating a zero length match (execute the code) https://godbolt.org/z/YvT7hb4f4

regex 101 sample demonstrating a zero length regex & match https://regex101.com/r/dzUrRp/1

Related: #88203

[llvmbot]

@llvm/issue-subscribers-clang-tidy

Author: None (sadan4)

given the following code ```cpp #include <regex> void foo() { std::smatch match; if (match.length()); } ``` clang-tidy will suggest to replace `match.length()` with `!match.empty()`; however, this is invalid because for successful matches of zero length, `match.length() == 0` and `match.empty() == false`.

godbolt sample demonstrating clang-tidy's incorrect diagnostic https://godbolt.org/z/qnzc4Pcrc

godbolt sample demonstrating a zero length match (execute the code) https://godbolt.org/z/YvT7hb4f4

regex 101 sample demonstrating a zero length regex & match https://regex101.com/r/dzUrRp/1

Related: #88203

[vbvictor]

Regex's empty and length don't follow the interchangeability that the check implies, so the only way we can mitigate this is to introduce an option to exclude certain types (like smatch). We don't analyze bodies of methods (and I may say we won't do that).

For now, you can NOLINT the warning or disable the check.

[sadan4]

Regex's empty and length don't follow the interchangeability that the check implies, so the only way we can mitigate this is to introduce an option to exclude certain types (like smatch). We don't analyze bodies of methods (and I may say we won't do that).

For now, you can NOLINT the warning or disable the check.

@vbvictor According to the docs, there is already an option to exclude types from this check. I think the best fix for this is to exclude the regex match types by default.

[vbvictor]

Regex's empty and length don't follow the interchangeability that the check implies, so the only way we can mitigate this is to introduce an option to exclude certain types (like smatch). We don't analyze bodies of methods (and I may say we won't do that).
For now, you can NOLINT the warning or disable the check.

@vbvictor According to the docs, there is already an option to exclude types from this check. I think the best fix for this is to exclude the regex match types by default.

I recall it only works for such cases: value == Type();, it won't work for your case AFAIK.

A semicolon-separated list of regular expressions matching class names for which the check will ignore comparisons of objects with default-constructed objects of the same type.

If it does work, can you confirm.

[sadan4]

Regex's empty and length don't follow the interchangeability that the check implies, so the only way we can mitigate this is to introduce an option to exclude certain types (like smatch). We don't analyze bodies of methods (and I may say we won't do that).
For now, you can NOLINT the warning or disable the check.

@vbvictor According to the docs, there is already an option to exclude types from this check. I think the best fix for this is to exclude the regex match types by default.

I recall it only works for such cases: value == Type();, it won't work for your case AFAIK.

A semicolon-separated list of regular expressions matching class names for which the check will ignore comparisons of objects with default-constructed objects of the same type.

If it does work, can you confirm.

It does not work, I misread the docs.