r313386 - [Sema] Error out early for tags defined inside an enumeration.

[Richard Smith via cfe-commits]

On 15 September 2017 at 12:51, Volodymyr Sapsai via cfe-commits <
cfe-commits at lists.llvm.org> wrote:

> Author: vsapsai
> Date: Fri Sep 15 12:51:42 2017
> New Revision: 313386
>
> URL: http://llvm.org/viewvc/llvm-project?rev=313386&view=rev
> Log:
> [Sema] Error out early for tags defined inside an enumeration.
>
> This fixes PR28903 by avoiding access check for inner enum constant. We
> are performing access check because one enum constant references another
> and because enum is defined in CXXRecordDecl. But access check doesn't
> work because FindDeclaringClass doesn't expect more than one EnumDecl
> and because inner enum has access AS_none due to not being an immediate
> child of a record.
>
> The change detects an enum is defined in wrong place and allows to skip
> parsing its body. Access check is skipped together with body parsing.
> There was no crash in C, added test case to cover the new error.
>
> rdar://problem/28530809
>
> Reviewers: rnk, doug.gregor, rsmith
>
> Reviewed By: doug.gregor
>
> Subscribers: cfe-commits
>
> Differential Revision: https://reviews.llvm.org/D37089
>
>
> Modified:
>     cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>     cfe/trunk/lib/Sema/SemaDecl.cpp
>     cfe/trunk/test/Sema/enum.c
>     cfe/trunk/test/SemaCXX/enum.cpp
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/
> DiagnosticSemaKinds.td?rev=313386&r1=313385&r2=313386&view=diff
> ============================================================
> ==================
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Sep 15
> 12:51:42 2017
> @@ -1335,6 +1335,8 @@ def err_type_defined_in_alias_template :
>    "%0 cannot be defined in a type alias template">;
>  def err_type_defined_in_condition : Error<
>    "%0 cannot be defined in a condition">;
> +def err_type_defined_in_enum : Error<
> +  "%0 cannot be defined in an enumeration">;
>
>  def note_pure_virtual_function : Note<
>    "unimplemented pure virtual method %0 in %1">;
>
> Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/
> SemaDecl.cpp?rev=313386&r1=313385&r2=313386&view=diff
> ============================================================
> ==================
> --- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDecl.cpp Fri Sep 15 12:51:42 2017
> @@ -13928,6 +13928,12 @@ CreateNewDecl:
>      Invalid = true;
>    }
>
> +  if (!Invalid && TUK == TUK_Definition && DC->getDeclKind() ==
> Decl::Enum) {
> +    Diag(New->getLocation(), diag::err_type_defined_in_enum)
> +      << Context.getTagDeclType(New);
> +    Invalid = true;
> +  }
>

This looks like the wrong fix. As noted elsewhere, this is wrong in C. And
in C++, the relevant context is a type-specifier, which should be rejected
due to the check 7 lines above.

It looks like the actual bug is that we don't consider the type within a
C99 compound literal to be a type-specifier. The fact that the context is
an enumeration is irrelevant.


> +
>    // Maybe add qualifier info.
>    if (SS.isNotEmpty()) {
>      if (SS.isSet()) {
>
> Modified: cfe/trunk/test/Sema/enum.c
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/
> enum.c?rev=313386&r1=313385&r2=313386&view=diff
> ============================================================
> ==================
> --- cfe/trunk/test/Sema/enum.c (original)
> +++ cfe/trunk/test/Sema/enum.c Fri Sep 15 12:51:42 2017
> @@ -123,3 +123,14 @@ int NegativeShortTest[NegativeShort == -
>  // PR24610
>  enum Color { Red, Green, Blue }; // expected-note{{previous use is here}}
>  typedef struct Color NewColor; // expected-error {{use of 'Color' with
> tag type that does not match previous declaration}}
> +
> +// PR28903
> +struct PR28903 {
> +  enum {
> +    PR28903_A = (enum { // expected-error-re {{'enum PR28903::(anonymous
> at {{.*}})' cannot be defined in an enumeration}}
> +      PR28903_B,
> +      PR28903_C = PR28903_B
> +    })0
> +  };
> +  int makeStructNonEmpty;
> +};
>
> Modified: cfe/trunk/test/SemaCXX/enum.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/
> SemaCXX/enum.cpp?rev=313386&r1=313385&r2=313386&view=diff
> ============================================================
> ==================
> --- cfe/trunk/test/SemaCXX/enum.cpp (original)
> +++ cfe/trunk/test/SemaCXX/enum.cpp Fri Sep 15 12:51:42 2017
> @@ -110,3 +110,13 @@ enum { overflow = 123456 * 234567 };
>  // expected-warning at -2 {{not an integral constant expression}}
>  // expected-note at -3 {{value 28958703552 is outside the range of
> representable values}}
>  #endif
> +
> +// PR28903
> +struct PR28903 {
> +  enum {
> +    PR28903_A = (enum { // expected-error-re {{'PR28903::(anonymous enum
> at {{.*}})' cannot be defined in an enumeration}}
> +      PR28903_B,
> +      PR28903_C = PR28903_B
> +    })
> +  };
> +};
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170921/8bbad989/attachment-0001.html>