================
@@ -10397,6 +10347,71 @@ void Sema::CheckMemaccessArguments(const CallExpr
*Call,
}
}
+bool Sema::CheckSizeOfExpression(const Expr *LenExpr, const Expr *Dest,
+ llvm::FoldingSetNodeID SizeOfArgID,
+ IdentifierInfo *FnName) {
+ const Expr *SizeOfArg = getSizeOfExprArg(LenExpr);
----------------
ojhunt wrote:
~~The old code had a SizeOfArg != nullptr check, I can't see how that's not
necessary any more sorry - either an assertion on non-null-ness or a null check
would be warranted here.
If the null check is necessary it would be worth adding a test that faults due
to a null sifeofexprarg - my method for making such tests is to get a test to
fail without fixing the bug (e.g the potential need for a sizeofexpr != null
check) and then add the fix.~~
Oh I see the check is much further down. I think early returns are the better
choice here as they make it clearer what is happening.
I would perform these checks earlier
```cpp
bool Sema::CheckSizeOfExpression(const Expr *LenExpr, const Expr *Dest,
llvm::FoldingSetNodeID SizeOfArgID,
IdentifierInfo *FnName) {
const Expr *SizeOfArg = getSizeOfExprArg(LenExpr);
if (!SizeOfArg)
return false;
if (Diags.isIgnored(diag::warn_sizeof_pointer_expr_memaccess,
SizeOfArg->getExprLoc())
return false;
const PointerType *DestPtrTy = DestTy->getAs<PointerType>();
if (!DestPtrTy)
return false;
...
```
In general for trivial branches that lead to `return`/`break`/`continue` I
believe an early `return`/`break`/`continue` is always the better option as it
makes the `else` side of the check much more immediately obvious, keeps the
evaluation more overtly straight line, and ameliorates the excessively low
column limit in llvm/clang.
https://github.com/llvm/llvm-project/pull/170637
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
