[gcjx] Patch: FYI: make imports lazy
Tom Tromey
tromey@redhat.com
Tue Oct 25 19:29:00 GMT 2005
I'm checking this in on the gcjx branch.
The recent javax.imageio changes in Classpath triggered a bug in gcjx,
noticed by the Classpath automated regression tester.
The problem can most easily be seen with these files:
package p1;
public class base
{
public static class base_inner extends other_base
{
}
}
package p1;
import p1.base.base_inner;
public class other_base
{
public Object doit()
{
return new base_inner();
}
}
If you compile these with 'gcjx p1/other_base.java p1/base.java'
(order matters), you will get a circularity error.
The bug here is that we don't need to resolve the import in the
second file until we're actually resolving the body of 'doit'.
This patch changes the resolution of import statements to be lazy. We
still resolve them all, in order to do the required checks, but we
only do this after resolving the class body.
Tom
Index: ChangeLog
from Tom Tromey <tromey@redhat.com>
* model/unit.hh (model_unit_source::simple_name_map): New field.
(model_unit_source::name_map): Removed.
(model_unit_source::resolving): New field.
(model_unit_source): Initialize it.
(model_unit_source::resolve): Declare.
* model/import.cc (check_referenced): Removed old comment.
* model/unit.cc (check_imports): Removed old comment.
(resolve): Initialize simple_name_map. Don't resolve imports.
(check_imports): Resolve all imports here.
(look_up_name): Lazily resolve imports.
(resolve): New overload.
Index: model/import.cc
===================================================================
RCS file: /cvs/gcc/gcc/gcjx/model/Attic/import.cc,v
retrieving revision 1.1.2.3
diff -u -r1.1.2.3 import.cc
--- model/import.cc 11 Oct 2005 05:36:13 -0000 1.1.2.3
+++ model/import.cc 25 Oct 2005 19:21:53 -0000
@@ -34,7 +34,6 @@
model_import::check_referenced ()
{
if (! used && global->get_compiler ()->warn_unused_import ())
- // FIXME: the text of the import...?
std::cerr << warn (global->get_compiler ()->warn_unused_import (),
"unused %<import%> statement");
}
Index: model/unit.cc
===================================================================
RCS file: /cvs/gcc/gcc/gcjx/model/Attic/unit.cc,v
retrieving revision 1.1.2.3
diff -u -r1.1.2.3 unit.cc
--- model/unit.cc 11 Oct 2005 05:36:13 -0000 1.1.2.3
+++ model/unit.cc 25 Oct 2005 19:21:53 -0000
@@ -58,56 +58,30 @@
{
if (resolved)
return;
- resolution_scope::push_iscope self_holder (scope, this);
- // Note that we compute the name map here, then later assign it to
- // the corresponding field. This is done in case we have a
- // situation like this, from Jacks:
- // import java.util.Vector;
- // import Vector.Mosquito;
- // In this case, the simple name of the first import is the package
- // name of the second import -- but the first import should not be
- // in scope when resolving the second import.
- name_map_type local_name_map;
-
- // First resolve the imports.
+ // Initialize the simple name map. We will resolve elements from
+ // this map on demand.
for (std::list<ref_import>::const_iterator i = imports.begin ();
i != imports.end ();
++i)
{
- (*i)->resolve (scope);
- // This will be NULL if the import doesn't resolve to a single
- // class.
- model_class *k = (*i)->get_class_declaration ();
- if (k != NULL && accessible_p (k, package))
- check_dups (k->get_name (), (*i).get (), k, local_name_map);
- }
-
- // Now check for duplicates among the types.
- model_class *pub = NULL;
- for (std::list<ref_class>::const_iterator i = types.begin ();
- i != types.end ();
- ++i)
- {
- std::string sname = (*i)->get_name ();
- check_dups (sname, NULL, (*i).get (), local_name_map);
- if (((*i)->get_modifiers () & ACC_PUBLIC) != 0)
- {
- if (pub)
- std::cerr << (*i)->error ("%1 cannot be public in a compilation "
- "unit already containing "
- "public class %2")
- % (*i)->get_pretty_name () % pub->get_pretty_name ();
- else
- pub = (*i).get ();
- }
+ if ((*i)->single_import_p ())
+ simple_name_map.insert (std::make_pair ((*i)->get_simple_name (),
+ (*i).get ()));
}
- name_map = local_name_map;
resolved = true;
}
void
+model_unit_source::resolve (resolution_scope *scope, model_import *imp)
+{
+ resolving = true;
+ imp->resolve (scope);
+ resolving = false;
+}
+
+void
model_unit_source::look_up_name (const std::string &name,
std::set<model_class *> &result,
IContext *context,
@@ -115,18 +89,35 @@
{
// Check single-type-import declarations, static single-type-import
// declarations that resolve to a member type, and types defined in
- // this compilation unit. Note that if we haven't been resolved,
- // this won't find anything.
- name_map_type::const_iterator it = name_map.find (name);
- if (it != name_map.end ())
- {
- assert (resolved);
- if (accessible_p ((*it).second.second, context))
+ // this compilation unit. However, we only want to do if we've been
+ // resolved. If RESOLVING is set then we are resolving an import
+ // statement; in this case we don't want to use this code as it
+ // would lead to infinite recursion, and anyway other imports should
+ // not be in scope.
+ if (resolved && ! resolving)
+ {
+ // We lazily resolve the imports, and for that we need a scope.
+ // The scope doesn't need to know anything other than its
+ // compilation unit.
+ resolution_scope scope;
+ push_on_scope (&scope);
+
+ for (std::multimap<std::string, model_import *>::const_iterator i
+ = simple_name_map.begin ();
+ i != simple_name_map.end ();
+ ++i)
{
- if ((*it).second.first)
- (*it).second.first->set_used ();
- result.insert ((*it).second.second);
- return;
+ if (name != (*i).first)
+ continue;
+ model_import *imp = (*i).second;
+ resolve (&scope, imp);
+ model_class *klass = imp->get_class_declaration ();
+ if (accessible_p (klass, context))
+ {
+ imp->set_used ();
+ result.insert (klass);
+ return;
+ }
}
}
@@ -142,15 +133,22 @@
// If we haven't been resolved yet, we won't find anything by
// asking the imports, since they won't have been hooked up.
- if (! resolved)
+ if (! resolved || resolving)
return;
+ // We lazily resolve the imports, and for that we need a scope. The
+ // scope doesn't need to know anything other than its compilation
+ // unit.
+ resolution_scope scope;
+ push_on_scope (&scope);
+
// Finally look at on-demand imports.
model_import *result_import = NULL;
for (std::list<ref_import>::const_iterator i = imports.begin ();
i != imports.end ();
++i)
{
+ resolve (&scope, (*i).get ());
if (! (*i)->single_import_p ())
{
// FIXME: imports should probably cache negative results.
@@ -228,13 +226,51 @@
void
model_unit_source::check_imports ()
{
+ resolution_scope scope;
+ push_on_scope (&scope);
+
+ name_map_type local_name_map;
+
+ // Resolve the imports.
+ for (std::list<ref_import>::const_iterator i = imports.begin ();
+ i != imports.end ();
+ ++i)
+ {
+ resolve (&scope, (*i).get ());
+ // This will be NULL if the import doesn't resolve to a single
+ // class.
+ model_class *k = (*i)->get_class_declaration ();
+ if (k != NULL && accessible_p (k, package))
+ check_dups (k->get_name (), (*i).get (), k, local_name_map);
+ }
+
+ // Check for duplicates among the types.
+ model_class *pub = NULL;
+ for (std::list<ref_class>::const_iterator i = types.begin ();
+ i != types.end ();
+ ++i)
+ {
+ std::string sname = (*i)->get_name ();
+ check_dups (sname, NULL, (*i).get (), local_name_map);
+ if (((*i)->get_modifiers () & ACC_PUBLIC) != 0)
+ {
+ if (pub)
+ std::cerr << (*i)->error ("%1 cannot be public in a compilation "
+ "unit already containing "
+ "public class %2")
+ % (*i)->get_pretty_name () % pub->get_pretty_name ();
+ else
+ pub = (*i).get ();
+ }
+ }
+
+ // Check to see if each import is used.
if (! global->get_compiler ()->warn_unused_import ())
return;
for (std::list<ref_import>::const_iterator i = imports.begin ();
i != imports.end ();
++i)
- // FIXME: should pass warning flag here, it could be an error...
(*i)->check_referenced ();
}
Index: model/unit.hh
===================================================================
RCS file: /cvs/gcc/gcc/gcjx/model/Attic/unit.hh,v
retrieving revision 1.1.2.3
diff -u -r1.1.2.3 unit.hh
--- model/unit.hh 11 Oct 2005 05:36:13 -0000 1.1.2.3
+++ model/unit.hh 25 Oct 2005 19:21:53 -0000
@@ -140,21 +140,28 @@
// Imports.
std::list<ref_import> imports;
+ // Map simple names to imports. Imports are resolved lazily;
+ // resolving them eagerly can result in circularity errors where
+ // none are needed. Note that duplicate checking of simple names is
+ // not done until relatively late.
+ std::multimap<std::string, model_import *> simple_name_map;
+
+ // We set this when resolving an import, so that we don't recurse.
+ bool resolving;
+
typedef std::map<std::string, std::pair<model_import *, model_class *> >
name_map_type;
- // Map simple names to types. This is constructed during
- // resolution.
- name_map_type name_map;
-
- // A helper function when resolving.
+ // Helper function for resolving.
void check_dups (const std::string &, model_import *,
model_class *, name_map_type &);
+ void resolve (resolution_scope *, model_import *);
public:
model_unit_source (const location &w)
- : model_unit (w)
+ : model_unit (w),
+ resolving (false)
{
}
More information about the Java-patches
mailing list