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

Bound checking errors in small features #45334

Closed
johnniwinther opened this issue Mar 16, 2021 · 11 comments
Closed

Bound checking errors in small features #45334

johnniwinther opened this issue Mar 16, 2021 · 11 comments
Assignees
Labels
area-front-end Use area-front-end for front end / CFE / kernel format related issues.

Comments

@johnniwinther
Copy link
Member

These tests fail due to discrepancies wrt. bound checking:

language/generic/generic_function_type_argument_test
co19/Language/Generics/Superbounded_types/typedef3_A01_t06/01
co19/Language/Generics/Superbounded_types/typedef3_A01_t06/02
co19/Language/Generics/typedef_A06_t04
co19/Language/Generics/typedef_A06_t12
co19/LanguageFeatures/Instantiate-to-bound/nonfunction_typedef/static/nonfunction_l1_t03
co19/LanguageFeatures/Instantiate-to-bound/nonfunction_typedef/static/nonfunction_l1_t04
co19/LanguageFeatures/Instantiate-to-bound/nonfunction_typedef/static/nonfunction_l4_t01
co19/LanguageFeatures/regression/34560_t02
@johnniwinther johnniwinther added the area-front-end Use area-front-end for front end / CFE / kernel format related issues. label Mar 16, 2021
@chloestefantsova
Copy link
Contributor

  • I think co19/Language/Generics/Superbounded_types/typedef3_A01_t06/02 shouldn't be a compile-time error: it's a case of a super-bounded type.
  • co19/LanguageFeatures/Instantiate-to-bound/nonfunction_typedef/static/nonfunction_l1_t03 complains about an unexpected compile-time error, but I think it should be reported.
  • co19/LanguageFeatures/Instantiate-to-bound/nonfunction_typedef/static/nonfunction_l1_t04 seems that the errors are expected; although, the location of one of the three errors is different for Analyzer and the CFE because the CFE can't point inside of types yet. I guess the test should be updated adjusting the locations of the errors.
  • co19/LanguageFeatures/Instantiate-to-bound/nonfunction_typedef/static/nonfunction_l4_t01 complains about an unexpected compile-time error, but I think it should be reported.

@leafpetersen
Copy link
Member

leafpetersen commented Mar 18, 2021

Sorry, @stefantsov I'm not sure I totally parsed which of your examples were problems with tests and which were CFE issues.

I think you're saying the last three are bad tests, but the first is ok?

@chloestefantsova
Copy link
Contributor

@leafpetersen I think all four are issues with tests, and in the first one in the list a compile-time error is expected in the test, but I believe that it's not an error. Sorry for not being clear.

@chloestefantsova
Copy link
Contributor

cc @eernstg

@chloestefantsova
Copy link
Contributor

  • Also, I think the test co19/Language/Generics/typedef_A06_t04 should be corrected too. The right-hand side of the typedef there is a regular-bounded type (it's not even an application of a generic type to type arguments), and all parts of it are well-bounded.

@eernstg
Copy link
Member

eernstg commented Mar 18, 2021

Taking a look, I can't verify two of the issues, but I agree on the remaining ones:

  • Agree: co19/Language/Generics/Superbounded_types/typedef3_A01_t06/02 is not a compile-time error, B<A<A<A<B<A<dynamic>>>>>> is super-bounded.
  • Can't verify the problem with co19/LanguageFeatures/Instantiate-to-bound/nonfunction_typedef/static/nonfunction_l1_t03, it has no failures according to https://dart-ci.firebaseapp.com/current_results/#/filter=co19/LanguageFeatures/Instantiate-to-bound/nonfunction_typedef/static/nonfunction_l1_t03&showAll
  • Agree: co19/LanguageFeatures/Instantiate-to-bound/nonfunction_typedef/static/nonfunction_l4_t01 does look like it's just the location associated with the error message that differs for the CFE.
  • Can't verify the problem with co19/LanguageFeatures/Instantiate-to-bound/nonfunction_typedef/static/nonfunction_l4_t01.
  • Agree on co19/Language/Generics/typedef_A06_t04 as well.

So maybe the 2nd and 4th issue have been fixed in a roll already?

@chloestefantsova
Copy link
Contributor

I filed a co19 issue for the three tests that we agree should be updated: dart-lang/co19#1029.

@chloestefantsova
Copy link
Contributor

Some of the remaining test failures are being worked on here: https://dart-review.googlesource.com/c/sdk/+/192146.

@eernstg
Copy link
Member

eernstg commented Mar 23, 2021

About co19/Language/Generics/typedef_A06_t04: The front end does not report the error about the raw bound, but this is covered by #42435.

dart-bot pushed a commit that referenced this issue Mar 25, 2021
Bug: #45334
Change-Id: Ia2a9d154dba21d445f8f821988580fbf4249cf24
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/192925
Commit-Queue: Dmitry Stefantsov <dmitryas@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
dart-bot pushed a commit that referenced this issue Mar 25, 2021
Bug: #45334
Change-Id: Idbc823ecc688f2679134c757dcf7a7ad12a02e24
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/192146
Commit-Queue: Dmitry Stefantsov <dmitryas@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
dart-bot pushed a commit that referenced this issue Mar 29, 2021
This reverts commit ce841aa.

Reason for revert: a crash in google3.

Original change's description:
> [cfe] Remember built TypedefTypes for late bounds checks
>
> Bug: #45334
> Change-Id: Idbc823ecc688f2679134c757dcf7a7ad12a02e24
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/192146
> Commit-Queue: Dmitry Stefantsov <dmitryas@google.com>
> Reviewed-by: Johnni Winther <johnniwinther@google.com>

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: #45334
Change-Id: I9ba9d02a29e17b58ddbf313dce70af8c9498c1a0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/193305
Reviewed-by: Dmitry Stefantsov <dmitryas@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Commit-Queue: Dmitry Stefantsov <dmitryas@google.com>
@chloestefantsova
Copy link
Contributor

I have a status update for the remaining tests that are currently failing if run locally at the HEAD fo Dart SDK:

  • co19/Language/Generics/Superbounded_types/typedef3_A01_t06/02 — still marked as compile-time error, but shouldn't be; the test should be updated;
  • co19/Language/Generics/typedef_A06_t04 — the fix is in work at https://dart-review.googlesource.com/c/sdk/+/193662;
  • co19/Language/Generics/typedef_A06_t12 — the fix is in work at https://dart-review.googlesource.com/c/sdk/+/193662;
  • co19/LanguageFeatures/Instantiate-to-bound/nonfunction_typedef/static/nonfunction_l1_t04 — still a failure because of the error locations; the locations should be updated in the test.

So, after https://dart-review.googlesource.com/c/sdk/+/193662 lands, the issue for the CFE may be closed.

@chloestefantsova
Copy link
Contributor

https://dart-review.googlesource.com/c/sdk/+/193662 landed. I'm closing the issue. Please reopen if needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-front-end Use area-front-end for front end / CFE / kernel format related issues.
Projects
None yet
Development

No branches or pull requests

4 participants