AaronBallman wrote:
> The part I'm confused about is that this commit doesn't appear to simply be
> adding "or `alignas`" to some code that said "if `_Alignas`". Yes, the newly
> added `isAlignas()` checks do cover both attributes, but `_Alignas` was still
> working even after
> [b9cf7f1](https://github.com/llvm/llvm-project/commit/b9cf7f1066cc7ec11c6074d10b0f2ec9a3ae72d9).
>
> That is, in clang trunk, this test does still pass when parsed as -std=c23:
>
> ```
> struct X {
> _Alignas(8) char n;
> };
>
> static_assert(_Alignof(struct X) == 8, "");
> ```
>
> So why does this PR need to add new code to move `alignas`, instead of having
> `alignas` use whatever codepath was already handling `_Alignas`? Or,
> alternatively remove whatever code was formerly handling `_Alignas`, in favor
> of this new code?
>
> Or, are `alignas` and `_Alignas` actually getting parsed differently, even
> still, so only `alignas` needs to be moved?
Ah, thank you for the further details! The issue boils down to the fact that
`alignas` is not an attribute in C23 (it's a type-specifier-qualifier) but it
is an attribute in C++. So we hit awkward situations where we claim the
attribute is a C++11-style attribute, such as in `ProcessDeclAttribute()`. We
explicitly disallow C++11 attributes on the declaration specifier:
```
// Apply decl attributes from the DeclSpec if present.
if (!PD.getDeclSpec().getAttributes().empty()) {
ProcessDeclAttributeList(S, D, PD.getDeclSpec().getAttributes(),
ProcessDeclAttributeOptions()
.WithIncludeCXX11Attributes(false)
.WithIgnoreTypeAttributes(true));
}
```
but when the attribute is on the declaration itself, we allow C++11 attributes:
```
ProcessDeclAttributeList(S, D, NonSlidingAttrs);
```
So there *is* a better way to address this and I think it actually fixes a bug
that I didn't notice until now with non-structure use. The changes look a bit
odd because I also had to make an adjustment to ensure that `matrix_type`
continues to work (it's an odd beast that already required special handling,
that special handling is now needed in two places).
https://github.com/llvm/llvm-project/pull/98642
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
