dart-lang / macros

A Dart mono-repo for macro development.
BSD 3-Clause "New" or "Revised" License
60 stars 5 forks source link

Build and run with macros for e2e testing #80

Open davidmorgan opened 1 week ago

davidmorgan commented 1 week ago

Hi @johnniwinther, I'm running into what I think might be a CFE issue, could you take a look please?

What I'm trying to do is to take the code output by a new dart_model macro, dump it to disk, then run it with the CFE. So from the CFE's point of view they are "just" augmentations; the only weird point is that I have to explicitly add the import augment that the CFE would add invisibly when running as a macro.

What I think I see is that the CFE-generated augmentation works but the equivalent augmentation without macros doesn't.

All the files below are in this draft PR.

So this works (with CFE build at head today):

# declare_define_constructor.dart
import 'declare_define_constructor_macro.dart';

@DeclareDefineConstructor()
class A {}

void main() => print(A.x());
# declare_define_constructor_macro.dart
import 'package:macros/macros.dart';

macro class DeclareDefineConstructor implements ClassDeclarationsMacro, ClassDefinitionMacro {
  const DeclareDefineConstructor();

  Future<void> buildDeclarationsForClass(
      ClassDeclaration clazz, MemberDeclarationBuilder builder) async {
      builder.declareInType(DeclarationCode.fromParts([
        'external ',
        clazz.identifier.name,
        '.x();'
      ]));
  }

  Future<void> buildDefinitionForClass(
      ClassDeclaration clazz, TypeDefinitionBuilder typeBuilder) async {
    final constructors = await typeBuilder.constructorsOf(clazz);
    final x =
        constructors.singleWhere((c) => c.identifier.name == 'x');
    final builder = await typeBuilder.buildConstructor(x.identifier);
    builder.augment();
  }
}
dart --enable-experiment=macros --enable-experiment=enhanced-parts declare_define_constructor.dart
Instance of 'A'

Looking at this code in the debugger I can see the augmentations that get produced:

augment library 'file:///usr/local/google/home/davidmorgan/git/macros/scratch/declare_define_constructor.dart';

augment class A {
external A.x();
  augment A.x();
}

and then my equivalent dart_model macro produces something very similar, and trying to run (which means adding an explicit import augment) it I end up with this on disk:

# declare_define_constructor.dart
import augment 'declare_define_constructor.dart.macro_tool_output'; // added by macro_tool
// Copyright (c) 2024, the Dart project authors.  Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

import 'package:_test_macros/declare_define_constructor.dart';

@DeclareDefineConstructor()
class A {}

void main() => print(A.x());
# declare_define_constructor.dart.macro_tool_output
augment library 'package:foo/declare_define_constructor.dart';

augment class A {
external A.x();
augment A.x();
}

but running it fails:

dart --enable-experiment=macros --enable-experiment=enhanced-parts run lib/declare_define_constructor.dart
lib/declare_define_constructor.dart.macro_tool_output:5:9: Error: Augmentation constructor 'A.x' doesn't match a constructor in the augmented class.
Try changing the name to an existing constructor or removing the 'augment' modifier.
augment A.x();
        ^
lib/declare_define_constructor.dart:11:24: Error: Member not found: 'A.x'.
void main() => print(A.x());

I tried using file references instead of package references in this code, but it doesn't seem to make any difference.

Any ideas, please?

I realize that using "external" in this way is probably not what we want in the end, so if there is some newer/better recommended way then that's good too :)

Thanks.

johnniwinther commented 1 week ago

It looks like we currently don't support both declaring the constructor and augmenting it within the same class body. Is this blocking you?

davidmorgan commented 1 week ago

Hmmmm. Confirmed if I split the augmentation file in two it works:

# declare_define_constructor.dart.macro_tool_output1
augment library 'package:foo/declare_define_constructor.dart';

augment class A {
external A.x();
}
# declare_define_constructor.dart.macro_tool_output2
augment library 'package:foo/declare_define_constructor.dart';

augment class A {
augment A.x();
}

but then, how is it that it works if it's a macro generating the same augmentation with clashing external+augment?

Perhaps it's to do with the output being evaluated phase by phase instead of in one shot from the file?

Whether I'm blocked / how high priority this is, is a good question :) ... I'm trying to use the CFE to test the output from the analyzer on one particular macro, I think I can reasonably work around it and focus on other things for now by putting the external declaration in the main file, I'll try that for now.

Thanks.

davidmorgan commented 1 week ago

Yeah, that seems to work okay--will aim to land an e2e test with that workaround tomorrow, hopefully that gets us enough scope to work on json_codable that we have plenty to do for a while :)

davidmorgan commented 1 week ago

There seems to be a similar issue with methods but with a different symptom, approximately, the CFE can run this if it's macro output:

augment class A {
  external Map<String, Object?>  toJson();
  augment Map<String, Object?> toJson() => {};
}

but if I supply it as a file on disk I get an error that doesn't make sense:

  NoSuchMethodError: No static method 'toJson' declared in class 'F'.
  Receiver: F
  Tried calling: toJson()

workaround is the same, move the external to the checked in code, e.g. here and it works fine.