[PATCH] D143632: [clang] Handle __declspec() attributes in using

[Aaron Ballman via Phabricator via cfe-commits]

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