thakis closed this revision.
thakis marked an inline comment as done.
thakis added a comment.
Thanks! Landed in r280574 r280575 r280576 280578.
https://reviews.llvm.org/D23895
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm
rsmith accepted this revision.
rsmith added a comment.
For submit, can you separate the changes to generally support `[]` attribute
syntax and the changes to parse `[uuid(...)]` into distinct commits? (I'm OK
with the `Microsoft` attribute support piece landing with no tests since the
`uuid` pa
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.
LGTM with one small test case nit.
Comment at: test/Parser/ms-square-bracket-attributes.mm:43
@@ +42,3 @@
+// expected-error@+1 {{expected '('}}
+[uuid{"00A0
thakis marked an inline comment as done.
Comment at: lib/Parse/ParseDeclCXX.cpp:4037
@@ -3945,1 +4036,3 @@
+}
+
T.consumeClose();
Aha, I had forgotten that SkipUntil() handles paren nesting intelligently, so I
think just requiring a '(' after 'uuid' with
thakis updated this revision to Diff 70197.
thakis added a comment.
Require '('.
https://reviews.llvm.org/D23895
Files:
include/clang/Basic/Attr.td
include/clang/Basic/Attributes.h
include/clang/Parse/Parser.h
include/clang/Sema/AttributeList.h
lib/Parse/ParseDecl.cpp
lib/Parse/Pars
aaron.ballman added inline comments.
Comment at: include/clang/Basic/Attr.td:201
@@ -200,2 +200,3 @@
class Declspec : Spelling;
+class Microsoft : Spelling;
class CXX11
aaron.ballman wrote:
> thakis wrote:
> > rsmith wrote:
> > > Given that MS have deprecated th
thakis added inline comments.
Comment at: lib/Parse/ParseDeclCXX.cpp:4031
@@ +4030,3 @@
+ ConsumeToken();
+ if (Name->getName() == "uuid" && Tok.is(tok::l_paren))
+ParseMicrosoftUuidAttributeArgs(Name, Loc, attrs);
aaron.ballman wrote:
> thakis w
aaron.ballman added inline comments.
Comment at: lib/Parse/ParseDeclCXX.cpp:4031
@@ +4030,3 @@
+ ConsumeToken();
+ if (Name->getName() == "uuid" && Tok.is(tok::l_paren))
+ParseMicrosoftUuidAttributeArgs(Name, Loc, attrs);
thakis wrote:
> aaron.ba
thakis added inline comments.
Comment at: lib/Parse/ParseDeclCXX.cpp:4031
@@ +4030,3 @@
+ ConsumeToken();
+ if (Name->getName() == "uuid" && Tok.is(tok::l_paren))
+ParseMicrosoftUuidAttributeArgs(Name, Loc, attrs);
aaron.ballman wrote:
> thakis w
aaron.ballman added inline comments.
Comment at: include/clang/Basic/Attr.td:201
@@ -200,2 +200,3 @@
class Declspec : Spelling;
+class Microsoft : Spelling;
class CXX11
thakis wrote:
> rsmith wrote:
> > Given that MS have deprecated this syntax and are trying to
thakis added inline comments.
Comment at: include/clang/Basic/Attr.td:201
@@ -200,2 +200,3 @@
class Declspec : Spelling;
+class Microsoft : Spelling;
class CXX11
rsmith wrote:
> Given that MS have deprecated this syntax and are trying to move away from it
> tow
rsmith added inline comments.
Comment at: include/clang/Basic/Attr.td:201
@@ -200,2 +200,3 @@
class Declspec : Spelling;
+class Microsoft : Spelling;
class CXX11
Given that MS have deprecated this syntax and are trying to move away from it
towards `__declspec`,
aaron.ballman added inline comments.
Comment at: lib/Parse/ParseDeclCXX.cpp:4031
@@ +4030,3 @@
+ ConsumeToken();
+ if (Name->getName() == "uuid" && Tok.is(tok::l_paren))
+ParseMicrosoftUuidAttributeArgs(Name, Loc, attrs);
thakis wrote:
> aaron.ba
thakis added inline comments.
Comment at: lib/Parse/ParseDeclCXX.cpp:4031
@@ +4030,3 @@
+ ConsumeToken();
+ if (Name->getName() == "uuid" && Tok.is(tok::l_paren))
+ParseMicrosoftUuidAttributeArgs(Name, Loc, attrs);
aaron.ballman wrote:
> Silently
aaron.ballman added inline comments.
Comment at: lib/Parse/ParseDeclCXX.cpp:4031
@@ +4030,3 @@
+ ConsumeToken();
+ if (Name->getName() == "uuid" && Tok.is(tok::l_paren))
+ParseMicrosoftUuidAttributeArgs(Name, Loc, attrs);
Silently ignoring seems
thakis added inline comments.
Comment at: include/clang/Parse/Parser.h:2217
@@ -2215,1 +2216,3 @@
+ SourceLocation UuidLoc,
+ ParsedAttributes &attrs);
void ParseMicrosoftAttributes(ParsedAttributes &at
thakis updated this revision to Diff 70036.
thakis marked 6 inline comments as done.
thakis added a comment.
Address comments.
https://reviews.llvm.org/D23895
Files:
include/clang/Basic/Attr.td
include/clang/Basic/Attributes.h
include/clang/Parse/Parser.h
include/clang/Sema/AttributeLis
aaron.ballman added inline comments.
Comment at: include/clang/Parse/Parser.h:2217
@@ -2215,1 +2216,3 @@
+ SourceLocation UuidLoc,
+ ParsedAttributes &attrs);
void ParseMicrosoftAttributes(ParsedAttribu
rnk added inline comments.
Comment at: lib/Parse/ParseDeclCXX.cpp:3943
@@ +3942,3 @@
+ } else {
+// something like uuid({00A0---C000-0049}) -- no
+// quotes in the parens. Just append the spelling of all tokens encountered
This isn't s
thakis updated this revision to Diff 70013.
thakis marked an inline comment as done.
thakis added a comment.
Sorry about the delay! Now with custom parsing code that accepts uuid() without
quotes. I thought this would be complicated, but ended up being not so bad in
the end. See the tests for se
thakis marked 2 inline comments as done.
thakis added a comment.
(Just wanted to ask if I'm missing something about these three bullets. I do
need to make a few other changes: More tests, parsing for uuid(1-2-3) without
quotes + tests, and I also want to add a deprecation warning with a fixit,
aaron.ballman added a comment.
In https://reviews.llvm.org/D23895#529275, @thakis wrote:
> > There are things missing from this patch as well. To wit:
>
> >
>
> > - Attributes.h needs to know about this attribute syntax, and
> > `hasAttribute()` needs to support it (this hooks into `__has_attri
thakis marked 3 inline comments as done.
thakis added a comment.
> There are things missing from this patch as well. To wit:
>
> - Attributes.h needs to know about this attribute syntax, and
> `hasAttribute()` needs to support it (this hooks into `__has_attribute`
> support).
> - AttributeLi
On Mon, Aug 29, 2016 at 7:52 PM, Reid Kleckner wrote:
> rnk added a comment.
>
> In https://reviews.llvm.org/D23895#528208, @aaron.ballman wrote:
>
>> You are removing a lot of instances of `MaybeParseMicrosoftAttributes()`,
>> but I'm not certain I understand why. Can you describe why you need t
rnk added a comment.
In https://reviews.llvm.org/D23895#528208, @aaron.ballman wrote:
> You are removing a lot of instances of `MaybeParseMicrosoftAttributes()`, but
> I'm not certain I understand why. Can you describe why you need to remove
> those?
That shouldn't change functionality, Nico
aaron.ballman requested changes to this revision.
aaron.ballman added a comment.
This revision now requires changes to proceed.
You are removing a lot of instances of `MaybeParseMicrosoftAttributes()`, but
I'm not certain I understand why. Can you describe why you need to remove those?
There are
rnk added a comment.
The code looks good and I'm happy to leave the naming as it is for now.
Apparently MSVC is warning on these attributes and calling them "ATL
attributes" now: https://msdn.microsoft.com/en-us/library/mt723604.aspx I'm not
sure if that warning applies to all square-bracketed
thakis added a comment.
rnk: ping
https://reviews.llvm.org/D23895
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
thakis updated this revision to Diff 69419.
thakis added a comment.
Aaron's right that these are currently called "Microsoft" in the source. I
agree that that's a confusing name and we should probably rename it, but on
second thought that seems unrelated to this change. If y'all agree on a name,
Microsoft used to call their IDL dialect 'MIDL' (which is kind of punny),
don't know if it makes sense to stick with that over 'MSIDL'.
- Kim
Den 26 aug. 2016 4:09 fm skrev "Nico Weber via cfe-commits" <
cfe-commits@lists.llvm.org>:
> thakis updated this revision to Diff 69312.
> thakis marked a
thakis updated this revision to Diff 69312.
thakis marked an inline comment as done.
thakis added a comment.
comments round 1
https://reviews.llvm.org/D23895
Files:
include/clang/Basic/Attr.td
include/clang/Basic/Attributes.h
include/clang/Parse/Parser.h
include/clang/Sema/AttributeList
thakis marked an inline comment as done.
thakis added a comment.
Thanks for the fast review and the good comments! I'll leave it to y'all to
agree on some name if you don't like the one I picked.
> Also, doesn't this introduce ambiguities into the grammar? Something like
> this:
>
> void u
On Thu, Aug 25, 2016 at 9:16 PM, Richard Smith wrote:
> On Thu, Aug 25, 2016 at 5:46 PM, Reid Kleckner wrote:
>>
>> rnk added a comment.
>>
>> I think these are known as "IDL attributes":
>> https://msdn.microsoft.com/en-us/library/8tesw2eh.aspx
>>
>> Let's update the naming to use that terminolo
On Thu, Aug 25, 2016 at 5:46 PM, Reid Kleckner wrote:
> rnk added a comment.
>
> I think these are known as "IDL attributes":
> https://msdn.microsoft.com/en-us/library/8tesw2eh.aspx
>
> Let's update the naming to use that terminology, so AS_MS should be
> AS_IDL, and MaybeParseMicrosoftAttribute
rnk added a comment.
I think these are known as "IDL attributes":
https://msdn.microsoft.com/en-us/library/8tesw2eh.aspx
Let's update the naming to use that terminology, so AS_MS should be AS_IDL, and
MaybeParseMicrosoftAttributes should be MaybeParseMicrosoftIDLAttributes, etc.
Also, doesn't t
rsmith added inline comments.
Comment at: include/clang/Basic/Attr.td:201
@@ -200,2 +200,3 @@
class Declspec : Spelling;
+class MS : Spelling;
class CXX11
Is there some better description for this than MS? `__declspec` is also an MS
attribute. Is it fair to cal
36 matches
Mail list logo
