Hi,

On 14/12/18 21:19, Jason Merrill wrote:
On 12/14/18 1:44 PM, Paolo Carlini wrote:
Hi,

On 13/12/18 22:03, Jason Merrill wrote:
On 10/30/18 9:22 PM, Paolo Carlini wrote:
Hi,

On 30/10/18 21:37, Jason Merrill wrote:
On 10/26/18 2:02 PM, Paolo Carlini wrote:
On 26/10/18 17:18, Jason Merrill wrote:
On Fri, Oct 26, 2018 at 4:52 AM Paolo Carlini <paolo.carl...@oracle.com> wrote:
On 24/10/18 22:41, Jason Merrill wrote:
On 10/15/18 12:45 PM, Paolo Carlini wrote:
         && ((TREE_CODE (declspecs->type) != TYPENAME_TYPE
+       && TREE_CODE (declspecs->type) != DECLTYPE_TYPE
          && MAYBE_CLASS_TYPE_P (declspecs->type))
I would think that the MAYBE_CLASS_TYPE_P here should be CLASS_TYPE_P,
and then we can remove the TYPENAME_TYPE check. Or do we want to
allow template type parameters for some reason?
Indeed, it would be nice to just use OVERLOAD_TYPE_P. However it seems we at least want to let through TEMPLATE_TYPE_PARMs representing 'auto'
- otherwise Dodji's check a few lines below which fixed c++/51473
doesn't work anymore - and also BOUND_TEMPLATE_TEMPLATE_PARM, otherwise we regress on template/spec32.C and template/ttp22.C because we don't diagnose the shadowing anymore. Thus, I would say either we keep on using MAYBE_CLASS_TYPE_P or we pick what we need, possibly we add a comment?
Aha.  I guess the answer is not to restrict that test any more, but
instead to fix the code further down so it gives a proper diagnostic
rather than call warn_misplaced_attr_for_class_type.

I see. Thus something like the below? It passes testing on x86_64-linux.

+  if ((!declared_type || TREE_CODE (declared_type) == DECLTYPE_TYPE)
+      && ! saw_friend && !error_p)
     permerror (input_location, "declaration does not declare anything");

I see no reason to make this specific to decltype.  Maybe move this diagnostic into the final 'else' block with the other declspec diagnostics and not look at declared_type at all?

I'm not sure to fully understand: if we do that we still want to at least minimally check that declared_type is null, like we already do, and then we simply accept the new testcase. Is that Ok? Because, as I probably mentioned at some point, all the other compilers I have at hand issue a "does not declare anything" diagnostic, and we likewise do that for the legacy __typeof. Not looking into declared_type *at all* doesn't work with plain class types and enums, of course. Or you meant something entirely different??

+  if (declspecs->attributes && warn_attributes && declared_type
+      && TREE_CODE (declared_type) != DECLTYPE_TYPE)

I think we do want to give a diagnostic about useless attributes, not skip it.

Agreed. FWIW the attached tests fine.

The problem here is that the code toward the bottom expects "declared_type" to be the tagged type declared by a declaration with no declarator, and in this testcase it's ending up as a DECLTYPE_TYPE.

I think once we've checked for 'auto' we don't want declared_type to be anything that isn't OVERLOAD_TYPE_P.  We can arrange that either by checking for 'auto' first and then changing the code that sets declared_type to use OVERLOAD_TYPE_P, or by clearing declared_type after checking for 'auto' if it isn't OVERLOAD_TYPE_P.

Thanks. I'm slowly catching up on this issue... Any suggestion about BOUND_TEMPLATE_TEMPLATE_PARM? If we don't let through such tree nodes - which are MAYBE_CLASS_TYPE_P and aren't OVERLOAD_TYPE_P - we regress on template/spec32.C, we don't reject it anymore.
If we clear declared_type for a BOUND_TEMPLATE_TEMPLATE_PARM, we should get the "does not declare anything" error.

Ah, now I see, I didn't realize that we would also change the errors we issue for those testcases. Thus the below is finishing testing, appears to work fine.

Thanks, Paolo.

///////////////////

Index: cp/decl.c
===================================================================
--- cp/decl.c   (revision 267131)
+++ cp/decl.c   (working copy)
@@ -4803,9 +4803,8 @@ check_tag_decl (cp_decl_specifier_seq *declspecs,
     declared_type = declspecs->type;
   else if (declspecs->type == error_mark_node)
     error_p = true;
-  if (declared_type == NULL_TREE && ! saw_friend && !error_p)
-    permerror (input_location, "declaration does not declare anything");
-  else if (declared_type != NULL_TREE && type_uses_auto (declared_type))
+
+  if (type_uses_auto (declared_type))
     {
       error_at (declspecs->locations[ds_type_spec],
                "%<auto%> can only be specified for variables "
@@ -4812,6 +4811,12 @@ check_tag_decl (cp_decl_specifier_seq *declspecs,
                "or function declarations");
       return error_mark_node;
     }
+
+  if (declared_type && !OVERLOAD_TYPE_P (declared_type))
+    declared_type = NULL_TREE;
+
+  if (!declared_type && !saw_friend && !error_p)
+    permerror (input_location, "declaration does not declare anything");
   /* Check for an anonymous union.  */
   else if (declared_type && RECORD_OR_UNION_CODE_P (TREE_CODE (declared_type))
           && TYPE_UNNAMED_P (declared_type))
Index: testsuite/g++.dg/cpp0x/decltype-33838.C
===================================================================
--- testsuite/g++.dg/cpp0x/decltype-33838.C     (revision 267127)
+++ testsuite/g++.dg/cpp0x/decltype-33838.C     (working copy)
@@ -2,5 +2,5 @@
 // PR c++/33838
 template<typename T> struct A
 {
-  __decltype (T* foo()); // { dg-error "expected|no arguments|accept" }
+  __decltype (T* foo()); // { dg-error "expected|no arguments|declaration" }
 };
Index: testsuite/g++.dg/cpp0x/decltype68.C
===================================================================
--- testsuite/g++.dg/cpp0x/decltype68.C (nonexistent)
+++ testsuite/g++.dg/cpp0x/decltype68.C (working copy)
@@ -0,0 +1,7 @@
+// PR c++/84644
+// { dg-do compile { target c++11 } }
+
+template<int a>
+struct b {
+  decltype(a) __attribute__((break));  // { dg-error "declaration does not 
declare anything" }
+};
Index: testsuite/g++.dg/template/spec32.C
===================================================================
--- testsuite/g++.dg/template/spec32.C  (revision 267127)
+++ testsuite/g++.dg/template/spec32.C  (working copy)
@@ -2,5 +2,5 @@
 
 struct A
 {
-  template<template<int> class B> struct B<0>;  // { dg-error "name of class 
shadows" }
+  template<template<int> class B> struct B<0>;  // { dg-error "declaration 
does not declare anything" }
 };
Index: testsuite/g++.dg/template/ttp22.C
===================================================================
--- testsuite/g++.dg/template/ttp22.C   (revision 267127)
+++ testsuite/g++.dg/template/ttp22.C   (working copy)
@@ -2,7 +2,7 @@
 // { dg-do compile }
 
 template<template<int> class A>
-class A<0>;  // { dg-error "shadows template template parameter" }
+class A<0>;  // { dg-error "declaration does not declare anything" }
 
 template<template<int> class B>
 class B<0> {};  // { dg-error "shadows template template parameter" }

Reply via email to