Skip to content
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

patched_dart2js_sdk has no _CompileTimeError class #29661

Closed
peter-ahe-google opened this issue May 19, 2017 · 5 comments
Closed

patched_dart2js_sdk has no _CompileTimeError class #29661

peter-ahe-google opened this issue May 19, 2017 · 5 comments
Assignees

Comments

@peter-ahe-google
Copy link
Contributor

The class _CompileTimeError is missing from patched_dart2js_sdk and leads to a crash. See #29660.

@sigmundch
Copy link
Member

seems like this type just be declared in sdk/lib/core/errors.dart instead of the patch files, correct?

Basically, I don't expect fasta to know about anything that comes from individual patch implementations.

@peter-ahe-google
Copy link
Contributor Author

That's one option. Another option is to make Fasta more flexible so the backend can choose how compile-time errors are reported. Similarly to how I added instantiateInvocation to VmTarget.

@sigmundch
Copy link
Member

Good point - I slightly prefer to avoid that if we can share the code in core lib, though. Mainly I'd like to be able to be able to produce kernel for dart2js or for ddc using the same target/options, which might require that at least those two platforms have the exact same Target (from package:kernel) and agree on a common place for CompileTimeError.

To unblock you, here is a CL that just adds _CompileTimeError to the dart2js core patch: https://codereview.chromium.org/2901323002

If we want to right away move towards that shared location, I like the idea of adding it to dart:_internal as a public class, instead of a private class in dart:core. Here is a CL that takes that approach: https://codereview.chromium.org/2897363002

I sent out the former CL, but decided to wait to hear from you before sending the latter (it has changes in the VM, so I rather bug them only if we agree on the direction)

@peter-ahe-google
Copy link
Contributor Author

I agree. It's nicer to share library code.

sigmundch added a commit that referenced this issue May 24, 2017
@sigmundch
Copy link
Member

submitted the simple fix for now

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