Debugger returns empty string as URI when paused inside a macro-generated script

[DanTup]

If I step into a macro-generated script with the debugger, the uri or the script is an empty string:

"topFrame": {
	"type": "Frame",
	"kind": "Regular",
	"location": {
		"type": "SourceLocation",
		"script": {
			"type": "@Script",
			"fixedId": true,
			"id": "libraries\/@19252696\/scripts\/\/18d790e6a87",
			"uri": ""
		},
		"tokenPos": 117
	},

We use these uris in the debug adapter to map back to source code to display to the user. It's not clear to me if this is a) a bug, b) just a known issue/incomplete or c) expected and the debug adapter will need to handle it.

@jakemac53 @bkonyi

Edit: I think actually we definitely needs these URIs, because it's the only way we can map back to the source code the user already has in the editor from the analyzer (otherwise we'd have to download the source from the VM which we generally don't want for VS Code because it looks like a duplicated file to the user).

[DanTup]

Unsure if related, but the String source is also missing if I send a getObject() request for that script:

{
	"jsonrpc": "2.0",
	"result": {
		"type": "Script",
		"class": { /* ... */ },
		"size": 80,
		"fixedId": true,
		"id": "libraries\/@19252696\/scripts\/\/18d792687ca",
		"uri": "",
		"_kind": "kernel",
		"_loadTime": 1707134584778,
		"library": {
			"type": "@Library",
			"fixedId": true,
			"id": "libraries\/@19252696",
			"name": "",
			"uri": "file:\/\/\/C:\/Dev\/Google\/dart-language\/working\/macros\/example\/bin\/observable_main.dart"
		},
		"lineOffset": 0,
		"columnOffset": 0
	},
	"id": "38"
}

(Also unsure if there should be two slashes in the ID scripts\/\/18d792687ca?)

[bkonyi]

If the Script responses from the service are missing source and uri properties, it's likely that they're not present in the loaded kernel. @johnniwinther I think the CFE isn't providing these details for macros, can you confirm?

Also unsure if there should be two slashes in the ID scripts//18d792687ca?

The script ID takes the form of libraries/$libId/scripts/$encodedScriptUri/$loadedTimestamp, so the double slash is due to the missing URI.

[johnniwinther]

The CFE doesn't currently generate the merged augmentation uri with the correct dart-macro+ uri. A CL is in progress: https://dart-review.googlesource.com/c/sdk/+/349864

[DanTup]

I'm seeing URIs with that change, thanks!

"topFrame": {
	"type": "Frame",
	"kind": "Regular",
	"location": {
		"type": "SourceLocation",
		"script": {
			"type": "@Script",
			"fixedId": true,
			"id": "libraries/@19252696/scripts/dart-macro%2Bfile%3A%2F%2F%2FC%3A%2FDev%2FGoogle%2Fdart-language%2Fworking%2Fmacros%2Fexample%2Fbin%2Fobservable_main.dart/18d843c57a3",
			"uri": "dart-macro+file:///C:/Dev/Google/dart-language/working/macros/example/bin/observable_main.dart"
		},
		"tokenPos": 117,
		"line": 5,
		"column": 21
	},

[DanTup]

Although, lookupResolvedPackageUris isn't returning what I'd expect. I would expect it to map dart-macro+package URIs into dart-macro+file in the same way it maps package: URIs into file: URIs.

Here's what I currently see:

==> {
	"jsonrpc": "2.0",
	"id": "19",
	"method": "lookupResolvedPackageUris",
	"params": {
		"isolateId": "isolates/2646600707904379",
		"uris": [
			"dart-macro+package:macro_proposal/observable_lib.dart"
		],
		"local": true
	}
}

<== {
	"jsonrpc": "2.0",
	"result": {
		"type": "UriList",
		"uris": [
			// This is not what I expected, I expected the `package:` part to be resolved to a `file:`
			"dart-macro+package:macro_proposal/observable_lib.dart"
		]
	},
	"id": "19"
}

@bkonyi is my expectation correct, and if so, is it related to this issue or should I file another? (I'm unfamiliar with where the data comes from that's used for these mappings).

[bkonyi]

The data for the mappings comes from the kernel's SourceInfo for the script, so it's likely something else that needs to be updated in the CFE.

If we expect that dart-macro+package: URIs map to dart-macro+file: URIS (I'm not sure where we landed on this), uriUtf8Bytes should be different from importUriUtf8Bytes in the SourceInfo for the script (see the kernel spec for details).

[DanTup]

If we expect that dart-macro+package: URIs map to dart-macro+file: URIS (I'm not sure where we landed on this),

I don't know if this has been specifically agreed, but I believe that we need this for the behaviour we're targetting.

The IDEs are unable to handle the mapping of package:s to file:s which is why we have lookupResolvedPackageUris. The debug adapter takes URIs from the VM (that may be package:) and maps them back to absolute paths that the editor is using for files.

The dart-macro+ prefix needs the same functionality.. the VM is using dart-macro+package: for things inside lib/ but we still need to map them to a dart-macro+file: equiv to match what the editor (and LSP) are using.

Without this, files outside of lib/ when debugged will use the analyzer-provided version but those inside lib/ would have to be downloaded from the VM (which is inconsistent and leads to confusion because the user can see multiple versions of the file - the one already in the editor and another from the debug session).

Should I file a separate issue about this (perhaps it needs some further discussion?).

[jakemac53]

It does sound to me like package:package_config and possibly the related spec need to be updated to handle dart-macro+ schemes.

This goes both ways, resolving a dart-macro+package: uri should give the resolved uri for the original library, with the dart-macro+ prefix. And, asking to look up the package for a dart-macro+<resolved-uri> should also give back the package of the <resolved-uri>.

cc @lrhn

[bkonyi]

The IDEs are unable to handle the mapping of package:s to file:s which is why we have lookupResolvedPackageUris. The debug adapter takes URIs from the VM (that may be package:) and maps them back to absolute paths that the editor is using for files.

The dart-macro+ prefix needs the same functionality.. the VM is using dart-macro+package: for things inside lib/ but we still need to map them to a dart-macro+file: equiv to match what the editor (and LSP) are using.

We definitely should be able to support this with the lookupResolvedPackageUris API, I just wasn't sure if we had decided on having both package and file URIs for generated macro scripts. If that's something we need to do, the CFE will need to provide both in the generated kernel and then it should just work with the lookupResolvedPackageUris API.

Without this, files outside of lib/ when debugged will use the analyzer-provided version but those inside lib/ would have to be downloaded from the VM (which is inconsistent and leads to confusion because the user can see multiple versions of the file - the one already in the editor and another from the debug session).

I was under the impression that both the analyzer generated code should be identical to the code generated by the CFE. Is that not the case?

Should I file a separate issue about this (perhaps it needs some further discussion?).

Maybe? I do think it's related to this issue and probably only requires a small change somewhere in this CFE CL (probably at this line) to support. I'll leave a comment.

[DanTup]

We definitely should be able to support this with the lookupResolvedPackageUris API, I just wasn't sure if we had decided on having both package and file URIs for generated macro scripts.

Oh, I see. I assumed that they'd both be needed for the same reason as the originals, but my knowledge of this area is poor :-)

I'm proceeding for now on the assumption that we will have both, and will wait for @johnniwinther's response. If he agrees and thinks it should be included in his fix, no need for another issue. If it's not clear that's the right direction, I'll file an issue for further discussion.

I was under the impression that both the analyzer generated code should be identical to the code generated by the CFE. Is that not the case?

Yep, that's my expectation (although I just discovered it's not currently - I filed https://github.com/dart-lang/sdk/issues/54848 about that). However if we don't make these URIs match, VS Code doesn't know that they're the same file - it just knows it has an analyzer-backed file with one URI, and a VM-downloaded file with another URI, and will show them to the user as if they are distinct files. We need both DAP and LSP to be using the same URIs for this to work correctly (and for LSP we must use unique file URIs, because package: URIs are not globally unique because the workspace can contain multiple versions of the same packages).

[lrhn]

I'd definitely prefer if we didn't need to resolve dart-macro+package:foo/bar.dart to anything. If we even can.

The URI is an identifier (I in URI) of generated code, it's not a locator for it (L in URL). It doesn't say where to find the code. If we resolve dart-macro+package:foo/bar.dart to dart-macro+file:///users/me/dart/foo/lib/bar.dart, then we still haven't given a file location. It's still an opaque resource identifier that doesn't point to the actual file (the actual bar.dart file isn't it, that's the source file the macro augmentation was created for). So we're replacing one resource identifier with another, without actually bringing anyone closer to finding a file. That sounds like a waste of work.

If we need to find "the macro augmentation for file:///.../bar.dart", I'd rather go the other way and convert bar.dart to a package: URI first, if possible, then add the dart-macro+ prefix to its scheme.

You can still have macro-augmentations for files outside of lib/, but each Dart library, identified by its URI, will only have one macro augmentation file URI. Which is not a package: URI.

I think everyone would be happiest if the code of an augmentation doesn't need to exist on disk at all. Only write it out if some tool is unable to work if it doesn't have physical files. And those files should not be inside the lib/ directory anyway, so they won't really have a package: URI. (That does mean that package_config needs to be updated to recognize dart-macro+package:foo/ to be in the foo package, because otherwise the file won't have a default language version.)

[DanTup]

To be clear, the reason we need this is not because we need to find files on disk, or even that we need to tie this script back to the original source file. We need it because it happens to be the unique URI that the editor is using to identify this content and we need the debugger and the editor to use the same reference.

So, it would be fine if we used dart-macro+file: everywhere IMO. I assumed that wouldn't be possible for the same reason that the VM is using package: URIs (and not file: URIs) today, but I don't know if that's accurate.

However it's not fine for us to only have dart-macro+package:foo/foo.dart because in the editor/analyzer, we might have multiple versions of package:foo (because you can have multiple projects open) and therefore this URI is not unique (whereas having a URI that is based on the file is, because the filesystem is).

That said, I don't know how (or if) this interacts with package_config/etc.. these URIs from my perspective are only required for mapping in the debugger (eg. via lookupResolvedPackageUris) to translate between what the client/editor is using and what the VM is using.

I guess in theory, I could drop the prefix from the scheme, use the existing mappings, and then put the prefix back.. however this feels like an assumption we might not want to bake into lots of places.

[DanTup]

these URIs from my perspective are only required for mapping in the debugger

Actually, it occurs to me that they may also be printed in stack traces in stderr.. Ignoring the ambiguity of the IDE wants to linkify this output (if there are multiple versions of a package in the workspace), the package: variants would be shorter than the file variants, so probably that's another reason to not use file variants everywhere (and therefore be able to map back and forth).

[lrhn]

I've implemented the minimum support for dart-macro+ in package:package_config, which is to recognize that dart-macro+uri belongs to the same package as uri. That's what's necessary to get a language version. (https://github.com/dart-lang/package_config/pull/148)

I can allow resolve to "resolve" a dart-macro+package:foo/bar to dart-macro+file:///somewhere/foo/lib/bar and let toPackageUri go in the other direction. But it doesn't feel right, because toPackageUri is defined as returning a package: URI, and dart-macro+package: is not a package URI. Similarly resolve accepts only package URIs, and that's not a package URI.

I don't want to start claiming that dart-macro+package is a valid package URI, because it's not.

I would actually prefer if we don't change package_config, and let the tools that actually know what a dart-macro+package URI is be the ones to remove the dart-macro+ before asking something about the related Dart file. The package_config package has one job: Read and understand .dart_tool/package_config.json, and that file has no relation to dart-macro+ URIs. It should be handled at another level, by tools which do know what those URIs mean.

[jakemac53]

It should be handled at another level, by tools which do know what those URIs mean.

The main thing is that we want all the tools to do this consistently, and if we handle it in this package we get a consistent implementation that handles any URI any tool might encounter. Otherwise should we publish another package that just removes the prefix before calling this package, and then adds it back? That doesn't seem right either.

I am still a bit confused about the exact issue that is happening here though, it does feel wrong to me for tools to be resolving the dart-macro+package uris in this way. It seems like more just a quirk of the fact that IDEs really want to deal in file URIs. So maybe it really is only the editor plugins that have to deal with this, and the logic should live there?

[DanTup]

I agree with most of the above.. I don't think this really affects package_config. I also think that ideally no tools should should be removing and re-adding these scheme prefixes. I think that you could easily get from dart-macro+file:///foo/bar.dart back to /foo/bar.dart is just an implementation detail that should be assumed only by the code that's generating these scripts.

It seems like more just a quirk of the fact that IDEs really want to deal in file URIs.

To be clear, the issue here is not about getting to file URIs (or back to the original script that caused this one to be generated), we just need to get to a unique URI for the script that is the same both inside the analyzer and the debug adapter. It doesn't matter whether this looks like a file URI or not.

When you click "Go to Definition" in your IDE (when there is no running app) you will get to a copy of the generated macro code. When you add a breakpoint in this file, we need to be able to send that breakpoint to the runtime when you start debugging, and that means we need a unique URI to refer to it by.

Today, we are working on the basis that this URI will be dart-macro+file://path/to/source/file.dart.. it doesn't particularly matter what it is, as long as the analyzer can generate it, and the debug adapter can translate back and forth between it and whatever the runtime is using. To me, these feels exactly like what we have with file paths and package: URIs (the runtime uses package:, but the editor/analysis server use file paths.. the debug adapter translates between them using lookupResolvedUris and lookupResolvedPackageUris).

[lrhn]

think that you could easily get from dart-macro+file:///foo/bar.dart back to /foo/bar.dart is just an implementation detail that should be assumed only by the code that's generating these scripts.

With one, sadly rather significant, exception. The semantics of

// library URI: package:foo/bar.dart
augment 'dart-macro+package:foo/bar.dart'; // Inserted by macro generation
// ...

which is the semantics that we want the post-macro-generation program to have, depends on dart-macro+package:foo/bar.dart to have the same language version as package:foo/bar.dart. The language version is determined by the package that dart-macro+package:foo/bar.dart belongs to. The package it belongs to is defined as:

• If it's a package URI, the package with that name in the active package configuration.
• If not, the (most deeply nested) package in the package configuration, whose root URI the URI is inside.

This is not a package URI, and it's not inside any file: URI, so it will not belong to any package.

The hack, which I have implemented, is to allow packageOf in package:package_config to recognize dart-macro+ and ask for the package of th rest of the URI. But it is a hack, it's hard-coding a convention into resolution. We can do that (it's just adding a "URI is dart-macro+scheme:rest, then belongs to same package as scheme:rest" rule above), but it is exposing the intermediate files of macro generation.

(I'd rather allow package URIs with queries, like package:foo/bar.dart?macro-generated as the way to define a URI for a macro-generated file. That's still a package: URI, not one which is currently valid, but we could change that - and then not allow users to write such URIs in imports anyway. And it can survives resolve and toPackageUri if we want it to. A query is part of the URI identifier, unlike a fragment, so it is a differnt URI than the one without the query. We just generally don't allow queries, so the syntactic space is free.)

[DanTup]

I'd rather allow package URIs with queries, like package:foo/bar.dart?macro-generated

I don't personally have any opinion on these URIs, but for the URIs we'll use to communicate with the editor, we couldn't do the same thing (eg. we can't use file:///foo/bar?macro-generated) because in order to provide these "virtual files" in the editor, they need to be a custom scheme. Anything starting file:/// is handled by the editor and expected to be on disk, whereas a custom scheme we can provide the implementation of.

There's certainly no reason we couldn't map between dart-macro+file:/// and package:...?macro-generated, although there is something nice and symmetrical with the prefix.

Whatever the specifics of the URIs, we do need some way to map between them in the debug adapter. Using lookupResolveUris and lookupResolvePackageUris feels natural to me (because it already handles package:<->file:/ mappings and this feels like the same kind of thing), but if there's a reason that won't work and we're happy to encode these rules into the debug adapter, that will probably work (and I'll probably do this locally for now to let me continue testing debugging).

[lrhn]

I do like the idea of using a query suffix instead of a scheme prefix. My one worry is that valid file URIs cannot contain queries.

Our Uri class can build strings starting with file: and ending with ?foo, but that doesn't make them valid file: URLs, and it may give us problems if such an URL ever flows into a tool that assumes, or enforces, valid file URLs.

I think the WhatWG URL spec does accept a query component on a file URL, though, which was my first worry. We do want to move Uri to a WhatWG URL compliant behavior, so we can use the built-in functionality in the browser. And probably stay there, so if that spec allows file URIs with queries, I wouldn't worry about using them.

Basically someUri?dart-macro is a URI that is adjacent to someUri, but different from it. Whatever we do to read such a file needs to be aware of the difference. Which may be a problem if an IDE sees a file has URI file:///..foo.dart?dart-macro and thinks "I can read that from disk!". If that's an issue, using a file: URI with a query suffix is probably not viable. (That's the "tool expecting valid file: URIs to be valid" case again, rather than throwing because it's invalid, it ignores the part that is invalid and misinterprets the URI.)

All in all, I think we can choose to say that file:///users/me/dart/pkgs/foo/tool/tool.dart?dart-macro is a valid Dart file-reference URI, but that users are not allowed to specify queries in import/export/part/augment URIs. The biggest issue is how other tools react to such an URI.

It would have the added advantage that an import inside a generated file will be resolved relative to the original file, because a base URI of package:foo/src/bar.dart?dart-macro used to resolve a reference of ../../baz.dart (notice one .. too many) directly gives package:foo/baz.dart, because resolving a relative URI against a base URI will discard the base query, and because it's resolved as a known package URI with all the necessary special casing we have made for package URIs. Without that, our compilers will need to recognize the base URI of a macro-generated augmentation file specially, and remove the dart-macro+ prefix before resolving against it. Otherwise a macro file dart-macro+package:foo/src/bar.dart with an import of ../baz.dart would resolve to dart-macro+package:foo/baz.dart, which is a macro-generated file, not the file we tried to import. (I assume that's already happening, but it means one more place where the compilers need to be aware that macro-generated files with weird URIs exist, and need special casing. It can't just use the normal semantics for anything related to the URIs of such files.

We could also use a fragment, #dart-macro. Those have the advantage that they apply to any URI, because they are not really a part of it. They're metadata-on-the-end of a URI> It's a little more iffy to distinguish import 'foo.dart'; and import 'foo.dart#dart-macro';, because at the URI level they identify the same resource, and the #dart-macro is an internal reference into that resource, which is not the case here.

[DanTup]

For the URI that is used to communicate between LSP and the editor, and the editor and the debug adapter, we definitely cannot use file:///. It's not just that the IDE might try to read from disk, we also have no way to support the contents of files unless they are for a custom URI scheme that we have registered.

So whatever is being used inside the runtime, we must ultimately convert this to a custom-scheme URI in the debug adapter to go to the client (and the LSP server must also use those same custom-scheme URIs to provide content and functionality like navigation/diagnostics).

With one, sadly rather significant, exception.

It occurs to me we probably have another exception here. In the debug adapter, we have settings to allow the user to debug "My code", "My code + external packages" or "My code + external packages + sdk". In order to determine what is "My code", we look at whether the resolved file URI is inside a set of workspace paths.

To handle the same for macros, we'll need to strip the dart-macro+ prefix from the custom-file URI to be able to do this same kind of check.

[lrhn]

So, generally, we need the dart-macro+url to behave like url in a number of cases:

• Is it inside the workspace.
• Is it inside a particular package in a package_config.
• Resolving file references inside it (import/export/part/augment)

Any time it matters where a Dart file is, a dart-macro+url should claim to be at the position of the url.

If we actually want to read the file, then that's not where it is (there is a completely different file there).

The question is then who should be stripping the dart-macro+ before asking? It has to be someone who knows the URIs exist. And we don't want everybody to have to know that.

The obvious choices are the tools that macro-compilation is integrated into: Compilers and the analyzer. Maybe the LSP.

The compilers and analyzer are the ones that need to ask about the package of a dart-macro+ URI. They can strip the prefix before asking, knowing that they are assuming that the macro-generated file has the same answer as the corresponding non-prefixed-URI file. (Which the macro engine should then ensure.) They are also the ones to resolve import URIs. I think those tools they should be the ones removing prefixes, at the point where they need to ask a question depending on location on a macro-prefixed URI. Downstream tools and packages shouldn't need to know.

To see whether the file is inside a specific workspace, the debug adapter may need to know to strip the prefix, and ask about another file, and again assume the answer to be the same. That's more annoying, but unless the analysis server starts providing a service to check whether a URI is included below another URI, so it can do the prefix-twiddling, the code that actually knows what the the workspace location is, will have to know about macro-URIs too, if it wants them to be included. (Exposing a general "Is this URI morally inside that URI" functionlity, which can account for things like resolving package URIs or removing macro-prefixes, might be an idea. The "does this file have a package URI" functionality is an example of this, which is language and package-config dependent.)

[DanTup]

The obvious choices are the tools that macro-compilation is integrated into: Compilers and the analyzer. Maybe the LSP.

I don't think the LSP server needs to know this directly (it wraps the analyzer which should take care of most of this, minus some custom mapping because the analyzer currently uses paths and not URIs in many APIs). The debug adapter does need to know, only for the "is in workspace" check.

One thing that might be helpful in these discussions is defining some terms for these different kinds of URIs. Originally in the debug adapter, we just had two things:

1. File paths - these are what the editor always used. When the debugger breaks at a breakpoint, we had to tell the editor a file path (something physically on disk) for it to open that file (ignoring a slight niggle of downloading source code from the VM, but that uses internal incrementing integer references unrelated to any paths/URIs from the VM)
2. URIs - these were used in the VM (for example we might see package:foo/foo.dart or dart:io)

We ended up with some helpers to convert between Paths and URIs and it seemed fairly simple. Then we started mapping SDK sources and started dealing with org-dartlang-sdk URIs. Today, file paths are really going away entirely and instead we need to use URIs everywhere (because the client is no longer restricted to physical files).

So now we have at least three "kinds" of URIs in the debug adapter. I don't know if these already have names, but I think it would be useful to define them (so we can name APIs the same across tools and be on the same page in discussions). For example:

1. The URIs in the VM and used for importing
   • dart:io
   • package:foo/foo.dart
   • (maybe file:/// for things outside of lib/??)
   • dart-macro+package:/// ?
2. The URIs that the VM returns when we ask it to resolve those URIs (via lookupResolvedPackageUris).. for non-SDK sources, these are generally file:/// URIs, but for SDK sources they are org-dartlang-sdk URIs
   • org-dartlang-sdk:///...
   • file:///...
   • dart-macro+file:/// ?
3. The URIs we can send to the client. In the debug adapter I'm currently referring to these as "file-like URIs". Previously they would be Strings and called "paths", but that doesn't work now. These are generally the same as (2) but for SDK sources the org-dartlang-sdk URI is converted to a real file path for the source inside the SDK (the same place the user would end up if they navigated into SDK sources via the analyzer)
   • file:///my/dart-sdk/core/iterable.dart
   • file:///my/projects/foo/lib/foo.dart
   • dart-macro+file:///my/projects/foo/lib/foo.dart

(1) is what we see in the VM and will show up in things like call stacks in the debugger view. In the debug adapter, we will convert them to (2) using lookupResolvedPackageUris and then to (3) using some mappings in the debug adapter. Those are what we'll use to communicate with the editor so it can open those files.

In the opposite direction, the editor will send us (3) (for example when the user adds a breakpoint), we'll convert it to (2) with the built-in mappings in the debug adapter, and then use lookupPackageUris to convert back to (1) to send to the VM (having "package" in the name of that API is a bit confusing, but it is what it is).

The check the debug adapter does for whether a URI is "my code" will take (3) and just do the (unfortunate, but simple and probably necessary) stripping of the URI prefix and then compare the result against the workspace roots it knows about.

[lrhn]

I don't thing we need to "resolve" a dart-macro+package:foo/bar to dart-macro+file://home/me/dart/pkg/foo/lib/bar.

It's a non-package: URI, so you can just use it directly. If the macro generated files are never saved to disk, then it doesn't matter whether you ask the source for dart-macro+package:... or dart-macro+file:...`, they are both opaque URIs, and if anything, things might get harder for ourselves if we try to give them two URIs instead of just one.

In a way, dart-macro+package:... and dart-macro+file:... are magical URIs just like org-dartlang-sdk:.... They only make sense when passed used with tools that understand what they mean, and in this case, only if the tools actually know how to generate the file content, if it hasn't already. If we're asking the analyzer about the URI that a front-end compiler generated, the analyzer will just have to generate the same content.

There is no benefit in converting dart-macro+package:... to dart-macro+file:... if the corresponding file doesn't exist anyway. There will be dart-macro+file:... URIs too, from non-lib/-dir libraries.

I think the main reason we have the org-dartlang-sdk: URIs is so we can give a URI to part files. If we could name those as dart:io/file_system_entity.dart, and just disallow references to URIs of that form from non-platform libraries, I don't think we need the extra org-dartlang-sdk: scheme.

(If a macro generated file contains a macro application, will it get a dart-macro+dart-macro+package:... URI? I guess the plan is to flatten macro generated files for a main library into one level. That requires the files to be entirely generated with all imports uniquely prefixed, otherwise we can't flatten augment files with different imports.)_

[DanTup]

I don't thing we need to "resolve" a dart-macro+package:foo/bar to dart-macro+file://home/me/dart/pkg/foo/lib/bar.

I'm not sure exactly what "resolve" means here. If you mean support in package_config or an import, then you may be correct. However, for the debugger and analysis server integration I believe we do need to handle this for the same reason that we already convert package:foo/bar.dart to a file:/// URI (via lookupResolvedPackageUris) in the debug adapter.

The editor needs to communicate with both the debug adapter and analysis server and they must all have a shared understanding of the same unique URIs for any script. dart-macro+package:foo/bar.dart is not unique because in a workspace with multiple projects, it is ambiguous.

Eg.:

• When the user invokes Go-to-Definition, the analysis server must return a URI to the editor
• At any point, the editor may ask the analysis server for the content of the script with that URI (no other context - such as which project - is given)
• The user may add a breakpoint in this file
• When a debug session starts, the editor will send this URI to the debug adapter and ask it to add a breakpoint (the user may have breakpoints in multiple different versions of the same package in their workspace, and the editor doesn't know the difference, so it cannot selectively only send the breakpoints of files that will be loaded by the application being run)

If we're asking the analyzer about the URI that a front-end compiler generated, the analyzer will just have to generate the same content.

Sure, but the analysis server can't resolve package:bar with no context. If the debugger says "Execution paused in dart-macro+package:foo/bar.dart on line 6" and the analyzer has 2 copies of package:foo in two different contexts (because the user has two projects open in the same session), it can't know which one to generate.

I think the main reason we have the org-dartlang-sdk: URIs is so we can give a URI to part files. If we could name those as dart:io/file_system_entity.dart, and just disallow references to URIs of that form from non-platform libraries, I don't think we need the extra org-dartlang-sdk: scheme.

Maybe, but we still need to convert those URIs back to file:/// URIs to send to the editor, because if the user did Go-to-Definition into an SDK source, it would go to the version on-disk in the SDK (there may be no VM running). We keep a mapping of file paths to URIs for this purpose:

https://github.com/dart-lang/sdk/blob/78dbc184b151440684faa525f63155c5230411ed/pkg/dds/lib/src/dap/adapters/dart.dart#L500

To me, there's nothing different in mapping dart-macro+package<->dart-macro+file to what we're already doing for non- macros (package:<->file:/ and dart:foo/org-dartlang-sdk<->file:/), it's just more of the same for the new scheme.

If a macro generated file contains a macro application, will it get a dart-macro+dart-macro+package:... URI? I guess the plan is to flatten macro generated files for a main library into one level.

I believe they are flattened.. @jakemac53 is probably best to comment on that though.

[jakemac53]

Yes macro generated files are all flattened into one at the end.

[lrhn]

The editor needs to communicate with both the debug adapter and analysis server and they must all have a shared understanding of the same unique URIs for any script. dart-macro+package:foo/bar.dart is not unique because in a workspace with multiple projects, it is ambiguous.

That does make sense. Converting it to a dart-macro+file: URI will place it uniquely next to the actual file that generated it, which puts it into a workspace.

(An option could perhaps be to have files represented internally in the compiler(s) as something other than a URI, a URI + more, and keep the ID of a macro generated library as the URI of the original file plus a "macro-generated" flag. Then only convert that to dart-macro+.... when a textual represenation is needed for some reason. The compiler won't need to switch back and forth between dart-macro+package: and dart-macro+file: URIs, all URI operations are just performed on the URI-part of the library ID, and the "macro-generated" flag is retained. Other tools which understand the notation can have the same separation internally, parsing a dart-macro+package:... URI denoting a file into a file-locator of package:...+ macro-generated flag. If anything, if this isn't possible, we're leaking details from the tools that know about macros to tools that probably shouldn't.)

[DanTup]

Do you mean that the VM Service would return the dart-macro+file:/// URI in things like the stack trace?

If yes, I think that would work but it feels odd to use them for these when we don't use them for non-generated package: URIs (the VM returns package: URIs and then the debug adapter maps them with lookupResolvedPackageUris).

If no, then it's not clear to me what they would return - the APIs today have URIs, but if we need a URI + more to unique indicate the script, we'd need to add the +more to many APIs (and change all code that consumes them).

I'm not familiar enough with the internals to have a real opinion here (beyond needing a unique URI consistent URI from both analysis_server and runtime), I'd be interested in @jakemac53 and @bkonyi's thoughts as I think they both understand my requirement, but also have a much better understanding of the other parts than I.

[jakemac53]

Based on the context from above, repeating some things I am sure, my logic here goes as follows:

Identifying URIs (for IDEs)

• In a context with multiple package configs (workspaces, effectively here), package: URIs are ambiguous.
• Therefore, any tool which has to deal with multiple package configs must resolve package: to their actual URI (usually, a file: URI), using the appropriate package config based on the context.
• A dart-macro+package: URI is also ambiguous, it's corresponding contents may not even be the same across projects.
• Therefore, these tools must also resolve dart+macro-package: URIs to some unambiguous URI.
• The obvious unambiguous URI to choose for this, is the "same" one that the equivalent package: URI would resolve to, with dart-macro+ prefix.
  • obvious because it is user intuitive, will look nice in the IDE, and it just makes sense.

Stack traces

• Macro generated code URIs should always be easy to match up with the URI of the library they apply to.
• Therefore, the URI that should appear there should depend on what the tool is generally doing. Most tools, most of the time show package: URIs, and so they should also show the corresponding dart-macro+package: URIs. But if they show the full resolved URIs, they should show the macro URIs resolved as well.

[lrhn]

• Therefore, any tool which has to deal with multiple package configs must resolve package: to their actual URI (usually, a file: URI), using the appropriate package config based on the context.

That's not a given, just an implementation choice. (Possibly one that's hard to change, though.)

A Dart package: URI is a package configuration local identifier. Disambiguating it means identifying the package configuration it's local to. Then that uniquely defines what the package: URI means, what its contents are. That happens to be implemented by resolving the package: URI to a file: URI and reading the file.

If a tool can resolve it to a file: URI, then it has an associated package configuration. That package configuration comes from a package configuration file. The pair of package: URI and package configuration file (location) unambiguously identify what a package URI refers to. If a dart-macro+package: URI is also combined with a package configuration, then that should also disambiguate what it refers to (the macro generated code for the unambiguous file).

(We could consider instrumenting all package: URIs with a query containing the path to the package configuration file. Then package: URIs, post instrumentation, are unique, and are still just a single URI. Not as pretty, but we don't have to print that query.)

Therefore, these tools must also resolve dart+macro-package: URIs to some unambiguous URI.

Or not use a plain URI as file identifier when there is more than one package configuration in play. If that is possible.

Now, if the IDE needs a single URI, then we can't just use a URI+config-path pair as identifier. We can embed the configuration in the URI. It's ugly, but consistent. Converting to a file: URI is useful too, and when the Dart file is a file-system file, it's probably more efficient to point the IDE to that. For a Dart dart-macro+package: URI which is not backed by a file:-file, converting its URI to a file:-like thing is an indirection. Combining it with the location of the package-configuration would disambiguate, and would not change it to something it's not, and derive properties indirectly from the underlying file: path.

(If I have one Pub package that has a path dependency on another package, and I have both packages open in the same IDE, then the same file should be able to have different package resolutions depending on which project, which package configuration, it's considered part of. I guess we just choose one of them. If we could identify files per package-configuration, then we could treat that one file as two files with the same path. I'm sure that would confuse the IDE horrendously, though, especially if we start editing both.)

[jakemac53]

Sorry I didn't finish reading your comment before I was responding.

While yes we could include the package config path in the canonical URI we give to the IDE, we cannot reasonably hide that from users, because it would have to appear in stack traces etc?

We also do want to tell the IDE the actual file path, when a real file exists, because it should be writable often times.