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

Deprecate avoid_as? #57894

Closed
a14n opened this issue Jan 31, 2019 · 9 comments · Fixed by dart-lang/linter#2439
Closed

Deprecate avoid_as? #57894

a14n opened this issue Jan 31, 2019 · 9 comments · Fixed by dart-lang/linter#2439
Assignees
Labels
analyzer-linter Issues with the analyzer's support for the linter package area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) type-code-health Internal changes to our tools and workflows to make them cleaner, simpler, or more maintainable type-documentation A request to add or improve documentation

Comments

@a14n
Copy link
Contributor

a14n commented Jan 31, 2019

As of #34097 (comment)

We've unified the implementation of implicit/explicit as checks in the VM to make them perform equally fast. We decided to retain the two different exceptions being thrown without extra penalty.

Not sure how dart2js handles explicit as checks. Is there an extra penalty ? Perhaps @sigmundch knows.

@pq
Copy link
Member

pq commented Jan 31, 2019

Interesting. I totally missed this discussion. Thanks!

@pq pq added the type-question A question about expected behavior or functionality label Jan 31, 2019
@sigmundch
Copy link
Member

For dart2js it still is uneven: we haven't invested in optimizing away checks because many customers use --omit-implicit-checks (included also with -O3 and -O4). We are however starting some changes to our runtime type representation to prepare for it in the future.

At this time, the --omit-implicit-checks flag removes the implicit casts, but keeps around the explicit casts, so the former have no cost but the latter do. An alternative we've considered instead of optimizing away the checks is to omit all casts. For that purpose we have added a --omit-as-casts flag, but we haven't experimented too much with it. If we decide to include it as part of -O3 and we validate with our customers that it works as expected, then we would be in a position to say that explicit and implicit checks have the same cost in dart2js as well.

@lexaknyazev
Copy link
Contributor

My casts look like:

// Iterator<num> iterator;
// In case when values are guarantied to be ints:
while (hasNext) {
  final int value = iterator.current;

Before Dart SDK 2.1.1, analyzer's implicit-casts didn't cover "declaration" casts that avoid_as description recommends:

... If you know the type is correct, use an assertion or assign to a more narrowly-typed variable ...

With the latest SDK version, I have to either enable implicit-casts or to use as (and disable the lint). The comment above about dart2js suggests that it's worth performance-wise to choose the former even if it makes code less type safe, right?

@nstrelow
Copy link

nstrelow commented Dec 7, 2020

Can this lint rule now be deprecated or does it still apply in dart2js?

Using this rule for a Flutter app which runs also in web.

Might be completely unrelated, but
I have a weird issue with google/json_serializable.dart#656, where as String breaks Flutter web release builds. toString() works

@bwilkerson
Copy link
Member

@sigmundch @rakudrama

@sigmundch
Copy link
Member

For dart2js there is still a benefit when using --omit-implicit-checks, but with null-safety implicit casts are no longer allowed unless the input has type dynamic. So I think deprecating the lint is a good choice at this time. Otherwise, it should at least be fixed to only trigger when the input is dynamic if the enclosing library has opted into null safety.

@mraleph
Copy link
Member

mraleph commented Dec 19, 2020

I got a question about this lint on Twitter in context of Flutter - we should definitely either deprecate it or at least make it explicit why it is there and which platforms it applies to. People are confused because this lint is enforced in Flutter and it sends a confusing message that implicit casting is somehow cheaper than as, which makes no sense on VM platform as it never unsoundly omits any checks.

Recommendation to use is instead of as is also somewhat confusing (I guess concern was about code size for exception throwing?).

/cc @Hixie

@Hixie
Copy link
Contributor

Hixie commented Dec 19, 2020

Yeah this is obsolete, we should just remove it, at least as a default if it's still enabled anywhere.

@pq
Copy link
Member

pq commented Dec 20, 2020

+1 for deprecating and I'll take that on.

(FWIW I don't see it enabled anywhere in flutter but it definitely used to be so the confusion could be lingering or the user is on an old branch.)

@pq pq self-assigned this Dec 20, 2020
@pq pq added type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) type-code-health Internal changes to our tools and workflows to make them cleaner, simpler, or more maintainable type-documentation A request to add or improve documentation and removed type-question A question about expected behavior or functionality labels Dec 20, 2020
@devoncarew devoncarew added analyzer-linter Issues with the analyzer's support for the linter package area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. labels Nov 18, 2024
@devoncarew devoncarew transferred this issue from dart-lang/linter Nov 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analyzer-linter Issues with the analyzer's support for the linter package area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) type-code-health Internal changes to our tools and workflows to make them cleaner, simpler, or more maintainable type-documentation A request to add or improve documentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants