Skip to content
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,7 @@ Bug Fixes in This Version
a function without arguments caused us to try to access a non-existent argument.
(#GH159080)
- Fixed a failed assertion with empty filename arguments in ``__has_embed``. (#GH159898)
- Accept empty enum in MSVC-compatible C. (#GH114402)

Bug Fixes to Compiler Builtins
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand Down
4 changes: 3 additions & 1 deletion clang/include/clang/Basic/DiagnosticGroups.td
Original file line number Diff line number Diff line change
Expand Up @@ -1435,6 +1435,7 @@ def MicrosoftDrectveSection : DiagGroup<"microsoft-drectve-section">;
def MicrosoftInclude : DiagGroup<"microsoft-include">;
def MicrosoftCppMacro : DiagGroup<"microsoft-cpp-macro">;
def MicrosoftFixedEnum : DiagGroup<"microsoft-fixed-enum">;
def MicrosoftEmptyEnum : DiagGroup<"microsoft-empty-enum">;
def MicrosoftSealed : DiagGroup<"microsoft-sealed">;
def MicrosoftAbstract : DiagGroup<"microsoft-abstract">;
def MicrosoftUnqualifiedFriend : DiagGroup<"microsoft-unqualified-friend">;
Expand Down Expand Up @@ -1488,7 +1489,8 @@ def Microsoft : DiagGroup<"microsoft",
MicrosoftConstInit, MicrosoftVoidPseudoDtor, MicrosoftAnonTag,
MicrosoftCommentPaste, MicrosoftEndOfFile,
MicrosoftInitFromPredefined, MicrosoftStringLiteralFromPredefined,
MicrosoftInconsistentDllImport, MicrosoftInlineOnNonFunction]>;
MicrosoftInconsistentDllImport, MicrosoftInlineOnNonFunction,
MicrosoftEmptyEnum]>;

def ClangClPch : DiagGroup<"clang-cl-pch">;

Expand Down
3 changes: 3 additions & 0 deletions clang/include/clang/Basic/DiagnosticParseKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,9 @@ def err_enumerator_unnamed_no_def : Error<
def ext_ms_c_enum_fixed_underlying_type : Extension<
"enumeration types with a fixed underlying type are a Microsoft extension">,
InGroup<MicrosoftFixedEnum>;
def ext_ms_c_empty_enum_type : Extension<
"empty enumeration types are a Microsoft extension">,
InGroup<MicrosoftEmptyEnum>;
def ext_c23_enum_fixed_underlying_type : Extension<
"enumeration types with a fixed underlying type are a C23 extension">,
InGroup<C23>;
Expand Down
9 changes: 7 additions & 2 deletions clang/lib/Parse/ParseDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5360,8 +5360,13 @@ void Parser::ParseEnumBody(SourceLocation StartLoc, Decl *EnumDecl,
T.consumeOpen();

// C does not allow an empty enumerator-list, C++ does [dcl.enum].
if (Tok.is(tok::r_brace) && !getLangOpts().CPlusPlus)
Diag(Tok, diag::err_empty_enum);
if (Tok.is(tok::r_brace) && !getLangOpts().CPlusPlus) {
if (getLangOpts().MicrosoftExt)
Diag(T.getOpenLocation(), diag::ext_ms_c_empty_enum_type)
<< SourceRange(T.getOpenLocation(), Tok.getLocation());
else
Diag(Tok, diag::err_empty_enum);
}

SmallVector<Decl *, 32> EnumConstantDecls;
SmallVector<SuppressAccessChecks, 32> EnumAvailabilityDiags;
Expand Down
6 changes: 6 additions & 0 deletions clang/test/Parser/ms-empty-enum.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// RUN: %clang_cc1 %s -fsyntax-only -Wmicrosoft -verify -fms-extensions

typedef enum tag1 { } A; // expected-warning {{empty enumeration types are a Microsoft extension}}
typedef enum tag2 { } B; // expected-warning {{empty enumeration types are a Microsoft extension}}
typedef enum : unsigned { } C; // expected-warning {{enumeration types with a fixed underlying type are a Microsoft extension}}\
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't warn 2x, the 'with a fixed underlying type' version is sufficient.

Also, isn't this a C23 extension, not a microsoft extension (or at least, as well)? See: ext_c23_enum_fixed_underlying_type.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review!

Also, isn't this a C23 extension, not a microsoft extension (or at least, as well)? See: ext_c23_enum_fixed_underlying_type.

In microsoft-extension mode, it's ext_ms_c_enum_fixed_underlying_type but not ext_ms_c_enum_fixed_underlying_type.

This shouldn't warn 2x, the 'with a fixed underlying type' version is sufficient.

Should we suppress the 2nd empty enumeration types are a Microsoft extension warning? Seems they are 2 different issue.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a little confused here as to the order of warnings/what we should do. I suspect @AaronBallman and I need to spend some time figuring out/discussing what we're looking for.

I'm not sure about the suppressing the 2nd warning, there is perhaps a 'we should do both' kinda thing since one might be disabled, but it also looks silly.

Ping this in another ~week and a half, and Aaron and I can discuss it (he should be back by then!).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think emitting two diagnostics is fine (one for the underlying type and one for the empty enum) because they are separate extensions. But I think it's a bit odd that we claim the underlying type is a Microsoft extension rather than a C23 extension -- that seems to be a preexisting issue which could be handled in a separate PR: https://godbolt.org/z/sMhGoE3qq

// expected-warning {{empty enumeration types are a Microsoft extension}}