aaron.ballman added a reviewer: erichkeane.
aaron.ballman added a comment.
Please be sure to add a release note for the fix.
================
Comment at: clang/lib/Parse/ParseDecl.cpp:61-63
+ for (ParsedAttr &AL : DS.getAttributes())
+ if (AL.isDeclspecAttribute())
+ ToBeMoved.push_back(&AL);
----------------
How about using an algorithm for this (would need to be reformatted for 80 col)?
================
Comment at: clang/lib/Parse/ParseDecl.cpp:65-66
+
+ for (ParsedAttr *AL : ToBeMoved)
+ Attrs->takeOneFrom(DS.getAttributes(), AL);
+
----------------
Similar here, but this one might be less of a win in terms of readability.
================
Comment at: clang/test/SemaCXX/using-declspec.cpp:4
+
+struct Test { int a; };
+using AlignedTest = __declspec(align(16)) const Test;
----------------
Because we're touching `ParseTypeName()` and that is called in a lot of
contexts, I think we should have extra testing in non-alias declaration cases
and verify that MSVC behaves the same way. e.g., (search for `ParseTypeName()`,
see where we parse type names, devise some test cases)
```
// I don't think the align attribute has a declaration to move onto in this
case...
auto func() -> __declspec(align(16)) int;
// So I *think* the alignment of the return type isn't changed?
static_assert(alignof(decltype(func())) == alignof(int));
// Same here, no declaration to shift to
int i = (__declspec(align(16))int)12;
// But there is a declaration here!
typedef __declspec(align(16)) int Foo;
static_assert(alignof(Foo) == 16);
```
(There are possibly other interesting tests to consider.)
I suspect we should be issuing some "attribute ignored" diagnostics for the
cases where the attribute has no effect (even if MSVC does not warn).
================
Comment at: clang/test/SemaCXX/using-declspec.cpp:7
+static_assert(alignof(AlignedTest) == 16, "error");
\ No newline at end of file
----------------
Please add a newline to the end of the file
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D143632/new/
https://reviews.llvm.org/D143632
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
