Re: Patch: PR java/20031 - ICE on missing files

[Andrew Haley <aph at redhat dot com>]

Ranjit Mathew writes:
 > -----BEGIN PGP SIGNED MESSAGE-----
 > Hash: SHA1
 > 
 > Hi,
 > 
 >   This patch fixes both the problems reported in PR java/20031,
 > which was a 4.0/4.1 regression.
 > 
 > It is a follow-up to the discussion in:
 > 
 >   http://gcc.gnu.org/ml/java/2005-09/msg00051.html
 > 
 > Tested on i686-pc-linux-gnu, Jacks included, and introduces
 > no new failures.
 > 
 > The contentious bit would probably be my proposal to use
 > TYPE_DUMMY on a RECORD_TYPE node to indicate a failed
 > loading of the class, even in the non-indirect-dispatch
 > case when VERBOSE is true and to use this to not bother
 > to lay out a class's methods if it failed loading.
 > 
 > As I understand it, VERBOSE is set to 1 for load_class()
 > when we expect the class to be found and consider it an
 > error if it is not. In this case, if the class failed
 > loading, there does not seem to be any hope of finding it
 > later and therefore it is better to "poison" the RECORD_TYPE
 > denoting the class. I would like to know if this assumption
 > is invalid.
 > 
 > I noticed that the "if (!class_loaded)" loop did anything
 > useful only when VERBOSE is set, so I simplified the conditional
 > statements a bit. The diff output might be a bit unreadable,
 > so here is the new structure:
 > - --------------------------- 8< ---------------------------
 >   if (!class_loaded && verbose)
 >     {
 >       if (flag_verify_invocations || ! flag_indirect_dispatch
 >           || flag_emit_class_files)
 >         {
 >           error ("cannot find file for class %s", IDENTIFIER_POINTER (saved));
 >         }
 >       else if (!quiet_flag)
 >         /* This is just a diagnostic during testing, not a real problem.  */
 >         warning (0, "cannot find file for class %s",
 >                  IDENTIFIER_POINTER (saved));
 > 
 >       /* Fake it.  */
 >       if (TREE_CODE (class_or_name) == RECORD_TYPE)
 >         {
 >           set_super_info (0, class_or_name, object_type_node, 0);
 >           TYPE_DUMMY (class_or_name) = 1;
 >           /* We won't be able to output any debug info for this class.  */
 >           DECL_IGNORED_P (TYPE_NAME (class_or_name)) = 1;
 >         }
 >     }
 > - --------------------------- 8< ---------------------------
 > 
 > OK for mainline?

I can't say it fills me with happiness.  Is there really no better way
to do this than to (ab)use TYPE_DUMMY ?  OK, if you really want to do
this go ahead, but at least add some commentary to the effect that
when compiling without indirect dispatch this really is a fatal error,
but we're continuing in case we need to generate further disgnostics.

And in particular, this line:

 > +          if (!TYPE_DUMMY (cls))

really does need a comment.

 > +	    layout_class_methods (cls);

Andrew.