ZarkoCA marked 8 inline comments as done.
ZarkoCA added inline comments.
================
Comment at: clang/include/clang/Sema/Sema.h:12693-12695
void CheckArgAlignment(SourceLocation Loc, NamedDecl *FDecl,
- StringRef ParamName, QualType ArgTy, QualType
ParamTy);
+ StringRef ParamName, QualType ArgTy, QualType ParamTy,
+ const Expr *Arg = nullptr);
----------------
ZarkoCA wrote:
> aaron.ballman wrote:
> > I'm not keen on passing both `Arg` and `ArgTy` such that they can get out
> > of sync. Do all of the places calling `CheckArgAlignment()` have access to
> > the `Expr` so that we can require it be passed (and drop the `ArgTy`
> > parameter)?
> Thanks, that is something I overlooked.
>
> It seems like I can do this everywhere except the call from
> `Sema::CheckConstructorCall`. Trying to figure out whether it's something I'm
> missing.
Thanks for the through review, I think I addressed everything but this comment.
I agree with your concern about having `Arg` and `ArgTy` getting out of sync. I
need to spend more time on that particular call from
`Sema::CheckConstructorCall` and see what can be done.
================
Comment at: clang/lib/Sema/SemaChecking.cpp:5220
+ // Using AArg so as to not modify Arg for the rest of the function.
+ const Expr *AArg = Arg->IgnoreParens();
+ if (AArg->getStmtClass() == Stmt::ImplicitCastExprClass) {
----------------
aaron.ballman wrote:
> Are there other things you want to ignore here (such as
> `IgnoreParenNoopCasts()`)? (I don't have an opinion the current code is
> wrong, just checking if those sort of cases were considered or not.)
Yes, thanks. I do have suspicions that `IgnoreParens()` may be too broad but I
am worried that we may miss cases to diagnose otherwise.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D118350/new/
https://reviews.llvm.org/D118350
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
