hokein added inline comments.
================
Comment at: clang/lib/AST/DeclBase.cpp:400
+ for (; I != E; ++I) {
+ if (!I->isAlignmentDependent())
+ Align = std::max(Align, I->getAlignment(Ctx));
----------------
sammccall wrote:
> This doesn't seem great - previously if e.g. codegen ends up needing the
> alignment of a dependent decl somehow, then an assert will catch that
> programming error.
> Now it'll be silently swallowed. Seems better to check
> isAlignmentErrorDependent() here (you'd need to add that function) and
> continue to assert in other dependent cases.
ah, right. I think we should probably fix other similar places (where I added
FIXMEs). We don't have problems at the moment -- because the recovery-expr is
dependent, and they have been bailed out by isAlignmentDependent(). If we start
capture the concrete type for recovery-expr, the `isAlignmentDependent` bailout
will fail.
The `isAlignmentDependent` seems a bit confusing now, what does the `dependent`
mean? if we generalize it, it could mean the attr depends on a template
parameter (type, value, instantiation) or an error, in this sense
`isAlignmentDependent` should cover the error-dependent case. I think here it
only indicates the type-, value-, instantiation- dependent.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D78085/new/
https://reviews.llvm.org/D78085
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
