On 03/17/2015 11:39 AM, Alan Bateman wrote:
On 17/03/2015 09:57, Chris Hegarty wrote:
Peter, Alan,
After further thought I think that requiring all final fields to be
set is reasonable behaviour. Since there is no compile time checking,
this is a reasonable runtime effort to catch what is likely to be
developer errors. To address Alan’s comments, I beefed up the API
docs and added examples to typical usage.
Updated specdiff (and webrev):
http://cr.openjdk.java.net/~chegar/8071472/01/
<http://cr.openjdk.java.net/%7Echegar/8071472/01/>
This version also includes a number of changes to readObject
implementations in the base module, to replace unsafe usage with this
field setter API.
The rename to FieldSetter looks good, also good to have an example in
the javadoc.
It is good to catch cases where final fields aren't set but the issue
is the surprising behavior that the check is tied to whether the
FieldSetter has been obtained or not. The other thing is that
fieldSetter's javadoc doesn't make this clear, you need to read
FieldSetter's javadoc to learn about this enforcement. It's hard to
know what the right thing to do here as ideally this enforcement would
be enabled by default. If my class with final fields has an empty
readObject, or it has code paths that don't call defaultReadObject for
example, then it's a bug that I'd like to know about. I realize there
is potential compatibility, maybe performance, impact of doing this
but it does feel like something that I should be able to opt-in or get
for free rather than it being a side effect.
Hi Alan,
I agree that not calling defaultReadObject() from readObject() and
having a final field is potentially a bug. But need not be in case some
other means of setting final fields was used (Unsafe or reflection).
Some readObject() implementations in base module that Chris changed to
use new API fall into this category. We can't track those usages, so to
keep backwards compatibility, this checking has to be opt-in. Is there a
more elegant way to opt-in? A @CheckFinalsAssignment annotation on the
readObject() method?
Regards,
Peter
One other thing that I wonder about is the exception and whether it
might be better to bend InvalidObjectException instead.
-Alan
