[C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix)

[C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix)

[Giovanni Bajo]

Hello,

this is a partial fix for PR2204, which is about checking if PARM_DECLs in
function declarations have abstract types. Currently, the check is being
performed in grokparms, which is fine for many cases, but it's way too early if
the type is the class being defined, or an incomplete type. In that case, we
need to defer the check later, when/if the type is completed.

To do this, I allow PARM_DECLS to be registered in the incomplete_vars list (by
grokparms) and check for abstractness upon type completition (finish_struct_1,
which calls complete_vars). Notice that we cannot check it in start_function,
because the program is ill-formed even if there is no definition for the
function.

Alas, this solution is partial, mainly because it doesn't get into account
array types. In fact, parameters with array type are decayed into pointer types
within grokdeclarator (even before a PARM_DECL is built), so they never get
registered into the incomplete_vars list. A solution for this might be
extracting the standard decays out of grokdeclarator, and having grokparms call
it. There is also another user of grokdeclarator(PARM) (in pt.c, to build
template parameters), which should be updated as well. Comments on this plan
are highly appreciated.

Another case we don't handle yet is when the type of the parameter is
dependent. In that case, we probably need to check for abstractness upon
instantiation, while tsubsting. I'll investigate.

Meanwhile, this tested succesfully on i686-pc-linux-gnu. The testcase have a
handful of xfails for the cases we don't handle yet. OK for mainline?

Giovanni Bajo



2004-02-10  Giovanni Bajo  <giovannibajo@gcc.gnu.org>

        PR c++/2204
        * decl.c (cp_finish_decl): Extract logic to strip array and pointer
        to array types into...
        (strip_array_and_pointer_to_array_types): New function.
        (grokparms): Register PARM_DECLs of incomplete types for later check,
        using...
        (maybe_register_incomplete_parms): New function.
        (complete_vars): When completing the type for a PARM_DECL, check
        for abstractness. Fix a typo in the comment.
        * typeck2.c (abstract_virtual_errors): Reword diagnostics, make them
        appear at the correct location.

2004-02-10  Giovanni Bajo  <giovannibajo@gcc.gnu.org>

        PR c++/2204
        * g++.dg/other/abstract2.C: New test.


Index: decl.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cp/decl.c,v
retrieving revision 1.1179
diff -c -3 -p -r1.1179 decl.c
*** decl.c 2 Feb 2004 16:53:02 -0000 1.1179
--- decl.c 9 Feb 2004 23:27:43 -0000
*************** static void expand_static_init (tree, tr
*** 120,125 ****
--- 120,127 ----
  static tree next_initializable_field (tree);
  static tree reshape_init (tree, tree *);
  static tree build_typename_type (tree, tree, tree);
+ static void maybe_register_incomplete_parm (tree);
+ static tree strip_array_and_pointer_to_array_types (tree);

  /* Erroneous argument lists can use this *IFF* they do not modify it.  */
  tree error_mark_list;
*************** cp_finish_decl (tree decl, tree init, tr
*** 4883,4901 ****
     || TREE_CODE (type) == METHOD_TYPE)
   abstract_virtuals_error (decl,
       strip_array_types (TREE_TYPE (type)));
!       else if (POINTER_TYPE_P (type) || TREE_CODE (type) == ARRAY_TYPE)
!       {
!  /* If it's either a pointer or an array type, strip through all
!     of them but the last one. If the last is an array type, issue
!     an error if the element type is abstract.  */
!  while (POINTER_TYPE_P (TREE_TYPE (type))
!         || TREE_CODE (TREE_TYPE (type)) == ARRAY_TYPE)
!    type = TREE_TYPE (type);
!  if (TREE_CODE (type) == ARRAY_TYPE)
!    abstract_virtuals_error (decl, TREE_TYPE (type));
!       }
!       else
!  abstract_virtuals_error (decl, type);

        if (TREE_CODE (decl) == FUNCTION_DECL
     || TREE_TYPE (decl) == error_mark_node)
--- 4885,4894 ----
     || TREE_CODE (type) == METHOD_TYPE)
   abstract_virtuals_error (decl,
       strip_array_types (TREE_TYPE (type)));
!       else
!  abstract_virtuals_error (decl,
!      strip_array_and_pointer_to_array_types
!       (type));

        if (TREE_CODE (decl) == FUNCTION_DECL
     || TREE_TYPE (decl) == error_mark_node)
*************** grokparms (tree first_parm)
*** 8669,8674 ****
--- 8660,8667 ----
         type = build_pointer_type (type);
         TREE_TYPE (decl) = type;
       }
+    else if (!COMPLETE_TYPE_P (type) || currently_open_class (type))
+      maybe_register_incomplete_parm (decl);
     else if (abstract_virtuals_error (decl, type))
       any_error = 1;  /* Seems like a good idea.  */
     else if (POINTER_TYPE_P (type))
*************** finish_method (tree decl)
*** 11049,11055 ****

    return decl;
  }
! \f

  /* VAR is a VAR_DECL.  If its type is incomplete, remember VAR so that
     we can lay it out later, when and if its type becomes complete.  */
--- 11042,11098 ----

    return decl;
  }
!
! /* Strip all the array/pointer-to-array types from TYPE. This is used to
!    extract the underlying type from declarations such as:
!
!    A a[2];
!    A (*a)[2];
!    A (**a[2])[2];
!
!    which are invalid if 'A' is an abstract type.  */
!
! static tree
! strip_array_and_pointer_to_array_types (tree type)
! {
!   if (POINTER_TYPE_P (type) || TREE_CODE (type) == ARRAY_TYPE)
!     {
!       /* If it's either a pointer or an array type, strip through all
!   of them but the last one. If the last is an array type, then
!   we can return its element type.  */
!       while (POINTER_TYPE_P (TREE_TYPE (type))
!       || TREE_CODE (TREE_TYPE (type)) == ARRAY_TYPE)
!  type = TREE_TYPE (type);
!       if (TREE_CODE (type) == ARRAY_TYPE)
!  return TREE_TYPE (type);
!     }
!   return type;
! }
!
! /* PARM is a PARM_DECL, whose type is incomplete at the point of parsing.
!    We keep track of these because we must issue an error if, upon
!    completition, the type ends up being abstract.  */
!
! static void
! maybe_register_incomplete_parm (tree var)
! {
!   my_friendly_assert (TREE_CODE (var) == PARM_DECL, 20040208);
!
!   /* If the type is dependent, there is nothing we can do here. The
!      diagnostic will have to be issued during template substitution.  */
!   if (!dependent_type_p (TREE_TYPE (var)))
!     {
!       tree inner_type = TREE_TYPE (var);
!
!       /* FIXME: We currently don't get any ARRAY_TYPE because grokdeclarator
!   decays them to POINTER_TYPE too early.  */
!       inner_type = strip_array_and_pointer_to_array_types (inner_type);
!       inner_type = TYPE_MAIN_VARIANT (inner_type);
!
!       if (!COMPLETE_TYPE_P (inner_type) && CLASS_TYPE_P (inner_type))
!  incomplete_vars = tree_cons (inner_type, var, incomplete_vars);
!     }
! }

  /* VAR is a VAR_DECL.  If its type is incomplete, remember VAR so that
     we can lay it out later, when and if its type becomes complete.  */
*************** maybe_register_incomplete_var (tree var)
*** 11078,11084 ****
  }

  /* Called when a class type (given by TYPE) is defined.  If there are
!    any existing VAR_DECLs whose type hsa been completed by this
     declaration, update them now.  */

  void
--- 11121,11127 ----
  }

  /* Called when a class type (given by TYPE) is defined.  If there are
!    any existing VAR_DECLs whose type has been completed by this
     declaration, update them now.  */

  void
*************** complete_vars (tree type)
*** 11095,11100 ****
--- 11138,11148 ----
     /* Complete the type of the variable.  The VAR_DECL itself
        will be laid out in expand_expr.  */
     complete_type (TREE_TYPE (var));
+    /* If it is a PARM_DECL, we need to emit an error if the
+       type is abstract, because this is why we registered it
+       here.  */
+    if (TREE_CODE (var) == PARM_DECL)
+      abstract_virtuals_error (var, TREE_TYPE (var));
     /* Remove this entry from the list.  */
     *list = TREE_CHAIN (*list);
   }
Index: typeck2.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cp/typeck2.c,v
retrieving revision 1.154
diff -c -3 -p -r1.154 typeck2.c
*** typeck2.c 22 Jan 2004 00:03:52 -0000 1.154
--- typeck2.c 9 Feb 2004 23:27:46 -0000
*************** abstract_virtuals_error (tree decl, tree
*** 149,182 ****
   return 0;

        if (TREE_CODE (decl) == VAR_DECL)
!  error ("cannot declare variable `%D' to be of type `%T'",
!       decl, type);
        else if (TREE_CODE (decl) == PARM_DECL)
!  error ("cannot declare parameter `%D' to be of type `%T'",
!       decl, type);
        else if (TREE_CODE (decl) == FIELD_DECL)
!  error ("cannot declare field `%D' to be of type `%T'",
!       decl, type);
        else if (TREE_CODE (decl) == FUNCTION_DECL
          && TREE_CODE (TREE_TYPE (decl)) == METHOD_TYPE)
!  error ("invalid return type for member function `%#D'", decl);
        else if (TREE_CODE (decl) == FUNCTION_DECL)
!  error ("invalid return type for function `%#D'", decl);
      }
    else
!     error ("cannot allocate an object of type `%T'", type);

    /* Only go through this once.  */
    if (TREE_PURPOSE (u) == NULL_TREE)
      {
        TREE_PURPOSE (u) = error_mark_node;

!       error ("  because the following virtual functions are abstract:");
        for (tu = u; tu; tu = TREE_CHAIN (tu))
!  cp_error_at ("\t%#D", TREE_VALUE (tu));
      }
    else
!     error ("  since type `%T' has abstract virtual functions", type);

    return 1;
  }
--- 149,196 ----
   return 0;

        if (TREE_CODE (decl) == VAR_DECL)
!  error ("%Jcannot declare variable `%D' to be of abstract type `%T'",
!         decl, decl, type);
        else if (TREE_CODE (decl) == PARM_DECL)
!  error ("%Jcannot declare parameter `%D' to be of abstract type `%T'",
!         decl, decl, type);
        else if (TREE_CODE (decl) == FIELD_DECL)
!  error ("%Jcannot declare field `%D' to be of abstract type `%T'",
!         decl, decl, type);
        else if (TREE_CODE (decl) == FUNCTION_DECL
          && TREE_CODE (TREE_TYPE (decl)) == METHOD_TYPE)
!  error ("%Jinvalid abstract return type for member function `%#D'",
!         decl, decl);
        else if (TREE_CODE (decl) == FUNCTION_DECL)
!  error ("%Jinvalid abstract return type for function `%#D'",
!         decl, decl);
      }
    else
!     error ("cannot allocate an object of abstract type `%T'", type);

    /* Only go through this once.  */
    if (TREE_PURPOSE (u) == NULL_TREE)
      {
        TREE_PURPOSE (u) = error_mark_node;

!       if (decl)
!  inform ("%J  because the following virtual functions are pure:",
!   decl);
!       else
!  inform ("  because the following virtual functions are pure:");
!
        for (tu = u; tu; tu = TREE_CHAIN (tu))
!  inform ("%J\t%#D", TREE_VALUE (tu), TREE_VALUE (tu));
      }
    else
!   {
!     if (decl)
!       inform ("%J  since type `%T' has pure virtual functions",
!        decl, type);
!     else
!       inform ("  since type `%T' has pure virtual functions",
!        type);
!   }

    return 1;
  }




// { dg-do compile }
// Contributed by Gabriel Dos Reis <gdr at integrable-solutions dot net>
// PR c++/2204: Check for parameters of abstract type in function declarations.

namespace N1 {
  struct X;

  struct Y1 {
    void g(X parm1);         // { dg-error "abstract" }
    void g(X parm2[2]);      // { dg-error "abstract" "" { xfail *-*-* } }
  };

  template <int N>
  struct Y2 {
    void g(X parm3);         // { dg-error "abstract" }
    void g(X parm4[2]);      // { dg-error "abstract" "" { xfail *-*-* } }
  };

  struct X {
    virtual void xfunc(void) = 0;  // { dg-error "note" }
  };
}

namespace N2 {
  struct X1 {
    virtual void xfunc(void) = 0;  // { dg-error "note" }
    void g(X1 parm5);        // { dg-error "abstract" }
    void g(X1 parm6[2]);     // { dg-error "abstract" "" { xfail *-*-* } }
  };

  template <int N>
  struct X2 {
    virtual void xfunc(void) = 0;  // { dg-error "note" "" { xfail *-*-* } }
    void g(X2 parm7);        // { dg-error "abstract" "" { xfail *-*-* } }
    void g(X2 parm8[2]);     // { dg-error "abstract" "" { xfail *-*-* } }
  };
}

namespace N3 {
  struct X {
    virtual void xfunc(void) = 0;  // { dg-error "note" }
  };
  void g(X parm9);           // { dg-error "abstract" }
  void g(X parm10[2]);       // { dg-error "abstract" "" { xfail *-*-* } }
  template <int N>
  void g(X parm11);          // { dg-error "abstract" }
  template <int N>
  void g(X parm12[2]);       // { dg-error "abstract" "" { xfail *-*-* } }
}

// { dg-excess-errors "following virtual functions|has pure virtual
functions" }

Re: [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix)

[Gabriel Dos Reis]

"Giovanni Bajo" <giovannibajo@libero.it> writes:

|         if (TREE_CODE (decl) == VAR_DECL)
| !  error ("%Jcannot declare variable `%D' to be of abstract type `%T'",
| !         decl, decl, type);

That is an abuse of %J.  Use error_at instead.  E.g.

  error_at ("cannot declare variable `%+D' to be of abstract type `%T'",
            decl, type);


| !  error ("%Jcannot declare parameter `%D' to be of abstract type `%T'",
| !         decl, decl, type);

Ditto.

|         else if (TREE_CODE (decl) == FIELD_DECL)
| !  error ("%Jcannot declare field `%D' to be of abstract type `%T'",
| !         decl, decl, type);

Ditto.

|         else if (TREE_CODE (decl) == FUNCTION_DECL
|           && TREE_CODE (TREE_TYPE (decl)) == METHOD_TYPE)
| !  error ("%Jinvalid abstract return type for member function `%#D'",
| !         decl, decl);

Ditto.

|         else if (TREE_CODE (decl) == FUNCTION_DECL)
| !  error ("%Jinvalid abstract return type for function `%#D'",
| !         decl, decl);

Ditto.

As a side comment, I don't understand why you went through that
change.

-- Gaby

Re: [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix)

[Zack Weinberg]

Gabriel Dos Reis <gdr@integrable-solutions.net> writes:

> "Giovanni Bajo" <giovannibajo@libero.it> writes:
>
> |         if (TREE_CODE (decl) == VAR_DECL)
> | !  error ("%Jcannot declare variable `%D' to be of abstract type `%T'",
> | !         decl, decl, type);
>
> That is an abuse of %J.  Use error_at instead.
[etc]

I told Giovanni that error/warning/pedwarn_at were to be eliminated in
favor of %J.  Did I misunderstand what you've said about these in the
past?

zw

Re: [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix)

[Gabriel Dos Reis]

"Zack Weinberg" <zack@codesourcery.com> writes:

| Gabriel Dos Reis <gdr@integrable-solutions.net> writes:
| 
| > "Giovanni Bajo" <giovannibajo@libero.it> writes:
| >
| > |         if (TREE_CODE (decl) == VAR_DECL)
| > | !  error ("%Jcannot declare variable `%D' to be of abstract type `%T'",
| > | !         decl, decl, type);
| >
| > That is an abuse of %J.  Use error_at instead.
| [etc]
| 
| I told Giovanni that error/warning/pedwarn_at were to be eliminated in
| favor of %J.  Did I misunderstand what you've said about these in the
| past?

I don't know exactly what you're referring to.  But, the repetition of
the "decl" is not right and if any removal should lead to that, then
that removal should not happen.

There are some uses of error_at which are wrong.  E.g.

  error_at ("foo is nonsensical", decl);        // WRONG

  error ("%Jfoo is nonsensical", decl);        // Good


  error ("%Jdeclaration '%D' is nonsensical", decl, decl); // WRONG

  error_at ("declaration '%+D' is nonsensical", decl); // Good

-- Gaby

Re: [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix)

[Giovanni Bajo]

Gabriel Dos Reis wrote:

> As a side comment, I don't understand why you went through that
> change.

This is the rationale for my changes to abstract_virtual_errors.

* Use of %J (will be error_at):
With my patch, abstract_virtual_errors can be deferred to an indefinite moment
(when the type is completed). Without this change, the error message appears on
the last line of the completed type, which is wrong: we need to show the user
where is the parameter whose type is abstract. Then, the other messages about
the pure virtual functions will point her to the functions which need to be
overridden, and thus to the type.

* Rewording
There is no such thing as an "abstract virtual function". Standard calls it
"pure virtual function".

* Use of inform vs error
Those messages are just additional information provided to help the user, they
are not part of the error message proper. Moreover, I heard someone wanted to
have a -fno-notes mode one day, so it makes sense to be able to shut those off.
This is also why I added the word "abstract" on the first error line, so that
it is now a complete error message, and the notes are really optional. Before,
the first line would have been unclear, were the notes disabled for some
reason.

Sorry, I should have mentioned these things in the patch message.

Giovanni Bajo

Re: [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix)

[Gabriel Dos Reis]

"Giovanni Bajo" <giovannibajo@libero.it> writes:

| Gabriel Dos Reis wrote:
| 
| > As a side comment, I don't understand why you went through that
| > change.
| 
| This is the rationale for my changes to abstract_virtual_errors.
| 
| * Use of %J (will be error_at):
| With my patch, abstract_virtual_errors can be deferred to an indefinite moment
| (when the type is completed). Without this change, the error message appears on
| the last line of the completed type, which is wrong: we need to show the user
| where is the parameter whose type is abstract. Then, the other messages about
| the pure virtual functions will point her to the functions which need to be
| overridden, and thus to the type.

