[PATCH] Recognize __unaligned in more cases

[Aaron Ballman]

On Mon, Nov 24, 2014 at 4:57 PM, Nico Rieck <nico.rieck at gmail.com> wrote:
> On 24.11.2014 20:21, Aaron Ballman wrote:
>> When I compile with MSVC 2013, this example gives me:
>>
>> main.cpp(5): warning C4228: nonstandard extension used : qualifiers
>> after comma in declarator list are ignored
>
> I've noticed the SDK headers explicitly silence this warning, so this
> shouldn't actually add any attributes.
>
>> Why not handle other type qualifiers that Microsoft supports? Does
>> that turn out to be messy for some reason? It seems like we could just
>> call Parser::ParseMicrosoftTypeAttributes?
>
> I've looked further into this. MSVC also accepts const and volatile
> there and these keywords should just be ignored. So
> ParseMicrosoftTypeAttributes doesn't quite fit. This also deserves a
> warning. I've updated the patch accordingly and split this extension
> into a separate commit.

recognized-__unaligned-after-type-specifier.patch LGTM.
ms-ext-ignored-qualifiers.patch comments below.

> diff --git a/include/clang/Basic/DiagnosticParseKinds.td b/include/clang/Basic/DiagnosticParseKinds.td
> index 6bf1910..106b716 100644
> --- a/include/clang/Basic/DiagnosticParseKinds.td
> +++ b/include/clang/Basic/DiagnosticParseKinds.td
> @@ -108,6 +108,8 @@ def ext_alignof_expr : ExtWarn<
>  def warn_microsoft_dependent_exists : Warning<
>    "dependent %select{__if_not_exists|__if_exists}0 declarations are ignored">,
>    InGroup<DiagGroup<"microsoft-exists">>;
> +def warn_microsoft_qualifiers_ignored : Warning<
> +  "qualifiers after comma in declarator list are ignored">;

This should be in the IgnoredAttributes warning group.

