RFR: 8273408: java.lang.AssertionError: typeSig ERROR on generated class property of record

[Guoxiong Li]

On Wed, 15 Sep 2021 02:34:10 GMT, Vicente Romero <vromero at openjdk.org> wrote:

> could you please add a link to where this is done in the compiler for other cases different from records? thanks

The compiler would [parse the newly generated source files](https://github.com/openjdk/jdk/blob/febcc72a549e973de4649503fc686fc520e3b3cd/src/jdk.compiler/share/classes/com/sun/tools/javac/processing/JavacProcessingEnvironment.java#L1393)  and [enter all the source files again](https://github.com/openjdk/jdk/blob/febcc72a549e973de4649503fc686fc520e3b3cd/src/jdk.compiler/share/classes/com/sun/tools/javac/processing/JavacProcessingEnvironment.java#L1133). Many dummy types or symbols, such as ErrorType, would be replaced with the actually right ones.

> But when the feature record is added, the convention is broken. 

This description may be excessive. Actually, new features always obey the convention, so as record. But the method `getRecordComponent` in this bug is an example which obscures the convention.

In detail, please read the following source code, currently in the main line.


// Symbol.java
        public RecordComponent getRecordComponent(JCVariableDecl var, boolean addIfMissing, List<JCAnnotation> annotations) {
            for (RecordComponent rc : recordComponents) {
                /* it could be that a record erroneously declares two record components with the same name, in that
                 * case we need to use the position to disambiguate
                 */
                if (rc.name == var.name && var.pos == rc.pos) {
                    return rc;
                }
            }
            RecordComponent rc = null;
            if (addIfMissing) {
                recordComponents = recordComponents.append(rc = new RecordComponent(var.sym, annotations));
            }
            return rc;
        }


Before the first processing round, the `var` is not right at all, because `var.sym.type` is `ErrorType`. So the `getRecordComponent` would generate a not good `RecordComponent` by using the wrong `var.sym` whose type is `ErrorType`.

After the first processing round, the new source files are created and we can catch the right type from the newly generated source files. When the compiler invokes `getRecordComponent` to get the `RecordComponent`, it only can get the wrong one(the type is `ErrorType`). At this time, the `var.sym.type` is the right `ClassType` which is created correctly earier. So we should use current argument `var` and the right `var.sym.type` instead of the old wrong one.

So you can see my patch, I judge whether the type is right. If it is right, which means the `RecordComponent` created earier is right, I reuse it. If the type is `ErrorType`, we shouldn't keep it and should create a new one by using the right `var` whose `var.sym.type` is not `ErrorType`.


                    if (rc.type.hasTag(TypeTag.ERROR) && !var.sym.type.hasTag(TypeTag.ERROR)) {
                        // Found a wrong record component: save it so that we can remove it later.
                        // If the class type of the record component is generated by annotation processor, it should
                        // use the new actual class type and symbol instead of the old dummy ErrorType.
                        toRemove = rc;
                    } else {
                        // Found a good record component: just return.
                        return rc;
                    }


You can put breakpoints at the method `getRecordComponent` to watch the `var.sym.type` and `rc.type`. You can observe that the `var.sym.type` is an `ErrorType` at first and is a right `ClassType` after first processing round.

-------------

PR: https://git.openjdk.java.net/jdk/pull/5511