That makes sense.

| * Rewording
| There is no such thing as an "abstract virtual function". Standard calls it
| "pure virtual function".


I agree too.  I've been seeing those messages these last days (personal
experience forgetting to define dozens of pure virtual  functions in a
very complex class hierarchy).  They look indeed indeed very odd.

| * Use of inform vs error
| Those messages are just additional information provided to help the user, they
| are not part of the error message proper. Moreover, I heard someone wanted to
| have a -fno-notes mode one day, so it makes sense to be able to shut those off.
| This is also why I added the word "abstract" on the first error line, so that
| it is now a complete error message, and the notes are really optional. Before,
| the first line would have been unclear, were the notes disabled for some
| reason.

This makes sense.

| Sorry, I should have mentioned these things in the patch message.

No need to sorry.  What I don't understand is the use of '%J' in lieu
of xxx_at.  

Thanks,

-- Gaby

Re: [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix)

[Zack Weinberg]

Gabriel Dos Reis <gdr@integrable-solutions.net> writes:

> "Zack Weinberg" <zack@codesourcery.com> writes:
>
> | Gabriel Dos Reis <gdr@integrable-solutions.net> writes:
> | 
> | > "Giovanni Bajo" <giovannibajo@libero.it> writes:
> | >
> | > |         if (TREE_CODE (decl) == VAR_DECL)
> | > | !  error ("%Jcannot declare variable `%D' to be of abstract type `%T'",
> | > | !         decl, decl, type);
> | >
> | > That is an abuse of %J.  Use error_at instead.
> | [etc]
> | 
> | I told Giovanni that error/warning/pedwarn_at were to be eliminated in
> | favor of %J.  Did I misunderstand what you've said about these in the
> | past?
>
> I don't know exactly what you're referring to.  But, the repetition of
> the "decl" is not right and if any removal should lead to that, then
> that removal should not happen.

I don't understand this.  The %J consumes one argument and the %D
consumes another one.  Why is this "wrong"?

>   error ("%Jdeclaration '%D' is nonsensical", decl, decl); // WRONG
>   error_at ("declaration '%+D' is nonsensical", decl); // Good

The %+D notation is tidier, but I had understood you to be of the
opinion that it was not worth the implementation mess, nor the baggage
of carrying around two different sets of diagnostic functions.

(If we ever get Chiaki's indexed-parameters-notation patch laid down,
it will also allow implementing %+x cleanly and for all languages.)

zw

Re: [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix)

[Gabriel Dos Reis]

"Zack Weinberg" <zack@codesourcery.com> writes:

| Gabriel Dos Reis <gdr@integrable-solutions.net> writes:
| 
| > "Zack Weinberg" <zack@codesourcery.com> writes:
| >
| > | Gabriel Dos Reis <gdr@integrable-solutions.net> writes:
| > | 
| > | > "Giovanni Bajo" <giovannibajo@libero.it> writes:
| > | >
| > | > |         if (TREE_CODE (decl) == VAR_DECL)
| > | > | !  error ("%Jcannot declare variable `%D' to be of abstract type `%T'",
| > | > | !         decl, decl, type);
| > | >
| > | > That is an abuse of %J.  Use error_at instead.
| > | [etc]
| > | 
| > | I told Giovanni that error/warning/pedwarn_at were to be eliminated in
| > | favor of %J.  Did I misunderstand what you've said about these in the
| > | past?
| >
| > I don't know exactly what you're referring to.  But, the repetition of
| > the "decl" is not right and if any removal should lead to that, then
| > that removal should not happen.
| 
| I don't understand this.  The %J consumes one argument and the %D
| consumes another one.  Why is this "wrong"?

The repetition oan information that is already there.  Any scheme that
requires repetition of information is wrong. 

| >   error ("%Jdeclaration '%D' is nonsensical", decl, decl); // WRONG
| >   error_at ("declaration '%+D' is nonsensical", decl); // Good
| 
| The %+D notation is tidier, but I had understood you to be of the
| opinion that it was not worth the implementation mess, nor the baggage
| of carrying around two different sets of diagnostic functions.

If you have error() recognizing '%+' properly then error_at() should
go.  The whole purpose of xxx_at() begin there is that xxx() does not
support yet '%+'.

-- Gaby

Re: [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix)

[Zack Weinberg]

Gabriel Dos Reis <gdr@integrable-solutions.net> writes:
> "Zack Weinberg" <zack@codesourcery.com> writes:
> | Gabriel Dos Reis <gdr@integrable-solutions.net> writes:
> | > I don't know exactly what you're referring to.  But, the repetition of
> | > the "decl" is not right and if any removal should lead to that, then
> | > that removal should not happen.
> | 
> | I don't understand this.  The %J consumes one argument and the %D
> | consumes another one.  Why is this "wrong"?
>
> The repetition oan information that is already there.  Any scheme that
> requires repetition of information is wrong. 

Well, this is not consistent with what you have said before, and I
wonder why you didn't object to %J when it was originally added.  We
have %J...%D all over the C front end these days.

zw

Re: [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix)

[Gabriel Dos Reis]

"Zack Weinberg" <zack@codesourcery.com> writes:

| Gabriel Dos Reis <gdr@integrable-solutions.net> writes:
| > "Zack Weinberg" <zack@codesourcery.com> writes:
| > | Gabriel Dos Reis <gdr@integrable-solutions.net> writes:
| > | > I don't know exactly what you're referring to.  But, the repetition of
| > | > the "decl" is not right and if any removal should lead to that, then
| > | > that removal should not happen.
| > | 
| > | I don't understand this.  The %J consumes one argument and the %D
| > | consumes another one.  Why is this "wrong"?
| >
| > The repetition oan information that is already there.  Any scheme that
| > requires repetition of information is wrong. 
| 
| Well, this is not consistent with what you have said before, and I

Again, I don't understand what you're talking about.
I have never been a fan of information/code duplication.  Did you see
me advocating for information/code duplication in the past?

I want to get rid of xxx_at() if xxx() works correctly.

| wonder why you didn't object to %J when it was originally added.  We

Most certainly because when it was added, it was to replace previous
attempts to remove xxx_with_decl() that where using %s when actually a
tree was being passed.

| have %J...%D all over the C front end these days.

Yes, but I do not believe the situation is comparable.   The C
front-end never had xxx_at().

-- Gaby

Re: [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix)

[Zack Weinberg]

Gabriel Dos Reis <gdr@integrable-solutions.net> writes:
> | Gabriel Dos Reis <gdr@integrable-solutions.net> writes:
> | > The repetition oan information that is already there.  Any scheme that
> | > requires repetition of information is wrong. 
> | 
> | Well, this is not consistent with what you have said before, and I
>
> Again, I don't understand what you're talking about.

You were quite adamant in the past that xxx_at() must go.  So adamant,
in fact, that I thought it was already dead.

My position is:

 * locate_error is a nasty thing that should go away.
 * ", decl, decl" isn't pretty but it isn't as nasty as locate_error.
 * Consistency between the C and C++ front ends is desirable.

and most importantly

 * Why are we holding up a bug fix with this stupid argument anyway?

zw

Re: [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix)

[Gabriel Dos Reis]

"Zack Weinberg" <zack@codesourcery.com> writes:

| Gabriel Dos Reis <gdr@integrable-solutions.net> writes:
| > | Gabriel Dos Reis <gdr@integrable-solutions.net> writes:
| > | > The repetition oan information that is already there.  Any scheme that
| > | > requires repetition of information is wrong. 
| > | 
| > | Well, this is not consistent with what you have said before, and I
| >
| > Again, I don't understand what you're talking about.
| 
| You were quite adamant in the past that xxx_at() must go.  So adamant,
| in fact, that I thought it was already dead.
| 
| My position is:
| 
|  * locate_error is a nasty thing that should go away.
|  * ", decl, decl" isn't pretty but it isn't as nasty as locate_error.
|  * Consistency between the C and C++ front ends is desirable.
| 
| and most importantly
| 
|  * Why are we holding up a bug fix with this stupid argument anyway?

What I find stupid is your current position. Quite frankly, it is
incomprehensible to me. You came in keeping saying that I used to be
yyy in the past so zzz must be.  Even when I explained what I mean.

In fact, it is -you- holding up a patch with this stupid argument that
I used to be xxx.  
I think that xxx_at()  should be used until xxx() supports '%+', in
particular I recommend Giovanni makes the modification I suggested.

-- Gaby

Re: [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix)

[Zack Weinberg]

If you changed your mind you could just say so, rather than screaming
at me.

zw

Re: [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix)

[Gabriel Dos Reis]

"Zack Weinberg" <zack@codesourcery.com> writes:

| If you changed your mind you could just say so, rather than screaming
| at me.

  (1) I'm not screaming at you.
  (2) I did not change my mind.  I'm still of the opinion that xxx_at
      must go, but not before xxxx() supports '%+'.
  (3) Why do you keep saying I used to be xxx?

-- Gaby

[C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix) - Take 2

[Giovanni Bajo]

Gabriel Dos Reis wrote:

> What I don't understand is the use of '%J' in lieu of xxx_at.

This is a revised patch, which uses the form suggested by you. It also now
emits the first note line ("because the following virtual..." / "since it has
pure...") on the declaration of the type which is found to be abstract, which
can be a good reference for the user: in fact, the pure virtual function
declarations could be in a differnt point, many files away from the type which
the user is trying to instantiate.

I can happily revert back to %J if it turns out to be better for some reason.
Actually, everybody can in a few seconds, and I reckon the discussion about it
is orthogonal to my patch.

Retested on i686-pc-linux-gnu with no new regressions. OK for mainline?

---------- quote -----------
Alas, this solution is partial, mainly because it doesn't get into account
array types. In fact, parameters with array type are decayed into pointer types
within grokdeclarator (even before a PARM_DECL is built), so they never get
registered into the incomplete_vars list. A solution for this might be
extracting the standard decays out of grokdeclarator, and having grokparms call
it. There is also another user of grokdeclarator(PARM) (in pt.c, to build
template parameters), which should be updated as well. Comments on this plan
are highly appreciated.

Another case we don't handle yet is when the type of the parameter is
dependent. In that case, we probably need to check for abstractness upon
instantiation, while tsubsting. I'll investigate.
---------- end quote -----------
Any comments on this?

Thanks,
Giovanni Bajo



2004-02-10  Giovanni Bajo  <giovannibajo@gcc.gnu.org>

        PR c++/2204
        * decl.c (cp_finish_decl): Extract logic to strip array and pointer
        to array types into...
        (strip_array_and_pointer_to_array_types): New function.
        (grokparms): Register PARM_DECLs of incomplete types for later check,
        using...
        (maybe_register_incomplete_parms): New function.
        (complete_vars): When completing the type for a PARM_DECL, check
        for abstractness. Fix a typo in the comment.
        * typeck2.c (abstract_virtual_errors): Reword diagnostics, make them
        appear at the correct location.

2004-02-10  Giovanni Bajo  <giovannibajo@gcc.gnu.org>

        PR c++/2204
        * g++.dg/other/abstract2.C: New test.
        * g++.old-deja/g++.robertl/eb4.C: Adjust error markers.
        * g++.old-deja/g++.other/decl3.C: Likewise.


Index: cp/decl.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cp/decl.c,v
retrieving revision 1.1179
diff -c -3 -p -r1.1179 decl.c
*** cp/decl.c 2 Feb 2004 16:53:02 -0000 1.1179
--- cp/decl.c 10 Feb 2004 17:10:04 -0000
*************** static void expand_static_init (tree, tr
*** 120,125 ****
--- 120,127 ----
  static tree next_initializable_field (tree);
  static tree reshape_init (tree, tree *);
  static tree build_typename_type (tree, tree, tree);
+ static void maybe_register_incomplete_parm (tree);
+ static tree strip_array_and_pointer_to_array_types (tree);

  /* Erroneous argument lists can use this *IFF* they do not modify it.  */
  tree error_mark_list;
*************** cp_finish_decl (tree decl, tree init, tr
*** 4883,4901 ****
     || TREE_CODE (type) == METHOD_TYPE)
   abstract_virtuals_error (decl,
       strip_array_types (TREE_TYPE (type)));
!       else if (POINTER_TYPE_P (type) || TREE_CODE (type) == ARRAY_TYPE)
!       {
!  /* If it's either a pointer or an array type, strip through all
!     of them but the last one. If the last is an array type, issue
!     an error if the element type is abstract.  */
!  while (POINTER_TYPE_P (TREE_TYPE (type))
!         || TREE_CODE (TREE_TYPE (type)) == ARRAY_TYPE)
!    type = TREE_TYPE (type);
!  if (TREE_CODE (type) == ARRAY_TYPE)
!    abstract_virtuals_error (decl, TREE_TYPE (type));
!       }
!       else
!  abstract_virtuals_error (decl, type);

        if (TREE_CODE (decl) == FUNCTION_DECL
     || TREE_TYPE (decl) == error_mark_node)
--- 4885,4894 ----
     || TREE_CODE (type) == METHOD_TYPE)
   abstract_virtuals_error (decl,
       strip_array_types (TREE_TYPE (type)));
!       else
!  abstract_virtuals_error (decl,
!      strip_array_and_pointer_to_array_types
!       (type));

        if (TREE_CODE (decl) == FUNCTION_DECL
     || TREE_TYPE (decl) == error_mark_node)
*************** grokparms (tree first_parm)
*** 8669,8674 ****
--- 8660,8667 ----
         type = build_pointer_type (type);
         TREE_TYPE (decl) = type;
       }
+    else if (!COMPLETE_TYPE_P (type) || currently_open_class (type))
+      maybe_register_incomplete_parm (decl);
     else if (abstract_virtuals_error (decl, type))
       any_error = 1;  /* Seems like a good idea.  */
     else if (POINTER_TYPE_P (type))
*************** finish_method (tree decl)
*** 11049,11055 ****

    return decl;
  }
! \f

  /* VAR is a VAR_DECL.  If its type is incomplete, remember VAR so that
     we can lay it out later, when and if its type becomes complete.  */
--- 11042,11098 ----

    return decl;
  }
!
! /* Strip all the array/pointer-to-array types from TYPE. This is used to
!    extract the underlying type from declarations such as:
!
!    A a[2];
!    A (*a)[2];
!    A (**a[2])[2];
!
!    which are invalid if 'A' is an abstract type.  */
!
! static tree
! strip_array_and_pointer_to_array_types (tree type)
! {
!   if (POINTER_TYPE_P (type) || TREE_CODE (type) == ARRAY_TYPE)
!     {
!       /* If it's either a pointer or an array type, strip through all
!   of them but the last one. If the last is an array type, then
!   we can return its element type.  */
!       while (POINTER_TYPE_P (TREE_TYPE (type))
!       || TREE_CODE (TREE_TYPE (type)) == ARRAY_TYPE)
!  type = TREE_TYPE (type);
!       if (TREE_CODE (type) == ARRAY_TYPE)
!  return TREE_TYPE (type);
!     }
!   return type;
! }
!
! /* PARM is a PARM_DECL, whose type is incomplete at the point of parsing.
!    We keep track of these because we must issue an error if, upon
!    completition, the type ends up being abstract.  */
!
! static void
! maybe_register_incomplete_parm (tree var)
! {
!   my_friendly_assert (TREE_CODE (var) == PARM_DECL, 20040208);
!
!   /* If the type is dependent, there is nothing we can do here. The
!      diagnostic will have to be issued during template substitution.  */
!   if (!dependent_type_p (TREE_TYPE (var)))
!     {
!       tree inner_type = TREE_TYPE (var);
!
!       /* FIXME: We currently don't get any ARRAY_TYPE because grokdeclarator
!   decays them to POINTER_TYPE too early.  */
!       inner_type = strip_array_and_pointer_to_array_types (inner_type);
!       inner_type = TYPE_MAIN_VARIANT (inner_type);
!
!       if (!COMPLETE_TYPE_P (inner_type) && CLASS_TYPE_P (inner_type))
!  incomplete_vars = tree_cons (inner_type, var, incomplete_vars);
!     }
! }

  /* VAR is a VAR_DECL.  If its type is incomplete, remember VAR so that
     we can lay it out later, when and if its type becomes complete.  */
*************** maybe_register_incomplete_var (tree var)
*** 11064,11074 ****
        && DECL_EXTERNAL (var))
      {
        tree inner_type = TREE_TYPE (var);
!
        while (TREE_CODE (inner_type) == ARRAY_TYPE)
   inner_type = TREE_TYPE (inner_type);
        inner_type = TYPE_MAIN_VARIANT (inner_type);
!
        if ((!COMPLETE_TYPE_P (inner_type) && CLASS_TYPE_P (inner_type))
     /* RTTI TD entries are created while defining the type_info.  */
     || (TYPE_LANG_SPECIFIC (inner_type)
--- 11107,11117 ----
        && DECL_EXTERNAL (var))
      {
        tree inner_type = TREE_TYPE (var);
!
        while (TREE_CODE (inner_type) == ARRAY_TYPE)
   inner_type = TREE_TYPE (inner_type);
        inner_type = TYPE_MAIN_VARIANT (inner_type);
!
        if ((!COMPLETE_TYPE_P (inner_type) && CLASS_TYPE_P (inner_type))
     /* RTTI TD entries are created while defining the type_info.  */
     || (TYPE_LANG_SPECIFIC (inner_type)
*************** maybe_register_incomplete_var (tree var)
*** 11078,11084 ****
  }

  /* Called when a class type (given by TYPE) is defined.  If there are
!    any existing VAR_DECLs whose type hsa been completed by this
     declaration, update them now.  */

  void
--- 11121,11127 ----
  }

  /* Called when a class type (given by TYPE) is defined.  If there are
!    any existing VAR_DECLs whose type has been completed by this
     declaration, update them now.  */

  void
*************** complete_vars (tree type)
*** 11095,11100 ****
--- 11138,11148 ----
     /* Complete the type of the variable.  The VAR_DECL itself
        will be laid out in expand_expr.  */
     complete_type (TREE_TYPE (var));
+    /* If it is a PARM_DECL, we need to emit an error if the
+       type is abstract, because this is why we registered it
+       here.  */
+    if (TREE_CODE (var) == PARM_DECL)
+      abstract_virtuals_error (var, TREE_TYPE (var));
     /* Remove this entry from the list.  */
     *list = TREE_CHAIN (*list);
   }
Index: cp/typeck2.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cp/typeck2.c,v
retrieving revision 1.154
diff -c -3 -p -r1.154 typeck2.c
*** cp/typeck2.c 22 Jan 2004 00:03:52 -0000 1.154
--- cp/typeck2.c 10 Feb 2004 17:10:07 -0000
*************** abstract_virtuals_error (tree decl, tree
*** 149,182 ****
   return 0;

        if (TREE_CODE (decl) == VAR_DECL)
!  error ("cannot declare variable `%D' to be of type `%T'",
!       decl, type);
        else if (TREE_CODE (decl) == PARM_DECL)
!  error ("cannot declare parameter `%D' to be of type `%T'",
!       decl, type);
        else if (TREE_CODE (decl) == FIELD_DECL)
!  error ("cannot declare field `%D' to be of type `%T'",
!       decl, type);
        else if (TREE_CODE (decl) == FUNCTION_DECL
          && TREE_CODE (TREE_TYPE (decl)) == METHOD_TYPE)
!  error ("invalid return type for member function `%#D'", decl);
        else if (TREE_CODE (decl) == FUNCTION_DECL)
!  error ("invalid return type for function `%#D'", decl);
      }
    else
!     error ("cannot allocate an object of type `%T'", type);

    /* Only go through this once.  */
    if (TREE_PURPOSE (u) == NULL_TREE)
      {
        TREE_PURPOSE (u) = error_mark_node;

!       error ("  because the following virtual functions are abstract:");
        for (tu = u; tu; tu = TREE_CHAIN (tu))
!  cp_error_at ("\t%#D", TREE_VALUE (tu));
      }
    else
!     error ("  since type `%T' has abstract virtual functions", type);

    return 1;
  }
--- 149,187 ----
   return 0;

        if (TREE_CODE (decl) == VAR_DECL)
!  cp_error_at ("cannot declare variable `%+D' to be of abstract "
!        "type `%T'", decl, type);
        else if (TREE_CODE (decl) == PARM_DECL)
!  cp_error_at ("cannot declare parameter `%+D' to be of abstract "
!        "type `%T'", decl, type);
        else if (TREE_CODE (decl) == FIELD_DECL)
!  cp_error_at ("cannot declare field `%+D' to be of abstract "
!        "type `%T'", decl, type);
        else if (TREE_CODE (decl) == FUNCTION_DECL
          && TREE_CODE (TREE_TYPE (decl)) == METHOD_TYPE)
!  cp_error_at ("invalid abstract return type for member function `%+#D'",
!        decl);
        else if (TREE_CODE (decl) == FUNCTION_DECL)
!  cp_error_at ("invalid abstract return type for function `%+#D'",
!        decl);
      }
    else
!     error ("cannot allocate an object of abstract type `%T'", type);

    /* Only go through this once.  */
    if (TREE_PURPOSE (u) == NULL_TREE)
      {
        TREE_PURPOSE (u) = error_mark_node;

!       inform ("%J  because the following virtual functions are pure "
!        "within `%T':", TYPE_MAIN_DECL (type), type);
!
        for (tu = u; tu; tu = TREE_CHAIN (tu))
!  inform ("%J\t%#D", TREE_VALUE (tu), TREE_VALUE (tu));
      }
    else
!     inform ("%J  since type `%T' has pure virtual functions",
!      TYPE_MAIN_DECL (type), type);

    return 1;
  }
Index: testsuite/g++.old-deja/g++.other/decl3.C
===================================================================
RCS file: /cvs/gcc/gcc/gcc/testsuite/g++.old-deja/g++.other/decl3.C,v
retrieving revision 1.4
diff -c -3 -p -r1.4 decl3.C
*** testsuite/g++.old-deja/g++.other/decl3.C 1 May 2003 02:02:49 -0000 1.4
--- testsuite/g++.old-deja/g++.other/decl3.C 10 Feb 2004 17:10:07 -0000
***************
*** 6,13 ****

  // We should not allow arrays of abstract type. [class.abstract/2]

! struct cow_t {
!   virtual void f()=0; // { dg-error "" } abstract
  };


--- 6,13 ----

  // We should not allow arrays of abstract type. [class.abstract/2]

! struct cow_t { // { dg-error "" } note
!   virtual void f()=0; // { dg-error "" } pure
  };


Index: testsuite/g++.old-deja/g++.robertl/eb4.C
===================================================================
RCS file: /cvs/gcc/gcc/gcc/testsuite/g++.old-deja/g++.robertl/eb4.C,v
retrieving revision 1.6
diff -c -3 -p -r1.6 eb4.C
*** testsuite/g++.old-deja/g++.robertl/eb4.C 1 May 2003 02:02:58 -0000 1.6
--- testsuite/g++.old-deja/g++.robertl/eb4.C 10 Feb 2004 17:10:07 -0000
*************** public:
*** 17,23 ****
          };

  class some_derived : public some_base
!         {
  public:
          class derived_func_args;
          void func(derived_func_args &);
--- 17,23 ----
          };

  class some_derived : public some_base
!         {  // { dg-error "" } note
  public:
          class derived_func_args;
          void func(derived_func_args &);







// { dg-do compile }
// Contributed by Gabriel Dos Reis <gdr at integrable-solutions dot net>
// PR c++/2204: Check for parameters of abstract type in function declarations.

namespace N1 {
  struct X;

  struct Y1 {
    void g(X parm1);         // { dg-error "abstract" }
    void g(X parm2[2]);      // { dg-error "abstract" "" { xfail *-*-* } }
  };

  template <int N>
  struct Y2 {
    void g(X parm3);         // { dg-error "abstract" }
    void g(X parm4[2]);      // { dg-error "abstract" "" { xfail *-*-* } }
  };

  struct X {  // { dg-error "note" }
    virtual void xfunc(void) = 0;  // { dg-error "note" }
  };
}

namespace N2 {
  struct X1 { // { dg-error "note" }
    virtual void xfunc(void) = 0;  // { dg-error "note" }
    void g(X1 parm5);        // { dg-error "abstract" }
    void g(X1 parm6[2]);     // { dg-error "abstract" "" { xfail *-*-* } }
  };

  template <int N>
  struct X2 { // { dg-error "note" "" { xfail *-*-* } }
    virtual void xfunc(void) = 0;  // { dg-error "note" "" { xfail *-*-* } }
    void g(X2 parm7);        // { dg-error "abstract" "" { xfail *-*-* } }
    void g(X2 parm8[2]);     // { dg-error "abstract" "" { xfail *-*-* } }
  };
}

namespace N3 {
  struct X { // { dg-error "note" "" }
    virtual void xfunc(void) = 0;  // { dg-error "note" }
  };
  void g(X parm9);           // { dg-error "abstract" }
  void g(X parm10[2]);       // { dg-error "abstract" "" { xfail *-*-* } }
  template <int N>
  void g(X parm11);          // { dg-error "abstract" }
  template <int N>
  void g(X parm12[2]);       // { dg-error "abstract" "" { xfail *-*-* } }
}

Re: [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix) - Take 2

[Gabriel Dos Reis]

"Giovanni Bajo" <giovannibajo@libero.it> writes:

| Gabriel Dos Reis wrote:
| 
| > What I don't understand is the use of '%J' in lieu of xxx_at.
| 
| This is a revised patch, which uses the form suggested by you. It also now
| emits the first note line ("because the following virtual..." / "since it has
| pure...") on the declaration of the type which is found to be abstract, which
| can be a good reference for the user: in fact, the pure virtual function
| declarations could be in a differnt point, many files away from the type which
| the user is trying to instantiate.

I totally agree (at least, that has been my case these last days).

| I can happily revert back to %J if it turns out to be better for some reason.
| Actually, everybody can in a few seconds, and I reckon the discussion about it
| is orthogonal to my patch.
| 
| Retested on i686-pc-linux-gnu with no new regressions. OK for mainline?

The diagnostic part is OK with me (I think that the rest also makes
sense).  However, you'll need Jason or Mark's approval for the rest.

| ---------- quote -----------
| Alas, this solution is partial, mainly because it doesn't get into account
| array types. In fact, parameters with array type are decayed into pointer types
| within grokdeclarator (even before a PARM_DECL is built), so they never get
| registered into the incomplete_vars list. A solution for this might be
| extracting the standard decays out of grokdeclarator, and having grokparms call
| it. There is also another user of grokdeclarator(PARM) (in pt.c, to build
| template parameters), which should be updated as well. Comments on this plan
| are highly appreciated.
| 
| Another case we don't handle yet is when the type of the parameter is
| dependent. In that case, we probably need to check for abstractness upon
| instantiation, while tsubsting. I'll investigate.
| ---------- end quote -----------
| Any comments on this?

I think I would have gone with Jason's suggestion -- putting those
parameter types in a separate list -- but your  partial solution does
no harm I suppose. 

-- Gaby

Re: [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix)

[Gabriel Dos Reis]

"Giovanni Bajo" <giovannibajo@libero.it> writes:

|         if (TREE_CODE (decl) == VAR_DECL)
| !  error ("%Jcannot declare variable `%D' to be of abstract type `%T'",
| !         decl, decl, type);

That is an abuse of %J.  Use error_at instead.  E.g.

  error_at ("cannot declare variable `%+D' to be of abstract type `%T'",
            decl, type);


| !  error ("%Jcannot declare parameter `%D' to be of abstract type `%T'",
| !         decl, decl, type);

Ditto.

|         else if (TREE_CODE (decl) == FIELD_DECL)
| !  error ("%Jcannot declare field `%D' to be of abstract type `%T'",
| !         decl, decl, type);

Ditto.

|         else if (TREE_CODE (decl) == FUNCTION_DECL
|           && TREE_CODE (TREE_TYPE (decl)) == METHOD_TYPE)
| !  error ("%Jinvalid abstract return type for member function `%#D'",
| !         decl, decl);

Ditto.

|         else if (TREE_CODE (decl) == FUNCTION_DECL)
| !  error ("%Jinvalid abstract return type for function `%#D'",
| !         decl, decl);

Ditto.

As a side comment, I don't understand why you went through that
change.

-- Gaby

Re: [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix) - Take 2

[Gabriel Dos Reis]

"Giovanni Bajo" <giovannibajo@libero.it> writes:

| Gabriel Dos Reis wrote:
| 
| > What I don't understand is the use of '%J' in lieu of xxx_at.
| 
| This is a revised patch, which uses the form suggested by you. It also now
| emits the first note line ("because the following virtual..." / "since it has
| pure...") on the declaration of the type which is found to be abstract, which
| can be a good reference for the user: in fact, the pure virtual function
| declarations could be in a differnt point, many files away from the type which
| the user is trying to instantiate.

I totally agree (at least, that has been my case these last days).

| I can happily revert back to %J if it turns out to be better for some reason.
| Actually, everybody can in a few seconds, and I reckon the discussion about it
| is orthogonal to my patch.
| 
| Retested on i686-pc-linux-gnu with no new regressions. OK for mainline?

The diagnostic part is OK with me (I think that the rest also makes
sense).  However, you'll need Jason or Mark's approval for the rest.

| ---------- quote -----------
| Alas, this solution is partial, mainly because it doesn't get into account
| array types. In fact, parameters with array type are decayed into pointer types
| within grokdeclarator (even before a PARM_DECL is built), so they never get
| registered into the incomplete_vars list. A solution for this might be
| extracting the standard decays out of grokdeclarator, and having grokparms call
| it. There is also another user of grokdeclarator(PARM) (in pt.c, to build
| template parameters), which should be updated as well. Comments on this plan
| are highly appreciated.
| 
| Another case we don't handle yet is when the type of the parameter is
| dependent. In that case, we probably need to check for abstractness upon
| instantiation, while tsubsting. I'll investigate.
| ---------- end quote -----------
| Any comments on this?

I think I would have gone with Jason's suggestion -- putting those
parameter types in a separate list -- but your  partial solution does
no harm I suppose. 

-- Gaby

Re: [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix)

[Zack Weinberg]

Gabriel Dos Reis <gdr@integrable-solutions.net> writes:
> | Gabriel Dos Reis <gdr@integrable-solutions.net> writes:
> | > The repetition oan information that is already there.  Any scheme that
> | > requires repetition of information is wrong. 
> | 
> | Well, this is not consistent with what you have said before, and I
>
> Again, I don't understand what you're talking about.

You were quite adamant in the past that xxx_at() must go.  So adamant,
in fact, that I thought it was already dead.

My position is:

 * locate_error is a nasty thing that should go away.
 * ", decl, decl" isn't pretty but it isn't as nasty as locate_error.
 * Consistency between the C and C++ front ends is desirable.

and most importantly

 * Why are we holding up a bug fix with this stupid argument anyway?

zw

Re: [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix)

[Gabriel Dos Reis]

"Zack Weinberg" <zack@codesourcery.com> writes:

| Gabriel Dos Reis <gdr@integrable-solutions.net> writes:
| 
| > "Giovanni Bajo" <giovannibajo@libero.it> writes:
| >
| > |         if (TREE_CODE (decl) == VAR_DECL)
| > | !  error ("%Jcannot declare variable `%D' to be of abstract type `%T'",
| > | !         decl, decl, type);
| >
| > That is an abuse of %J.  Use error_at instead.
| [etc]
| 
| I told Giovanni that error/warning/pedwarn_at were to be eliminated in
| favor of %J.  Did I misunderstand what you've said about these in the
| past?

I don't know exactly what you're referring to.  But, the repetition of
the "decl" is not right and if any removal should lead to that, then
that removal should not happen.

There are some uses of error_at which are wrong.  E.g.

  error_at ("foo is nonsensical", decl);        // WRONG

  error ("%Jfoo is nonsensical", decl);        // Good


  error ("%Jdeclaration '%D' is nonsensical", decl, decl); // WRONG

  error_at ("declaration '%+D' is nonsensical", decl); // Good

-- Gaby

[C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix)

[Giovanni Bajo]

Hello,

this is a partial fix for PR2204, which is about checking if PARM_DECLs in
function declarations have abstract types. Currently, the check is being
performed in grokparms, which is fine for many cases, but it's way too early if
the type is the class being defined, or an incomplete type. In that case, we
need to defer the check later, when/if the type is completed.

To do this, I allow PARM_DECLS to be registered in the incomplete_vars list (by
grokparms) and check for abstractness upon type completition (finish_struct_1,
which calls complete_vars). Notice that we cannot check it in start_function,
because the program is ill-formed even if there is no definition for the
function.

Alas, this solution is partial, mainly because it doesn't get into account
array types. In fact, parameters with array type are decayed into pointer types
within grokdeclarator (even before a PARM_DECL is built), so they never get
registered into the incomplete_vars list. A solution for this might be
extracting the standard decays out of grokdeclarator, and having grokparms call
it. There is also another user of grokdeclarator(PARM) (in pt.c, to build
template parameters), which should be updated as well. Comments on this plan
are highly appreciated.

Another case we don't handle yet is when the type of the parameter is
dependent. In that case, we probably need to check for abstractness upon
instantiation, while tsubsting. I'll investigate.

Meanwhile, this tested succesfully on i686-pc-linux-gnu. The testcase have a
handful of xfails for the cases we don't handle yet. OK for mainline?

Giovanni Bajo



2004-02-10  Giovanni Bajo  <giovannibajo@gcc.gnu.org>

        PR c++/2204
        * decl.c (cp_finish_decl): Extract logic to strip array and pointer
        to array types into...
        (strip_array_and_pointer_to_array_types): New function.
        (grokparms): Register PARM_DECLs of incomplete types for later check,
        using...
        (maybe_register_incomplete_parms): New function.
        (complete_vars): When completing the type for a PARM_DECL, check
        for abstractness. Fix a typo in the comment.
        * typeck2.c (abstract_virtual_errors): Reword diagnostics, make them
        appear at the correct location.

2004-02-10  Giovanni Bajo  <giovannibajo@gcc.gnu.org>

        PR c++/2204
        * g++.dg/other/abstract2.C: New test.


Index: decl.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cp/decl.c,v
retrieving revision 1.1179
diff -c -3 -p -r1.1179 decl.c
*** decl.c 2 Feb 2004 16:53:02 -0000 1.1179
--- decl.c 9 Feb 2004 23:27:43 -0000
*************** static void expand_static_init (tree, tr
*** 120,125 ****
--- 120,127 ----
  static tree next_initializable_field (tree);
  static tree reshape_init (tree, tree *);
  static tree build_typename_type (tree, tree, tree);
+ static void maybe_register_incomplete_parm (tree);
+ static tree strip_array_and_pointer_to_array_types (tree);

  /* Erroneous argument lists can use this *IFF* they do not modify it.  */
  tree error_mark_list;
*************** cp_finish_decl (tree decl, tree init, tr
*** 4883,4901 ****
     || TREE_CODE (type) == METHOD_TYPE)
   abstract_virtuals_error (decl,
       strip_array_types (TREE_TYPE (type)));
!       else if (POINTER_TYPE_P (type) || TREE_CODE (type) == ARRAY_TYPE)
!       {
!  /* If it's either a pointer or an array type, strip through all
!     of them but the last one. If the last is an array type, issue
!     an error if the element type is abstract.  */
!  while (POINTER_TYPE_P (TREE_TYPE (type))
!         || TREE_CODE (TREE_TYPE (type)) == ARRAY_TYPE)
!    type = TREE_TYPE (type);
!  if (TREE_CODE (type) == ARRAY_TYPE)
!    abstract_virtuals_error (decl, TREE_TYPE (type));
!       }
!       else
!  abstract_virtuals_error (decl, type);

        if (TREE_CODE (decl) == FUNCTION_DECL
     || TREE_TYPE (decl) == error_mark_node)
--- 4885,4894 ----
     || TREE_CODE (type) == METHOD_TYPE)
   abstract_virtuals_error (decl,
       strip_array_types (TREE_TYPE (type)));
!       else
!  abstract_virtuals_error (decl,
!      strip_array_and_pointer_to_array_types
!       (type));

        if (TREE_CODE (decl) == FUNCTION_DECL
     || TREE_TYPE (decl) == error_mark_node)
*************** grokparms (tree first_parm)
*** 8669,8674 ****
--- 8660,8667 ----
         type = build_pointer_type (type);
         TREE_TYPE (decl) = type;
       }
+    else if (!COMPLETE_TYPE_P (type) || currently_open_class (type))
+      maybe_register_incomplete_parm (decl);
     else if (abstract_virtuals_error (decl, type))
       any_error = 1;  /* Seems like a good idea.  */
     else if (POINTER_TYPE_P (type))
*************** finish_method (tree decl)
*** 11049,11055 ****

    return decl;
  }
! \f

  /* VAR is a VAR_DECL.  If its type is incomplete, remember VAR so that
     we can lay it out later, when and if its type becomes complete.  */
--- 11042,11098 ----

    return decl;
  }
