================
@@ -1533,9 +1533,9 @@ const Stmt *LabelStmt::getInnermostLabeledStmt() const {
}
const Stmt *LoopControlStmt::getNamedLoopOrSwitch() const {
- if (!hasLabelTarget())
- return nullptr;
- return getLabelDecl()->getStmt()->getInnermostLabeledStmt();
+ assert(hasLabelTarget());
+ LabelStmt *Label = getLabelDecl()->getStmt();
+ return Label ? Label->getInnermostLabeledStmt() : nullptr;
----------------
Expertcoderz wrote:
`getNamedLoopOrSwitch()` can fail for two different reasons:
1. the `break`/`continue` statement points to an unnamed loop (i.e.
`hasLabelTarget() == false`)
2. the loop is a named loop but its enclosing `LabelStmt` has not yet been
created and thus the loop statement cannot be returned
The null return value is unable to distinguish between those two reasons.
Treating a failure due to reason 2 as if it were due to reason 1 may lead to
bugs whereby a named `break`/`continue` statement is mistaken for its unnamed
equivalent, thus it is safer to require callers to rule out reason 1 before
calling `getNamedLoopOrSwitch()`. (Please refer to this discussion from the
original PR:
https://github.com/llvm/llvm-project/pull/226754#discussion_r4124227800)
Furthermore, existing callers, such as in `CodeGen/CGStmt.cpp` and
`AST/TextNodeDumper.cpp` already call `hasLabelTarget()` beforehand to handle
reason 1 specifically; the ones that I changed in this PR are the cases where
reasons 1 and 2 both happen to lead to the same handling outcome. Hence in many
cases, the `hasLabelTarget()` check performed in `getNamedLoopOrSwitch()` is
effectively redundant and moving it out of the function avoids the duplication
of the check.
https://github.com/llvm/llvm-project/pull/228655
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
