Match against all plugins when parsing microsoft attributes

[apache-hb]

Fixes #86422

[github-actions]

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be
notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write
permissions for the repository. In which case you can instead tag reviewers by
name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review
by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate
is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Elliot (apache-hb)

Changes

Fixes #86422

---

Full diff: https://github.com/llvm/llvm-project/pull/86426.diff

1 Files Affected:

• (modified) clang/lib/Parse/ParseDeclCXX.cpp (+4-3)

diff --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp
index 63fe678cbb29e2..d05b3a455f7f63 100644
--- a/clang/lib/Parse/ParseDeclCXX.cpp
+++ b/clang/lib/Parse/ParseDeclCXX.cpp
@@ -5061,11 +5061,12 @@ void Parser::ParseMicrosoftAttributes(ParsedAttributes &Attrs) {
         IdentifierInfo *II = Tok.getIdentifierInfo();
         SourceLocation NameLoc = Tok.getLocation();
         ConsumeToken();
-        ParsedAttr::Kind AttrKind =
-            ParsedAttr::getParsedKind(II, nullptr, ParsedAttr::AS_Microsoft);
+
         // For HLSL we want to handle all attributes, but for MSVC compat, we
         // silently ignore unknown Microsoft attributes.
-        if (getLangOpts().HLSL || AttrKind != ParsedAttr::UnknownAttribute) {
+        AttributeCommonInfo Info{II, NameLoc, AttributeCommonInfo::Form::Microsoft()};
+        const ParsedAttrInfo& AttrInfo = ParsedAttrInfo::get(Info);
+        if (getLangOpts().HLSL || AttrInfo.hasSpelling(AttributeCommonInfo::AS_Microsoft, II->getName())) {
           bool AttrParsed = false;
           if (Tok.is(tok::l_paren)) {
             CachedTokens OpenMPTokens;

[github-actions]

⚠️ We detected that you are using a GitHub private e-mail address to contribute to the repo.
Please turn off Keep my email addresses private setting in your account.
See LLVM Discourse for more information.

[Sirraide]

In addition to the comment below, this also needs a release note as well as some tests.

Also, as the bot has already pointed out, you need to set your email address to public.

[to268]

CC: @erichkeane, since it's a PR about attributes.
I can't add the extension::microsoftlabel to the PR myself.

[erichkeane]

This has enough to do with parsing that i think Aaron should take a look too.

Also, this needs a parser test, not just the plugin test. The plugins/examples aren't built often enough, so this needs to be a full lit test.

[github-actions]

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff 5d7fd6a04a6748936dece9d90481b2ba4ec97e53 6ba913fb0d3efdb17ae481ccad63ccde2170d4e2 -- clang/examples/MicrosoftAttributes/MicrosoftAttributes.cpp clang/test/Frontend/ms-attributes.cpp clang/lib/Parse/ParseDeclCXX.cpp

View the diff from clang-format here.

diff --git a/clang/examples/MicrosoftAttributes/MicrosoftAttributes.cpp b/clang/examples/MicrosoftAttributes/MicrosoftAttributes.cpp
index 571d6f8624..dd252f7d5b 100644
--- a/clang/examples/MicrosoftAttributes/MicrosoftAttributes.cpp
+++ b/clang/examples/MicrosoftAttributes/MicrosoftAttributes.cpp
@@ -25,34 +25,35 @@ using namespace clang;
 namespace {
 
 struct ExampleAttrInfo : public ParsedAttrInfo {
-    ExampleAttrInfo() {
-        // Can take up to 1 optional argument.
-        OptArgs = 1;
-
-        // Only Microsoft-style [ms_example] supported.
-        // e.g.:
-        // [ms_example] class C {};
-        // [ms_example] void f() {}
-        static constexpr Spelling S[] = {
-            {ParsedAttr::AS_Microsoft, "ms_example"}
-        };
-        Spellings = S;
-    }
-
-    bool diagAppertainsToDecl(Sema &S, const ParsedAttr &Attr, const Decl *D) const override {
-        // This attribute can be applied to any declaration.
-        if (!isa<CXXRecordDecl>(D)) {
-            S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type_str)
-                << Attr << Attr.isRegularKeywordAttribute() << "classes";
-        }
-        return true;
-    }
-
-    AttrHandling handleDeclAttribute(Sema &S, Decl *D, const ParsedAttr &Attr) const override {
-        // Add an annotation to the declaration.
-        D->addAttr(AnnotateAttr::Create(S.Context, "ms_example", nullptr, 0, Attr.getRange()));
-        return AttributeApplied;
+  ExampleAttrInfo() {
+    // Can take up to 1 optional argument.
+    OptArgs = 1;
+
+    // Only Microsoft-style [ms_example] supported.
+    // e.g.:
+    // [ms_example] class C {};
+    // [ms_example] void f() {}
+    static constexpr Spelling S[] = {{ParsedAttr::AS_Microsoft, "ms_example"}};
+    Spellings = S;
+  }
+
+  bool diagAppertainsToDecl(Sema &S, const ParsedAttr &Attr,
+                            const Decl *D) const override {
+    // This attribute can be applied to any declaration.
+    if (!isa<CXXRecordDecl>(D)) {
+      S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type_str)
+          << Attr << Attr.isRegularKeywordAttribute() << "classes";
     }
+    return true;
+  }
+
+  AttrHandling handleDeclAttribute(Sema &S, Decl *D,
+                                   const ParsedAttr &Attr) const override {
+    // Add an annotation to the declaration.
+    D->addAttr(AnnotateAttr::Create(S.Context, "ms_example", nullptr, 0,
+                                    Attr.getRange()));
+    return AttributeApplied;
+  }
 };
 
 } // namespace
diff --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp
index 619f7f88bf..b323f053c4 100644
--- a/clang/lib/Parse/ParseDeclCXX.cpp
+++ b/clang/lib/Parse/ParseDeclCXX.cpp
@@ -5064,8 +5064,8 @@ void Parser::ParseMicrosoftAttributes(ParsedAttributes &Attrs) {
 
         // For HLSL we want to handle all attributes, but for MSVC compat, we
         // silently ignore unknown Microsoft attributes.
-        int Attr = hasAttribute(AttributeCommonInfo::Syntax::AS_Microsoft, nullptr,
-                                II, getTargetInfo(), getLangOpts());
+        int Attr = hasAttribute(AttributeCommonInfo::Syntax::AS_Microsoft,
+                                nullptr, II, getTargetInfo(), getLangOpts());
         if (getLangOpts().HLSL || Attr != 0) {
           bool AttrParsed = false;
           if (Tok.is(tok::l_paren)) {

[AaronBallman]

I'm really sorry for not realizing this was held up on me -- mea culpa!

This LGTM (modulo the request for a parser test @erichkeane suggested) with a caveat: Clang has ignored Microsoft style attributes for a long time, so it's entirely possible that we don't correctly implement their parsing in all cases.

[AaronBallman]

Suggested change

	//===- Attribute.cpp ------------------------------------------------------===//
	//===- MicrosoftAttributes.cpp --------------------------------------------===//