!
! /* Strip all the array/pointer-to-array types from TYPE. This is used to
!    extract the underlying type from declarations such as:
!
!    A a[2];
!    A (*a)[2];
!    A (**a[2])[2];
!
!    which are invalid if 'A' is an abstract type.  */
!
! static tree
! strip_array_and_pointer_to_array_types (tree type)
! {
!   if (POINTER_TYPE_P (type) || TREE_CODE (type) == ARRAY_TYPE)
!     {
!       /* If it's either a pointer or an array type, strip through all
!   of them but the last one. If the last is an array type, then
!   we can return its element type.  */
!       while (POINTER_TYPE_P (TREE_TYPE (type))
!       || TREE_CODE (TREE_TYPE (type)) == ARRAY_TYPE)
!  type = TREE_TYPE (type);
!       if (TREE_CODE (type) == ARRAY_TYPE)
!  return TREE_TYPE (type);
!     }
!   return type;
! }
!
! /* PARM is a PARM_DECL, whose type is incomplete at the point of parsing.
!    We keep track of these because we must issue an error if, upon
!    completition, the type ends up being abstract.  */
!
! static void
! maybe_register_incomplete_parm (tree var)
! {
!   my_friendly_assert (TREE_CODE (var) == PARM_DECL, 20040208);
!
!   /* If the type is dependent, there is nothing we can do here. The
!      diagnostic will have to be issued during template substitution.  */
!   if (!dependent_type_p (TREE_TYPE (var)))
!     {
!       tree inner_type = TREE_TYPE (var);
!
!       /* FIXME: We currently don't get any ARRAY_TYPE because grokdeclarator
!   decays them to POINTER_TYPE too early.  */
!       inner_type = strip_array_and_pointer_to_array_types (inner_type);
!       inner_type = TYPE_MAIN_VARIANT (inner_type);
!
!       if (!COMPLETE_TYPE_P (inner_type) && CLASS_TYPE_P (inner_type))
!  incomplete_vars = tree_cons (inner_type, var, incomplete_vars);
!     }
! }

  /* VAR is a VAR_DECL.  If its type is incomplete, remember VAR so that
     we can lay it out later, when and if its type becomes complete.  */
*************** maybe_register_incomplete_var (tree var)
*** 11078,11084 ****
  }

  /* Called when a class type (given by TYPE) is defined.  If there are
!    any existing VAR_DECLs whose type hsa been completed by this
     declaration, update them now.  */

  void
--- 11121,11127 ----
  }

  /* Called when a class type (given by TYPE) is defined.  If there are
!    any existing VAR_DECLs whose type has been completed by this
     declaration, update them now.  */

  void
*************** complete_vars (tree type)
*** 11095,11100 ****
--- 11138,11148 ----
     /* Complete the type of the variable.  The VAR_DECL itself
        will be laid out in expand_expr.  */
     complete_type (TREE_TYPE (var));
+    /* If it is a PARM_DECL, we need to emit an error if the
+       type is abstract, because this is why we registered it
+       here.  */
+    if (TREE_CODE (var) == PARM_DECL)
+      abstract_virtuals_error (var, TREE_TYPE (var));
     /* Remove this entry from the list.  */
     *list = TREE_CHAIN (*list);
   }
Index: typeck2.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cp/typeck2.c,v
retrieving revision 1.154
diff -c -3 -p -r1.154 typeck2.c
*** typeck2.c 22 Jan 2004 00:03:52 -0000 1.154
--- typeck2.c 9 Feb 2004 23:27:46 -0000
*************** abstract_virtuals_error (tree decl, tree
*** 149,182 ****
   return 0;

        if (TREE_CODE (decl) == VAR_DECL)
!  error ("cannot declare variable `%D' to be of type `%T'",
!       decl, type);
        else if (TREE_CODE (decl) == PARM_DECL)
!  error ("cannot declare parameter `%D' to be of type `%T'",
!       decl, type);
        else if (TREE_CODE (decl) == FIELD_DECL)
!  error ("cannot declare field `%D' to be of type `%T'",
!       decl, type);
        else if (TREE_CODE (decl) == FUNCTION_DECL
          && TREE_CODE (TREE_TYPE (decl)) == METHOD_TYPE)
!  error ("invalid return type for member function `%#D'", decl);
        else if (TREE_CODE (decl) == FUNCTION_DECL)
!  error ("invalid return type for function `%#D'", decl);
      }
    else
!     error ("cannot allocate an object of type `%T'", type);

    /* Only go through this once.  */
    if (TREE_PURPOSE (u) == NULL_TREE)
      {
        TREE_PURPOSE (u) = error_mark_node;

!       error ("  because the following virtual functions are abstract:");
        for (tu = u; tu; tu = TREE_CHAIN (tu))
!  cp_error_at ("\t%#D", TREE_VALUE (tu));
      }
    else
!     error ("  since type `%T' has abstract virtual functions", type);

    return 1;
  }
--- 149,196 ----
   return 0;

        if (TREE_CODE (decl) == VAR_DECL)
!  error ("%Jcannot declare variable `%D' to be of abstract type `%T'",
!         decl, decl, type);
        else if (TREE_CODE (decl) == PARM_DECL)
!  error ("%Jcannot declare parameter `%D' to be of abstract type `%T'",
!         decl, decl, type);
        else if (TREE_CODE (decl) == FIELD_DECL)
!  error ("%Jcannot declare field `%D' to be of abstract type `%T'",
!         decl, decl, type);
        else if (TREE_CODE (decl) == FUNCTION_DECL
          && TREE_CODE (TREE_TYPE (decl)) == METHOD_TYPE)
!  error ("%Jinvalid abstract return type for member function `%#D'",
!         decl, decl);
        else if (TREE_CODE (decl) == FUNCTION_DECL)
!  error ("%Jinvalid abstract return type for function `%#D'",
!         decl, decl);
      }
    else
!     error ("cannot allocate an object of abstract type `%T'", type);

    /* Only go through this once.  */
    if (TREE_PURPOSE (u) == NULL_TREE)
      {
        TREE_PURPOSE (u) = error_mark_node;

!       if (decl)
!  inform ("%J  because the following virtual functions are pure:",
!   decl);
!       else
!  inform ("  because the following virtual functions are pure:");
!
        for (tu = u; tu; tu = TREE_CHAIN (tu))
!  inform ("%J\t%#D", TREE_VALUE (tu), TREE_VALUE (tu));
      }
    else
!   {
!     if (decl)
!       inform ("%J  since type `%T' has pure virtual functions",
!        decl, type);
!     else
!       inform ("  since type `%T' has pure virtual functions",
!        type);
!   }

    return 1;
  }




// { dg-do compile }
// Contributed by Gabriel Dos Reis <gdr at integrable-solutions dot net>
// PR c++/2204: Check for parameters of abstract type in function declarations.

namespace N1 {
  struct X;

  struct Y1 {
    void g(X parm1);         // { dg-error "abstract" }
    void g(X parm2[2]);      // { dg-error "abstract" "" { xfail *-*-* } }
  };

  template <int N>
  struct Y2 {
    void g(X parm3);         // { dg-error "abstract" }
    void g(X parm4[2]);      // { dg-error "abstract" "" { xfail *-*-* } }
  };

  struct X {
    virtual void xfunc(void) = 0;  // { dg-error "note" }
  };
}

namespace N2 {
  struct X1 {
    virtual void xfunc(void) = 0;  // { dg-error "note" }
    void g(X1 parm5);        // { dg-error "abstract" }
    void g(X1 parm6[2]);     // { dg-error "abstract" "" { xfail *-*-* } }
  };

  template <int N>
  struct X2 {
    virtual void xfunc(void) = 0;  // { dg-error "note" "" { xfail *-*-* } }
    void g(X2 parm7);        // { dg-error "abstract" "" { xfail *-*-* } }
    void g(X2 parm8[2]);     // { dg-error "abstract" "" { xfail *-*-* } }
  };
}

namespace N3 {
  struct X {
    virtual void xfunc(void) = 0;  // { dg-error "note" }
  };
  void g(X parm9);           // { dg-error "abstract" }
  void g(X parm10[2]);       // { dg-error "abstract" "" { xfail *-*-* } }
  template <int N>
  void g(X parm11);          // { dg-error "abstract" }
  template <int N>
  void g(X parm12[2]);       // { dg-error "abstract" "" { xfail *-*-* } }
}

// { dg-excess-errors "following virtual functions|has pure virtual
functions" }

Re: [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix)

[Zack Weinberg]

Gabriel Dos Reis <gdr@integrable-solutions.net> writes:
> "Zack Weinberg" <zack@codesourcery.com> writes:
> | Gabriel Dos Reis <gdr@integrable-solutions.net> writes:
> | > I don't know exactly what you're referring to.  But, the repetition of
> | > the "decl" is not right and if any removal should lead to that, then
> | > that removal should not happen.
> | 
> | I don't understand this.  The %J consumes one argument and the %D
> | consumes another one.  Why is this "wrong"?
>
> The repetition oan information that is already there.  Any scheme that
> requires repetition of information is wrong. 

Well, this is not consistent with what you have said before, and I
wonder why you didn't object to %J when it was originally added.  We
have %J...%D all over the C front end these days.

zw

[C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix) - Take 2

[Giovanni Bajo]

Gabriel Dos Reis wrote:

> What I don't understand is the use of '%J' in lieu of xxx_at.

This is a revised patch, which uses the form suggested by you. It also now
emits the first note line ("because the following virtual..." / "since it has
pure...") on the declaration of the type which is found to be abstract, which
can be a good reference for the user: in fact, the pure virtual function
declarations could be in a differnt point, many files away from the type which
the user is trying to instantiate.

I can happily revert back to %J if it turns out to be better for some reason.
Actually, everybody can in a few seconds, and I reckon the discussion about it
is orthogonal to my patch.

Retested on i686-pc-linux-gnu with no new regressions. OK for mainline?

---------- quote -----------
Alas, this solution is partial, mainly because it doesn't get into account
array types. In fact, parameters with array type are decayed into pointer types
within grokdeclarator (even before a PARM_DECL is built), so they never get
registered into the incomplete_vars list. A solution for this might be
extracting the standard decays out of grokdeclarator, and having grokparms call
it. There is also another user of grokdeclarator(PARM) (in pt.c, to build
template parameters), which should be updated as well. Comments on this plan
are highly appreciated.

Another case we don't handle yet is when the type of the parameter is
dependent. In that case, we probably need to check for abstractness upon
instantiation, while tsubsting. I'll investigate.
---------- end quote -----------
Any comments on this?

Thanks,
Giovanni Bajo



2004-02-10  Giovanni Bajo  <giovannibajo@gcc.gnu.org>

        PR c++/2204
        * decl.c (cp_finish_decl): Extract logic to strip array and pointer
        to array types into...
        (strip_array_and_pointer_to_array_types): New function.
        (grokparms): Register PARM_DECLs of incomplete types for later check,
        using...
        (maybe_register_incomplete_parms): New function.
        (complete_vars): When completing the type for a PARM_DECL, check
        for abstractness. Fix a typo in the comment.
        * typeck2.c (abstract_virtual_errors): Reword diagnostics, make them
        appear at the correct location.

2004-02-10  Giovanni Bajo  <giovannibajo@gcc.gnu.org>

        PR c++/2204
        * g++.dg/other/abstract2.C: New test.
        * g++.old-deja/g++.robertl/eb4.C: Adjust error markers.
        * g++.old-deja/g++.other/decl3.C: Likewise.


Index: cp/decl.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cp/decl.c,v
retrieving revision 1.1179
diff -c -3 -p -r1.1179 decl.c
*** cp/decl.c 2 Feb 2004 16:53:02 -0000 1.1179
--- cp/decl.c 10 Feb 2004 17:10:04 -0000
*************** static void expand_static_init (tree, tr
*** 120,125 ****
--- 120,127 ----
  static tree next_initializable_field (tree);
  static tree reshape_init (tree, tree *);
  static tree build_typename_type (tree, tree, tree);
+ static void maybe_register_incomplete_parm (tree);
+ static tree strip_array_and_pointer_to_array_types (tree);

  /* Erroneous argument lists can use this *IFF* they do not modify it.  */
  tree error_mark_list;
*************** cp_finish_decl (tree decl, tree init, tr
*** 4883,4901 ****
     || TREE_CODE (type) == METHOD_TYPE)
   abstract_virtuals_error (decl,
       strip_array_types (TREE_TYPE (type)));
!       else if (POINTER_TYPE_P (type) || TREE_CODE (type) == ARRAY_TYPE)
!       {
!  /* If it's either a pointer or an array type, strip through all
!     of them but the last one. If the last is an array type, issue
!     an error if the element type is abstract.  */
!  while (POINTER_TYPE_P (TREE_TYPE (type))
!         || TREE_CODE (TREE_TYPE (type)) == ARRAY_TYPE)
!    type = TREE_TYPE (type);
!  if (TREE_CODE (type) == ARRAY_TYPE)
!    abstract_virtuals_error (decl, TREE_TYPE (type));
!       }
!       else
!  abstract_virtuals_error (decl, type);

        if (TREE_CODE (decl) == FUNCTION_DECL
     || TREE_TYPE (decl) == error_mark_node)
--- 4885,4894 ----
     || TREE_CODE (type) == METHOD_TYPE)
   abstract_virtuals_error (decl,
       strip_array_types (TREE_TYPE (type)));
!       else
!  abstract_virtuals_error (decl,
!      strip_array_and_pointer_to_array_types
!       (type));

        if (TREE_CODE (decl) == FUNCTION_DECL
     || TREE_TYPE (decl) == error_mark_node)
*************** grokparms (tree first_parm)
*** 8669,8674 ****
--- 8660,8667 ----
         type = build_pointer_type (type);
         TREE_TYPE (decl) = type;
       }
+    else if (!COMPLETE_TYPE_P (type) || currently_open_class (type))
+      maybe_register_incomplete_parm (decl);
     else if (abstract_virtuals_error (decl, type))
       any_error = 1;  /* Seems like a good idea.  */
     else if (POINTER_TYPE_P (type))
*************** finish_method (tree decl)
*** 11049,11055 ****

    return decl;
  }
! \f

  /* VAR is a VAR_DECL.  If its type is incomplete, remember VAR so that
     we can lay it out later, when and if its type becomes complete.  */
--- 11042,11098 ----

    return decl;
  }
!
! /* Strip all the array/pointer-to-array types from TYPE. This is used to
!    extract the underlying type from declarations such as:
!
!    A a[2];
!    A (*a)[2];
!    A (**a[2])[2];
!
!    which are invalid if 'A' is an abstract type.  */
!
! static tree
! strip_array_and_pointer_to_array_types (tree type)
! {
!   if (POINTER_TYPE_P (type) || TREE_CODE (type) == ARRAY_TYPE)
!     {
!       /* If it's either a pointer or an array type, strip through all
!   of them but the last one. If the last is an array type, then
!   we can return its element type.  */
!       while (POINTER_TYPE_P (TREE_TYPE (type))
!       || TREE_CODE (TREE_TYPE (type)) == ARRAY_TYPE)
!  type = TREE_TYPE (type);
!       if (TREE_CODE (type) == ARRAY_TYPE)
!  return TREE_TYPE (type);
!     }
!   return type;
! }
!
! /* PARM is a PARM_DECL, whose type is incomplete at the point of parsing.
!    We keep track of these because we must issue an error if, upon
!    completition, the type ends up being abstract.  */
!
! static void
! maybe_register_incomplete_parm (tree var)
! {
!   my_friendly_assert (TREE_CODE (var) == PARM_DECL, 20040208);
!
!   /* If the type is dependent, there is nothing we can do here. The
!      diagnostic will have to be issued during template substitution.  */
!   if (!dependent_type_p (TREE_TYPE (var)))
!     {
!       tree inner_type = TREE_TYPE (var);
!
!       /* FIXME: We currently don't get any ARRAY_TYPE because grokdeclarator
!   decays them to POINTER_TYPE too early.  */
!       inner_type = strip_array_and_pointer_to_array_types (inner_type);
!       inner_type = TYPE_MAIN_VARIANT (inner_type);
!
!       if (!COMPLETE_TYPE_P (inner_type) && CLASS_TYPE_P (inner_type))
!  incomplete_vars = tree_cons (inner_type, var, incomplete_vars);
!     }
! }

  /* VAR is a VAR_DECL.  If its type is incomplete, remember VAR so that
     we can lay it out later, when and if its type becomes complete.  */
