[PATCH] D103825: [clang] Do not crash when ArgTy is null in CheckArgAlignment
Haojian Wu via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Jun 8 23:24:27 PDT 2021
hokein added a comment.
In D103825#2805760 <https://reviews.llvm.org/D103825#2805760>, @adamcz wrote:
> FYI The ArgTy.isNull() check is sufficient to fix this. The Arg->containsErrors() is not - it's false in this case, since it seems CXXDefaultArgExpr with RecoveryExpr inside seems to not contains errors, according to containsError(). I can't tell if that's by design, or a bug.
>
> Haojian, please let me know if you think Arg->containsError() should be true or if we should be testing somewhere else, or just drop it and accept that CheckArgAlignment may be called with RecoveryExpr and have it check for ArgTy->isNull().
>
> Basically, let me know what you think about this. Thanks!
Thanks for the analysis.
Yeah, I think this is a bug in clang when computing the dependence <https://github.com/llvm/llvm-project/blob/main/clang/include/clang/AST/ExprCXX.h#L1260> of CXXDefaultArgExpr -- the CXXDefaultArgExpr should respect the dependence of its actual argument (`getExpr()`). I think we should fix it first.
Regarding to test it, the best way is to dump the AST node and verify the containsError is in the dump string.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D103825/new/
https://reviews.llvm.org/D103825
More information about the cfe-commits
mailing list