Skip to content

More compact lowering of late variable error messages by front-end #43995

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
rakudrama opened this issue Oct 30, 2020 · 3 comments
Open

More compact lowering of late variable error messages by front-end #43995

rakudrama opened this issue Oct 30, 2020 · 3 comments

Comments

@rakudrama
Copy link
Member

The front-end lowering of late variables is too large for dart2js (see #43361) and DDC.

One of the reasons is that the lowering contains comprehensive error messages.
These could be factored so that the call to the error contains only the name, and the rest of the message is provided by the constructor.
Instead of:

throw LateInitializationErrorImpl("Field 'queryParameters' has been assigned during initialization.");
throw LateInitializationErrorImpl("Local 'result' has not been initialized.");

generate:

throw LateInitializationErrorImpl.fieldAssignedDuringInitialization('queryParameters');
throw LateInitializationErrorImpl.localNotInitialized('result');

There should also be a target configuration option to omit the names as the name can be guessed from the stack-trace.
For the best code from dart2js, the LateInitializationErrorImpl constructor without the name should be a separate named constructor (not an optional name argument, as the default occurs at the call site).

/cc @johnniwinther

@johnniwinther
Copy link
Member

Does using nameless constructors for errors on locals? I would imagine that the variable in question is not easily deducible from the stack trace.

@rakudrama
Copy link
Member Author

The error will happen in a getter or setter or direct code that implements =local and/or local=. The source location for that variable reference should be the source location for the lowered call / code.

@rakudrama
Copy link
Member Author

Perhaps we could combine the API for this request and #43993 by the Target having a method that takes e.g. an enum for the LateLowering context and returns a choice between named, unnamed and no lowering.
If the source location of a local proved to be too difficult to use in production, then we could elect to used the name.
This also provides a mechanism for removing source names from a production binary.

enum LateLowering {
  nullableUninitializedNonFinalLocal,
  nonNullableUninitializedNonFinalLocal,
  ...
}
enum LateLoweringAction {
  namedLowering,
  unnamedLowering,
  noLowering
}

class Target {
  ...
  LateLoweringAction lateLowering(LateLowering type);
  ...
}

dart-bot pushed a commit that referenced this issue Nov 3, 2020
This CL adds predefined constructors to LateInitializationErrorImpl that
only take the name of the late field/variable as argument. This can reduce
the size of the generated code for ddc/dart2js size because the long
message isn't repeated at all call sites.

In response to #43995

Change-Id: Ie276dd1455feab1655da3a503856da425b20f04f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/169883
Commit-Queue: Johnni Winther <[email protected]>
Reviewed-by: Stephen Adams <[email protected]>
Reviewed-by: Dmitry Stefantsov <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants