Patchwork C++0x, virt-specifier (final/override) support for member functions (parser and analysis both)

login
register
mail settings
Submitter ville
Date May 7, 2011, 11:48 p.m.
Message ID <871v0akr6p.wl%ville@ville-laptop>
Download mbox | patch
Permalink /patch/94525/
State New
Headers show

Comments

ville - May 7, 2011, 11:48 p.m.
Tested on Linux/X86-32. Final on class is not yet supported,
that's probably what I'll be working on next. Changelog and
patch follow. Please holler if anything's wrong, as far as
I can see this works as it's supposed to.

2011-05-08 Ville Voutilainen <ville.voutilainen@gmail.com> <ville.voutilainen@symbio.com>
           Implement final/override for member functions.
           * class.c (check_for_override): Check for DECL_OVERRIDE_P if the fndecl doesn't override anything and diagnose.
           * cp-tree.h: Add DECL_OVERRIDE_P and DECL_FINAL_P, add a new typedef cp_virt_specifiers, use cp_virt_specifiers in cp_declarator.
           * decl.c (set_virt_specifiers): New. Used for setting the virt-specifiers from cp_declarator to DECL_OVERRIDE_P and DECL_FINAL_P.
           * decl.c (grokdeclarator): ..and use them here. Diagnose virt-specifiers on non-fields here.
           * parser.c (make_call_declarator): add the virt-specifiers as a new parameter.
           * parser.c (cp_parser_lambda_declarator_opt, cp_parser_direct_declarator): ..and change the calls to make_call_declarator
           * parser.c (cp_parser_virt_specifier_seq_opt): New. Parses a virt-specifier-seq for a member function.
           * search.c (check_final_overrider): Diagnose attempts to override a final member function.
           * virtual9.C: New. Tests for virt-specifiers.
           * tree.h: Convert the #defines for cv-qualifier values into an enum, add a new enum for virt-specifier values.
Jason Merrill - May 8, 2011, 2:19 a.m.
On 05/07/2011 07:48 PM, Ville Voutilainen wrote:
> +static tree set_virt_specifiers (tree decl, cp_virt_specifiers specifiers)

The name of the function needs to be at the beginning of the line.

> +      else
> +          virt_specifier = VIRT_SPEC_UNSPECIFIED;
> +
> +      if (!virt_specifier)
> +	break;

Using a named constant one one line and then testing for a boolean value 
on the next line is unclear.  How about just "else break;"?

> +/* Non-static member functions have an optional virt-specifier-seq.
> +   There is a VIRT_SPEC value for each virt-specifier.
> +   They can be combined by bitwise-or to form the complete set of
> +   virt-specifiers for a member function.  */
> +enum virt_specifier
> +  {
> +    VIRT_SPEC_UNSPECIFIED = 0x0,
> +    VIRT_SPEC_FINAL       = 0x1,
> +    VIRT_SPEC_OVERRIDE    = 0x2
> +  };

This should be in cp/cp-tree.h.

Otherwise, looks pretty good.

Jason
Jason Merrill - May 8, 2011, 2:20 a.m.
I might as well fix these issues myself, actually.

Jason

Patch

diff --git a/gcc/cp/class.c b/gcc/cp/class.c
index 9af238b..0c42281 100644
--- a/gcc/cp/class.c
+++ b/gcc/cp/class.c
@@ -2477,6 +2477,11 @@  check_for_override (tree decl, tree ctype)
       if (DECL_DESTRUCTOR_P (decl))
 	TYPE_HAS_NONTRIVIAL_DESTRUCTOR (ctype) = true;
     }
+  else if (DECL_OVERRIDE_P (decl))
+    {
+      DECL_VINDEX (decl) = error_mark_node;
+      error ("%q+#D marked override, but does not override", decl);
+    }
 }
 
 /* Warn about hidden virtual functions that are not overridden in t.
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index a65998d..b729a3c 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -73,6 +73,7 @@  c-common.h, not after.
       LAMBDA_EXPR_CAPTURES_THIS_P (in LAMBDA_EXPR)
       DECLTYPE_FOR_LAMBDA_CAPTURE (in DECLTYPE_TYPE)
       VEC_INIT_EXPR_IS_CONSTEXPR (in VEC_INIT_EXPR)
+      DECL_OVERRIDE_P (in FUNCTION_DECL)
    1: IDENTIFIER_VIRTUAL_P (in IDENTIFIER_NODE)
       TI_PENDING_TEMPLATE_FLAG.
       TEMPLATE_PARMS_FOR_INLINE.
@@ -85,6 +86,7 @@  c-common.h, not after.
       TARGET_EXPR_LIST_INIT_P (in TARGET_EXPR)
       LAMBDA_EXPR_MUTABLE_P (in LAMBDA_EXPR)
       DECLTYPE_FOR_LAMBDA_RETURN (in DECLTYPE_TYPE)
+      DECL_FINAL_P (in FUNCTION_DECL)
    2: IDENTIFIER_OPNAME_P (in IDENTIFIER_NODE)
       ICS_THIS_FLAG (in _CONV)
       DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P (in VAR_DECL)
@@ -2285,6 +2287,15 @@  struct GTY((variable_size)) lang_decl {
 #define DECL_INVALID_OVERRIDER_P(NODE) \
   (DECL_LANG_FLAG_4 (NODE))
 
+/* True (in a FUNCTION_DECL) if NODE is a function declared with
+   an override virt-specifier */
+#define DECL_OVERRIDE_P(NODE) (TREE_LANG_FLAG_0 (NODE))
+
+/* True (in a FUNCTION_DECL) if NODE is a function declared with
+   a final virt-specifier */
+#define DECL_FINAL_P(NODE) (TREE_LANG_FLAG_1 (NODE))
+
+
 /* The thunks associated with NODE, a FUNCTION_DECL.  */
 #define DECL_THUNKS(NODE) \
   (LANG_DECL_FN_CHECK (NODE)->context)
@@ -4426,6 +4437,11 @@  extern GTY(()) operator_name_info_t assignment_operator_name_info
 
 typedef int cp_cv_quals;
 
