thanks Joe,
Vicente
On 12/2/19 1:17 AM, Joe Darcy wrote:
Hi Vicente,
I took the liberty of adding the necessary directory-level jtreg
config files to enable the feature and updated the tests accordingly:
https://hg.openjdk.java.net/amber/amber/rev/c91826d62310
(The feature is enabled by default in for the langtools tests, but
disabled by default elsewhere.)
HTH,
-Joe
On 11/29/2019 3:12 PM, Vicente Romero wrote:
Hi Joe,
All the tests that have an explicit -source 14 are that way because
of, I think to remember, a bug in jtreg that doesn't expand the
${some.property} macro for those tests. I don't remember the details
though
Thanks,
Vicente
On 11/29/19 9:59 AM, Joe Darcy wrote:
Hi Vicente,
Please change all uses of
@compile --enable-preview -source 14
in jtreg tags to to
@compile --enable-preview -source ${jdk.version}
The former structure will spuriously fail when the JDK 14 -> 15
transition occurs.
Also, publishing delta-webrevs between iterations in additional to
full webrev would help track the incremental changes.
Thanks,
-Joe
On 11/28/2019 8:05 AM, Vicente Romero wrote:
Hi again,
Sorry but I realized that I forgot to remove some code on the
compiler side. The code removed is small, before we were issuing an
error if some serialization methods were declared as record
members. That section was removed from the spec. I have prepared
another iteration with this change at [1]
Thanks,
Vicente
[1]
http://cr.openjdk.java.net/~vromero/records.review/all_code/webrev.01/
On 11/27/19 11:37 PM, Vicente Romero wrote:
Hi,
Please review the code for the records feature at [1]. This webrev
includes all: APIs, runtime, compiler, serialization, javadoc, and
more! Must of the code has been reviewed but there have been some
changes since reviewers saw it. Also this is the first time an
integral webrev is sent out for review. Last changes on top of my
mind since last review iterations:
On the compiler implementation:
- it has been adapted to the last version of the language spec
[2], as a reference the JVM spec is at [3]. This implied some
changes in determining if a user defined constructor is the
canonical or not. Now if a constructor is override-equivalent to a
signature derived from the record components, then it is
considered the canonical constructor. And any canonical
constructor should satisfy a set of restrictions, see section
8.10.4 Record Constructor Declarations of the specification.
- It was also added a check to make sure that accessors are not
generic.
- And that the canonical constructor, if user defined, is not
explicitly invoking any other constructor.
- The list of forbidden record component names has also been updated.
- new error messages have been added
APIs:
- there have been some API editing in java.lang.Record,
java.lang.runtime.ObjectMethods and
java.lang.reflect.RecordComponent, java.io.ObjectInputStream,
javax.lang.model (some visitors were added)
On the JVM implementation:
- some logging capabilities have been added to classFileParser.cpp
to provide the reason for which the Record attribute has been ignored
Reflection:
- there are several new changes to the implementation of
java.lang.reflect.RecordComponent apart from the spec changes
mentioned before.
bug fixes in
- compiler
- serialization,
- JVM, etc
As a reference the last iteration of the previous reviews can be
found at [4] under folders: compiler, hotspot_runtime, javadoc,
reflection and serialization,
TIA,
Vicente
[1]
http://cr.openjdk.java.net/~vromero/records.review/all_code/webrev.00/
[2]
http://cr.openjdk.java.net/~gbierman/jep359/jep359-20191125/specs/records-jls.html
[3]
http://cr.openjdk.java.net/~gbierman/jep359/jep359-20191125/specs/records-jvms.html
[4] http://cr.openjdk.java.net/~vromero/records.review/
