[AST] More JSON AST dump improvements. #80498

allevato · 2025-04-03T16:13:13Z

These improvements came about through great back-and-forth in this forum thread. They are summarized as follows:

Instead of using a custom-rolled type transformer, call mapTypeOutOfContext() to replace any primary archetypes with type parameters when computing a type USR.
If the type still contains local archetypes, replace them with an equivalent existential.
When dumping type inheritance, print the derived semantic information (all protocol conformances, raw type for enums, superclass/inherited protocols, etc.).
Dump additional information about protocol conformances (safety, preconcurrency, isolation, etc.). (I also decided to do this in the default dump, since it seems useful there.)
Add a source-file-level mapping of opaque type USRs to the USR of the equivalent existential type.

- For type USRs, use `mapTypeOutOfContext()` to replace primary archetypes with type parameters. If the type contains local archetypes, replace them with their existential upper bounds. - For inheritance of types, print the derived semantic information instead of the inheritance list as it is written. - Dump conformance requirements as protocol decl USRs instead of type USRs, since the latter involves an existential conversion that loses suppressed protocols. - Fix a small bug in conformance dumping where the `MemberLoading` field wasn't propagated to the nested printer, which caused protocols to be dumped as simple names instead of USRs.

The earlier changes to ASTDumper only printed information like retroactiveness and safety when printing the `InheritedEntries` of a decl. Now that we print the actual resolved semantic information instead of the inheritance list as-written, we need to surface these values via the `ProtocolConformance`s.

allevato · 2025-04-03T16:13:52Z

@slavapestov Thanks for all the pointers for these improvements. Would you mind reviewing this change please?

lib/AST/ASTDumper.cpp

slavapestov · 2025-04-03T16:29:21Z

lib/AST/ASTDumper.cpp

