dart-lang / language

Design of the Dart language
Other
2.67k stars 205 forks source link

Augmenting declarations cannot occur outside parts, right? #3647

Closed eernstg closed 5 months ago

eernstg commented 8 months ago

[Update: At this time, May 2024, there are no library augmentation files. We will use parts to do this job. Changed the title accordingly.]

Thanks to @sgrekhov for bringing up this topic. I might have missed it, but I do not think the augmentation library specification specifies that

It is an error for an augmenting declaration to occur in any library which is not an augmentation library.

That is, a library that contains a library augment ... directive can have augment declarations, others can't.

eernstg commented 8 months ago

@jakemac53, @munificent, do you agree?

jakemac53 commented 8 months ago

This is the current spec yes - but there have been discussions about allowing them anywhere so it could change.

lrhn commented 8 months ago

I agree that that's what's specified. I disagree that it's necessary, and would like it changed: #3576. (Probably part of the discussion Jake refers to.)

You can have an empty "main" library file with a single "augment import "real_library.dart";` declaration, then declare all your stuff in that library augmentation file, and have both your main declarations and augmentation declarations in the same file.

If you can do that, we might as well allow declaring augmentations in the main library file too.

You can argue that you shouldn't need to have declarations and augmentations on top of those declarations in the same library, but I'm not absolutely certain there aren't things you can do with class+augment class that you can't do (or do as easily) with a single class.

Maybe someone just likes to do:

class Pomegranate { 
  // all the base stuff
}
// All the Iterable stuff here.
augment class Pomegranate with Iterable<Seed> {
  Iterator<Seed> get iterator => ...
}
// All the comparable stuff here.
augment class Pomegranate implements Comparable<Pomegranate> {
  int compareTo(Pomegranate other) => weight.compareTo(other.weight);
  static int compare(Pomegranate p1, Pomegranate p2) => p1.compareTo(p2);
}

Who are we to disallow that (or force them to have an extra spurious file to achieve it), if there is no technical reason for it. \</soapbox>

eernstg commented 8 months ago

So every library can contain augment declarations?

eernstg commented 6 months ago

I'd recommend allowing augmentations in a library as well as in a library augmentation, but each augmenting declaration should apply to an original declaration in the same library. (This is in line with the rule that every augmentation should apply to the original declaration, or to an augmenting declaration which occurs in the parent chain of library augmentations, or in the main library. That is, "you can only augment your parents, which includes yourself". ;-)

@dart-lang/language-team, WDYT?

jakemac53 commented 6 months ago

SGTM, I think this is in line with the recent in person discussions on this.

It does make me sad that you can't augment a declaration produced by a macro, given that macro augmentations always come last, but I don't see a reasonable way around that.

lrhn commented 6 months ago

Agree. If macro introspection can only see entire, fully augmented declarations, there is no way we can also have augmentations that occur after macros.

davidmorgan commented 5 months ago

I believe this is implemented in #3848

eernstg commented 5 months ago

This means that we have decided to allow augmenting declarations to occur in libraries (not just in parts).

@davidmorgan, did I get this right? I couldn't see it expressed explicitly in #3848 (or in any of the links from there). I might just have missed it, of course. However, I do have the impression that the language team supports this decision.

davidmorgan commented 5 months ago

@eernstg I'm 95% sure that's what I heard, but I'm not sure I've seen it written down anywhere :)

@lrnh do I have it correct that we agreed augmentations can occur in libraries? If so is that part of your in-progress spec change? Thanks.

lrhn commented 5 months ago

I believe we did agree to that (there is only one kind of Dart, the Dart kind!), and it is in my in-progress-and-needing-updates(-and-decisions) PR.

Or, rather, there is nothing in there saying that you cannot. Augmentations is defined as a language feature separate from "parts with imports", and augmentations do not say anything about where they can be declared - other than "after" the base declaration (in syntactic pre-order depth-first traversal of part files and sub-parts).

The removed phrase was:

Library augmentations may also contain declaration augmentations, which augment existing declarations from the library. Some

There is no new text added, so augmenting declarations can occur anywhere the declaration they augment could occur.