+/* A type-qualifier, or bitmask therefore, using the VIRT_SPEC
+   constants.  */
+
+typedef int cp_virt_specifiers;
+
 /* A storage class.  */
 
 typedef enum cp_storage_class {
@@ -4568,6 +4584,8 @@  struct cp_declarator {
       tree parameters;
       /* The cv-qualifiers for the function.  */
       cp_cv_quals qualifiers;
+      /* The virt-specifiers for the function.  */
+      cp_virt_specifiers virt_specifiers;
       /* The exception-specification for the function.  */
       tree exception_specification;
       /* The late-specified return type, if any.  */
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 5bf637e..4593244 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -7283,6 +7283,24 @@  grokfndecl (tree ctype,
   return decl;
 }
 
+/* decl is a FUNCTION_DECL.
+   specifiers are the parsed virt-specifiers.
+
+   Set flags to reflect the virt-specifiers.
+
+   Returns decl.  */
+
+static tree set_virt_specifiers (tree decl, cp_virt_specifiers specifiers)
+{
+  if (decl == NULL_TREE)
+    return decl;
+  if (specifiers & VIRT_SPEC_OVERRIDE)
+    DECL_OVERRIDE_P (decl) = 1;
+  if (specifiers & VIRT_SPEC_FINAL)
+    DECL_FINAL_P (decl) = 1;
+  return decl;
+}
+
 /* DECL is a VAR_DECL for a static data member.  Set flags to reflect
    the linkage that DECL will receive in the object file.  */
 
@@ -8057,6 +8075,9 @@  grokdeclarator (const cp_declarator *declarator,
   /* cv-qualifiers that apply to the declarator, for a declaration of
      a member function.  */
   cp_cv_quals memfn_quals = TYPE_UNQUALIFIED;
+  /* virt-specifiers that apply to the declarator, for a declaration of
+     a member function.  */
+  cp_virt_specifiers virt_specifiers = VIRT_SPEC_UNSPECIFIED;
   /* cv-qualifiers that apply to the type specified by the DECLSPECS.  */
   int type_quals;
   tree raises = NULL_TREE;
@@ -8817,7 +8838,8 @@  grokdeclarator (const cp_declarator *declarator,
 
 	    /* Pick up type qualifiers which should be applied to `this'.  */
 	    memfn_quals = declarator->u.function.qualifiers;
-
+	    /* Pick up virt-specifiers.  */
+            virt_specifiers = declarator->u.function.virt_specifiers;
 	    /* Pick up the exception specifications.  */
 	    raises = declarator->u.function.exception_specification;
 
@@ -9742,6 +9764,7 @@  grokdeclarator (const cp_declarator *declarator,
 			       sfk,
 			       funcdef_flag, template_count, in_namespace,
 			       attrlist, declarator->id_loc);
+            decl = set_virt_specifiers (decl, virt_specifiers);
 	    if (decl == NULL_TREE)
 	      return error_mark_node;
 #if 0
@@ -9935,6 +9958,8 @@  grokdeclarator (const cp_declarator *declarator,
 	else if (thread_p)
 	  error ("storage class %<__thread%> invalid for function %qs", name);
 
+        if (virt_specifiers)
+          error ("virt-specifiers in %qs not allowed outside a class definition", name);
 	/* Function declaration not at top level.
 	   Storage classes other than `extern' are not allowed
 	   and `extern' makes no difference.  */
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 89100aa..6dcbe20 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -937,7 +937,7 @@  clear_decl_specs (cp_decl_specifier_seq *decl_specs)
    VAR_DECLs or FUNCTION_DECLs) should do that directly.  */
 
 static cp_declarator *make_call_declarator
-  (cp_declarator *, tree, cp_cv_quals, tree, tree);
+  (cp_declarator *, tree, cp_cv_quals, cp_virt_specifiers, tree, tree);
 static cp_declarator *make_array_declarator
   (cp_declarator *, tree);
 static cp_declarator *make_pointer_declarator
@@ -1102,6 +1102,7 @@  cp_declarator *
 make_call_declarator (cp_declarator *target,
 		      tree parms,
 		      cp_cv_quals cv_qualifiers,
+		      cp_virt_specifiers virt_specifiers,
 		      tree exception_specification,
 		      tree late_return_type)
 {
@@ -1111,6 +1112,7 @@  make_call_declarator (cp_declarator *target,
   declarator->declarator = target;
   declarator->u.function.parameters = parms;
   declarator->u.function.qualifiers = cv_qualifiers;
+  declarator->u.function.virt_specifiers = virt_specifiers;
   declarator->u.function.exception_specification = exception_specification;
   declarator->u.function.late_return_type = late_return_type;
   if (target)
@@ -1690,6 +1692,8 @@  static enum tree_code cp_parser_ptr_operator
   (cp_parser *, tree *, cp_cv_quals *);
 static cp_cv_quals cp_parser_cv_qualifier_seq_opt
   (cp_parser *);
+static cp_virt_specifiers cp_parser_virt_specifier_seq_opt
+  (cp_parser *);
 static tree cp_parser_late_return_type_opt
   (cp_parser *);
 static tree cp_parser_declarator_id
@@ -7664,6 +7668,7 @@  cp_parser_lambda_declarator_opt (cp_parser* parser, tree lambda_expr)
     quals = (LAMBDA_EXPR_MUTABLE_P (lambda_expr)
 	     ? TYPE_UNQUALIFIED : TYPE_QUAL_CONST);
     declarator = make_call_declarator (declarator, param_list, quals,
+				       VIRT_SPEC_UNSPECIFIED,
 				       exception_spec,
                                        /*late_return_type=*/NULL_TREE);
     declarator->id_loc = LAMBDA_EXPR_LOCATION (lambda_expr);
@@ -14887,6 +14892,7 @@  cp_parser_direct_declarator (cp_parser* parser,
 	      if (member_p || cp_parser_parse_definitely (parser))
 		{
 		  cp_cv_quals cv_quals;
+		  cp_virt_specifiers virt_specifiers;
 		  tree exception_specification;
 		  tree late_return;
 
@@ -14903,6 +14909,8 @@  cp_parser_direct_declarator (cp_parser* parser,
 		  /* And the exception-specification.  */
 		  exception_specification
 		    = cp_parser_exception_specification_opt (parser);
+		  /* Parse the virt-specifier-seq.  */
+		  virt_specifiers = cp_parser_virt_specifier_seq_opt (parser);
 
 		  late_return
 		    = cp_parser_late_return_type_opt (parser);
@@ -14911,6 +14919,7 @@  cp_parser_direct_declarator (cp_parser* parser,
 		  declarator = make_call_declarator (declarator,
 						     params,
 						     cv_quals,
+						     virt_specifiers,
 						     exception_specification,
 						     late_return);
 		  /* Any subsequent parameter lists are to do with
@@ -15418,6 +15427,56 @@  cp_parser_cv_qualifier_seq_opt (cp_parser* parser)
   return cv_quals;
 }
 
+/* Parse an (optional) virt-specifier-seq.
+
+   virt-specifier-seq:
+     virt-specifier virt-specifier-seq [opt]
+
+   virt-specifier:
+     override
+     final
+
+   Returns a bitmask representing the virt-specifiers.  */
+
+static cp_virt_specifiers cp_parser_virt_specifier_seq_opt
+  (cp_parser* parser)
+{
+  cp_virt_specifiers virt_specifiers = VIRT_SPEC_UNSPECIFIED;
+
+  while (true)
+    {
+      cp_token *token;
+      cp_virt_specifiers virt_specifier;
+
+      /* Peek at the next token.  */
+      token = cp_lexer_peek_token (parser->lexer);
+      /* See if it's a virt-specifier-qualifier.  */
+      if (token->type != CPP_NAME)
+        break;
+      if (!strcmp (IDENTIFIER_POINTER(token->u.value), "override"))
+          virt_specifier = VIRT_SPEC_OVERRIDE;
+      else if (!strcmp (IDENTIFIER_POINTER(token->u.value), "final"))
+          virt_specifier = VIRT_SPEC_FINAL;
+      else
+          virt_specifier = VIRT_SPEC_UNSPECIFIED;
+
+      if (!virt_specifier)
+	break;
+
+      if (virt_specifiers & virt_specifier)
+	{
+	  error_at (token->location, "duplicate virt-specifier");
+	  cp_lexer_purge_token (parser->lexer);
+	}
+      else
+	{
+	  cp_lexer_consume_token (parser->lexer);
+	  virt_specifiers |= virt_specifier;
+	}
+    }
+  return virt_specifiers;
+}
+
 /* Parse a late-specified return type, if any.  This is not a separate
    non-terminal, but part of a function declarator, which looks like
 
diff --git a/gcc/cp/search.c b/gcc/cp/search.c
index e7d2048..cf0b1a0 100644
--- a/gcc/cp/search.c
+++ b/gcc/cp/search.c
@@ -1920,6 +1920,12 @@  check_final_overrider (tree overrider, tree basefn)
 	}
       return 0;
     }
+  if (DECL_FINAL_P (basefn))
+    {
+      error ("virtual function %q+D", overrider);
+      error ("overriding final function %q+D", basefn);
+      return 0;
+    }
   return 1;
 }
 
diff --git a/gcc/testsuite/g++.dg/inherit/virtual9.C b/gcc/testsuite/g++.dg/inherit/virtual9.C
new file mode 100644
index 0000000..139a0bc
--- /dev/null
+++ b/gcc/testsuite/g++.dg/inherit/virtual9.C
@@ -0,0 +1,39 @@ 
+// { dg-do compile }
+struct B 
+{
+  virtual void f() final {}
+  virtual void g() {}
+};
+
+struct B2 
+{
+  virtual void h() {}
+};
+
+struct D : B 
+{
+  virtual void g() override final {} // { dg-error "overriding" }
+};
+
+template <class T> struct D2 : T 
+{
+  void h() override {} // { dg-error "marked override, but does not override" }
+};
+
+struct D3 : D 
+{
+  void g() {} // { dg-error "virtual function" }
+};
+
+struct B3
+{
+  virtual void f() final final {} // { dg-error "duplicate virt-specifier" }
+};
+
+void g() override {} // { dg-error "virt-specifiers" }
+
+int main()
+{
+  D2<B> d2;
+  D2<B2> d3;
+}
diff --git a/gcc/tree.h b/gcc/tree.h
index 8baaf7b..72e50fc 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -2253,11 +2253,24 @@  extern enum machine_mode vector_type_mode (const_tree);
 /* There is a TYPE_QUAL value for each type qualifier.  They can be
    combined by bitwise-or to form the complete set of qualifiers for a
    type.  */
-
-#define TYPE_UNQUALIFIED   0x0
-#define TYPE_QUAL_CONST    0x1
-#define TYPE_QUAL_VOLATILE 0x2
-#define TYPE_QUAL_RESTRICT 0x4
+enum cv_qualifier 
+  {
+    TYPE_UNQUALIFIED   = 0x0,
+    TYPE_QUAL_CONST    = 0x1,
+    TYPE_QUAL_VOLATILE = 0x2,
+    TYPE_QUAL_RESTRICT = 0x4
+  };
+
+/* Non-static member functions have an optional virt-specifier-seq.
+   There is a VIRT_SPEC value for each virt-specifier.
+   They can be combined by bitwise-or to form the complete set of
+   virt-specifiers for a member function.  */
+enum virt_specifier
+  {
+    VIRT_SPEC_UNSPECIFIED = 0x0,
+    VIRT_SPEC_FINAL       = 0x1,
+    VIRT_SPEC_OVERRIDE    = 0x2
+  };
 
 /* Encode/decode the named memory support as part of the qualifier.  If more
    than 8 qualifiers are added, these macros need to be adjusted.  */