Primary constructors spec question

[bwilkerson]

In the specification of primary constructors there is text that reads:

The grammar is ambiguous with regard to the keyword factory. For example, factory() => C(); could be a method named factory with an implicitly inferred return type, or it could be a factory constructor whose name is the name of the enclosing class.

This ambiguity is resolved as follows: When a Dart parser expects to parse a <memberDeclaration>, and the beginning of the declaration is factory or one or more of the modifiers const, augment, or external followed by factory, it proceeds to parse the following input as a factory constructor.

It's my understanding that an instance method can't start with const, so I believe that the presence of a const modifier would already force the construct to be interpreted as a constructor (once the parser determines that it isn't a field). If that's true, then it would be better to remove the unnecessary const from the list to avoid confusion.

I don't know whether the augmentations feature is going to allow both augment and external to be applied to the same constructor declaration, but if const is removed from the list then the "or more" might not be needed.

[bwilkerson]

@eernstg

[eernstg]

Good points!

First, augment and external can coexist. The idea is that it should be possible to declare the signature of a function, and an augmenting declaration would then be able to provide a function body that implements this function, or it could provide the external modifier which will direct the compilation process or the runtime to somehow bind this name to an external implementation. So that's fine.

Next, it is true that const can't be the first token in the declaration of an instance method (at least until #2222 or something similar is accepted).

I'm not quite sure which form is less confusing:

The disambiguation rule says that we are definitely parsing a factory constructor when we're at the beginning of a member declaration and we're seeing factory, const factory, augment factory, augment const factory, external factory, external const factory, augment external factory, or augment external const factory.

The rule that you're suggesting says that we're definitely parsing a factory constructor when we're at the beginning of a member declaration and we're seeing factory, augment factory, external factory, or augment external factory (assuming that the parser is already decides that it is definitely a factory constructor when we are seeing any of the other four keyword sequences).

Your rule may be more economical because it avoids stating that certain declarations are definitely declaring a factory constructor in a situation where that is already unambiguously the case according to the grammar. However, it isn't wrong for a disambiguation rule to classify certain token sequences, even in the case where they are unambiguous (and the disambiguation rule just confirms something that is already known to be true).

It might even be easier to use the slightly-too-broad rule that we currently have in the feature specification: 'parser_impl.dart' has code like this:

// In `parseClassOrMixinOrExtensionOrEnumMemberImpl`:
            ...
          } else if (next.isA(Keyword.CONST) && covariantToken == null) {
            varFinalOrConst = token = next;
            next = token.next!;
          } ...

and I suspect that it is a typical situation that we're encountering the keyword factory, and then we can detect whether or not there is a const keyword in front of it by looking at local variables like varFinalOrConst, but it's actually easier if we rely on the fact that it doesn't matter: We're parsing a factory constructor both in the case where that local variable is null and when it points to a const token.

In summary, I think it's OK to keep the current wording. However, if it comes across as confusing then we can certainly change it as you proposed.

[bwilkerson]

Thanks.

FWIW, if I try to write an external augmentation today

class C {
  augment external factory();
}

I get an error telling me that "Members can't be declared to be both 'external' and 'augment'."

I'll assume for now that that's a limitation in the analyzer's implementation of one or both of the language features.

[scheglov]

Strictly speaking this reported from the parser, not the analyzer.

[eernstg]

The grammar certainly works hard to allow for augment external on getters, setters, functions, and variables (here for top-level declarations and here for members).

The version 1.35 change log entry also mentions this combination as something which is supported.

Finally, I haven't spotted anything saying that it is not supported.

[lrhn]

ACK. An external declaration is a complete declaration when augmenting, it adds implementation.
You should be able to do:

augment external <decl>

If you can augment that declaration otherwise, and if you could make that declaration external without any augments. The two are orthogonal.

So augment external const factory is possible. We allow external const factory C() today, and I think you can augment the incomplete const factory C();.

(An external const factory usually means "this thing is magic", because if it's just redirecting, you could have written that, and if it's not, the language doesn't otherwise allow a non-redirecting const factory.)