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

[Aaron Ballman via Phabricator via llvm-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM aside from some minor nits that you can fix when landing, thank you for the fix!



================
Comment at: clang/docs/ReleaseNotes.rst:112-114
+__declspec attributes can now be used together with the using keyword. Before
+the attributes on __declspec was ignored, while now it will be forwarded to the
+point where the alias is used.
----------------



================
Comment at: clang/lib/Parse/ParseDecl.cpp:60-61
+  // Move declspec attributes to ParsedAttributes
+  if (Attrs)
+  {
+    llvm::SmallVector<ParsedAttr*, 1> ToBeMoved;
----------------
Formatting is off here.


================
Comment at: clang/lib/Parse/ParseDecl.cpp:61-63
+  for (ParsedAttr &AL : DS.getAttributes())
+    if (AL.isDeclspecAttribute())
+      ToBeMoved.push_back(&AL);
----------------
thieta wrote:
> aaron.ballman wrote:
> > How about using an algorithm for this (would need to be reformatted for 80 col)?
> I spent a while on trying to figure this out. I am not that great with the algorithms in the standard library since we don't really use that at work. But from my testing I couldn't get it to work:
> 
> - DS.getAttributes() returns a `ParsedAttr&`, but ToBeMoved needs a pointer to ParsedAttr. So your example code above fails because it can't convert ParsedAttr& to ParsedAttr* in the back_inserter()
> - I tried to change ToBeMoved to be a list of references instead, but ParsedAttr disallows copying (I think, the error message was a bit confusing).
> - I could do transform() to a list of pointers and then copy_if() but that seemed to really make it harder to understand.
> - A transform_if() would solve the problem or I guess replace back_inserter() with some out iterator that could transform. But I couldn't find anything used like this browsing the LLVM source code.
> 
> So yeah - I might totally be missing something obvious here, feel free to point it out.
> So yeah - I might totally be missing something obvious here, feel free to point it out.

Thank you for trying it out, I'd say let's leave it as-is (but fix the formatting issues).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D143632/new/

https://reviews.llvm.org/D143632