Fwd: Re: RFR: JEP 359-Records: compiler code

[Vicente Romero]

Hi all,

Thanks for all the feedback so far. I have published another iteration 
at [1]. New in this iteration:

Flags.java:

- I have sync the javadoc and the comment on the RECORD flag
- renamed MemberRecordClassFlags -> MemberRecordFlags

com.sun.tools.javac.code.Kinds.java:
- in order to generate better error messages I have added two new kinds: 
RECORD_COMPONENT and RECORD

Symbols.java
- moved method isRecord to ClassSymbol for the rest of the symbols moved 
back to using the bitwise test
- removed method ::isDynamic from MethodSymbol

Attr.java
- implemented a spec change to relax that constraint that every 
non-canonical constructor should call the canonical constructor. Now the 
spec forces every non-canonical constructor to invoke a different 
constructor as its first statement. This actually implied removing some 
code.

Check.java
- at method ::duplicateError, there is new code to avoid generating two 
errors when the user declares a record with duplicated record component 
names. If this happens, then a compiler generated canonical constructor 
would have duplicated parameters. This code is preventing the compiler 
to generate a second error referring to a method that is not visible to 
the user. Another option is to not generate the canonical constructor at 
all if there are duplicate record components
- at method ::checkUnique, there is new code to improve the error 
message when the user defines a constructor with the same erasure as the 
canonical constructor. Still the error message is not perfect because it 
shows in both cases the erasure of the methods, so they seem to be the 
same. But this is not related to records code, that's a simplification 
that happens at the moment of reporting the error.

TypeEnter
- removed some commented code
- ::addRecordMembersIfNeeded here I added new code to avoid generating 
accessor methods for forbidden record component names. This is to avoid 
spurious errors later as any way the compiler will complain because of 
the forbidden record component names. The issue with this solution is 
that the list of forbidden record component names has been duplicated 
here. The other location is Attr. We can probably find a common place to 
define it. Probably a new utility class?

JavacParser:
- fixed the bug that the compiler was not allowing final local records
- removed the check for abstract modifiers in records, this code was 
redundant as Check was already checking this
- removed the check for duplicated record component names
- added a check to forbid instance initializers in records

compiler.properties
- I hope I got the error messages right, I tried to reflect all the 
comments, suggestions, etc. I was proposed to create a `master` message 
for canonical records and use different `causes`, aka fragments, to be 
more specific. I decided to follow the same pattern for accessor 
methods. Regarding the suggestion of placing the caret in the throws 
clause for the corresponding error message, not possible. We would have 
to move the check for that error to the parser.

New in this iteration:
- I have included jdeps related code, I was thinking about putting it in 
a separate review bucket but, it is small, it kind of fits with the rest 
of the review
- a series of supporting libraries for tests under: 
test/langtools/lib/combo/tools/javac/combo with also the tests that 
needed to be modified due to changes in the lib. These changes were 
created by Brian
- a minor change done at ToolBox.java

Thanks for all the feedback,
Vicente


[1] http://cr.openjdk.java.net/~vromero/records.review/compiler/webrev.03/