language icon indicating copy to clipboard operation
language copied to clipboard

Augmenting declarations cannot occur outside parts, right?

Open eernstg opened this issue 1 year ago • 7 comments

[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 avatar Mar 11 '24 09:03 eernstg

@jakemac53, @munificent, do you agree?

eernstg avatar Mar 11 '24 09:03 eernstg

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

jakemac53 avatar Mar 11 '24 14:03 jakemac53

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>

lrhn avatar Mar 11 '24 14:03 lrhn

So every library can contain augment declarations?

eernstg avatar Mar 11 '24 14:03 eernstg

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?

eernstg avatar Apr 29 '24 08:04 eernstg

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.

jakemac53 avatar Apr 29 '24 19:04 jakemac53

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

lrhn avatar Apr 30 '24 20:04 lrhn

I believe this is implemented in #3848

davidmorgan avatar Jun 03 '24 16:06 davidmorgan

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.

eernstg avatar Jun 12 '24 17:06 eernstg

@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.

davidmorgan avatar Jun 13 '24 07:06 davidmorgan

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.

lrhn avatar Jun 13 '24 13:06 lrhn