[PATCH] D103825: [clang] Do not crash when ArgTy is null in CheckArgAlignment

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