Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>,
Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>,
Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>,
Danny =?utf-8?q?Mösch?= <danny.moe...@icloud.com>
Message-ID:
In-Reply-To: <llvm.org/llvm/llvm-project/pull/81...@github.com>
================
@@ -42,10 +44,30 @@ void AvoidReturnWithVoidValueCheck::check(
const auto *VoidReturn = Result.Nodes.getNodeAs<ReturnStmt>("void_return");
if (IgnoreMacros && VoidReturn->getBeginLoc().isMacroID())
return;
- if (!StrictMode && !Result.Nodes.getNodeAs<CompoundStmt>("compound_parent"))
+ const auto *SurroundingBlock =
+ Result.Nodes.getNodeAs<CompoundStmt>("compound_parent");
+ if (!StrictMode && !SurroundingBlock)
return;
- diag(VoidReturn->getBeginLoc(), "return statement within a void function "
- "should not have a specified return value");
+ DiagnosticBuilder Diag = diag(VoidReturn->getBeginLoc(),
+ "return statement within a void function "
+ "should not have a specified return value");
+ const SourceLocation SemicolonPos = utils::lexer::findNextTerminator(
+ VoidReturn->getEndLoc(), *Result.SourceManager, getLangOpts());
+ if (SemicolonPos.isInvalid())
+ return;
+ if (!SurroundingBlock) {
+ const auto BraceInsertionHints = utils::getBraceInsertionsHints(
+ VoidReturn, getLangOpts(), *Result.SourceManager,
+ VoidReturn->getBeginLoc());
+ if (BraceInsertionHints)
+ Diag << BraceInsertionHints.openingBraceFixIt()
+ << BraceInsertionHints.closingBraceFixIt();
+ }
+ Diag << FixItHint::CreateRemoval(VoidReturn->getReturnLoc());
+ if (!Result.Nodes.getNodeAs<FunctionDecl>("function_parent") ||
+ SurroundingBlock->body_back() != VoidReturn)
----------------
mikerice1969 wrote:
Hi @SimplyDanny, we are getting a static verifier hit on this code now since
there is a nullptr check of SurroundingBlock followed by a use of
SurroundingBlock without a check.
It seems when there is a function parent SurroundingBlock won't be a nullptr
since it will be set to the function's CompoundStmt, but that's not completely
obvious to the reader of the code.
You might consider adding an assert to make it clear for anyone modifying in
the future. Maybe:
```
const auto *FunctionParent =
Result.Nodes.getNodeAs<FunctionDecl>("function_parent");
assert((!FunctionParent || SurroundingBlock) &&
"missing surrounding block when function parent");
if (!FunctionParent || SurroundingBlock->body_back() != VoidReturn)
```
https://github.com/llvm/llvm-project/pull/81420
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
