[PATCH] Recover from errors in enum definition.
Alp Toker
alp at nuanti.com
Mon Dec 30 00:06:13 PST 2013
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
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
--
http://www.nuanti.com
the browser experts
More information about the cfe-commits
mailing list