*************** maybe_register_incomplete_var (tree var)
*** 11064,11074 ****
        && DECL_EXTERNAL (var))
      {
        tree inner_type = TREE_TYPE (var);
!
        while (TREE_CODE (inner_type) == ARRAY_TYPE)
   inner_type = TREE_TYPE (inner_type);
        inner_type = TYPE_MAIN_VARIANT (inner_type);
!
        if ((!COMPLETE_TYPE_P (inner_type) && CLASS_TYPE_P (inner_type))
     /* RTTI TD entries are created while defining the type_info.  */
     || (TYPE_LANG_SPECIFIC (inner_type)
--- 11107,11117 ----
        && DECL_EXTERNAL (var))
      {
        tree inner_type = TREE_TYPE (var);
!
        while (TREE_CODE (inner_type) == ARRAY_TYPE)
   inner_type = TREE_TYPE (inner_type);
        inner_type = TYPE_MAIN_VARIANT (inner_type);
!
        if ((!COMPLETE_TYPE_P (inner_type) && CLASS_TYPE_P (inner_type))
     /* RTTI TD entries are created while defining the type_info.  */
     || (TYPE_LANG_SPECIFIC (inner_type)
*************** maybe_register_incomplete_var (tree var)
*** 11078,11084 ****
  }

  /* Called when a class type (given by TYPE) is defined.  If there are
!    any existing VAR_DECLs whose type hsa been completed by this
     declaration, update them now.  */

  void
--- 11121,11127 ----
  }

  /* Called when a class type (given by TYPE) is defined.  If there are
!    any existing VAR_DECLs whose type has been completed by this
     declaration, update them now.  */

  void
*************** complete_vars (tree type)
*** 11095,11100 ****
--- 11138,11148 ----
     /* Complete the type of the variable.  The VAR_DECL itself
        will be laid out in expand_expr.  */
     complete_type (TREE_TYPE (var));
+    /* If it is a PARM_DECL, we need to emit an error if the
+       type is abstract, because this is why we registered it
+       here.  */
+    if (TREE_CODE (var) == PARM_DECL)
+      abstract_virtuals_error (var, TREE_TYPE (var));
     /* Remove this entry from the list.  */
     *list = TREE_CHAIN (*list);
   }
Index: cp/typeck2.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cp/typeck2.c,v
retrieving revision 1.154
diff -c -3 -p -r1.154 typeck2.c
*** cp/typeck2.c 22 Jan 2004 00:03:52 -0000 1.154
--- cp/typeck2.c 10 Feb 2004 17:10:07 -0000
*************** abstract_virtuals_error (tree decl, tree
*** 149,182 ****
   return 0;

        if (TREE_CODE (decl) == VAR_DECL)
!  error ("cannot declare variable `%D' to be of type `%T'",
!       decl, type);
        else if (TREE_CODE (decl) == PARM_DECL)
!  error ("cannot declare parameter `%D' to be of type `%T'",
!       decl, type);
        else if (TREE_CODE (decl) == FIELD_DECL)
!  error ("cannot declare field `%D' to be of type `%T'",
!       decl, type);
        else if (TREE_CODE (decl) == FUNCTION_DECL
          && TREE_CODE (TREE_TYPE (decl)) == METHOD_TYPE)
!  error ("invalid return type for member function `%#D'", decl);
        else if (TREE_CODE (decl) == FUNCTION_DECL)
!  error ("invalid return type for function `%#D'", decl);
      }
    else
!     error ("cannot allocate an object of type `%T'", type);

    /* Only go through this once.  */
    if (TREE_PURPOSE (u) == NULL_TREE)
      {
        TREE_PURPOSE (u) = error_mark_node;

!       error ("  because the following virtual functions are abstract:");
        for (tu = u; tu; tu = TREE_CHAIN (tu))
!  cp_error_at ("\t%#D", TREE_VALUE (tu));
      }
    else
!     error ("  since type `%T' has abstract virtual functions", type);

    return 1;
  }
--- 149,187 ----
   return 0;

        if (TREE_CODE (decl) == VAR_DECL)
!  cp_error_at ("cannot declare variable `%+D' to be of abstract "
!        "type `%T'", decl, type);
        else if (TREE_CODE (decl) == PARM_DECL)
!  cp_error_at ("cannot declare parameter `%+D' to be of abstract "
!        "type `%T'", decl, type);
        else if (TREE_CODE (decl) == FIELD_DECL)
!  cp_error_at ("cannot declare field `%+D' to be of abstract "
!        "type `%T'", decl, type);
        else if (TREE_CODE (decl) == FUNCTION_DECL
          && TREE_CODE (TREE_TYPE (decl)) == METHOD_TYPE)
!  cp_error_at ("invalid abstract return type for member function `%+#D'",
!        decl);
        else if (TREE_CODE (decl) == FUNCTION_DECL)
!  cp_error_at ("invalid abstract return type for function `%+#D'",
!        decl);
      }
    else
!     error ("cannot allocate an object of abstract type `%T'", type);

    /* Only go through this once.  */
    if (TREE_PURPOSE (u) == NULL_TREE)
      {
        TREE_PURPOSE (u) = error_mark_node;

!       inform ("%J  because the following virtual functions are pure "
!        "within `%T':", TYPE_MAIN_DECL (type), type);
!
        for (tu = u; tu; tu = TREE_CHAIN (tu))
!  inform ("%J\t%#D", TREE_VALUE (tu), TREE_VALUE (tu));
      }
    else
!     inform ("%J  since type `%T' has pure virtual functions",
!      TYPE_MAIN_DECL (type), type);

    return 1;
  }
Index: testsuite/g++.old-deja/g++.other/decl3.C
===================================================================
RCS file: /cvs/gcc/gcc/gcc/testsuite/g++.old-deja/g++.other/decl3.C,v
retrieving revision 1.4
diff -c -3 -p -r1.4 decl3.C
*** testsuite/g++.old-deja/g++.other/decl3.C 1 May 2003 02:02:49 -0000 1.4
--- testsuite/g++.old-deja/g++.other/decl3.C 10 Feb 2004 17:10:07 -0000
***************
*** 6,13 ****

  // We should not allow arrays of abstract type. [class.abstract/2]

! struct cow_t {
!   virtual void f()=0; // { dg-error "" } abstract
  };


--- 6,13 ----

  // We should not allow arrays of abstract type. [class.abstract/2]

! struct cow_t { // { dg-error "" } note
!   virtual void f()=0; // { dg-error "" } pure
  };


Index: testsuite/g++.old-deja/g++.robertl/eb4.C
===================================================================
RCS file: /cvs/gcc/gcc/gcc/testsuite/g++.old-deja/g++.robertl/eb4.C,v
retrieving revision 1.6
diff -c -3 -p -r1.6 eb4.C
*** testsuite/g++.old-deja/g++.robertl/eb4.C 1 May 2003 02:02:58 -0000 1.6
--- testsuite/g++.old-deja/g++.robertl/eb4.C 10 Feb 2004 17:10:07 -0000
*************** public:
*** 17,23 ****
          };

  class some_derived : public some_base
!         {
  public:
          class derived_func_args;
          void func(derived_func_args &);
--- 17,23 ----
          };

  class some_derived : public some_base
!         {  // { dg-error "" } note
  public:
          class derived_func_args;
          void func(derived_func_args &);







// { dg-do compile }
// Contributed by Gabriel Dos Reis <gdr at integrable-solutions dot net>
// PR c++/2204: Check for parameters of abstract type in function declarations.

namespace N1 {
  struct X;

  struct Y1 {
    void g(X parm1);         // { dg-error "abstract" }
    void g(X parm2[2]);      // { dg-error "abstract" "" { xfail *-*-* } }
  };

  template <int N>
  struct Y2 {
    void g(X parm3);         // { dg-error "abstract" }
    void g(X parm4[2]);      // { dg-error "abstract" "" { xfail *-*-* } }
  };

  struct X {  // { dg-error "note" }
    virtual void xfunc(void) = 0;  // { dg-error "note" }
  };
}

namespace N2 {
  struct X1 { // { dg-error "note" }
    virtual void xfunc(void) = 0;  // { dg-error "note" }
    void g(X1 parm5);        // { dg-error "abstract" }
    void g(X1 parm6[2]);     // { dg-error "abstract" "" { xfail *-*-* } }
  };

  template <int N>
  struct X2 { // { dg-error "note" "" { xfail *-*-* } }
    virtual void xfunc(void) = 0;  // { dg-error "note" "" { xfail *-*-* } }
    void g(X2 parm7);        // { dg-error "abstract" "" { xfail *-*-* } }
    void g(X2 parm8[2]);     // { dg-error "abstract" "" { xfail *-*-* } }
  };
}

namespace N3 {
  struct X { // { dg-error "note" "" }
    virtual void xfunc(void) = 0;  // { dg-error "note" }
  };
  void g(X parm9);           // { dg-error "abstract" }
  void g(X parm10[2]);       // { dg-error "abstract" "" { xfail *-*-* } }
  template <int N>
  void g(X parm11);          // { dg-error "abstract" }
  template <int N>
  void g(X parm12[2]);       // { dg-error "abstract" "" { xfail *-*-* } }
}

Re: [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix)

[Gabriel Dos Reis]

"Giovanni Bajo" <giovannibajo@libero.it> writes:

| Gabriel Dos Reis wrote:
| 
| > As a side comment, I don't understand why you went through that
| > change.
| 
| This is the rationale for my changes to abstract_virtual_errors.
| 
| * Use of %J (will be error_at):
| With my patch, abstract_virtual_errors can be deferred to an indefinite moment
| (when the type is completed). Without this change, the error message appears on
| the last line of the completed type, which is wrong: we need to show the user
| where is the parameter whose type is abstract. Then, the other messages about
| the pure virtual functions will point her to the functions which need to be
| overridden, and thus to the type.

That makes sense.

| * Rewording
| There is no such thing as an "abstract virtual function". Standard calls it
| "pure virtual function".


I agree too.  I've been seeing those messages these last days (personal
experience forgetting to define dozens of pure virtual  functions in a
very complex class hierarchy).  They look indeed indeed very odd.

| * Use of inform vs error
| Those messages are just additional information provided to help the user, they
| are not part of the error message proper. Moreover, I heard someone wanted to
| have a -fno-notes mode one day, so it makes sense to be able to shut those off.
| This is also why I added the word "abstract" on the first error line, so that
| it is now a complete error message, and the notes are really optional. Before,
| the first line would have been unclear, were the notes disabled for some
| reason.

This makes sense.

| Sorry, I should have mentioned these things in the patch message.

No need to sorry.  What I don't understand is the use of '%J' in lieu
of xxx_at.  

Thanks,

-- Gaby

Re: [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix)

[Zack Weinberg]

If you changed your mind you could just say so, rather than screaming
at me.

zw

Re: [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix)

[Giovanni Bajo]

Gabriel Dos Reis wrote:

> As a side comment, I don't understand why you went through that
> change.

This is the rationale for my changes to abstract_virtual_errors.

* Use of %J (will be error_at):
With my patch, abstract_virtual_errors can be deferred to an indefinite moment
(when the type is completed). Without this change, the error message appears on
the last line of the completed type, which is wrong: we need to show the user
where is the parameter whose type is abstract. Then, the other messages about
the pure virtual functions will point her to the functions which need to be
overridden, and thus to the type.

* Rewording
There is no such thing as an "abstract virtual function". Standard calls it
"pure virtual function".

* Use of inform vs error
Those messages are just additional information provided to help the user, they
are not part of the error message proper. Moreover, I heard someone wanted to
have a -fno-notes mode one day, so it makes sense to be able to shut those off.
This is also why I added the word "abstract" on the first error line, so that
it is now a complete error message, and the notes are really optional. Before,
the first line would have been unclear, were the notes disabled for some
reason.

Sorry, I should have mentioned these things in the patch message.

Giovanni Bajo

Re: [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix)

[Zack Weinberg]

Gabriel Dos Reis <gdr@integrable-solutions.net> writes:

> "Zack Weinberg" <zack@codesourcery.com> writes:
>
> | Gabriel Dos Reis <gdr@integrable-solutions.net> writes:
> | 
> | > "Giovanni Bajo" <giovannibajo@libero.it> writes:
> | >
> | > |         if (TREE_CODE (decl) == VAR_DECL)
> | > | !  error ("%Jcannot declare variable `%D' to be of abstract type `%T'",
> | > | !         decl, decl, type);
> | >
> | > That is an abuse of %J.  Use error_at instead.
> | [etc]
> | 
> | I told Giovanni that error/warning/pedwarn_at were to be eliminated in
> | favor of %J.  Did I misunderstand what you've said about these in the
> | past?
>
> I don't know exactly what you're referring to.  But, the repetition of
> the "decl" is not right and if any removal should lead to that, then
> that removal should not happen.

I don't understand this.  The %J consumes one argument and the %D
consumes another one.  Why is this "wrong"?

>   error ("%Jdeclaration '%D' is nonsensical", decl, decl); // WRONG
>   error_at ("declaration '%+D' is nonsensical", decl); // Good

The %+D notation is tidier, but I had understood you to be of the
opinion that it was not worth the implementation mess, nor the baggage
of carrying around two different sets of diagnostic functions.

(If we ever get Chiaki's indexed-parameters-notation patch laid down,
it will also allow implementing %+x cleanly and for all languages.)

zw

Re: [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix)

[Gabriel Dos Reis]

"Zack Weinberg" <zack@codesourcery.com> writes:

| If you changed your mind you could just say so, rather than screaming
| at me.

  (1) I'm not screaming at you.
  (2) I did not change my mind.  I'm still of the opinion that xxx_at
      must go, but not before xxxx() supports '%+'.
  (3) Why do you keep saying I used to be xxx?

-- Gaby

Re: [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix)

[Zack Weinberg]

Gabriel Dos Reis <gdr@integrable-solutions.net> writes:

> "Giovanni Bajo" <giovannibajo@libero.it> writes:
>
> |         if (TREE_CODE (decl) == VAR_DECL)
> | !  error ("%Jcannot declare variable `%D' to be of abstract type `%T'",
> | !         decl, decl, type);
>
> That is an abuse of %J.  Use error_at instead.
[etc]

I told Giovanni that error/warning/pedwarn_at were to be eliminated in
favor of %J.  Did I misunderstand what you've said about these in the
past?

zw

Re: [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix)

[Gabriel Dos Reis]

"Zack Weinberg" <zack@codesourcery.com> writes:

| Gabriel Dos Reis <gdr@integrable-solutions.net> writes:
| > "Zack Weinberg" <zack@codesourcery.com> writes:
| > | Gabriel Dos Reis <gdr@integrable-solutions.net> writes:
| > | > I don't know exactly what you're referring to.  But, the repetition of
| > | > the "decl" is not right and if any removal should lead to that, then
| > | > that removal should not happen.
| > | 
| > | I don't understand this.  The %J consumes one argument and the %D
| > | consumes another one.  Why is this "wrong"?
| >
| > The repetition oan information that is already there.  Any scheme that
| > requires repetition of information is wrong. 
| 
| Well, this is not consistent with what you have said before, and I

Again, I don't understand what you're talking about.
I have never been a fan of information/code duplication.  Did you see
me advocating for information/code duplication in the past?

I want to get rid of xxx_at() if xxx() works correctly.

| wonder why you didn't object to %J when it was originally added.  We

Most certainly because when it was added, it was to replace previous
attempts to remove xxx_with_decl() that where using %s when actually a
tree was being passed.

| have %J...%D all over the C front end these days.

Yes, but I do not believe the situation is comparable.   The C
front-end never had xxx_at().

-- Gaby

Re: [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix)

[Gabriel Dos Reis]

"Zack Weinberg" <zack@codesourcery.com> writes:

| Gabriel Dos Reis <gdr@integrable-solutions.net> writes:
| > | Gabriel Dos Reis <gdr@integrable-solutions.net> writes:
| > | > The repetition oan information that is already there.  Any scheme that
| > | > requires repetition of information is wrong. 
| > | 
| > | Well, this is not consistent with what you have said before, and I
| >
| > Again, I don't understand what you're talking about.
| 
| You were quite adamant in the past that xxx_at() must go.  So adamant,
| in fact, that I thought it was already dead.
| 
| My position is:
| 
|  * locate_error is a nasty thing that should go away.
|  * ", decl, decl" isn't pretty but it isn't as nasty as locate_error.
|  * Consistency between the C and C++ front ends is desirable.
| 
| and most importantly
| 
|  * Why are we holding up a bug fix with this stupid argument anyway?

What I find stupid is your current position. Quite frankly, it is
incomprehensible to me. You came in keeping saying that I used to be
yyy in the past so zzz must be.  Even when I explained what I mean.

In fact, it is -you- holding up a patch with this stupid argument that
I used to be xxx.  
I think that xxx_at()  should be used until xxx() supports '%+', in
particular I recommend Giovanni makes the modification I suggested.

-- Gaby

Re: [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix)

[Gabriel Dos Reis]

"Zack Weinberg" <zack@codesourcery.com> writes:

| Gabriel Dos Reis <gdr@integrable-solutions.net> writes:
| 
| > "Zack Weinberg" <zack@codesourcery.com> writes:
| >
| > | Gabriel Dos Reis <gdr@integrable-solutions.net> writes:
| > | 
| > | > "Giovanni Bajo" <giovannibajo@libero.it> writes:
| > | >
| > | > |         if (TREE_CODE (decl) == VAR_DECL)
| > | > | !  error ("%Jcannot declare variable `%D' to be of abstract type `%T'",
| > | > | !         decl, decl, type);
| > | >
| > | > That is an abuse of %J.  Use error_at instead.
| > | [etc]
| > | 
| > | I told Giovanni that error/warning/pedwarn_at were to be eliminated in
| > | favor of %J.  Did I misunderstand what you've said about these in the
| > | past?
| >
| > I don't know exactly what you're referring to.  But, the repetition of
| > the "decl" is not right and if any removal should lead to that, then
| > that removal should not happen.
| 
| I don't understand this.  The %J consumes one argument and the %D
| consumes another one.  Why is this "wrong"?

