1732574 - Go to definition should automatically look-through/bypass IPDL's auto-generated aliasing typedefs

Status: NEW

(Webtools :: Searchfox, enhancement)

Description

[Andrew Sutherland [:asuth] (he/him)]

Compiled IPDL generates aliasing typedefs inside class definitions like:

typedef mozilla::ipc::PrincipalInfo PrincipalInfo;

which searchfox considers as the definitions to "go to definition" for when the type is referenced in the class/subclasses.

While it's potentially notable that the typedef exists, I don't think there's any useful benefit to dropping someone at that typedef and requiring them to "go to definition" again, which is what they probably wanted. That said, it could make sense to follow the wiki idiom of adding a parenthetical "(Redirected from Thing you actually typed)" which could be exposed as "(Bypassed aliasing type definition at PBackgroundSessionStorageCacheParent.h:66)".

Exactly how this happens is TBD because of potential upcoming changes to the jumps/searches lists and potentially richer semantic linking info, but I think the above UX makes sense.

Relatedly, it would be preferable for PrincipalInfo in particular to go to the IDL definition in PBackgroundSharedTypes.ipdlh rather than the current location in the generated PBackgroundSharedTypes.h. I think my current thinking might be that we would have "Go to IDL Definition", "Go to C++ Impl Definition", and then maybe variations for other extant language bindings. Or maybe the list is just: Go to IDL def, Search IDL def (which lists all the impl defs), go to current lang impl def.

Also, I expect this general rationale to still hold even if IPDL compilation were to change its exact output here.

Comment 1

[Andrew Sutherland [:asuth] (he/him)]

I'm running into aspects of this for class diagram purposes where the current hacky mechanism for pointer type resolution uses pretty identifiers which can get tricked by these convenience typedefs. While that won't be a problem longterm as it makes sense to have the MozsearchIndexer just emit a structured type rep that uses symbols instead, I'm digging into this a little more. Here be details:

For something like:

    typedef ::nsIContentSecurityPolicy nsIContentSecurityPolicy;

We see analysis records (without a structured record) like:

{"loc":"00149:14-38","target":1,"kind":"use","pretty":"nsIContentSecurityPolicy","sym":"T_nsIContentSecurityPolicy","context":"mozilla::dom::PMessagePortChild","contextsym":"T_mozilla::dom::PMessagePortChild"}
{"loc":"00149:39-63","target":1,"kind":"def","pretty":"mozilla::dom::PMessagePortChild::nsIContentSecurityPolicy","sym":"T_mozilla::dom::PMessagePortChild::nsIContentSecurityPolicy","context":"mozilla::dom::PMessagePortChild","contextsym":"T_mozilla::dom::PMessagePortChild"}
{"loc":"00149:14-38","source":1,"syntax":"type,use","pretty":"type nsIContentSecurityPolicy","sym":"T_nsIContentSecurityPolicy","type":"class nsIContentSecurityPolicy","typesym":"T_nsIContentSecurityPolicy"}
{"loc":"00149:39-63","source":1,"syntax":"def,type","pretty":"type mozilla::dom::PMessagePortChild::nsIContentSecurityPolicy","sym":"T_mozilla::dom::PMessagePortChild::nsIContentSecurityPolicy"}

Compare with the actual definition:

class nsIContentSecurityPolicy : public nsISerializable {

Which gives us source/target records (eliding structured record):

{"loc":"00050:6-30","target":1,"kind":"def","pretty":"nsIContentSecurityPolicy","sym":"T_nsIContentSecurityPolicy","peekRange":"50-50"}
{"loc":"00050:40-55","target":1,"kind":"use","pretty":"nsISerializable","sym":"T_nsISerializable","context":"nsIContentSecurityPolicy","contextsym":"T_nsIContentSecurityPolicy"}
{"loc":"00050:6-30","source":1,"syntax":"def,type","pretty":"type nsIContentSecurityPolicy","sym":"T_nsIContentSecurityPolicy","nestingRange":"50:56-170:0"}
{"loc":"00050:40-55","source":1,"syntax":"type,use","pretty":"type nsISerializable","sym":"T_nsISerializable","type":"class nsISerializable","typesym":"T_nsISerializable"}

In MozsearchIndexer.cpp we explicitly decide that typedefs count as "def"s of a "type" which is also exactly what we do for non-forward tag decls. We also mark uses of a typedef as a "use" of a "type". We also mangle them just like tags with a "T_" prefix. Notably, since we don't generate any structured records for tyepdefs and the target records lack the "typesym" , we have nothing to glue the typedef alias name with its target type which makes it hard to pierce the alias.

Brief investigation into what happens for a use of such a typedef is that for our source records, our "pretty" and "type" are both the typedef's pretty (although the "type" lacks the "type" prefix of the "pretty"), but the "typesym" does end up referring to the underlying type. This makes sense because our emission logic explicitly uses getCanonicalType for the "typesym" but we intentionally wanted exactly what was typed for a given definition for "type". We do not have a "typesym" for the "def" of the typedef because in VisitNamedDecl we only initialize the qtype that would be the MaybeType used to compute the "typesym" if we can cast the NamedDecl to a ValueDecl which the typedef of course is not.

Straightforward-ish changes:

• Change the mangling for typedefs to have a "TA_" prefix instead of a "T_" prefix. This addresses the pointer type info hack which currently looks for "T_" prefixes.
• Change the "def" kind for typedef definitions to a new "alias" kind that is below "def" and "decl" for ordering purposes on the "search" endpoint UI. In general there aren't really any problems right now because the IPDL typedefs are all under the "generated" pathkind and what people care about is the IDL definition under the "normal" pathkind. But in our example of "nsIContentSecurityPolicy", right now there are 35 "defs" under "generated", with 34 of them being the IPDL typedefs and 1 being the XPIDL generated nsIContentSecurityPolicy impl def, and arguably that should not be mixed in with the aliases. This also implies:
  • Create an "aliases" crossref key.
  • Teach the helpers in crossref_converter.rs to add an "alias" jumpref. This is also where we'd contribute the target symbol to "extra_syms" so the context menu could potentially pierce the alias, but as noted above, we don't have that info right now. (Except in source records which crossref can't/won't consult.)

More complex things that need more thought:

• Make searchfox specifically aware of the alias relationship. Possibilities include:
  • Generate a "typesym" for the "alias" kind target record that allows crossref to understand the relationship between the types.
  • Generate a structured record for the type alias / "using" aliases. We might potentially want to break this down into "boring" aliases like the IPDL case where we're just propagating a type name into the local scope and "notable"/"novel"/"interesting" aliases like when we define a type where the alias name is a more useful descriptor than its expansion which is an invocation of a template with specific parameters.
    • For boring aliases we'd want to bias towards piercing the type.
    • For notable aliases we might want to create a Petname-ish system where we potentially would want to go out of our way to be able to map back from the expanded type to the alias and/or tightly associate any specific template parameters when operating in the context of the type. For example, nsString and nsCString are both typedefs (nsTString<char16_t> and nsTString<char>, respectively). This involves thinking about/improving searchfox's handling of templates as well, though.