Difference in output for `DECLARES_INSTANCE_FIELD` in extension and extension type

[FMorschel]

I was working on https://dart-review.googlesource.com/c/sdk/+/402622 (#59831) and I got to this code:

extension E on int {
  final int foo = 1, bar = 2;
}

Which has a similar case as:

extension type A(int i) {
  final int foo = 1, bar = 2;
}

But we have different outputs here:

To start, the error on extension type is a CompileTimeErrorCode and the error on extension is a ParserErrorCode. I'm unsure of what is the differentiation between them but I don't see any semantic differentiation here that would make them different.

But the problem I opened this to discuss is actually the following:

Why do we get more errors on one than on the other? That doesn't seem coherent to me.

CC @bwilkerson @scheglov @srawlins

[bwilkerson]

A ParserErrorCode is used for errors produced by the parser; a CompileTimeErrorCode is used for errors produced by later stages of the compiler. From a semantic perspective they're both errors and hence there's no interesting difference. Both will prevent code from being compiled.

Yes, the differences in reporting these diagnostics is a bit odd. I suspect that the only reason they're different is because they're being reported in different pieces of code written by different people. There's certainly no inherent reason for the differences.

[FMorschel]

Should we keep this open to make the error on extension to trigger for all variables?

[bwilkerson]

Yes, I think triggering on every variable is the better approach because it's also more consistent with what we'd do if each field were declared in a separate declaration.

[FMorschel]

I was trying to make this change myself but I could not find where is this being processed for generating the error. Can someone point me to where I could look? I did find the tests at pkg\analyzer\test\src\diagnostics\extension_declares_field_test.dart.

[bwilkerson]

The diagnostic extension_declares_instance_field is being produced by the Parser which is in sdk/pkg/_fe_analyzer_shared/lib/src/parser/parser_impl.dart. Search for messageExtensionDeclaresInstanceField.

[FMorschel]

Here is the CL https://dart-review.googlesource.com/c/sdk/+/407080

[FMorschel]

I got an answer from jensjoha that the approach I took was not the best. I'm trying to now convert the existing ParserErrorCode to a CompileTimeErrorCode. But I'm not sure what should I do at pkg\front_end\messages.yaml to ExtensionDeclaresInstanceField. I've tried removing ParserErrorCode. to make it similar to EXTENSION_TYPE_DECLARES_INSTANCE_FIELD below but I got an error that asked for all analyzerCodes to have ParserErrorCode at the beginning (which I'm a bit confused since just below we have the other error without it):

ExtensionDeclaresInstanceField:
  index: 93
  problemMessage: "Extensions can't declare instance fields"
  correctionMessage: "Try removing the field declaration or making it a static field"
  analyzerCode: ParserErrorCode.EXTENSION_DECLARES_INSTANCE_FIELD
  script: |
    extension E on int {
      int? field;
    }

# ...

ExtensionTypeDeclaresInstanceField:
  problemMessage: "Extension types can't declare instance fields"
  correctionMessage: "Try removing the field declaration or making it a static field"
  analyzerCode: EXTENSION_TYPE_DECLARES_INSTANCE_FIELD
  script: |
    extension type ET(int id) {
      int? field;
    }

I could simply create the CompileTimeErrorCode at pkg\analyzer\messages.yaml but then I'm not sure what to do at pkg\_fe_analyzer_shared\lib\src\parser\parser_impl.dart. I see that below we have extension types triggering the CompileTimeErrorCode I mentioned at the yaml above. But the one for extension is still a ParserErrorCode because of the reason mentioned above. Any help with this would be appreciated.

[FMorschel]

As a quick reference to jensjoha response to my attempt to make the current error trigger for every field name:

I don't like this. It will make every parsing which includes fields slower.
Measuring an AOT compiled CFE compiling itself I get these differences (statistics on 25 runs for each version):

msec task-clock:u: 0.4178% +/- 0.2860% (25.52 +/- 17.47)
cycles:u: 0.4530% +/- 0.2973% (115706758.20 +/- 75945506.12)
instructions:u: 0.4278% +/- 0.0006% (133637230.28 +/- 179097.98)
seconds time elapsed: 0.4204% +/- 0.2834% (0.03 +/- 0.02)
seconds user: 0.6492% +/- 0.4040% (0.04 +/- 0.02)

I think this reporting should be moved to the CFE and Analyzer where the information must already be available..

[bwilkerson]

I could simply create the CompileTimeErrorCode at pkg\analyzer\messages.yaml ...

That is the right place to define the error code.

... but then I'm not sure what to do at pkg\_fe_analyzer_shared\lib\src\parser\parser_impl.dart.

@jensjoha

[jensjoha]

I think the trick is to remove the index.

[FMorschel]

@jensjoha I tried but then it complains the indexes are not sequencial. Should I manually fix every index in that case or what?