On 30/12/2013 03:45, Serge Pavlov wrote:
Updated the patch for new interface of SkipUntil.
Hi Serge,
You've had this patch for a while so I'll try to get the ball rolling.
There's some trailing whitespace and formatting which you can fix with
clang-format but I'll focus on the implementation from here out..
http://llvm-reviews.chandlerc.com/D2116
CHANGE SINCE LAST DIFF
http://llvm-reviews.chandlerc.com/D2116?vs=5391&id=6309#toc
Files:
include/clang/Basic/DiagnosticCommonKinds.td
lib/Parse/ParseDecl.cpp
test/Parser/cxx0x-ambig.cpp
test/Parser/declarators.c
D2116.2.patch
Index: include/clang/Basic/DiagnosticCommonKinds.td
===================================================================
--- include/clang/Basic/DiagnosticCommonKinds.td
+++ include/clang/Basic/DiagnosticCommonKinds.td
@@ -62,6 +62,7 @@
def err_expected : Error<"expected %0">;
def err_expected_either : Error<"expected %0 or %1">;
+def err_expected_one_of_three : Error<"expected %0 or %1 or %2">;
def err_expected_after : Error<"expected %1 after %0">;
def err_param_redefinition : Error<"redefinition of parameter %0">;
Index: lib/Parse/ParseDecl.cpp
===================================================================
--- lib/Parse/ParseDecl.cpp
+++ lib/Parse/ParseDecl.cpp
@@ -3888,58 +3888,82 @@
Decl *LastEnumConstDecl = 0;
// Parse the enumerator-list.
- while (Tok.is(tok::identifier)) {
- IdentifierInfo *Ident = Tok.getIdentifierInfo();
- SourceLocation IdentLoc = ConsumeToken();
-
- // If attributes exist after the enumerator, parse them.
- ParsedAttributesWithRange attrs(AttrFactory);
- MaybeParseGNUAttributes(attrs);
- MaybeParseCXX11Attributes(attrs);
- ProhibitAttributes(attrs);
-
- SourceLocation EqualLoc;
- ExprResult AssignedVal;
- ParsingDeclRAIIObject PD(*this, ParsingDeclRAIIObject::NoParent);
-
- if (TryConsumeToken(tok::equal, EqualLoc)) {
- AssignedVal = ParseConstantExpression();
- if (AssignedVal.isInvalid())
- SkipUntil(tok::comma, tok::r_brace, StopAtSemi | StopBeforeMatch);
- }
-
- // Install the enumerator constant into EnumDecl.
- Decl *EnumConstDecl = Actions.ActOnEnumConstant(getCurScope(), EnumDecl,
- LastEnumConstDecl,
- IdentLoc, Ident,
- attrs.getList(), EqualLoc,
- AssignedVal.release());
- PD.complete(EnumConstDecl);
-
- EnumConstantDecls.push_back(EnumConstDecl);
- LastEnumConstDecl = EnumConstDecl;
-
- if (Tok.is(tok::identifier)) {
- // We're missing a comma between enumerators.
- SourceLocation Loc = PP.getLocForEndOfToken(PrevTokLocation);
- Diag(Loc, diag::err_enumerator_list_missing_comma)
- << FixItHint::CreateInsertion(Loc, ", ");
- continue;
- }
-
- SourceLocation CommaLoc;
- if (!TryConsumeToken(tok::comma, CommaLoc))
- break;
-
- if (Tok.isNot(tok::identifier)) {
- if (!getLangOpts().C99 && !getLangOpts().CPlusPlus11)
- Diag(CommaLoc, getLangOpts().CPlusPlus ?
- diag::ext_enumerator_list_comma_cxx :
- diag::ext_enumerator_list_comma_c)
- << FixItHint::CreateRemoval(CommaLoc);
- else if (getLangOpts().CPlusPlus11)
- Diag(CommaLoc, diag::warn_cxx98_compat_enumerator_list_comma)
- << FixItHint::CreateRemoval(CommaLoc);
+ if (Tok.isNot(tok::r_brace)) {
Nesting is getting deep here. Can this loop be restructured so it's
easier to follow the flow as it iterates through the enumerator-list?
+ while (true) {
+ if (Tok.isNot(tok::identifier)) {
+ Diag(Tok.getLocation(), diag::err_expected) << tok::identifier;
+ if (SkipUntil(tok::comma, tok::r_brace, StopBeforeMatch) &&
+ TryConsumeToken(tok::comma)) continue;
+ break;
+ }
+ IdentifierInfo *Ident = Tok.getIdentifierInfo();
+ SourceLocation IdentLoc = ConsumeToken();
+
+ // If attributes exist after the enumerator, parse them.
+ ParsedAttributesWithRange attrs(AttrFactory);
+ MaybeParseGNUAttributes(attrs);
+ MaybeParseCXX11Attributes(attrs);
+ ProhibitAttributes(attrs);
+
+ SourceLocation EqualLoc;
+ ExprResult AssignedVal;
+ ParsingDeclRAIIObject PD(*this, ParsingDeclRAIIObject::NoParent);
+
+ if (TryConsumeToken(tok::equal, EqualLoc)) {
+ AssignedVal = ParseConstantExpression();
+ if (AssignedVal.isInvalid())
+ SkipUntil(tok::comma, tok::r_brace, StopBeforeMatch);
+ }
+
+ // Install the enumerator constant into EnumDecl.
+ Decl *EnumConstDecl = Actions.ActOnEnumConstant(getCurScope(), EnumDecl,
+ LastEnumConstDecl,
+ IdentLoc, Ident,
+ attrs.getList(),
EqualLoc,
+ AssignedVal.release());
+ PD.complete(EnumConstDecl);
+
+ EnumConstantDecls.push_back(EnumConstDecl);
+ LastEnumConstDecl = EnumConstDecl;
+
+ if (Tok.is(tok::identifier)) {
+ // We're missing a comma between enumerators.
+ SourceLocation Loc = PP.getLocForEndOfToken(PrevTokLocation);
+ Diag(Loc, diag::err_enumerator_list_missing_comma)
+ << FixItHint::CreateInsertion(Loc, ", ");
+ continue;
+ }
+
+ if (Tok.is(tok::r_brace))
+ break;
+
+ SourceLocation CommaLoc;
+ if (!TryConsumeToken(tok::comma, CommaLoc)) {
+ if (EqualLoc.isValid())
+ Diag(Tok.getLocation(), diag::err_expected_either)
+ << tok::r_brace << tok::comma;
+ else
+ Diag(Tok.getLocation(), diag::err_expected_one_of_three)
+ << tok::r_brace << tok::comma << tok::equal;
Good work updating your patch to use the new diagnostic formatter.
Listing three kinds of tokens the parser wants here seems a little
brusque. In this instance it seems better to provide a custom diagnostic
tailored for enumerators.
+ if (SkipUntil(tok::comma, tok::r_brace, StopBeforeMatch)) {
+ if (TryConsumeToken(tok::comma, CommaLoc))
+ continue;
+ } else {
+ break;
+ }
+ }
+
+ if (Tok.is(tok::r_brace)) {
+ if (!getLangOpts().C99 && !getLangOpts().CPlusPlus11)
+ Diag(CommaLoc, getLangOpts().CPlusPlus ?
+ diag::ext_enumerator_list_comma_cxx :
+ diag::ext_enumerator_list_comma_c)
+ << FixItHint::CreateRemoval(CommaLoc);
+ else if (getLangOpts().CPlusPlus11)
+ Diag(CommaLoc, diag::warn_cxx98_compat_enumerator_list_comma)
+ << FixItHint::CreateRemoval(CommaLoc);
+ break;
+ }
}
}
Index: test/Parser/cxx0x-ambig.cpp
===================================================================
--- test/Parser/cxx0x-ambig.cpp
+++ test/Parser/cxx0x-ambig.cpp
@@ -48,7 +48,7 @@
};
// This could be a bit-field.
struct S2 {
- enum E : T { a = 1, b = 2, c = 3, 4 }; // expected-error {{non-integral
type}} expected-error {{expected '}'}} expected-note {{to match}}
+ enum E : T { a = 1, b = 2, c = 3, 4 }; // expected-error {{non-integral
type}} expected-error {{expected identifier}}
};
struct S3 {
enum E : int { a = 1, b = 2, c = 3, d }; // ok, defines an enum
@@ -64,7 +64,7 @@
};
// This could be a bit-field.
struct S6 {
- enum E : int { 1 }; // expected-error {{expected '}'}} expected-note {{to
match}}
+ enum E : int { 1 }; // expected-error {{expected identifier}}
Nice QOI improvement here.
Alp.
};
struct U {
Index: test/Parser/declarators.c
===================================================================
--- test/Parser/declarators.c
+++ test/Parser/declarators.c
@@ -113,3 +113,37 @@
struct S { int n; }: // expected-error {{expected ';'}}
};
+
+// PR10982
+enum E11 {
+ A1 = 1,
+};
+
+enum E12 {
+ , // expected-error{{expected identifier}}
+ A2
+};
+void func_E12(enum E12 *p) { *p = A2; }
+
+enum E13 {
+ 1D, // expected-error{{expected identifier}}
+ A3
+};
+void func_E13(enum E13 *p) { *p = A3; }
+
+enum E14 {
+ A4 12, // expected-error{{expected '}' or ',' or '='}}
+ A4a
+};
+void func_E14(enum E14 *p) { *p = A4a; }
+
+enum E15 {
+ A5=12 4, // expected-error{{expected '}' or ','}}
+ A5a
+};
+void func_E15(enum E15 *p) { *p = A5a; }
+
+enum E16 {
+ A6; // expected-error{{expected '}' or ',' or '='}}
+ A6a
+};
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
--
http://www.nuanti.com
the browser experts
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
