Use feature flags for extern definitions

[marzipankaiser]

This implements feature flags for externs.

Syntax

extern def infixAdd(a: Int, b: Int): Int =
     js "${a} + ${b}"
     llvm { someEffektStmt }
     default "${a} + ${b}"

(Note: You might not want to use default like this... Just an example.)

The old syntax should still work and is equivalent to having just a default clause.

This also exists for includes like this:

extern js """
...
"""
extern include js " ... "

Notes about implemented behaviour

Current behaviour is:

• Extern includes with non-matching flags are ignored
• Extern defs without matching flag (for current backend) give out a warning during naming (for now; ideally: Check if it is actually used!)
• Extern defs with an unannotated/default string will give out a warning

Future work

• Output warnings only if we (transitively) use unsupported externs from main
• Move generation of warnings from Namer into a separate phase, potentially with some other smaller checks. Alternatively: Move to ResolveExternDefs.
• Allow using interfaces/functions passed as block parameters (currently fails with a type error due to captures).

[marzipankaiser]

Design decision reg. Effekt code in extern definitions:
Currently, the syntax is just (expr) (forced parentheses for parsing) instead of a string.
They (all of them) are passed through to the backends and transformed on the way, the backend can then choose.

Upside: The backends can do whatever they want with it.

Downside: This has to be handled in the backends, which is non-trivial at least in LLVM.

Alternative: Choose the "correct" extern definition in core. Then make the pseudo-extern definitions normal definitions.
(Pro/con as above but reversed)

[b-studios]

I don't think I understand your proposal. Do you have an example of how this looks like?

[marzipankaiser]

I don't think I understand your proposal. Do you have an example of how this looks like?

Does this refer to the whole PR/the source syntax? - In this case, there is an example in the syntax section above.
To be more precise in this case, we could e.g. write:

extern def absDiff(x: Int, y: Int) =
    js "abs((${x}) - (${y}))"      // string for a backend
    chez ( abs(infixAdd(x, neg(y))) ) // effekt expression for a backend (e.g. bc. we can't use certain functions...)
    else ( abs(infixSub(x, y)) )    // effekt expression for default

Parenthesis are always required to make parsing feasible, currently they contain expressions. Alternatively, we could use {} with statements.

If it is about the (implementation) Design Decision:

• Currently, each IR (core, lifted, machine) has something like a Extern(..., bodies: List[ExternBody]) where ExternBody is something like:
  https://github.com/effekt-lang/effekt/blob/236a38dc873f78b1114394f6d37facd3d6c00515/effekt/shared/src/main/scala/effekt/lifted/Tree.scala#L61-L64
  (always adapted for the respective IR type)
  Then, the generators/backends search through those and "pick" what they want to use.
  This gives them more flexibility (and is how Koka does it). But makes it harder to implement in some (e.g., in LLVM, the calling conventions are different between Effekt defs and extern defs).
• An alternative would be to, in e.g. core, transform the Extern to either one with only String-based ExternBodies, or a "normal" core definition, depending on the current backend. absDiff from above would then be transformed to the equivalent of:

      def absDiff(x: Int, y: Int) = abs(infixAdd(x, neg(y)))
  

  in the chez backend. This allows the backends to be mostly agnostic about the selection process (and makes it consistent with the warnings generated), but also allows them a bit less flexibility.

[marzipankaiser]

Note to self: Add a simple phase on core to select & desugar extern defs (the backends can decide to use it or not)

[marzipankaiser]

Changed:

• Now uses { stmt } instead of ( expr ) for effekt-defined externs; this makes the second point easier and seems more natural to me
• Added a Core -> Core phase that selects the correct definitions and makes them "normal" definitions if they are defined using Effekt code

[marzipankaiser]

This is the phase resolving extern defs

[marzipankaiser]

At some point we might want to move the warnings at
https://github.com/effekt-lang/effekt/blob/5646f6731f7983dcead702a6e60321f9cc9186c2/effekt/shared/src/main/scala/effekt/Namer.scala#L294-L304
and
https://github.com/effekt-lang/effekt/blob/5646f6731f7983dcead702a6e60321f9cc9186c2/effekt/shared/src/main/scala/effekt/Namer.scala#L385-L389
from Namer to a separate phase with more of those smaller checks.
Maybe there are also other checks that would fit there?

[marzipankaiser]

Tests are failing because of tests that use definitions like
https://github.com/effekt-lang/effekt/blob/5646f6731f7983dcead702a6e60321f9cc9186c2/examples/pos/capture/resources.effekt#L3
which will potentially never work on some backends.
Potentially some can be rewritten to use Effekt definitions instead (i.e., extern def ... = else { ... }).

[b-studios]

This is just a bad test (already has been), not your fault

[marzipankaiser]

I tried to fix it up, and it found a bug: We cannot actually call any operations on f since the capture is not allowed by our current implementation of Typer here.
Just allowing this in typer, however, leads to crashes in the backends (more precisely: undefined variables). I think this might be related to not passing the captures to the extern 🤔.

[marzipankaiser]

else -> default

[b-studios]

Potentially risking conflicts with String this could be shortened to

enum ExternBody extends Tree {
  def featureFlag: FeatureFlag
  
  case String(featureFlag: FeatureFlag, contents: Template[Pure])
  case EffektExternBody(featureFlag: FeatureFlag, body: Stmt)
}

[marzipankaiser]

I'm not sure it's worth it given the potential conflicts. In particular, adding any helper methods on ExternBody that use Strings as parameters/return values would become awkward (and might not be that unlikely).
Maybe we can also come up with a better name (I couldn't, hence the not entirely helpful one... -s, actually).

[marzipankaiser]

https://github.com/effekt-lang/effekt/actions/runs/8736009035/job/23969998216?pr=427#step:9:718
Seems there is another calling-convention issue here.

[marzipankaiser]

Maybe it would be better to move the Resolving further to the front after all.
I would still prefer it to be after Typing, but we could do it in Middleend on Typechecked.

[marzipankaiser]

The approach of adding it after Typer does not work properly either: The calling-convention issue hinges on the symbol type used in core.Transformer:

effekt/effekt/shared/src/main/scala/effekt/core/Transformer.scala

Lines 635 to 638 in ee905df

	case f: ExternFunction if f.capture.pure =>
	PureApp(BlockVar(f), targs, vargsT)
	case f: ExternFunction if f.capture.pureOrIO =>
	DirectApp(BlockVar(f), targs, vargsT, bargsT)

[b-studios]

I reviewed the Scala source changes and they look mostly good to me. I only have a few remarks:

1. Use feature flags for extern definitions #427 (comment)
2. Use feature flags for extern definitions #427 (comment) (and for other backends)
3. Use feature flags for extern definitions #427 (comment)

[marzipankaiser]

(interpreted that as approval)