zinovy.nis marked 4 inline comments as done.
zinovy.nis added inline comments.
================
Comment at:
clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp:57-60
if (const auto *FuncDecl = CExpr->getDirectCallee()) {
if (FuncDecl->getDeclName().isIdentifier() &&
- FuncDecl->getName() == "__builtin_expect") // exceptions come here
+ IgnoredFunctions.contains(
+ FuncDecl->getName())) // exceptions come here
----------------
aaron.ballman wrote:
> This doesn't seem quite right to me (test coverage would help) in the case
> where the user is specifying a (potentially partially) qualified function
> name. e.g., imagine an `IgnoredFunctions` list of
> `my::fancy_func,::other_func,yet::another_func` where `my` is a namespace
> containing a function named `fancy_func`, and `yet` is a class with a static
> function named `another_func`. I think this code will only consider the name
> of the function itself, but not any part of its qualified name.
>
> I think we typically implement function name exclusions via the
> `matchesAnyListedName()` AST matcher, as in:
> https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp#L92
Thanks a lot!
Done.
BTW, now we have "," separator for macros and ";" for ignored functions.
Doesn't sound reasonable. What do you think?
================
Comment at:
clang-tools-extra/test/clang-tidy/checkers/bugprone-assert-side-effect.cpp:91
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: side effect in assert()
condition discarded in release builds
+ assert(mc.badButIgnoredFunc(0, 1));
assert(mc.goodFunc(0, 1));
----------------
aaron.ballman wrote:
> FWIW, I think this can be confusing in practice; the function called is
> `::MyClass:badButIgnoredFunc()`. Further, if I had a global function named
> `badButIgnoredFunc()` it would *also* be ignored and I'd have no way to
> configure to distinguish between the two.
Added a new test case for it.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D116478/new/
https://reviews.llvm.org/D116478
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
