donat.nagy added inline comments.
================
Comment at:
clang-tools-extra/clang-tidy/bugprone/MultipleNewInOneExpressionCheck.cpp:44-45
+ return BinOp->getOpcode() == BO_Assign &&
+ BinOp->getRHS()->IgnoreParenCasts() == E;
+
+ return isa<CallExpr, CXXConstructExpr>(ParentE);
----------------
In general, a value can be stored by passing it to a CompoundAssignOperator.
This is not relevant in the current application (pointers do not appear on the
RHS of compound assignment operators); but perhaps it's worth to mention this
in a short comment in case someone wishes to reuse this function in some other
checker.
================
Comment at:
clang-tools-extra/docs/clang-tidy/checks/bugprone/multiple-new-in-one-expression.rst:9-14
+C++ does often not specify the exact order of evaluation of the operands of an
operator or arguments of a function.
+Therefore if a first allocation succeeds and a second fails, in an exception
handler it is not possible to tell which allocation has failed and free the
memory.
+Even if the order is fixed the result of a first ``new`` may be stored in a
temporary location that is not reachable at the time when a second allocation
fails.
+It is best to avoid any expression that contains more than one operator
``new`` call, if exception handling is used to check for allocation errors.
+Different rules apply for are the short-circuit operators ``||`` and ``&&``
and the ``,`` operator, where evaluation of one side must be completed before
the other starts.
+Similarly, condition of a ``?`` operator is evaluated before the branches are
evaluated.
----------------
Eugene.Zelenko wrote:
> Please follow 80 characters limit.
I think the handling of the case of the short-circuiting operators is ambiguous
in this long paragraph: without the examples it's unclear whether the checker
reports e.g. `f2(new A) || f1(new B);` as an error ("Even if the order is fixed
... It is best to avoid any expression that contains more than one operator
``new`` call ... " suggests that even these cases are reported). Try to
restructure this description, perhaps by splitting it into two paragraphs: (1)
quote/state all the relevant rules of C++ evaluation order (2) describe how
"bad" ordering of the two allocation causes a memory leak.
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