+          // decl instead of the type. The type USR for a protocol is based on
+          // the equivalent expanded existential, which drops suppressed
+          // protocols.
+          if (requirement.getKind() == RequirementKind::Conformance) {


Maybe switch over the kind?

Done, and used a fallthrough instead of a separate else-branch to handle the non-parsable case, reducing some duplication.

Realized I had missed an opportunity to turn the entire larger conditional into part of the same switch—printing the protocol name instead of the type yields the same result as printing the type in non-parsable mode, so we don't need to treat isParsable() as a special case at all here.

slavapestov · 2025-04-03T16:30:39Z

lib/AST/ASTDumper.cpp

+                Label::always("conformances"));
+
+            if (auto CD = dyn_cast<ClassDecl>(DC); CD && CD->hasSuperclass()) {
+              printReferencedDeclField(CD->getSuperclassDecl(),


More generally there is a superclass type getSuperclass() which is not just a decl. Eg,

class G<T> {} class B: G<Int> {}

Switched to getSuperclass() for classes.

For a class-constrained ProtocolDecl, it looks like I don't have a Type-returning getSuperclass() there. But the full type is accessible in the requirement_signature that's being dumped; in your example there's a subclass_of requirement that contains the mangled G<Int>. So that'll just have to be a different access path for clients.

slavapestov · 2025-04-03T16:32:15Z

lib/AST/ASTDumper.cpp

+          [&](Label label) {
+            printHead("opaque_to_existential_mapping", FieldLabelColor, label);
+            for (const auto OTD : opaqueDecls) {
+              Type interfaceType = OTD->getDeclaredInterfaceType();


calling getDeclaredInterfaceType() on OpaqueTypeDecl should produce an assertion failure; are you sure you have test coverage for this?

Hmm, it seems to be working? When I dump an OpaqueTypeDecl in the context of the declaring function, declared_interface_type is one of the fields we dump (line ~2158). For this snippet:

func f() -> some Sequence { [] }

I get

"opaque_result_decl": { "_kind": "opaque_type", ... "declared_interface_type": "$s3lib1fQryFQOyQo_D",

and the top-level mapping is

"opaque_to_existential_mapping": { "_kind": "opaque_to_existential_mapping", "$s3lib1fQryFQOyQo_D": "$sST_pD" }

There are tests exercising opaque types in the omnibus Frontend/ast-dump-json-no-crash.swift test.

I used getDeclaredInterfaceType() because it seemed to be the most likely match for what would appear in expressions that use the type, but I'm also happy to switch it to something else more appropriate.

I see, I missed that we set it in OpaqueResultTypeRequest::evaluate(). My apologies.

However, my original concern is that opaque return type can appear in a nested position, and we might have more than one opaque return type, eg:

func f() -> (some Any, some Any)? {}

Indeed, it appears that the declared interface type of the opaque result declaration is the entire return type of the function in this case, (some Any, some Any)?, so your castTo<OpaqueTypeArchetypeType>() will fail. You probably want to walk the declared interface type to collect all opaque archetypes that it contains.

Ah, right. Updated this to use transformRec to replace any OpaqueTypeArchetypeTypes with their getExistentialType(), and added a handful of tests that cover different structural situations.

build/.cmake/api/v1/query/client-vscode/query.json

existentials.

allevato

Thanks!

build/.cmake/api/v1/query/client-vscode/query.json

lib/AST/ASTDumper.cpp

allevato · 2025-04-03T16:42:11Z

lib/AST/ASTDumper.cpp

+          // decl instead of the type. The type USR for a protocol is based on
+          // the equivalent expanded existential, which drops suppressed
+          // protocols.
+          if (requirement.getKind() == RequirementKind::Conformance) {


Done, and used a fallthrough instead of a separate else-branch to handle the non-parsable case, reducing some duplication.

allevato · 2025-04-03T16:54:57Z

lib/AST/ASTDumper.cpp

+          [&](Label label) {
+            printHead("opaque_to_existential_mapping", FieldLabelColor, label);
+            for (const auto OTD : opaqueDecls) {
+              Type interfaceType = OTD->getDeclaredInterfaceType();


Hmm, it seems to be working? When I dump an OpaqueTypeDecl in the context of the declaring function, declared_interface_type is one of the fields we dump (line ~2158). For this snippet:

func f() -> some Sequence { [] }

I get

"opaque_result_decl": { "_kind": "opaque_type", ... "declared_interface_type": "$s3lib1fQryFQOyQo_D",

and the top-level mapping is

"opaque_to_existential_mapping": { "_kind": "opaque_to_existential_mapping", "$s3lib1fQryFQOyQo_D": "$sST_pD" }

There are tests exercising opaque types in the omnibus Frontend/ast-dump-json-no-crash.swift test.

allevato · 2025-04-03T16:59:47Z

lib/AST/ASTDumper.cpp

+                Label::always("conformances"));
+
+            if (auto CD = dyn_cast<ClassDecl>(DC); CD && CD->hasSuperclass()) {
+              printReferencedDeclField(CD->getSuperclassDecl(),


Switched to getSuperclass() for classes.

For a class-constrained ProtocolDecl, it looks like I don't have a Type-returning getSuperclass() there. But the full type is accessible in the requirement_signature that's being dumped; in your example there's a subclass_of requirement that contains the mangled G<Int>. So that'll just have to be a different access path for clients.

allevato · 2025-04-08T13:22:07Z

@slavapestov Any other changes you'd like to see here?

allevato · 2025-04-11T15:51:04Z

@swift-ci please test

allevato · 2025-04-22T12:55:29Z

@swift-ci please smoke test

allevato · 2025-04-22T20:18:24Z

@swift-ci please smoke test

allevato · 2025-04-23T16:12:47Z

@swift-ci please smoke test Linux platform

[AST] More JSON AST dump improvements.

allevato added 2 commits April 2, 2025 08:55

allevato requested review from xymus, kavon, hborla, slavapestov, artemcm, tshortli and xedin as code owners April 3, 2025 16:13

slavapestov reviewed Apr 3, 2025

View reviewed changes

allevato added 2 commits April 3, 2025 12:37

Also show the new conformance details in the default dump.

fa1f829

Add a source-file-level mapping of opaque type USRs to equivalent

ef6352c

existentials.

allevato commented Apr 3, 2025

View reviewed changes

allevato force-pushed the json-usr-fixes branch 2 times, most recently from c2a4ec0 to 1ad6596 Compare April 3, 2025 19:57

Address review feedback.

5be2498

allevato force-pushed the json-usr-fixes branch from 1ad6596 to 5be2498 Compare April 4, 2025 13:12

slavapestov approved these changes Apr 22, 2025

View reviewed changes

allevato enabled auto-merge April 22, 2025 12:55

allevato merged commit a3993f9 into swiftlang:main Apr 23, 2025
5 checks passed

allevato deleted the json-usr-fixes branch April 24, 2025 11:50

allevato mentioned this pull request Apr 24, 2025

[🍒 6.2] [AST] More JSON AST dump improvements. #81069

Merged

allevato added a commit that referenced this pull request Apr 24, 2025

Merge pull request #80498 from allevato/json-usr-fixes

8ca0b41

[AST] More JSON AST dump improvements.

[AST] More JSON AST dump improvements. #80498

[AST] More JSON AST dump improvements. #80498

Uh oh!

Conversation

allevato commented Apr 3, 2025

Uh oh!

allevato commented Apr 3, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

allevato left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

allevato commented Apr 8, 2025

Uh oh!

allevato commented Apr 11, 2025

Uh oh!

allevato commented Apr 22, 2025

Uh oh!

allevato commented Apr 22, 2025

Uh oh!

allevato commented Apr 23, 2025

Uh oh!

Uh oh!

Uh oh!