>
>  def ext_c11_generic_selection : Extension<
>    "generic selections are a C11-specific feature">, InGroup<C11>;
> diff --git a/include/clang/Parse/Parser.h b/include/clang/Parse/Parser.h
> index 38073c3..d33d4c5 100644
> --- a/include/clang/Parse/Parser.h
> +++ b/include/clang/Parse/Parser.h
> @@ -2094,6 +2094,7 @@ private:
>                                    SourceLocation AttrNameLoc,
>                                    ParsedAttributes &Attrs);
>    void ParseMicrosoftTypeAttributes(ParsedAttributes &attrs);
> +  void SkipMicrosoftTypeAttributes();
>    void ParseMicrosoftInheritanceClassAttributes(ParsedAttributes &attrs);
>    void ParseBorlandTypeAttributes(ParsedAttributes &attrs);
>    void ParseOpenCLAttributes(ParsedAttributes &attrs);
> diff --git a/lib/Parse/ParseDecl.cpp b/lib/Parse/ParseDecl.cpp
> index fbf3337..5e87705 100644
> --- a/lib/Parse/ParseDecl.cpp
> +++ b/lib/Parse/ParseDecl.cpp
> @@ -614,6 +614,30 @@ void Parser::ParseMicrosoftTypeAttributes(ParsedAttributes &attrs) {
>    }
>  }
>
> +void Parser::SkipMicrosoftTypeAttributes() {
> +  while (true) {
> +    switch (Tok.getKind()) {
> +    case tok::kw_const:
> +    case tok::kw_volatile:
> +    case tok::kw___fastcall:
> +    case tok::kw___stdcall:
> +    case tok::kw___thiscall:
> +    case tok::kw___cdecl:
> +    case tok::kw___vectorcall:
> +    case tok::kw___ptr32:
> +    case tok::kw___ptr64:
> +    case tok::kw___w64:
> +    case tok::kw___unaligned:
> +    case tok::kw___sptr:
> +    case tok::kw___uptr:
> +      Diag(ConsumeToken(), diag::warn_microsoft_qualifiers_ignored);

I think the usual pattern for this is to make a Skip function that
parses and skips attribtues (returning the end location), and a
DiagnoseAndSkip which diagnoses if any attributes were skipped. We may
not need the separation, but it would be good to keep the
nomenclature.

> +      break;
> +    default:
> +      return;
> +    }
> +  }
> +}
> +
>  void Parser::ParseBorlandTypeAttributes(ParsedAttributes &attrs) {
>    // Treat these like attributes
>    while (Tok.is(tok::kw___pascal)) {
> @@ -1732,6 +1756,10 @@ Parser::DeclGroupPtrTy Parser::ParseDeclGroup(ParsingDeclSpec &DS,
>      //    short x, __attribute__((common)) var;    -> declarator
>      MaybeParseGNUAttributes(D);
>
> +    // MSVC parses but ignores qualifiers after the comma as an extensions.

"Extensions" should be "extension."

> +    if (getLangOpts().MicrosoftExt)
> +      SkipMicrosoftTypeAttributes();
> +
>      ParseDeclarator(D);
>      if (!D.isInvalidType()) {
>        Decl *ThisDecl = ParseDeclarationAfterDeclarator(D);
> diff --git a/test/Parser/MicrosoftExtensions.c b/test/Parser/MicrosoftExtensions.c
> index d7ea20b..fb09a1f 100644
> --- a/test/Parser/MicrosoftExtensions.c
> +++ b/test/Parser/MicrosoftExtensions.c
> @@ -85,3 +85,11 @@ int * __uptr __ptr64 pup64;
>
>  /* Legal to have nested pointer attributes */
>  int * __sptr * __ptr32 ppsp32;
> +
> +// Ignored type qualifiers after comma in declarator lists
> +typedef int ignored_quals_dummy1, const volatile __ptr32 __ptr64 __w64 __unaligned __sptr __uptr ignored_quals1; // expected-warning 8 {{qualifiers after comma in declarator list are ignored}}
> +typedef void(*ignored_quals_dummy2)(), __fastcall ignored_quals2; // expected-warning {{qualifiers after comma in declarator list are ignored}}
> +typedef void(*ignored_quals_dummy3)(), __stdcall ignored_quals3; // expected-warning {{qualifiers after comma in declarator list are ignored}}
> +typedef void(*ignored_quals_dummy4)(), __thiscall ignored_quals4; // expected-warning {{qualifiers after comma in declarator list are ignored}}
> +typedef void(*ignored_quals_dummy5)(), __cdecl ignored_quals5; // expected-warning {{qualifiers after comma in declarator list are ignored}}
> +typedef void(*ignored_quals_dummy6)(), __vectorcall ignored_quals6; // expected-warning {{qualifiers after comma in declarator list are ignored}}
> diff --git a/test/Parser/MicrosoftExtensions.cpp b/test/Parser/MicrosoftExtensions.cpp
> index 85ccdc5..96fdf44 100644
> --- a/test/Parser/MicrosoftExtensions.cpp
> +++ b/test/Parser/MicrosoftExtensions.cpp
> @@ -366,3 +366,11 @@ void foo(void) {
>  template <int *>
>  struct NullptrArg {};
>  NullptrArg<nullptr> a;
> +
> +// Ignored type qualifiers after comma in declarator lists
> +typedef int ignored_quals_dummy1, const volatile __ptr32 __ptr64 __w64 __unaligned __sptr __uptr ignored_quals1; // expected-warning 8 {{qualifiers after comma in declarator list are ignored}}
> +typedef void(*ignored_quals_dummy2)(), __fastcall ignored_quals2; // expected-warning {{qualifiers after comma in declarator list are ignored}}
> +typedef void(*ignored_quals_dummy3)(), __stdcall ignored_quals3; // expected-warning {{qualifiers after comma in declarator list are ignored}}
> +typedef void(*ignored_quals_dummy4)(), __thiscall ignored_quals4; // expected-warning {{qualifiers after comma in declarator list are ignored}}
> +typedef void(*ignored_quals_dummy5)(), __cdecl ignored_quals5; // expected-warning {{qualifiers after comma in declarator list are ignored}}
> +typedef void(*ignored_quals_dummy6)(), __vectorcall ignored_quals6; // expected-warning {{qualifiers after comma in declarator list are ignored}}
> --
> 1.9.4.msysgit.2
>
>

Thanks!

~Aaron