spicy icon indicating copy to clipboard operation
spicy copied to clipboard

`CodeGen::typeDependencies` does not emit all dependencies

Open bbannier opened this issue 4 years ago • 2 comments

We use CodeGen::typeDependencies to compute dependencies of a type, e.g., when emitting linker joins. The intention for this function is to recurse over a type and all its fields and emit a full list of dependencies. Looking at its output for e.g., types requiring recursion, it seems like that function never actually recurses and instead returns an empty list, e.g.,

module DNS;
type Name = unit {};

type Pointer = unit {
    name: Name;
};
type Label = unit {
    pointer: Pointer { $$.name; }
};

If passed Label the expectation would be that the typeDependencies returns a list containing cxx declarations for Label, Pointer, and Name.

I haven't ultimately tracked down the behavior, but what we see might be due to the use of a cache: To compute type dependencies we instantiate a VisitorDeclaration with a Cache held by CodeGen. When visiting a type::Struct we then try to get a result from the cache or compute an update via a lambda. The lambda mutates VisitorDeclaration::dependencies, but returns only the cxx::declaration::Type for the visited type::Struct. Since the dependencies are not explicitly tracked in the Cache it seems that only the first invocation of VisitorDeclaration will actually update dependencies while all later invocations will leave them empty (bypass of the lambda since we have a cache hit).

This does not seem to cause issues in existing code since we seem to currently emit enough information.

bbannier avatar Jul 02 '21 08:07 bbannier

@bbannier please see if this is still relevant (not immediately clear to me how to test)

rsmmr avatar Oct 06 '21 09:10 rsmmr

@rsmmr, I think this is still an issue. To check this I added the following code near the end of CodeGen::compileModule,

    for ( const auto& t : x ) {
        std::cerr << "Dependencies for type " << t << ": ";
        for ( const auto& tt : typeDependencies(t) )
            std::cerr << " " << tt.id;
        std::cerr << '\n';
    }

For above sample this yields the following:

Dependencies for type DNS::Name:
Dependencies for type DNS::Pointer:
Dependencies for type DNS::Label:

bbannier avatar Oct 21 '21 14:10 bbannier