Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Macros: resolveIdentifier does not resolve identifiers exported from other libraries #55910

Open
alexeyinkin opened this issue Jun 3, 2024 · 4 comments
Labels
area-pkg Used for miscellaneous pkg/ packages not associated with specific area- teams. cfe-feature-macros Implement macros features in the CFE feature-macros Implementation of the macros feature pkg-macros The experimental package:_macros library

Comments

@alexeyinkin
Copy link

If library main imports a library A, and A exports a library B, and a macro is augmenting main, then it can resolve identifiers from A but not B.

This example tries to resolve identifiers from args package. args/args.dart does not declare ArgParser class but exports it from src/arg_parser.dart. A macro fails to resolve this identifier in args/args.dart, but successfully resolves it in args/src/arg_parser.dart (and it's not a proper way because libraries under src are considered private).

main.dart:

import 'package:args/args.dart';
import 'macros.dart';

@Args()
class MyArgs {}

void main() {}

macro.dart:

import 'dart:async';
import 'package:macros/macros.dart';

final _argParserPrivateLibrary = Uri.parse('package:args/src/arg_parser.dart');
final _argsLibrary = Uri.parse('package:args/args.dart');

macro class Args implements ClassDeclarationsMacro {
  const Args();

  @override
  Future<void> buildDeclarationsForClass(ClassDeclaration clazz, MemberDeclarationBuilder builder) async {
    await builder.resolveIdentifier(_argParserPrivateLibrary, 'ArgParser'); // OK
    await builder.resolveIdentifier(_argsLibrary, 'ArgParserException'); // MacroImplementationExceptionImpl
  }
}

Output of dart --enable-experiment=macros run lib/main.dart:

lib/main.dart:4:2: Error: MacroImplementationExceptionImpl: Unable to find top level identifier "ArgParserException" in package:args/args.dart
null
@Args()
 ^
pubspec.yaml
name: args_macro_example

environment:
  sdk: ^3.5.0-164

dependencies:
  args: ^2.5.0
  meta: ^1.15.0
  macros: ^0.1.0-main.5
dart info
#### General info

- Dart 3.5.0-180.2.beta (beta) (Wed May 29 13:59:09 2024 +0000) on "macos_arm64"
- on macos / Version 13.6 (Build 22G120)
- locale is en-GE

#### Project info

- sdk constraint: '^3.5.0-164'
- dependencies: args, macros, meta
- dev_dependencies: 

#### Process info

| Memory |  CPU | Elapsed time | Command line                                                                               |
| -----: | ---: | -----------: | ------------------------------------------------------------------------------------------ |
|   9 MB | 0.0% |  17-00:11:03 | dart --enable-asserts --pause_isolates_on_start --enable-vm-service:51163 <path>/main.dart |
|   9 MB | 0.0% |  17-00:11:26 | dart --pause_isolates_on_start --enable-vm-service:51048 run test -r json <path>/main_test.dart |
|  10 MB | 0.0% |  72-17:35:11 | dart language-server --client-id=Android-Studio --client-version=AI-223.8836.35 --protocol=analyzer |
|  77 MB | 0.0% |  02-03:32:15 | dart language-server --client-id=Android-Studio --client-version=AI-223.8836.35 --protocol=analyzer |
| 175 MB | 0.0% |     04:48:58 | dart language-server --client-id=Android-Studio --client-version=AI-223.8836.35 --protocol=analyzer |
|  10 MB | 0.0% |  47-22:18:37 | dart language-server --client-id=Android-Studio --client-version=AI-223.8836.35 --protocol=analyzer |
|  10 MB | 0.0% | 237-19:22:21 | dart language-server --client-id=Android-Studio --client-version=AI-223.8836.35 --protocol=analyzer |
|  13 MB | 0.0% | 240-18:19:54 | dart language-server --client-id=Android-Studio --client-version=AI-223.8836.35 --protocol=analyzer |
| 479 MB | 0.0% |        32:47 | dart language-server --client-id=Android-Studio --client-version=AI-223.8836.35 --protocol=analyzer |
| 158 MB | 0.0% |  02-22:55:29 | dart language-server --client-id=Android-Studio --client-version=AI-223.8836.35 --protocol=analyzer |
|  13 MB | 0.0% |  18-18:31:53 | dart language-server --client-id=Android-Studio --client-version=AI-223.8836.35 --protocol=analyzer |
| 220 MB | 0.0% |  02-20:00:27 | dart language-server --client-id=Android-Studio --client-version=AI-223.8836.35 --protocol=analyzer |
|  18 MB | 0.0% |  05-23:20:06 | dart language-server --protocol=lsp --client-id=VS-Code --client-version=3.88.1            |
|  12 MB | 0.0% |  05-23:20:06 | dart tooling-daemon --machine                                                              |
|  56 MB | 0.4% |     04:48:59 | flutter_tools.snapshot daemon 

This is reproducible for me on:

  • The recent beta 3.5.0-180.2.beta.
  • The recent dev 3.5.0-202 with the latest macros package it supports: 0.1.1-main.0
@lrhn lrhn added area-pkg Used for miscellaneous pkg/ packages not associated with specific area- teams. feature-macros Implementation of the macros feature pkg-macros The experimental package:_macros library labels Jun 4, 2024
@davidmorgan davidmorgan added the M label Jun 6, 2024
@davidmorgan
Copy link
Contributor

Thanks! I had noticed this too, it is on a TODO list of mine somewhere to look into it :)

@scheglov @jakemac53 @johnniwinther I'm not sure where this sits in terms of specification / implementation; do we already specify that macros can refer to exported symbols instead of having to refer to the actual source location? If so--is this purely an implementation issue; and do we expect any problems? Thanks :)

@scheglov
Copy link
Contributor

scheglov commented Jun 6, 2024

Yes, AFAIK it should by exported symbols, not only declared symbols.
I know where the bug is in the analyzer :-)

@scheglov
Copy link
Contributor

scheglov commented Jun 6, 2024

copybara-service bot pushed a commit that referenced this issue Jun 6, 2024
Bug: #55910
Change-Id: Id1bf92ca2b95a33cd4feaf79e6c0a31f6d811c8d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/370102
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Phil Quitslund <pquitslund@google.com>
@davidmorgan davidmorgan added the cfe-feature-macros Implement macros features in the CFE label Jun 7, 2024
@davidmorgan
Copy link
Contributor

Thanks @scheglov! Then I think remaining is the corresponding CFE fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-pkg Used for miscellaneous pkg/ packages not associated with specific area- teams. cfe-feature-macros Implement macros features in the CFE feature-macros Implementation of the macros feature pkg-macros The experimental package:_macros library
Projects
Development

No branches or pull requests

4 participants