The repetition oan information that is already there.  Any scheme that
requires repetition of information is wrong. 

| >   error ("%Jdeclaration '%D' is nonsensical", decl, decl); // WRONG
| >   error_at ("declaration '%+D' is nonsensical", decl); // Good
| 
| The %+D notation is tidier, but I had understood you to be of the
| opinion that it was not worth the implementation mess, nor the baggage
| of carrying around two different sets of diagnostic functions.

If you have error() recognizing '%+' properly then error_at() should
go.  The whole purpose of xxx_at() begin there is that xxx() does not
support yet '%+'.

-- Gaby

Re: [C++ PATCH] [PR2204] Check for parameters of abstract types (partial fix) - Take 2

[Jason Merrill]

On Tue, 10 Feb 2004 18:37:31 +0100, "Giovanni Bajo" <giovannibajo@libero.it> wrote:

> Alas, this solution is partial, mainly because it doesn't get into account
> array types. In fact, parameters with array type are decayed into pointer types
> within grokdeclarator (even before a PARM_DECL is built), so they never get
> registered into the incomplete_vars list. A solution for this might be
> extracting the standard decays out of grokdeclarator, and having grokparms call
> it. There is also another user of grokdeclarator(PARM) (in pt.c, to build
> template parameters), which should be updated as well. Comments on this plan
> are highly appreciated.

There's a much simpler solution: just check for abstract types in
create_array_type_for_decl.

  8.3.4  Arrays                                              [dcl.array]

               T is called the array element type; this type  shall  not
  be a reference type, the (possibly cv-qualified) type void, a function
  type  or  an  abstract  class  type.

This should also make strip_array_and_pointer_to_array_types unnecessary.

> Another case we don't handle yet is when the type of the parameter is
> dependent. In that case, we probably need to check for abstractness upon
> instantiation, while tsubsting. I'll investigate.

Please.

Jason

[C++ PATCH] [PR2204] Check for parameters of abstract types - Take 3

[Giovanni Bajo]

Jason Merrill wrote:

[http://gcc.gnu.org/ml/gcc-patches/2004-02/msg00953.html]

>> Alas, this solution is partial, mainly because it doesn't get into
>> account
>> array types. In fact, parameters with array type are decayed into
>> pointer types within grokdeclarator (even before a PARM_DECL is
>> built), so they never get registered into the incomplete_vars list.
>> A solution for this might be extracting the standard decays out of
>> grokdeclarator, and having grokparms call it. There is also another
>> user of grokdeclarator(PARM) (in pt.c, to build template
>> parameters), which should be updated as well. Comments on this plan
>> are highly appreciated.
>
> There's a much simpler solution: just check for abstract types in
> create_array_type_for_decl.

I tried this but it occurred to me that if I checked for abstract types in
grokdeclarator immediatly after the big loop where the type for the declaration
is constructed (and where create_array_type_for_decl is called for the
ARRAY_REF case), I could catch both the array and the non-array case at the
same time, unifying the check. In fact, it works beautifully, so I removed the
other check for abstractness from grokparms. The only quirk is that I need to
create a PARM_DECL just for the purpose of storing it in the incomplete_vars
list (it cannot be done with the real PARM_DECL because that's already decayed
from array to pointer), but it doesn't seem too much of a problem to me.

>> Another case we don't handle yet is when the type of the parameter is
>> dependent. In that case, we probably need to check for abstractness
>> upon instantiation, while tsubsting. I'll investigate.
>
> Please.

I added support for simple cases, where the dependent type contains pure
virtual functions and it is used as a type for parameters. In this case, I am
emitting an error before instantiation (as soon as the type is completely
defined).

I still miss cases where the pure virtuals come from base classes or
specializations of the template (that is, non trivial cases). In this
situation, something will have to be done at instantiation time, but I will
leave this as a followup.

The hunk in abstract_virtual_errors is needed so that we emit diagnostic such
as "cannot define parameter 'parm' to be of type 'X (*)[2]' because 'X' is
abstract": basically, in that line of the diagnostic we want to print the real
type of the parameter, not the innest type which is abstract. Also, removing
the check for dependent_type_p does not hurt anything, because there was
already a check in place for CLASSTYPE_PURE_VIRTUALS(t), which is usually empty
for dependent types.

Tested on i686-pc-linux-gnu, OK for mainline?

Giovanni Bajo


cp/
        PR c++/2204
        * decl.c (cp_finish_decl): Extract logic to strip array and pointer
        to array types into...
        (strip_array_and_pointer_to_array_types): New function.
        (grokparms): Don't check for abstract types here.
        (grokdeclarator): Check for abstract types when grokking a
        parameter.
        (maybe_register_incomplete_parms): New function.
        (complete_vars): When completing the type for a PARM_DECL, check
        for abstractness. Fix a typo in the comment.
        * class.c (finish_struct): Construct CLASSTYPE_PURE_VIRTUALS for
        templates, and call complete_vars to check for abstractness.
        * typeck2.c (abstract_virtuals_error): Do not skip dependend types.
        Print real type of variables in diagnostic.

testsuite/
        PR c++/2204
        * g++.dg/other/abstract2.C: New test.


Index: decl.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cp/decl.c,v
retrieving revision 1.1206
diff -c -3 -p -r1.1206 decl.c
*** decl.c 13 May 2004 06:40:16 -0000 1.1206
--- decl.c 11 Jun 2004 10:45:08 -0000
*************** static void expand_static_init (tree, tr
*** 120,125 ****
--- 120,127 ----
  static tree next_initializable_field (tree);
  static tree reshape_init (tree, tree *);
  static tree build_typename_type (tree, tree, tree);
+ static void maybe_register_incomplete_parm (tree);
+ static tree strip_array_and_pointer_to_array_types (tree);

  /* Erroneous argument lists can use this *IFF* they do not modify it.  */
  tree error_mark_list;
*************** cp_finish_decl (tree decl, tree init, tr
*** 4833,4851 ****
     || TREE_CODE (type) == METHOD_TYPE)
   abstract_virtuals_error (decl,
       strip_array_types (TREE_TYPE (type)));
!       else if (POINTER_TYPE_P (type) || TREE_CODE (type) == ARRAY_TYPE)
!       {
!  /* If it's either a pointer or an array type, strip through all
!     of them but the last one. If the last is an array type, issue
!     an error if the element type is abstract.  */
!  while (POINTER_TYPE_P (TREE_TYPE (type))
!         || TREE_CODE (TREE_TYPE (type)) == ARRAY_TYPE)
!    type = TREE_TYPE (type);
!  if (TREE_CODE (type) == ARRAY_TYPE)
!    abstract_virtuals_error (decl, TREE_TYPE (type));
!       }
!       else
!  abstract_virtuals_error (decl, type);

        if (TREE_CODE (decl) == FUNCTION_DECL
     || TREE_TYPE (decl) == error_mark_node)
--- 4835,4844 ----
     || TREE_CODE (type) == METHOD_TYPE)
   abstract_virtuals_error (decl,
       strip_array_types (TREE_TYPE (type)));
!       else
!  abstract_virtuals_error (decl,
!      strip_array_and_pointer_to_array_types
!       (type));

        if (TREE_CODE (decl) == FUNCTION_DECL
     || TREE_TYPE (decl) == error_mark_node)
*************** grokdeclarator (tree declarator,
*** 7703,7708 ****
--- 7696,7718 ----
        type = error_mark_node;
      }

+   /* If it is a parameter, we need to check whether the type is abstract
+      or not, and issue an error if it is.  */
+   if ((decl_context == PARM || decl_context == CATCHPARM)
+       && type != error_mark_node)
+     {
+       tree t = strip_array_and_pointer_to_array_types (type);
+       tree decl = cp_build_parm_decl (declarator, type);
+
+       /* If the type is not complete at this point, register the parameter
+   declaration among the incomplete variables, so that it will get
+   checked when the type is completed.  */
+       if (!COMPLETE_TYPE_P (t) || currently_open_class (t))
+  maybe_register_incomplete_parm (decl);
+       else
+  abstract_virtuals_error (decl, t);
+     }
+
    if ((decl_context == FIELD || decl_context == PARM)
        && !processing_template_decl
        && variably_modified_type_p (type))
*************** grokparms (tree first_parm, tree *parms)
*** 8636,8643 ****
         type = build_pointer_type (type);
         TREE_TYPE (decl) = type;
       }
-    else if (abstract_virtuals_error (decl, type))
-      any_error = 1;  /* Seems like a good idea.  */
     else if (POINTER_TYPE_P (type))
       {
         /* [dcl.fct]/6, parameter types cannot contain pointers
--- 8646,8651 ----
*************** finish_method (tree decl)
*** 10966,10972 ****

    return decl;
  }
! \f

  /* VAR is a VAR_DECL.  If its type is incomplete, remember VAR so that
     we can lay it out later, when and if its type becomes complete.  */
--- 10974,11024 ----

    return decl;
  }
!
! /* Strip all the array/pointer-to-array types from TYPE. This is used to
!    extract the underlying type from declarations such as:
!
!    A a[2];
!    A (*a)[2];
!    A (**a[2])[2];
!
!    which are invalid if 'A' is an abstract type.  */
!
! static tree
! strip_array_and_pointer_to_array_types (tree type)
! {
!   if (POINTER_TYPE_P (type) || TREE_CODE (type) == ARRAY_TYPE)
!     {
!       /* If it's either a pointer or an array type, strip through all
!   of them but the last one. If the last is an array type, then
!   we can return its element type.  */
!       while (POINTER_TYPE_P (TREE_TYPE (type))
!       || TREE_CODE (TREE_TYPE (type)) == ARRAY_TYPE)
!  type = TREE_TYPE (type);
!       if (TREE_CODE (type) == ARRAY_TYPE)
!  return TREE_TYPE (type);
!     }
!   return type;
! }
!
! /* PARM is a PARM_DECL, whose type is incomplete at the point of parsing.
!    We keep track of these because we must issue an error if, upon
!    completition, the type ends up being abstract.  */
!
! static void
! maybe_register_incomplete_parm (tree var)
! {
!   tree inner_type;
!
!   my_friendly_assert (TREE_CODE (var) == PARM_DECL, 20040208);
!
!   inner_type = TREE_TYPE (var);
!   inner_type = strip_array_and_pointer_to_array_types (inner_type);
!   inner_type = TYPE_MAIN_VARIANT (inner_type);
!
!   if (!COMPLETE_TYPE_P (inner_type) && CLASS_TYPE_P (inner_type))
!     incomplete_vars = tree_cons (inner_type, var, incomplete_vars);
! }

  /* VAR is a VAR_DECL.  If its type is incomplete, remember VAR so that
     we can lay it out later, when and if its type becomes complete.  */
*************** maybe_register_incomplete_var (tree var)
*** 10981,10991 ****
        && DECL_EXTERNAL (var))
      {
        tree inner_type = TREE_TYPE (var);
!
        while (TREE_CODE (inner_type) == ARRAY_TYPE)
   inner_type = TREE_TYPE (inner_type);
        inner_type = TYPE_MAIN_VARIANT (inner_type);
!
        if ((!COMPLETE_TYPE_P (inner_type) && CLASS_TYPE_P (inner_type))
     /* RTTI TD entries are created while defining the type_info.  */
     || (TYPE_LANG_SPECIFIC (inner_type)
--- 11033,11043 ----
        && DECL_EXTERNAL (var))
      {
        tree inner_type = TREE_TYPE (var);
!
        while (TREE_CODE (inner_type) == ARRAY_TYPE)
   inner_type = TREE_TYPE (inner_type);
        inner_type = TYPE_MAIN_VARIANT (inner_type);
!
        if ((!COMPLETE_TYPE_P (inner_type) && CLASS_TYPE_P (inner_type))
     /* RTTI TD entries are created while defining the type_info.  */
     || (TYPE_LANG_SPECIFIC (inner_type)
*************** maybe_register_incomplete_var (tree var)
*** 10995,11001 ****
  }

  /* Called when a class type (given by TYPE) is defined.  If there are
!    any existing VAR_DECLs whose type hsa been completed by this
     declaration, update them now.  */

  void
--- 11047,11053 ----
  }

  /* Called when a class type (given by TYPE) is defined.  If there are
!    any existing VAR_DECLs whose type has been completed by this
     declaration, update them now.  */

  void
*************** complete_vars (tree type)
*** 11012,11017 ****
--- 11064,11076 ----
     /* Complete the type of the variable.  The VAR_DECL itself
        will be laid out in expand_expr.  */
     complete_type (TREE_TYPE (var));
+    /* If it is a PARM_DECL, we need to emit an error if the
+       type is abstract, because this is why we registered it
+       here.  */
+    if (TREE_CODE (var) == PARM_DECL)
+      abstract_virtuals_error (var,
+          strip_array_and_pointer_to_array_types
+           (TREE_TYPE (var)));
     /* Remove this entry from the list.  */
     *list = TREE_CHAIN (*list);
   }
Index: class.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cp/class.c,v
retrieving revision 1.611
diff -c -3 -p -r1.611 class.c
*** class.c 13 May 2004 06:40:13 -0000 1.611
--- class.c 11 Jun 2004 10:45:10 -0000
*************** finish_struct (tree t, tree attributes)
*** 5243,5250 ****
--- 5243,5266 ----

    if (processing_template_decl)
      {
+       tree x;
+
        finish_struct_methods (t);
        TYPE_SIZE (t) = bitsize_zero_node;
+
+       /* We need to emit an error message if this type was used as a
parameter
+   and it is an abstract type, even if it is a template. We construct
+   a simple CLASSTYPE_PURE_VIRTUALS list without taking bases into
+   account and we call complete_vars with this type, which will check
+   the PARM_DECLS. Note that while the type is being defined,
+   CLASSTYPE_PURE_VIRTUALS contains the list of the inline friends
+   (see CLASSTYPE_INLINE_FRIENDS) so we need to clear it.  */
+       CLASSTYPE_PURE_VIRTUALS (t) = NULL_TREE;
+       for (x = TYPE_METHODS (t); x; x = TREE_CHAIN (x))
+  if (DECL_PURE_VIRTUAL_P (x))
+    CLASSTYPE_PURE_VIRTUALS (t)
+      = tree_cons (NULL_TREE, x, CLASSTYPE_PURE_VIRTUALS (t));
+       complete_vars (t);
      }
    else
      finish_struct_1 (t);
Index: typeck2.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cp/typeck2.c,v
retrieving revision 1.159
diff -c -3 -p -r1.159 typeck2.c
*** typeck2.c 11 Jun 2004 03:11:05 -0000 1.159
--- typeck2.c 11 Jun 2004 03:32:56 -0000
*************** abstract_virtuals_error (tree decl, tree
*** 137,147 ****
         CLASSTYPE_PURE_VIRTUALS holds the inline friends.  */
      return 0;

-   if (dependent_type_p (type))
-     /* For a dependent type, we do not yet know which functions are pure
-        virtuals.  */
-     return 0;
-
    u = CLASSTYPE_PURE_VIRTUALS (type);
    if (decl)
      {
--- 137,142 ----
*************** abstract_virtuals_error (tree decl, tree
*** 150,162 ****

        if (TREE_CODE (decl) == VAR_DECL)
   cp_error_at ("cannot declare variable `%+D' to be of abstract "
!        "type `%T'", decl, type);
        else if (TREE_CODE (decl) == PARM_DECL)
   cp_error_at ("cannot declare parameter `%+D' to be of abstract "
!        "type `%T'", decl, type);
        else if (TREE_CODE (decl) == FIELD_DECL)
   cp_error_at ("cannot declare field `%+D' to be of abstract "
!        "type `%T'", decl, type);
        else if (TREE_CODE (decl) == FUNCTION_DECL
          && TREE_CODE (TREE_TYPE (decl)) == METHOD_TYPE)
   cp_error_at ("invalid abstract return type for member function `%+#D'",
--- 145,157 ----

        if (TREE_CODE (decl) == VAR_DECL)
   cp_error_at ("cannot declare variable `%+D' to be of abstract "
!        "type `%T'", decl, TREE_TYPE (decl));
        else if (TREE_CODE (decl) == PARM_DECL)
   cp_error_at ("cannot declare parameter `%+D' to be of abstract "
!        "type `%T'", decl, TREE_TYPE (decl));
        else if (TREE_CODE (decl) == FIELD_DECL)
   cp_error_at ("cannot declare field `%+D' to be of abstract "
!        "type `%T'", decl, TREE_TYPE (decl));
        else if (TREE_CODE (decl) == FUNCTION_DECL
          && TREE_CODE (TREE_TYPE (decl)) == METHOD_TYPE)
   cp_error_at ("invalid abstract return type for member function `%+#D'",




// { dg-do compile }
// Contributed by Gabriel Dos Reis <gdr at integrable-solutions dot net>
// PR c++/2204: Check for parameters of abstract type in function declarations.

namespace N1 {
  struct X;

  struct Y1 {
    void g(X parm1);         // { dg-error "abstract" }
    void g(X parm2[2]);      // { dg-error "abstract" }
    void g(X (*parm3)[2]);   // { dg-error "abstract" }
  };


  template <int N>
  struct Y2 {
    void g(X parm4);         // { dg-error "abstract" }
    void g(X parm5[2]);      // { dg-error "abstract" }
    void g(X (*parm6)[2]);   // { dg-error "abstract" }
  };

  struct X {  // { dg-error "note" }
    virtual void xfunc(void) = 0;  // { dg-error "note" }
  };
}

namespace N2 {
  struct X1 { // { dg-error "note" }
    virtual void xfunc(void) = 0;  // { dg-error "note" }
    void g(X1 parm7);        // { dg-error "abstract" }
    void g(X1 parm8[2]);     // { dg-error "abstract" }
    void g(X1 (*parm9)[2]);  // { dg-error "abstract" }
  };

  template <int N>
  struct X2 { // { dg-error "note" }
    virtual void xfunc(void) = 0; // { dg-error "note" }
    void g(X2 parm10);        // { dg-error "abstract" }
    void g(X2 parm11[2]);     // { dg-error "abstract" }
    void g(X2 (*parm12)[2]);  // { dg-error "abstract" }
  };
}

namespace N3 {
  struct X { // { dg-error "note" "" }
    virtual void xfunc(void) = 0;  // { dg-error "note" }
  };
  void g(X parm13);          // { dg-error "abstract" }
  void g(X parm14[2]);       // { dg-error "abstract" }
  void g(X (*parm15)[2]);    // { dg-error "abstract" }

  template <int N>
  void g(X parm16);          // { dg-error "abstract" }
  template <int N>
  void g(X parm17[2]);       // { dg-error "abstract" }
  template <int N>
  void g(X (*parm18)[2]);    // { dg-error "abstract" }
}

Re: [C++ PATCH] [PR2204] Check for parameters of abstract types - Take 3

[Mark Mitchell]

Giovanni Bajo wrote:

>Jason Merrill wrote:
>
>[http://gcc.gnu.org/ml/gcc-patches/2004-02/msg00953.html]
>
>  
>
>>>Alas, this solution is partial, mainly because it doesn't get into
>>>account
>>>array types. In fact, parameters with array type are decayed into
>>>pointer types within grokdeclarator (even before a PARM_DECL is
>>>built), so they never get registered into the incomplete_vars list.
>>>A solution for this might be extracting the standard decays out of
>>>grokdeclarator, and having grokparms call it. There is also another
>>>user of grokdeclarator(PARM) (in pt.c, to build template
>>>parameters), which should be updated as well. Comments on this plan
>>>are highly appreciated.
>>>      
>>>
>>There's a much simpler solution: just check for abstract types in
>>create_array_type_for_decl.
>>    
>>
>
>I tried this but it occurred to me that if I checked for abstract types in
>grokdeclarator immediatly after the big loop where the type for the declaration
>is constructed (and where create_array_type_for_decl is called for the
>ARRAY_REF case), I could catch both the array and the non-array case at the
>same time, unifying the check. In fact, it works beautifully, so I removed the
>other check for abstractness from grokparms. The only quirk is that I need to
>create a PARM_DECL just for the purpose of storing it in the incomplete_vars
>list (it cannot be done with the real PARM_DECL because that's already decayed
>from array to pointer), but it doesn't seem too much of a problem to me.
>  
>
It is a problem -- DECLs are big, and so creating extra DECLs is a problem.

We should find another way.

-- 
Mark Mitchell
CodeSourcery, LLC
(916) 791-8304
mark@codesourcery.com

Re: [C++ PATCH] [PR2204] Check for parameters of abstract types - Take 3

[Jason Merrill]

On Sun, 13 Jun 2004 01:05:16 -0700, Mark Mitchell <mark@codesourcery.com> wrote:

> Giovanni Bajo wrote:
>
>>Jason Merrill wrote:

>>>There's a much simpler solution: just check for abstract types in
>>>create_array_type_for_decl.
>>
>>I tried this but it occurred to me that if I checked for abstract types in
>>grokdeclarator immediatly after the big loop where the type for the declaration
>>is constructed (and where create_array_type_for_decl is called for the
>>ARRAY_REF case), I could catch both the array and the non-array case at the
>>same time, unifying the check. In fact, it works beautifully, so I removed the
>>other check for abstractness from grokparms. The only quirk is that I need to
>>create a PARM_DECL just for the purpose of storing it in the incomplete_vars
>>list (it cannot be done with the real PARM_DECL because that's already decayed
>>from array to pointer), but it doesn't seem too much of a problem to me.
>>
> It is a problem -- DECLs are big, and so creating extra DECLs is a problem.
>
> We should find another way.

I suggested another way.  It is in fact a constraint violation to have an
array of abstract types regardless of where it appears, so we really should
be checking in create_array_type_for_decl regardless of what we do for
parms.

  8.3.4  Arrays                                              [dcl.array]

1 In a declaration T D where D has the form
  
     D1 [constant-expressionopt]
  
  and the type of the identifier in the declaration T  D1  is  "derived-
  declarator-type-list  T,"  then  the type of the identifier of D is an
  array type.  T is called the array element type; this type  shall  not
  be a reference type, the (possibly cv-qualified) type void, a function
  type  or  an  abstract  class  type.

Jason

Re: [C++ PATCH] [PR2204] Check for parameters of abstract types - Take 3

[Giovanni Bajo]

Jason Merrill wrote:

> I suggested another way.  It is in fact a constraint violation to
> have an array of abstract types regardless of where it appears, so we really
> should be checking in create_array_type_for_decl regardless of what we do for
> parms.

Yes, that was my first approacch, but there was no declaration at that point
either, and I needed one to register the parameter in the incomplete_vars list.
So I moved the test outside, in the called for create_array_type_for_decl,
which is grokdeclarator, a few lines below (where we also have the complete
type of the symbol being declared). Of course, now that I know I must find a
solution which does not involve creating a declaration, I can move back the
test there.

I will have to create a different incomplete_vars list which stores couples of
IDENTIFIER_NODE and TREE_TYPE instead of declarations, for the purpose of
delayed evaluation of abstractness.

Giovanni Bajo

Re: [C++ PATCH] [PR2204] Check for parameters of abstract types - Take 3

[Mark Mitchell]

Giovanni Bajo wrote:

> I will have to create a different incomplete_vars list which stores couples of
> IDENTIFIER_NODE and TREE_TYPE instead of declarations, for the purpose of
> delayed evaluation of abstractness.

I'm sure I'm being dense, but can you back up and explain why we need to 
save away information at all?

Are we trying to detect cases like

   struct S;

   void f(S s);

   struct S {
     virtual void g() = 0;
   };

where "f" has a parameter with an abstract type?

Does the standard actually require us to diagnose this case? 
Presumably, we would already diagnose it if someone tried to call or 
define "f"?

Thanks,

-- 
Mark Mitchell
CodeSourcery, LLC
mark@codesourcery.com

Re: [C++ PATCH] [PR2204] Check for parameters of abstract types - Take 3

[Giovanni Bajo]

Mark Mitchell wrote:

>> I will have to create a different incomplete_vars list which stores
>> couples of IDENTIFIER_NODE and TREE_TYPE instead of declarations,
>> for the purpose of delayed evaluation of abstractness.
>
> I'm sure I'm being dense, but can you back up and explain why we need
> to save away information at all?

Yes. See also comment #3 in Bugzilla, which I quote:

------------------------
The problem is that we currently check for abstractness of parameter types in
gorkparms, which is way too early for the current class. Putting it into
start_function is a stop-gap, but 10.4/3 says that *declarations* are ill-
formed, there is no need for a definition.
------------------------

[class.abstract]/3:
------------------------
An abstract class shall not be used as a parameter type, as a function return
type, or as the type of an
explicit conversion. Pointers and references to an abstract class can be
declared. [Example:

shape x; // error: object of abstract class
shape* p; // OK
shape f(); // error
void g(shape); // error
shape& h(shape&); // OK

--- end example]
------------------------

It is not actually as explicit as I recalled, but I guess that the meaning is
obvious: the declarations are ill-formed, there is no need for definition.
Also, EDG agrees and diagnose such declarations.

> Are we trying to detect cases like
>
>    struct S;
>
>    void f(S s);
>
>    struct S {
>      virtual void g() = 0;
>    };
>
> where "f" has a parameter with an abstract type?

Exactly. If you look in the testcase of my patch, it checks for some slightly
different situations which require delayed evaluation of abstract types. The
testcase was carefully constructed thinking of different situations.

> Does the standard actually require us to diagnose this case?

This is my understanding.

> Presumably, we would already diagnose it if someone tried to call or
> define "f"?

Yes, we do that at the definition point. Calling f() is impossible: if use *s
with a S*, we get an error because we cannot allocate the temporary.

Giovanni Bajo

Re: [C++ PATCH] [PR2204] Check for parameters of abstract types - Take 3

[Mark Mitchell]

Giovanni Bajo wrote:

> Exactly. If you look in the testcase of my patch, it checks for some slightly
> different situations which require delayed evaluation of abstract types. The
> testcase was carefully constructed thinking of different situations.
>
>>Presumably, we would already diagnose it if someone tried to call or
>>define "f"?
> 
> 
> Yes, we do that at the definition point. Calling f() is impossible: if use *s
> with a S*, we get an error because we cannot allocate the temporary.

Yes, that's what I meant.

Curiously, EDG diagnoses the example I gave, but not:

   struct S;

   extern S s;

   struct S { virtual void g() = 0; };

Anyhow, why not just keep a list of all VAR_DECLs and FUNCTION_DECLs 
that have a parameter of an incomplete type, attached to that incomplete 
type?  Then, when a type is completed, if it is abstract, run down the 
list issuing error messages.  After the type is complete, clear the 
list.  To save space, use a hash table mapping class types to these 
lists, rather than using space in every class type.

-- 
Mark Mitchell
CodeSourcery, LLC
mark@codesourcery.com

Re: [C++ PATCH] [PR2204] Check for parameters of abstract types - Take 3

[Giovanni Bajo]

Mark Mitchell wrote:

> Curiously, EDG diagnoses the example I gave, but not:
>
>    struct S;
>
>    extern S s;
>
>    struct S { virtual void g() = 0; };

I would call this a bug in EDG.

> Anyhow, why not just keep a list of all VAR_DECLs and FUNCTION_DECLs
> that have a parameter of an incomplete type, attached to that
> incomplete type?  Then, when a type is completed, if it is abstract, run down
the
> list issuing error messages.  After the type is complete, clear the
> list.

This is basically what I am doing in my patch. Instead of using a list
attacched to each type, I'm reusing the existing global incomplete_vars list,
which keeps declaration of incomplete types, so that we can layout stuff when
the types are completed. It was something that was already in place, I just
added my own decls to them, and added the check for abstractness. Replacing the
list with a hash table is just an optimization which could be done as a follow
up patch I believe.

But there is a problem in reusing existing decls: we never construct a
PARM_DECL with array type, because it gets decayed to pointer before we
construct the declaration. My patch was coping with this by constructing a
"fake" PARM_DECL with array type to be inserted into the list (or attacched to
the type [or hash table], using your suggestion). Since this happens to be too
memory hungry, I was going to change the list into couples of [IDENTIFIER_NODE,
TYPE].

Giovanni Bajo

Re: [C++ PATCH] [PR2204] Check for parameters of abstract types - Take 3

[Jason Merrill]

On Mon, 14 Jun 2004 17:56:19 -0700, Mark Mitchell <mark@codesourcery.com> wrote:

> Anyhow, why not just keep a list of all VAR_DECLs and FUNCTION_DECLs that
> have a parameter of an incomplete type, attached to that incomplete type?
> Then, when a type is completed, if it is abstract, run down the list
> issuing error messages.  After the type is complete, clear the list.  To
> save space, use a hash table mapping class types to these lists, rather
> than using space in every class type.

Sounds like this checking could be combine with the existing use of
incomplete_vars.

Jason

Re: [C++ PATCH] [PR2204] Check for parameters of abstract types -Take 3

[Gabriel Dos Reis]

You Wrote Mark Mitchell
> Giovanni Bajo wrote:
>
>> I will have to create a different incomplete_vars list which stores
>> couples of
>> IDENTIFIER_NODE and TREE_TYPE instead of declarations, for the purpose
>> of
>> delayed evaluation of abstractness.
>
> I'm sure I'm being dense, but can you back up and explain why we need to
> save away information at all?
>
> Are we trying to detect cases like
>
>    struct S;
>
>    void f(S s);
>
>    struct S {
>      virtual void g() = 0;
>    };
>
> where "f" has a parameter with an abstract type?
>
> Does the standard actually require us to diagnose this case?

No, it does not require we diagnose the declaration of f -- and it
can't require that in a resaonably simple way.  I don't believe
we should try to diagnose that.

> Presumably, we would already diagnose it if someone tried to call or
> define "f"?

A call to f requires S to be both complete and non-abstract.  The
same goes for the definition of f.  But by the time user calls f,
we either error because f is undeclared, or S is incomplete or we
know S is abstract.  If all those cases, things follow naturally,

The trickier cases are when we have nested classes in templates.

Re: [C++ PATCH] [PR2204] Check for parameters of abstract types -Take 3

[Gabriel Dos Reis]

You Wrote Mark Mitchell
> Giovanni Bajo wrote:
>
>> Exactly. If you look in the testcase of my patch, it checks for some
>> slightly
>> different situations which require delayed evaluation of abstract types.
>> The
>> testcase was carefully constructed thinking of different situations.
>>
>>>Presumably, we would already diagnose it if someone tried to call or
>>>define "f"?
>>
>>
>> Yes, we do that at the definition point. Calling f() is impossible: if
>> use *s
>> with a S*, we get an error because we cannot allocate the temporary.
>
> Yes, that's what I meant.
>
> Curiously, EDG diagnoses the example I gave, but not:

I think that is a bug in EDG in diagnosing your earlier example.
Your very example can be expanded in combining two translation
units:

   // 1.C
   struct S;
   void f(S);

   // 2.C
   struct S { virtual void g() = 0; };

   int main() { return 0; }

How do you diagnose the declaration in 1.C, if you follow Giovanni's
reading?

Re: [C++ PATCH] [PR2204] Check for parameters of abstract types - Take 3

[Gabriel Dos Reis]

You Wrote Giovanni Bajo
> Mark Mitchell wrote:
>
>>> I will have to create a different incomplete_vars list which stores
>>> couples of IDENTIFIER_NODE and TREE_TYPE instead of declarations,
>>> for the purpose of delayed evaluation of abstractness.
>>
>> I'm sure I'm being dense, but can you back up and explain why we need
>> to save away information at all?
>
> Yes. See also comment #3 in Bugzilla, which I quote:
>
> ------------------------
> The problem is that we currently check for abstractness of parameter types
> in
> gorkparms, which is way too early for the current class. Putting it into
> start_function is a stop-gap, but 10.4/3 says that *declarations* are ill-
> formed, there is no need for a definition.

yes, but that paragraph does not apply to the example given by
Mark -- declarations at non-class scope are processed in there
order of appearance.  By the time you see the declaration for f,
lookup says that S is incomplete and we don't know it is abstract.
When S is completed, that happens after the declaration of f so
it does not affect the well-form-ness of that declaration.

[C++ PATCH] [PR2204] Check for parameters of abstract types - Take 4

[Giovanni Bajo]

Jason Merrill wrote:

>> Anyhow, why not just keep a list of all VAR_DECLs and FUNCTION_DECLs
>> that have a parameter of an incomplete type, attached to that
>> incomplete type? Then, when a type is completed, if it is abstract,
>> run down the list
>> issuing error messages.  After the type is complete, clear the list.
>> To
>> save space, use a hash table mapping class types to these lists,
>> rather
>> than using space in every class type.
>
> Sounds like this checking could be combine with the existing use of
> incomplete_vars.

Yup, in fact it is not so different from what I was doing before.

This is the updated patch. This time, I am hiding the deferral of the check for
abstractness in case of incomplete type within abstract_virtuals_error, so that
callers do not have to bother checking. I am also following Mark's advice to
use a hash table instead of a list, and Jason's advice to insert a check within
create_array_type_for_decl to handle array types (which creates a less accurate
diagnostic, but still good enough).

The new structure I created (struct pending_abstract_type) is needed basically
because I have to store the locus to emit a diagnostic at the right place. For
DECLs, the locus is already present within the DECL itself, but for
IDENTIFIER_NODEs it is not. In case you wonder, it is
create_array_type_for_decl which passes IDENTIFIER_NODEs to
abstract_virtuals_error. Also, there are some situations where the declaration
can be NULL (for instance, when creating an array type within a functional
cast).

As a follow-up patch, we can change incomplete_vars to use a hash table too, or
merge it with pending_abstract_type.

Bootstrapped & tested on i686-pc-linux-gnu with no new regressions, OK for
mainline?

Giovanni Bajo


cp/
        PR c++/2204
        * config-lang.in (gtfiles): Add typeck2.c.
        * Make-lang.in: Tweak typeck2.c dependencies, and add rule for
        gt-cp-typeck2.h.
        * cp-tree.h: Declare complete_type_check_abstract.
        * typeck2.c (pat_calc_hash, pat_compare,
        complete_type_check_abstract): New functions.
        (abstract_virtuals_error): If the type is abstract, register the
        declaration within abstract_pending_vars for further checks.
        Inspect also dependent types. Handle IDENTIFIER_NODEs as decl.
        * decl.c (cp_finish_decl): Do not strip array types.
        (create_array_type_for_decl): Check for abstractness of the element
        type.
        (complete_vars): Call complete_type_check_abstract.
        * class.c (finish_struct): Prepare a list of virtual functions for
        template types, and call complete_vars on it to check for abstractness.


testsuite/
        PR c++/2204
        * g++.dg/other/abstract2.C: New test.


Index: config-lang.in
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cp/config-lang.in,v
retrieving revision 1.21
diff -c -3 -p -r1.21 config-lang.in
*** config-lang.in 27 Feb 2004 00:31:47 -0000 1.21
--- config-lang.in 21 Jun 2004 10:32:45 -0000
*************** stagestuff="g++\$(exeext) g++-cross\$(ex
*** 34,37 ****

  target_libs="target-libstdc++-v3 target-gperf"

! gtfiles="\$(srcdir)/cp/mangle.c \$(srcdir)/cp/name-lookup.h
\$(srcdir)/cp/name-lookup.c \$(srcdir)/cp/cp-tree.h \$(srcdir)/cp/decl.h
\$(srcdir)/cp/lex.h \$(srcdir)/cp/call.c \$(srcdir)/cp/decl.c
\$(srcdir)/cp/decl2.c \$(srcdir)/cp/pt.c \$(srcdir)/cp/repo.c
\$(srcdir)/cp/semantics.c \$(srcdir)/cp/tree.c \$(srcdir)/cp/parser.c
\$(srcdir)/cp/method.c \$(srcdir)/c-common.c \$(srcdir)/c-common.h
\$(srcdir)/c-lex.c \$(srcdir)/c-pragma.c"
--- 34,37 ----

  target_libs="target-libstdc++-v3 target-gperf"

! gtfiles="\$(srcdir)/cp/mangle.c \$(srcdir)/cp/name-lookup.h
\$(srcdir)/cp/name-lookup.c \$(srcdir)/cp/cp-tree.h \$(srcdir)/cp/decl.h
\$(srcdir)/cp/lex.h \$(srcdir)/cp/call.c \$(srcdir)/cp/decl.c
\$(srcdir)/cp/decl2.c \$(srcdir)/cp/pt.c \$(srcdir)/cp/repo.c
\$(srcdir)/cp/semantics.c \$(srcdir)/cp/tree.c \$(srcdir)/cp/parser.c
\$(srcdir)/cp/method.c \$(srcdir)/cp/typeck2.c \$(srcdir)/c-common.c
\$(srcdir)/c-common.h \$(srcdir)/c-lex.c \$(srcdir)/c-pragma.c"
Index: Make-lang.in
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cp/Make-lang.in,v
retrieving revision 1.183
diff -c -3 -p -r1.183 Make-lang.in
*** Make-lang.in 24 May 2004 10:50:43 -0000 1.183
--- Make-lang.in 21 Jun 2004 10:32:45 -0000
*************** $(srcdir)/cp/cfns.h: $(srcdir)/cp/cfns.g
*** 99,105 ****

  gtype-cp.h gt-cp-call.h gt-cp-decl.h gt-cp-decl2.h : s-gtype; @true
  gt-cp-pt.h gt-cp-repo.h gt-cp-parser.h gt-cp-method.h : s-gtype; @true
! gt-cp-tree.h gt-cp-mangle.h gt-cp-name-lookup.h: s-gtype; @true

  #\f
  # Build hooks:
--- 99,105 ----

  gtype-cp.h gt-cp-call.h gt-cp-decl.h gt-cp-decl2.h : s-gtype; @true
  gt-cp-pt.h gt-cp-repo.h gt-cp-parser.h gt-cp-method.h : s-gtype; @true
! gt-cp-tree.h gt-cp-mangle.h gt-cp-name-lookup.h gt-cp-typeck2.h: s-gtype;
@true

  #\f
  # Build hooks:
*************** cp/decl.o: cp/decl.c $(CXX_TREE_H) $(TM_
*** 227,233 ****
  cp/decl2.o: cp/decl2.c $(CXX_TREE_H) $(TM_H) flags.h cp/lex.h cp/decl.h
$(EXPR_H) \
    output.h except.h toplev.h $(RTL_H) c-common.h gt-cp-decl2.h cgraph.h
  cp/typeck2.o: cp/typeck2.c $(CXX_TREE_H) $(TM_H) flags.h toplev.h output.h
$(TM_P_H) \
!    diagnostic.h
  cp/typeck.o: cp/typeck.c $(CXX_TREE_H) $(TM_H) flags.h $(RTL_H) $(EXPR_H)
toplev.h \
     diagnostic.h convert.h
  cp/class.o: cp/class.c $(CXX_TREE_H) $(TM_H) flags.h toplev.h $(RTL_H)
$(TARGET_H) convert.h
--- 227,233 ----
  cp/decl2.o: cp/decl2.c $(CXX_TREE_H) $(TM_H) flags.h cp/lex.h cp/decl.h
$(EXPR_H) \
    output.h except.h toplev.h $(RTL_H) c-common.h gt-cp-decl2.h cgraph.h
  cp/typeck2.o: cp/typeck2.c $(CXX_TREE_H) $(TM_H) flags.h toplev.h output.h
$(TM_P_H) \
!    diagnostic.h gt-cp-typeck2.h
  cp/typeck.o: cp/typeck.c $(CXX_TREE_H) $(TM_H) flags.h $(RTL_H) $(EXPR_H)
toplev.h \
     diagnostic.h convert.h
  cp/class.o: cp/class.c $(CXX_TREE_H) $(TM_H) flags.h toplev.h $(RTL_H)
$(TARGET_H) convert.h
Index: cp-tree.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cp/cp-tree.h,v
retrieving revision 1.977
diff -c -3 -p -r1.977 cp-tree.h
*** cp-tree.h 16 Jun 2004 01:21:29 -0000 1.977
--- cp-tree.h 21 Jun 2004 10:32:46 -0000
*************** extern void cxx_incomplete_type_error  (
*** 4275,4280 ****
--- 4275,4281 ----
  extern tree error_not_base_type   (tree, tree);
  extern tree binfo_or_else   (tree, tree);
  extern void readonly_error   (tree, const char *, int);
+ extern void complete_type_check_abstract (tree);
  extern int abstract_virtuals_error  (tree, tree);

  extern tree store_init_value   (tree, tree);
Index: typeck2.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cp/typeck2.c,v
retrieving revision 1.160
diff -c -3 -p -r1.160 typeck2.c
*** typeck2.c 16 Jun 2004 01:21:35 -0000 1.160
--- typeck2.c 21 Jun 2004 10:32:46 -0000
*************** readonly_error (tree arg, const char* st
*** 118,123 ****
--- 118,236 ----
      (*fn) ("%s of read-only location", string);
  }

+ \f
+ /* Structure that holds information about declarations whose type was
+    incomplete and we could not check whether it was abstract or not.  */
+
+ struct pending_abstract_type GTY((chain_next ("%h.next")))
+ {
+   /* Declaration which we are checking for abstractness. It is either
+      a DECL node, or an IDENTIFIER_NODE if we do not have a full
+      declaration available.  */
+   tree decl;
+
+   /* Type which will be checked for abstractness.  */
+   tree type;
+
+   /* Position of the declaration. This is only needed for IDENTIFIER_NODEs,
+      because DECLs already carry locus information.  */
+   location_t locus;
+
+   /* Link to the next element in list.  */
+   struct pending_abstract_type* next;
+ };
+
+
+ /* Compute the hash value of the node VAL. This function is used by the
+    hash table abstract_pending_vars.  */
+
+ static hashval_t
+ pat_calc_hash (const void* val)
+ {
+   const struct pending_abstract_type* pat = val;
+   return (hashval_t) TYPE_UID (pat->type);
+ }
+
+
+ /* Compare node VAL1 with the type VAL2. This function is used by the
+    hash table abstract_pending_vars.  */
+
+ static int
+ pat_compare (const void* val1, const void* val2)
+ {
+   const struct pending_abstract_type* pat1 = val1;
+   tree type2 = (tree)val2;
+
+   return (pat1->type == type2);
+ }
+
+ /* Hash table that maintains pending_abstract_type nodes, for which we still
+    need to check for type abstractness.  The key of the table is the type
+    of the declaration.  */
+ static GTY ((param_is (struct pending_abstract_type)))
+ htab_t abstract_pending_vars = NULL;
+
+
+ /* This function is called after TYPE is completed, and will check if there
+    are pending declarations for which we still need to verify the
abstractness
+    of TYPE, and emit a diagnostic (through abstract_virtuals_error) if TYPE
+    turned out to be incomplete.  */
+
+ void
+ complete_type_check_abstract (tree type)
+ {
+   void **slot;
+   struct pending_abstract_type *pat;
+   location_t cur_loc = input_location;
+
+   my_friendly_assert (COMPLETE_TYPE_P (type), 20040620_3);
+
+   if (!abstract_pending_vars)
+     return;
+
+   /* Retrieve the list of pending declarations for this type.  */
+   slot = htab_find_slot_with_hash (abstract_pending_vars, type,
+        (hashval_t)TYPE_UID (type), NO_INSERT);
+   if (!slot)
+     return;
+   pat = (struct pending_abstract_type*)*slot;
+   my_friendly_assert (pat, 20040620_2);
+
+   /* If the type is not abstract, do not do anything.  */
+   if (CLASS_TYPE_P (type) || CLASSTYPE_PURE_VIRTUALS (type))
+   {
+     struct pending_abstract_type *prev = 0, *next;
+
+     /* Reverse the list to emit the errors in top-down order.  */
+     for (; pat; pat = next)
+     {
+       next = pat->next;
+       pat->next = prev;
+       prev = pat;
+     }
+     pat = prev;
+
+     /* Go through the list, and call abstract_virtuals_error for each
+        element: it will issue a diagostic if the type is abstract.  */
+     while (pat)
+     {
+       my_friendly_assert (type == pat->type, 20040620_4);
+
+       /* Tweak input_location so that the diagnostic appears at the correct
+   location. Notice that this is only needed if the decl is an
+   IDENTIFIER_NODE, otherwise cp_error_at. */
+       input_location = pat->locus;
+       abstract_virtuals_error (pat->decl, pat->type);
+       pat = pat->next;
+     }
+   }
+
+   htab_clear_slot (abstract_pending_vars, slot);
+
+   input_location = cur_loc;
+ }
+
+
  /* If TYPE has abstract virtual functions, issue an error about trying
     to create an object of that type.  DECL is the object declared, or
     NULL_TREE if the declaration is unavailable.  Returns 1 if an error
*************** abstract_virtuals_error (tree decl, tree
*** 129,135 ****
    tree u;
    tree tu;

!   if (!CLASS_TYPE_P (type) || !CLASSTYPE_PURE_VIRTUALS (type))
      return 0;

    if (!TYPE_SIZE (type))
--- 242,286 ----
    tree u;
    tree tu;

!   /* This function applies only to classes. Any other entity can never
!      be abstract.  */
!   if (!CLASS_TYPE_P (type))
!     return 0;
!
!   /* If the type is incomplete, we register it within a hash table,
!      so that we can check again once it is completed. This makes sense
!      only for objects for which we have a declaration or at least a
!      name.  */
!   if (!COMPLETE_TYPE_P (type))
!   {
!     void **slot;
!     struct pending_abstract_type *pat;
!
!     my_friendly_assert (!decl || (DECL_P (decl)
!       || TREE_CODE (decl) == IDENTIFIER_NODE),
!    20040620_1);
!
!     if (!abstract_pending_vars)
!       abstract_pending_vars = htab_create_ggc (31, &pat_calc_hash,
!             &pat_compare, NULL);
!
!     slot = htab_find_slot_with_hash (abstract_pending_vars, type,
!          (hashval_t)TYPE_UID (type), INSERT);
!
!     pat = ggc_alloc (sizeof (struct pending_abstract_type));
!     pat->type = type;
!     pat->decl = decl;
!     pat->locus = ((decl && DECL_P (decl))
!     ? DECL_SOURCE_LOCATION (decl)
!     : input_location);
!
!     pat->next = *slot;
!     *slot = pat;
!
!     return 0;
!   }
!
!   if (!CLASSTYPE_PURE_VIRTUALS (type))
      return 0;

    if (!TYPE_SIZE (type))
*************** abstract_virtuals_error (tree decl, tree
*** 137,147 ****
         CLASSTYPE_PURE_VIRTUALS holds the inline friends.  */
      return 0;

-   if (dependent_type_p (type))
-     /* For a dependent type, we do not yet know which functions are pure
-        virtuals.  */
-     return 0;
-
    u = CLASSTYPE_PURE_VIRTUALS (type);
    if (decl)
      {
--- 288,293 ----
*************** abstract_virtuals_error (tree decl, tree
*** 164,169 ****
--- 310,319 ----
        else if (TREE_CODE (decl) == FUNCTION_DECL)
   cp_error_at ("invalid abstract return type for function `%+#D'",
         decl);
+       else if (TREE_CODE (decl) == IDENTIFIER_NODE)
+  /* Here we do not have location information, so use error instead
+     of cp_error_at.  */
+  error ("invalid abstract type `%T' for `%E'", type, decl);
        else
   cp_error_at ("invalid abstract type for `%+D'", decl);
      }
*************** require_complete_eh_spec_types (tree fnt
*** 1405,1407 ****
--- 1555,1560 ----
   }
      }
  }
+
+ \f
+ #include "gt-cp-typeck2.h"
Index: decl.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cp/decl.c,v
retrieving revision 1.1218
diff -c -3 -p -r1.1218 decl.c
*** decl.c 16 Jun 2004 01:21:30 -0000 1.1218
--- decl.c 21 Jun 2004 10:32:49 -0000
*************** cp_finish_decl (tree decl, tree init, tr
*** 4866,4886 ****

        make_rtl_for_nonlocal_decl (decl, init, asmspec);

        if (TREE_CODE (type) == FUNCTION_TYPE
     || TREE_CODE (type) == METHOD_TYPE)
!  abstract_virtuals_error (decl,
!      strip_array_types (TREE_TYPE (type)));
!       else if (POINTER_TYPE_P (type) || TREE_CODE (type) == ARRAY_TYPE)
!       {
!  /* If it's either a pointer or an array type, strip through all
!     of them but the last one. If the last is an array type, issue
!     an error if the element type is abstract.  */
!  while (POINTER_TYPE_P (TREE_TYPE (type))
!         || TREE_CODE (TREE_TYPE (type)) == ARRAY_TYPE)
!    type = TREE_TYPE (type);
!  if (TREE_CODE (type) == ARRAY_TYPE)
!    abstract_virtuals_error (decl, TREE_TYPE (type));
!       }
        else
   abstract_virtuals_error (decl, type);

--- 4866,4877 ----

        make_rtl_for_nonlocal_decl (decl, init, asmspec);

+       /* Check for abstractness of the type. Notice that there is no
+   need to strip array types here since the check for those types
+   is already done within create_array_type_for_decl.  */
        if (TREE_CODE (type) == FUNCTION_TYPE
     || TREE_CODE (type) == METHOD_TYPE)
!  abstract_virtuals_error (decl, TREE_TYPE (type));
        else
   abstract_virtuals_error (decl, type);

*************** create_array_type_for_decl (tree name, t
*** 6254,6259 ****
--- 6245,6255 ----
    if (size)
      itype = compute_array_index_type (name, size);

+   /* [dcl.array]
+      T is called the array element type; this type shall not be [...] an
+      abstract class type.  */
+   abstract_virtuals_error (name, type);
+
    return build_cplus_array_type (type, itype);
  }

*************** complete_vars (tree type)
*** 11051,11056 ****
--- 11047,11055 ----
        else
   list = &TREE_CHAIN (*list);
      }
+
+   /* Check for pending declarations which may have abstract type.  */
+   complete_type_check_abstract (type);
  }

  /* If DECL is of a type which needs a cleanup, build that cleanup
Index: class.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cp/class.c,v
retrieving revision 1.616
diff -c -3 -p -r1.616 class.c
*** class.c 31 May 2004 08:42:31 -0000 1.616
--- class.c 21 Jun 2004 10:32:51 -0000
*************** finish_struct (tree t, tree attributes)
*** 5260,5267 ****
--- 5260,5283 ----

    if (processing_template_decl)
      {
+       tree x;
+
        finish_struct_methods (t);
        TYPE_SIZE (t) = bitsize_zero_node;
+
+       /* We need to emit an error message if this type was used as a
parameter
+   and it is an abstract type, even if it is a template. We construct
+   a simple CLASSTYPE_PURE_VIRTUALS list without taking bases into
+   account and we call complete_vars with this type, which will check
+   the PARM_DECLS. Note that while the type is being defined,
+   CLASSTYPE_PURE_VIRTUALS contains the list of the inline friends
+   (see CLASSTYPE_INLINE_FRIENDS) so we need to clear it.  */
+       CLASSTYPE_PURE_VIRTUALS (t) = NULL_TREE;
+       for (x = TYPE_METHODS (t); x; x = TREE_CHAIN (x))
+  if (DECL_PURE_VIRTUAL_P (x))
+    CLASSTYPE_PURE_VIRTUALS (t)
+      = tree_cons (NULL_TREE, x, CLASSTYPE_PURE_VIRTUALS (t));
+       complete_vars (t);
      }
    else
      finish_struct_1 (t);



// { dg-do compile }
// Contributed by Gabriel Dos Reis <gdr at integrable-solutions dot net>
// PR c++/2204: Check for parameters of abstract type in function declarations.

namespace N1 {
  struct X;

  struct Y1 {
    void g(X parm1);         // { dg-error "abstract" }
    void g(X parm2[2]);      // { dg-error "abstract" }
    void g(X (*parm3)[2]);   // { dg-error "abstract" }
  };


  template <int N>
  struct Y2 {
    void g(X parm4);         // { dg-error "abstract" }
    void g(X parm5[2]);      // { dg-error "abstract" }
    void g(X (*parm6)[2]);   // { dg-error "abstract" }
  };

  struct X {  // { dg-error "note" }
    virtual void xfunc(void) = 0;  // { dg-error "note" }
  };
}

namespace N2 {

  struct X1 { // { dg-error "note" }
    virtual void xfunc(void) = 0;  // { dg-error "note" }
    void g(X1 parm7);        // { dg-error "abstract" }
    void g(X1 parm8[2]);     // { dg-error "abstract" }
    void g(X1 (*parm9)[2]);  // { dg-error "abstract" }
  };

  template <int N>
  struct X2 { // { dg-error "note" }
    virtual void xfunc(void) = 0; // { dg-error "note" }
    void g(X2 parm10);        // { dg-error "abstract" }
    void g(X2 parm11[2]);     // { dg-error "abstract" }
    void g(X2 (*parm12)[2]);  // { dg-error "abstract" }
  };
}

namespace N3 {
  struct X { // { dg-error "note" "" }
    virtual void xfunc(void) = 0;  // { dg-error "note" }
  };
  void g(X parm13);          // { dg-error "abstract" }
  void g(X parm14[2]);       // { dg-error "abstract" }
  void g(X (*parm15)[2]);    // { dg-error "abstract" }

  template <int N>
    void g(X parm16);          // { dg-error "abstract" }
  template <int N>
    void g(X parm17[2]);       // { dg-error "abstract" }
  template <int N>
    void g(X (*parm18)[2]);    // { dg-error "abstract" }
}