PR java/21436: imported type name for superclass, with multifile compilation

Ranjit Mathew rmathew@gmail.com
Tue May 10 09:18:00 GMT 2005


Paolo Bonzini wrote:
> The patch fully implements section 6.5.5.1, item 4, of the Java Language 
> Specification (thanks to Andrew Haley for the pointer): "If a type with 
> that name is declared in the current compilation unit [by] a 
> single-type-import declaration [...], then the simple type name denotes 
> that type".  If multifile compilation is active, the subclass is 
> compiled but the superclass is not, and the subclass specifies the 
> superclass with a simple type name, then gcj will not be able to find 
> the superclass.

Hi Paolo,

  I have a couple of quick comments:

1. Nicely explained. Kudos!

2. Did you test with Jacks (http://sources.redhat.com/mauve)
included in the tree as explained in
http://gcc.gnu.org/install/test.html? Any Java front-end change
needs to be tested against Jacks.

Thanks,
Ranjit.

> 
> The simplest failure mode was that of PR21436, but several others are 
> exposed by my patch to compile libjava one directory at a time.  In 
> order to fix them, several actions are needed:
> 
> 1) the first and most important is to really implement section 6.5.5.1/4 
> in do_resolve_class, find_in_imports and find_in_imports_demand.
> 
> 2) quite surprisingly for me, the compilation units are processed in 
> reverse order during pass 2 and 3.  The problem is that, if both class
> X and class Y are on the command line (like gcj "X.java Y.java"), and X 
> extends Y, read_class will do a java_pop_parser_context for Y before the 
> parsing of X is finished: so, unless classes are tacked to the *tail* of 
> the ctxp_for_generation linked list, subsequent passes through 
> ctxp_for_generation will process X before Y.
> 
> 3) the TYPE_IMPORT_LIST and TYPE_IMPORT_DEMAND_LIST are currently set at 
> the beginning of phase 2.  The problem is that phase 2 is done *before* 
> completing the parsing of the files on the command line, if gcj needs to 
> shuffle the order of files given on the command line.  So, take this 
> hierarchy
> 
> 
>    _______________
>   |               |
>   |   JComponent  |
>   |_______________|
>           |
>    _______V_______
>   |               |
>   |    JButton    |
>   |_______________|
>           |
>    _______V_______
>   |               |
>   | JToggleButton |
>   |_______________|
> 
> and a command line like "gcj JComponent.java JToggleButton.java 
> JButton.java".  Now gcj executes:
>    - phase 1 for JToggleButton
>    - phase 1 for JButton
>      - phase 1 for JButton
>      - phase 2 for JButton (**)
>      - phase 3 for JButton (**)
>      - then, phase 1 is finished for JButton
>    - phase 2 for JComponent (++)
>    - phase 3 for JComponent
>    - phase 2 for JToggleButton
>    - phase 3 for JToggleButton
> 
> and the phases marked (**) won't see the TYPE_IMPORT_LIST and 
> TYPE_IMPORT_DEMAND_LIST of JComponent, which are only set at time (++).
> 
> All these fixes are necessary.  PR21236 works if you apply only fix #2 
> and part of fix #1 (only the change to find_in_imports), or fixes #1 and 
> #3.  PR21242 on the other hand needs the complete fix for #1, and #3 
> too.  The case I just described with Swing classes, finally, requires 
> all the fixes (it is also the only one to involve three files on the 
> command line, rather than just two).
> 
> Tested with my Makefile patch and without it on mainline, bootstrapping 
> C/C++/Java and regtesting Java only (no new failures).  Ok?  I won't 
> propose this for 4.0, but it could reasonably go there after it's been 
> on mainline for a while.
> 
> Paolo
> 
> 
> 
> ------------------------------------------------------------------------
> 
> 2005-05-05  Paolo Bonzini  <bonzini@gnu.org>
> 
> 	PR java/21436
> 
> 	* class.c (maybe_layout_super_class): Look for imports in this_class.
> 	* parse.h (ctxp_for_generation_last): New.
> 	(do_resolve_class): Add a parameter.
> 	* parse.y (ctxp_for_generation_last): New.
> 	(java_pop_parser_context): Add at end of list.
> 	(find_in_imports, find_in_imports_on_demand): Look in ctxp
> 	if the TYPE_IMPORT_LIST or respectively the TYPE_IMPORT_DEMAND_LIST of
> 	the given type are NULL.
> 	(do_resolve_class): Look into the imports of the new second parameter.
> 	Adjust recursive calls.
> 	(resolve_class, resolve_inner_class, find_as_inner_class): Adjust
> 	calls to do_resolve_class.
> 	(create_class): Set the TYPE_IMPORT_LIST and TYPE_IMPORT_DEMAND_LIST.
> 	(java_complete_class): Do not do that here.
> 
> diff -rup gcc-40/gcc/gcc/java/class.c gcc/gcc/java/class.c
> --- gcc-40/gcc/gcc/java/class.c	2005-05-02 12:29:45.000000000 +0200
> +++ gcc/gcc/java/class.c	2005-05-09 11:54:30.000000000 +0200
> @@ -2085,7 +2085,7 @@ maybe_layout_super_class (tree super_cla
>  					  DECL_SOURCE_LINE (this_decl), 0);
>  #endif
>  	    }
> -	  super_class = do_resolve_class (NULL_TREE, /* FIXME? */
> +	  super_class = do_resolve_class (NULL_TREE, this_class,
>  					  super_class, NULL_TREE, this_wrap);
>  	  if (!super_class)
>  	    return NULL_TREE;	/* FIXME, NULL_TREE not checked by caller. */
> diff -rup gcc-40/gcc/gcc/java/parse.h gcc/gcc/java/parse.h
> --- gcc-40/gcc/gcc/java/parse.h	2005-03-06 13:57:57.000000000 +0100
> +++ gcc/gcc/java/parse.h	2005-05-09 12:10:44.000000000 +0200
> @@ -937,7 +937,7 @@ void java_layout_classes (void);
>  void java_reorder_fields (void);
>  tree java_method_add_stmt (tree, tree);
>  int java_report_errors (void);
> -extern tree do_resolve_class (tree, tree, tree, tree);
> +extern tree do_resolve_class (tree, tree, tree, tree, tree);
>  #endif
>  char *java_get_line_col (const char *, int, int);
>  extern void reset_report (void);
> @@ -960,5 +960,6 @@ extern void java_finish_classes (void);
>  
>  extern GTY(()) struct parser_ctxt *ctxp;
>  extern GTY(()) struct parser_ctxt *ctxp_for_generation;
> +extern GTY(()) struct parser_ctxt *ctxp_for_generation_last;
>  
>  #endif /* ! GCC_JAVA_PARSE_H */
> diff -rup gcc-40/gcc/gcc/java/parse.y gcc/gcc/java/parse.y
> --- gcc-40/gcc/gcc/java/parse.y	2005-04-27 13:01:15.000000000 +0200
> +++ gcc/gcc/java/parse.y	2005-05-09 18:18:42.000000000 +0200
> @@ -361,6 +361,7 @@ struct parser_ctxt *ctxp;
>  
>  /* List of things that were analyzed for which code will be generated */
>  struct parser_ctxt *ctxp_for_generation = NULL;
> +struct parser_ctxt *ctxp_for_generation_last = NULL;
>  
>  /* binop_lookup maps token to tree_code. It is used where binary
>     operations are involved and required by the parser. RDIV_EXPR
> @@ -2765,8 +2766,12 @@ java_pop_parser_context (int generate)
>       do is to just update a list of class names.  */
>    if (generate)
>      {
> -      ctxp->next = ctxp_for_generation;
> -      ctxp_for_generation = ctxp;
> +      if (ctxp_for_generation_last == NULL)
> +        ctxp_for_generation = ctxp;
> +      else
> +        ctxp_for_generation_last->next = ctxp;
> +      ctxp->next = NULL;
> +      ctxp_for_generation_last = ctxp;
>      }
>  
>    /* And restore those of the previous context */
> @@ -3706,7 +3711,7 @@ resolve_inner_class (htab_t circularity_
>          break;
>  
>        if (TREE_CODE (local_super) == POINTER_TYPE)
> -        local_super = do_resolve_class (NULL, local_super, NULL, NULL);
> +        local_super = do_resolve_class (NULL, NULL, local_super, NULL, NULL);
>        else
>  	local_super = TYPE_NAME (local_super);
>  
> @@ -3768,7 +3773,7 @@ find_as_inner_class (tree enclosing, tre
>  	  acc = merge_qualified_name (acc,
>  				      EXPR_WFL_NODE (TREE_PURPOSE (qual)));
>  	  BUILD_PTR_FROM_NAME (ptr, acc);
> -	  decl = do_resolve_class (NULL_TREE, ptr, NULL_TREE, cl);
> +	  decl = do_resolve_class (NULL_TREE, NULL_TREE, ptr, NULL_TREE, cl);
>  	}
>  
>        /* A NULL qual and a decl means that the search ended
> @@ -4177,6 +4182,12 @@ create_class (int flags, tree id, tree s
>       virtual function table in java.lang.object.  */
>    TYPE_VFIELD (TREE_TYPE (decl)) = TYPE_VFIELD (object_type_node);
>  
> +  /* We keep the compilation unit imports in the class so that
> +     they can be used later to resolve type dependencies that
> +     aren't necessary to solve now. */
> +  TYPE_IMPORT_LIST (TREE_TYPE (decl)) = ctxp->import_list;
> +  TYPE_IMPORT_DEMAND_LIST (TREE_TYPE (decl)) = ctxp->import_demand_list;
> +
>    /* Add the private this$<n> field, Replicate final locals still in
>       scope as private final fields mangled like val$<local_name>.
>       This does not occur for top level (static) inner classes. */
> @@ -5683,12 +5694,6 @@ java_complete_class (void)
>      {
>        jdep *dep;
>  
> -      /* We keep the compilation unit imports in the class so that
> -	 they can be used later to resolve type dependencies that
> -	 aren't necessary to solve now. */
> -      TYPE_IMPORT_LIST (TREE_TYPE (cclass)) = ctxp->import_list;
> -      TYPE_IMPORT_DEMAND_LIST (TREE_TYPE (cclass)) = ctxp->import_demand_list;
> -
>        for (dep = CLASSD_FIRST (cclassd); dep; dep = JDEP_CHAIN (dep))
>  	{
>  	  tree decl;
> @@ -5839,7 +5844,7 @@ resolve_class (tree enclosing, tree clas
>      WFL_STRIP_BRACKET (cl, cl);
>  
>    /* 2- Resolve the bare type */
> -  if (!(resolved_type_decl = do_resolve_class (enclosing, class_type,
> +  if (!(resolved_type_decl = do_resolve_class (enclosing, NULL_TREE, class_type,
>  					       decl, cl)))
>      return NULL_TREE;
>    resolved_type = TREE_TYPE (resolved_type_decl);
> @@ -5862,7 +5867,8 @@ resolve_class (tree enclosing, tree clas
>     and (but it doesn't really matter) qualify_and_find.  */
>  
>  tree
> -do_resolve_class (tree enclosing, tree class_type, tree decl, tree cl)
> +do_resolve_class (tree enclosing, tree import_type, tree class_type, tree decl,
> +		  tree cl)
>  {
>    tree new_class_decl = NULL_TREE, super = NULL_TREE;
>    tree saved_enclosing_type = enclosing ? TREE_TYPE (enclosing) : NULL_TREE;
> @@ -5879,7 +5885,7 @@ do_resolve_class (tree enclosing, tree c
>        if (split_qualified_name (&left, &right, TYPE_NAME (class_type)) == 0)
>  	{
>  	  BUILD_PTR_FROM_NAME (left_type, left);
> -	  q = do_resolve_class (enclosing, left_type, decl, cl);
> +	  q = do_resolve_class (enclosing, import_type, left_type, decl, cl);
>  	  if (q)
>  	    {
>  	      enclosing = q;
> @@ -5924,8 +5930,11 @@ do_resolve_class (tree enclosing, tree c
>  	return new_class_decl;
>      }
>  
> -  /* 1- Check for the type in single imports. This will change
> -     TYPE_NAME() if something relevant is found */
> +  /* 1- Check for the type in single imports.  Look at enclosing classes and,
> +     if we're laying out a superclass, at the import list for the subclass.
> +     This will change TYPE_NAME() if something relevant is found. */
> +  if (import_type && TYPE_IMPORT_LIST (import_type))
> +    find_in_imports (import_type, class_type);
>    find_in_imports (saved_enclosing_type, class_type);
>  
>    /* 2- And check for the type in the current compilation unit */
> @@ -5947,8 +5956,14 @@ do_resolve_class (tree enclosing, tree c
>    /* 4- Check the import on demands. Don't allow bar.baz to be
>       imported from foo.* */
>    if (!QUALIFIED_P (TYPE_NAME (class_type)))
> -    if (find_in_imports_on_demand (saved_enclosing_type, class_type))
> -      return NULL_TREE;
> +    {
> +      if (import_type
> +	  && TYPE_IMPORT_DEMAND_LIST (import_type)
> +	  && find_in_imports_on_demand (import_type, class_type))
> +        return NULL_TREE;
> +      if (find_in_imports_on_demand (saved_enclosing_type, class_type))
> +        return NULL_TREE;
> +    }
>  
>    /* If found in find_in_imports_on_demand, the type has already been
>       loaded. */
> @@ -6970,8 +6985,12 @@ process_imports (void)
>  static void
>  find_in_imports (tree enclosing_type, tree class_type)
>  {
> -  tree import = (enclosing_type ? TYPE_IMPORT_LIST (enclosing_type) :
> -		 ctxp->import_list);
> +  tree import;
> +  if (enclosing_type && TYPE_IMPORT_LIST (enclosing_type))
> +    import = TYPE_IMPORT_LIST (enclosing_type);
> +  else
> +    import = ctxp->import_list;
> +
>    while (import)
>      {
>        if (TREE_VALUE (import) == TYPE_NAME (class_type))
> @@ -7129,12 +7148,16 @@ static int
>  find_in_imports_on_demand (tree enclosing_type, tree class_type)
>  {
>    tree class_type_name = TYPE_NAME (class_type);
> -  tree import = (enclosing_type ? TYPE_IMPORT_DEMAND_LIST (enclosing_type) :
> -		  ctxp->import_demand_list);
>    tree cl = NULL_TREE;
>    int seen_once = -1;	/* -1 when not set, 1 if seen once, >1 otherwise. */
>    int to_return = -1;	/* -1 when not set, 0 or 1 otherwise */
>    tree node;
> +  tree import;
> +
> +  if (enclosing_type && TYPE_IMPORT_DEMAND_LIST (enclosing_type))
> +    import = TYPE_IMPORT_DEMAND_LIST (enclosing_type);
> +  else
> +    import = ctxp->import_demand_list;
>  
>    for (; import; import = TREE_CHAIN (import))
>      {


-- 
Ranjit Mathew      Email: rmathew AT gmail DOT com

Bangalore, INDIA.    Web: http://ranjitmathew.hostingzero.com/



More information about the Java-patches mailing list