This is the mail archive of the
java-patches@gcc.gnu.org
mailing list for the Java project.
[gcjx] Patch: FYI: static accessor methods
- From: Tom Tromey <tromey at redhat dot com>
- To: Java Patch List <java-patches at gcc dot gnu dot org>
- Date: 11 Jul 2005 10:38:16 -0600
- Subject: [gcjx] Patch: FYI: static accessor methods
- Reply-to: tromey at redhat dot com
I'm checking this in on the gcjx branch.
The synthetic accessor methods generated to handle access to private
members must themselves be 'static'. Otherwise, you can run into
inheritance problems if members in subclasses have the same names.
This patch fixes the problem.
The test case looks like this:
class base
{
private int p() { return 5; }
class bi
{
public int val() { return p(); }
}
}
public class t extends base
{
private int p() { return 7; }
class ti extends bi
{
public int val() { return p() + super.val(); }
}
public static void main(String[] args)
{
t v1 = new t();
ti v2 = v1.new ti();
System.out.println(v2.val());
}
}
Tom
Index: ChangeLog
from Tom Tromey <tromey@redhat.com>
* bytecode/generate.hh (bytecode_generator::handle_invocation):
Updated.
* bytecode/generate.cc (handle_invocation): Added 'use_accessor'
argument.
(visit_method_invocation): Updated.
(visit_type_qualified_invocation): Likewise.
(visit_super_invocation): Likewise.
(visit_field_ref): Likewise.
(dereference_left_hand_side): Updated for static accessors.
(emit_lhs_store): Likewise.
(visit_field_ref): Likewise.
* model/class.cc (get_accessor): Always make accessor static.
(get_accessor): Likewise.
Index: bytecode/generate.cc
===================================================================
RCS file: /cvs/gcc/gcc/gcjx/bytecode/Attic/generate.cc,v
retrieving revision 1.1.2.13
diff -u -r1.1.2.13 generate.cc
--- bytecode/generate.cc 18 May 2005 00:21:34 -0000 1.1.2.13
+++ bytecode/generate.cc 11 Jul 2005 16:39:47 -0000
@@ -1782,15 +1782,16 @@
model_class *k = dynamic_cast<model_class *> (info->type);
model_method *access = k->get_accessor (info->field, false);
// First duplicate.
- if (! access->static_p ())
+ if (! info->field->static_p ())
{
emit (op_dup);
increase_stack (k);
}
// Now dereference.
- emit (access->static_p () ? op_invokestatic : op_invokevirtual);
+ assert (access->static_p ());
+ emit (op_invokestatic);
emit2 (cpool->add (k, access));
- if (! access->static_p ())
+ if (! info->field->static_p ())
reduce_stack (k);
increase_stack (access->get_return_type ());
}
@@ -1872,9 +1873,10 @@
{
model_class *k = dynamic_cast<model_class *> (info->type);
model_method *access = k->get_accessor (info->field, true);
- emit (access->static_p () ? op_invokestatic : op_invokevirtual);
+ assert (access->static_p ());
+ emit (op_invokestatic);
emit2 (cpool->add (k, access));
- if (! access->static_p ())
+ if (! info->field->static_p ())
reduce_stack (k);
reduce_stack (rhs_type);
}
@@ -2683,6 +2685,7 @@
// FIXME: remove this cast.
= accessed->get_accessor (const_cast<model_field *> (field),
expr_target == LEFT_HAND_SIDE);
+ assert (tm->static_p ());
if (expr_target == LEFT_HAND_SIDE)
{
@@ -2696,10 +2699,8 @@
else
{
std::list<ref_expression> nullargs;
- handle_invocation (tm->static_p ()
- ? op_invokestatic
- : op_invokevirtual,
- ref->get_qualifying_class (), tm, nullargs);
+ handle_invocation (op_invokestatic, ref->get_qualifying_class (),
+ tm, nullargs, false, true);
}
}
else if (expr_target == LEFT_HAND_SIDE)
@@ -3065,7 +3066,8 @@
model_class *qualifier,
const model_method *meth,
const std::list<ref_expression> &args,
- bool null_check_semantics)
+ bool null_check_semantics,
+ bool use_accessor)
{
{
assert (expr_target == IGNORE
@@ -3111,7 +3113,7 @@
i != args.end ();
++i)
reduce_stack ((*i)->type ());
- if (! meth->static_p ())
+ if (! meth->static_p () || use_accessor)
reduce_stack (meth->get_declaring_class ());
if (meth->get_return_type () != primitive_void_type)
increase_stack (meth->get_return_type ());
@@ -3141,9 +3143,13 @@
this_expr->visit (this);
}
- model_class *accessed;
+ model_class *accessed = NULL;
+ bool nonstatic_accessor = false;
if (trampoline_required_p (meth, method->get_declaring_class (), &accessed))
- meth = accessed->get_accessor (const_cast<model_method *> (meth));
+ {
+ nonstatic_accessor = ! meth->static_p ();
+ meth = accessed->get_accessor (const_cast<model_method *> (meth));
+ }
java_opcode opcode;
if (meth->static_p ())
@@ -3154,7 +3160,8 @@
opcode = op_invokespecial;
else
opcode = op_invokevirtual;
- handle_invocation (opcode, inv->get_qualifying_class (), meth, args);
+ handle_invocation (opcode, inv->get_qualifying_class (), meth, args,
+ false, nonstatic_accessor);
}
void
@@ -3164,16 +3171,21 @@
const std::list<ref_expression> &args,
bool super)
{
+ if (! meth->static_p ())
+ emit_load (meth->get_declaring_class (), this_index);
+
// FIXME: duplicate code.
- model_class *accessed;
+ model_class *accessed = NULL;
+ bool nonstatic_accessor = false;
if (trampoline_required_p (meth, method->get_declaring_class (), &accessed))
- meth = accessed->get_accessor (const_cast<model_method *> (meth));
+ {
+ nonstatic_accessor = ! meth->static_p ();
+ meth = accessed->get_accessor (const_cast<model_method *> (meth));
+ }
- if (! meth->static_p ())
- emit_load (meth->get_declaring_class (), this_index);
handle_invocation (super ? op_invokespecial : op_invokestatic,
inv->get_qualifying_class (),
- meth, args);
+ meth, args, false, nonstatic_accessor);
}
void
@@ -3184,12 +3196,17 @@
{
// FIXME: duplicate code.
model_class *accessed;
+ bool nonstatic_accessor = false;
if (trampoline_required_p (meth, method->get_declaring_class (), &accessed))
- meth = accessed->get_accessor (const_cast<model_method *> (meth));
+ {
+ nonstatic_accessor = ! meth->static_p ();
+ meth = accessed->get_accessor (const_cast<model_method *> (meth));
+ }
emit_load (method->get_declaring_class (), this_index);
handle_invocation (op_invokespecial, inv->get_qualifying_class (),
- meth, args, inv->get_expression () != NULL);
+ meth, args, inv->get_expression () != NULL,
+ nonstatic_accessor);
}
void
@@ -3201,7 +3218,7 @@
// Note: an accessor can never be needed here.
emit_load (method->get_declaring_class (), this_index);
handle_invocation (op_invokespecial, inv->get_qualifying_class (),
- meth, args, inv->get_expression () != NULL);
+ meth, args, false, inv->get_expression () != NULL);
}
void
Index: bytecode/generate.hh
===================================================================
RCS file: /cvs/gcc/gcc/gcjx/bytecode/Attic/generate.hh,v
retrieving revision 1.1.2.7
diff -u -r1.1.2.7 generate.hh
--- bytecode/generate.hh 20 Apr 2005 16:05:01 -0000 1.1.2.7
+++ bytecode/generate.hh 11 Jul 2005 16:39:47 -0000
@@ -263,7 +263,7 @@
model_class *,
const model_method *,
const std::list<ref_expression> &,
- bool = false);
+ bool = false, bool = false);
void handle_logical_binary (const ref_expression &,
const ref_expression &,
bool);
Index: model/class.cc
===================================================================
RCS file: /cvs/gcc/gcc/gcjx/model/Attic/class.cc,v
retrieving revision 1.1.2.11
diff -u -r1.1.2.11 class.cc
--- model/class.cc 17 Apr 2005 21:33:57 -0000 1.1.2.11
+++ model/class.cc 11 Jul 2005 16:39:48 -0000
@@ -2134,18 +2134,32 @@
{
ref_method meth = new model_method (fld->get_location (), this);
- modifier_t mods = 0;
- if (fld->static_p ())
- mods |= ACC_STATIC;
- meth->set_modifiers (mods);
+ // An accessor must always be static so that it can't be
+ // accidentally overridden by a subclass.
+ meth->set_modifiers (ACC_STATIC);
meth->set_synthetic ();
- ref_expression expr;
- {
- ref_field_ref fref = new model_field_ref (fld->get_location ());
- fref->set_field (fld);
- expr = fref;
- }
+ ref_field_ref fref = new model_field_ref (fld->get_location ());
+ fref->set_field (fld);
+
+ std::list<ref_variable_decl> args;
+ // If we are handling an instance field we must pass in an
+ // object reference.
+ if (! fld->static_p ())
+ {
+ // Note that we use 'this' and not the field's declaring
+ // class here. Using the declaring class could result in a
+ // violation of the 'protected' access rules.
+ ref_forwarding_type fldt
+ = new model_forwarding_resolved (fld->get_location (), this);
+ ref_variable_decl arg
+ = new model_parameter_decl (fld->get_location (), "ref",
+ fldt, this);
+ args.push_back (arg);
+
+ fref->set_expression (new model_simple_variable_ref (fld->get_location (),
+ arg.get ()));
+ }
std::list<ref_stmt> statements;
if (writing)
@@ -2163,12 +2177,10 @@
"arg",
fld->get_declared_type (),
this);
- std::list<ref_variable_decl> args;
args.push_back (arg);
- meth->set_parameters (args);
ref_assignment assign = new model_assignment (fld->get_location ());
- assign->set_lhs (expr);
+ assign->set_lhs (fref);
assign->set_rhs (new model_simple_variable_ref (fld->get_location (),
arg.get ()));
@@ -2179,10 +2191,12 @@
{
meth->set_return_type (fld->get_declared_type ());
- // The body is 'return EXPR'.
- statements.push_back (new model_return (fld->get_location (), expr));
+ // The body is 'return FREF'.
+ statements.push_back (new model_return (fld->get_location (), fref));
}
+ meth->set_parameters (args);
+
ref_block block = new model_block (fld->get_location (), statements);
meth->set_body (block);
@@ -2209,6 +2223,12 @@
// This is only used in the constructor case.
int added_args = 0;
+ // Formal and actual arguments to the new method.
+ std::list<ref_variable_decl> args;
+ std::list<ref_expression> actual;
+ // Object reference, if needed.
+ ref_variable_decl arg;
+
// Note that constructors are special since we can't create one
// with a new name. So, we are forced to add dummy arguments to
// differentiate our constructors from ones that the user might
@@ -2233,22 +2253,36 @@
while (has_method_with_descriptor_p (name, descriptor));
accm = new model_constructor (cons);
+ accm->set_modifiers (0);
}
else
- accm = new model_method (where, this);
+ {
+ accm = new model_method (where, this);
+ // An accessor must always be static so that it can't be
+ // accidentally overridden by a subclass.
+ accm->set_modifiers (ACC_STATIC);
+
+ if (! meth->static_p ())
+ {
+ // Note that we use 'this' and not the method's
+ // declaring class here. Using the declaring class
+ // could result in a violation of the 'protected' access
+ // rules.
+ ref_forwarding_type declt
+ = new model_forwarding_resolved (meth->get_location (), this);
+ arg = new model_parameter_decl (meth->get_location (), "ref",
+ declt, this);
+ args.push_back (arg);
+ }
+ }
- modifier_t mods = 0;
- if (meth->static_p ())
- mods |= ACC_STATIC;
- accm->set_modifiers (mods);
accm->set_synthetic ();
accm->set_return_type (new model_forwarding_resolved (where,
meth->get_return_type ()));
// Compute the new formal arguments and actual arguments to the
// forwarding method call we create.
- std::list<ref_variable_decl> args, old_args = meth->get_parameters ();
- std::list<ref_expression> actual;
+ std::list<ref_variable_decl> old_args = meth->get_parameters ();
for (std::list<ref_variable_decl>::const_iterator i = old_args.begin ();
i != old_args.end ();
++i)
@@ -2260,8 +2294,7 @@
other->get_declared_type (),
this);
args.push_back (arg);
- actual.push_back (new model_simple_variable_ref (where,
- arg.get ()));
+ actual.push_back (new model_simple_variable_ref (where, arg.get ()));
}
// For a constructor we added arguments, so add those to the
@@ -2310,7 +2343,14 @@
thi->set_enclosing_class (this);
}
else
- inv = new model_method_invocation (where);
+ {
+ inv = new model_method_invocation (where);
+
+ if (arg)
+ inv->set_expression (new model_simple_variable_ref (meth->get_location (),
+ arg.get ()));
+ }
+
inv->set_arguments (actual);
inv->set_method (meth->get_name ());