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

[augmentation-libraries] Cover success case for aliased augmenting declarations #2574

Closed
eernstg opened this issue Mar 16, 2024 · 6 comments
Closed
Assignees

Comments

@eernstg
Copy link
Member

eernstg commented Mar 16, 2024

PR #2568 contains a test LanguageFeatures/Augmentation-libraries/augmenting_types_A01_t02.dart checking that we get the expected compile-time errors in the case where a type alias of a type declaration of a mismatched kind is augmented.

Do we have a test which is testing the same situation, except that the type aliases denote types of the same kind? (That is, the case where we should not get an error because there is no kind mismatch, but there might still be an error because a tool mistakenly treats the type alias as a new and different kind and fails to see that the type alias denotes a declaration of the right kind).

@sgrekhov sgrekhov self-assigned this Mar 25, 2024
sgrekhov added a commit to sgrekhov/co19 that referenced this issue Mar 27, 2024
@eernstg eernstg closed this as completed in 8d2cada Apr 2, 2024
copybara-service bot pushed a commit to dart-lang/sdk that referenced this issue Apr 5, 2024
2024-04-04 sgrekhov22@gmail.com Fixes dart-lang/co19#2589. Fix roll failure (dart-lang/co19#2590)
2024-04-03 sgrekhov22@gmail.com dart-lang/co19#2559. Add more augmenting types tests. Augment extends (dart-lang/co19#2588)
2024-04-03 sgrekhov22@gmail.com dart-lang/co19#2559. Rename and regroup augmentation types tests (dart-lang/co19#2587)
2024-04-02 sgrekhov22@gmail.com dart-lang/co19#2559. Add augmenting types tests. Part 5 (dart-lang/co19#2582)
2024-04-02 sgrekhov22@gmail.com dart-lang/co19#2559. Update augmenting libraries tests according dart-lang/co19#2583 (dart-lang/co19#2586)
2024-04-02 sgrekhov22@gmail.com Fixes dart-lang/co19#2574. Add more augmenting types tests (dart-lang/co19#2581)
2024-04-02 sgrekhov22@gmail.com dart-lang/co19#2559. Add augmenting types tests. Part 6 (dart-lang/co19#2583)
2024-04-01 49699333+dependabot[bot]@users.noreply.github.com Bump actions/checkout from 4.1.1 to 4.1.2 (dart-lang/co19#2585)
2024-04-01 49699333+dependabot[bot]@users.noreply.github.com Bump actions/setup-java from 4.1.0 to 4.2.1 (dart-lang/co19#2584)
2024-03-27 sgrekhov22@gmail.com Fixes dart-lang/co19#2577. Add more chained patterns assignment tests (dart-lang/co19#2580)
2024-03-26 sgrekhov22@gmail.com Fixes dart-lang/co19#2575. Remove null-aware warnings for CFE (dart-lang/co19#2578)
2024-03-26 sgrekhov22@gmail.com Fixes dart-lang/co19#2576. Replace `library augment` by `augment library` (dart-lang/co19#2579)
2024-03-25 sgrekhov22@gmail.com dart-lang/co19#2559. Add augmenting types tests. Part 2 (dart-lang/co19#2569)

R=brianwilkerson@google.com, vegorov@google.com

Change-Id: Ibf64d7fdcae0044b7317718ecf7e0852e0983f7d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/361161
Reviewed-by: Erik Ernst <eernst@google.com>
Auto-Submit: Sergey Grekhov <sgrekhov22@gmail.com>
Reviewed-by: Alexander Thomas <athom@google.com>
@scheglov
Copy link

Where the fact that type aliases can be used is described in the specification?

@eernstg
Copy link
Member Author

eernstg commented Apr 29, 2024

Ah, you're right, @scheglov, it is not supported that an augmenting declaration can use a type alias to refer to the augmented declaration. So this should be an error:

// --- Library 'main.dart'.
import augment 'augment.dart';

class B {}
class C {}
typedef CAlias = C;

// --- Library augmentation 'augment.dart'.
augment library 'main.dart';

augment class CAlias implements B {}

@sgrekhov, this implies that the tests introduced by #2581 need to be adjusted to expect errors: It is an error to augment class CAlias, but it is not because CAlias is an alias for something which isn't a class, it's an error because CAlias is itself a type alias and not a class.

This also implies that the meaning of the tests added in #2568, in particular LanguageFeatures/Augmentation-libraries/augmenting_types_A01_t02.dart and its augmentations, is different from what I was considering when reviewing them. (So augment mixin CAlias {} is still an error, but it isn't because CAlias is an alias of a class rather than an alias of a mixin, it's because CAlias is a type alias, full stop.) It is probably a good idea to take a look at the @description of these tests, and check that they do not convey anything which is wrong.

@sgrekhov
Copy link
Contributor

Reopened

@sgrekhov sgrekhov reopened this Apr 29, 2024
@sgrekhov
Copy link
Contributor

Interesting. @eernstg specification contains the following syntax

typeAlias ::= 'augment'? 'typedef' typeIdentifier typeParameters? '=' type ';'
  | 'augment'? 'typedef' functionTypeAlias

What exactly can be augmented here? Could you show any useful example? We cannot change typeParemeters section, cannot change type, cannot change anything in functionTypeAlias

@sgrekhov
Copy link
Contributor

Interesting. @eernstg specification contains the following syntax

typeAlias ::= 'augment'? 'typedef' typeIdentifier typeParameters? '=' type ';'
  | 'augment'? 'typedef' functionTypeAlias

What exactly can be augmented here? Could you show any useful example? We cannot change typeParemeters section, cannot change type, cannot change anything in functionTypeAlias

Ah, it's for adding metadata and DocComments. Ok

@eernstg
Copy link
Member Author

eernstg commented Apr 30, 2024

It is not unthinkable that a type alias augmentation could do something which is similar to variable declarations:

typedef F<X> = List<X>;
augment typedef F<X> = Map<augmented<X>, augmented<X>>; // means `Map<List<X>, List<X>>`.

I wouldn't recommend it, though. 😄

So let's stick to the DartDoc comments and metadata.

@eernstg eernstg closed this as completed in 194d6a6 May 1, 2024
copybara-service bot pushed a commit to dart-lang/sdk that referenced this issue May 3, 2024
2024-05-03 sgrekhov22@gmail.com Fixes dart-lang/co19#2635. Use null-aware operator to invoke function via nullable variable (dart-lang/co19#2636)
2024-05-02 sgrekhov22@gmail.com dart-lang/co19#2559. Add more augmented expression tests for local variables in field initializers (dart-lang/co19#2634)
2024-05-02 sgrekhov22@gmail.com dart-lang/co19#2559. Add more augmented expression tests for local variables inside of getters and setters (dart-lang/co19#2633)
2024-05-02 sgrekhov22@gmail.com dart-lang/co19#2559. Add augmented expression tests for functions (dart-lang/co19#2630)
2024-05-01 49699333+dependabot[bot]@users.noreply.github.com Bump actions/checkout from 4.1.2 to 4.1.4 (dart-lang/co19#2632)
2024-05-01 sgrekhov22@gmail.com dart-lang/co19#2559. Make more augmented expression tests stronger (dart-lang/co19#2629)
2024-05-01 sgrekhov22@gmail.com Fixes dart-lang/co19#2574. Expect an error in case of augmenting a type alias (dart-lang/co19#2627)
2024-04-30 sgrekhov22@gmail.com dart-lang/co19#2559. Add augmenting expression tests for fields with no initializers (dart-lang/co19#2626)
2024-04-30 sgrekhov22@gmail.com dart-lang/co19#2559. Make augmented expression tests stronger (dart-lang/co19#2628)
2024-04-29 sgrekhov22@gmail.com Fixes dart-lang/co19#2624. Add more extension type tests for union types (dart-lang/co19#2625)
2024-04-26 sgrekhov22@gmail.com dart-lang/co19#2559. Add augmenting expression tests for fields (dart-lang/co19#2621)

Change-Id: I8246f841afd24390742ba225e769ba881dccef9b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/365261
Reviewed-by: Alexander Thomas <athom@google.com>
Commit-Queue: Erik Ernst <eernst@google.com>
Reviewed-by: Erik Ernst <eernst@google.com>
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

3 participants