PiotrZSL added inline comments.
================
Comment at:
clang-tools-extra/clang-tidy/bugprone/MultipleNewInOneExpressionCheck.cpp:91
+ ExceptionType, ExceptionReferenceType)));
+ auto BadAllocCatchingTryBlock = cxxTryStmt(hasHandlerFor(CatchBadAllocType));
+
----------------
what about: ```catch(...)```
================
Comment at:
clang-tools-extra/clang-tidy/bugprone/MultipleNewInOneExpressionCheck.cpp:102
+ hasAnyArgument(
+ expr(HasNewExpr1, unless(equalsBoundNode("arg2"))).bind("arg1")),
+ hasAnyArgument(
----------------
this doesnt look valid, arg2 isn't known at this point yet, so this could be
removed.
and this may not work for case like this:
```callSomething({new A, new B});```
Most probably this equalsBoundNode should be on HasNewExpr2 level, to avoid
duplicated newExpr, not arguments of call
and image situation like this:
```
try {
something(std::shared_ptr<int>(new int), std::shared_ptr<int>(new int));
} catch(const std::bad_alloc&) {}
```
this in theory could also produce false-positive.
other issue is that first call could be anything, maloc, new, calloc, wzalloc,
operator new(), it doesn't need to be just new.
You could try to use some simple way of checking this like,
isBeforeInTransationUnit....
And this will also produce false-positive if we use `new(std::nothrow)` on
second.
There are some "utils" to check sequence order, maybe would be good to
investigate them.
================
Comment at:
clang-tools-extra/clang-tidy/bugprone/MultipleNewInOneExpressionCheck.cpp:105
+ expr(HasNewExpr2, unless(equalsBoundNode("arg1"))).bind("arg2")),
+ hasAncestor(BadAllocCatchingTryBlock)),
+ this);
----------------
To be honest, I don't see any reason, how this try-catch would change anything,
there can be one in parent function, and we going to heave a leak if we have
try-catch or not.
================
Comment at:
clang-tools-extra/clang-tidy/bugprone/MultipleNewInOneExpressionCheck.cpp:125
+ this);
+}
+
----------------
other issue I see is that same new could be matched multiple times by those
Matchers
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D138777/new/
https://reviews.llvm.org/D138777
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
