Analyzer summaries do not encode inferred args for constant constructor invocations

[stereotype441]

Consider the following code:

class C<T> {
  final T t;
  const C(this.t);
}
const Object x = const C(0);

Type inference infers that the constant constructor invocation is const C<int>(0). But this information is not recorded in the summary. Therefore, if x is reconstituted from a summary and constant evaluated, and the type of x's constant value is queried, the result will be C<dynamic>.

[stereotype441]

To fix this, we need to make the following modifications:

• The summary IDL needs a place to store an optional type inference slot associated with each const constructor invocation (maybe do it for non-const constructor invocations as well, just in case).*
• The unlinked summary builder needs to allocate a type inference slot whenever it sees a const constructor invocation that doesn't have explicit type parameters.
• The linker needs to store the inferred type of each a const constructor invocation in the type inference slot, if present.
• The resynthesizer needs to detect if there is an inferred type stored in the type inference slot; if there is, it needs to point the resynthesized constant AST to the appropriate ConstructorMember. Then the ConstantEvaluationEngine should do the right thing automatically.

*Note that this will need to be done carefully to avoid backward compatibility problems. E.g. you can't just add the slot to the UnlinkedExpr.ints list, because then old and new analyzers will disagree about where they are in the ints array when evaluating a constant. Probably we should just add a new field, UnlinkedExpr.constructorInvocationTypeSlots.

[matanlurey]

@stereotype441 @devoncarew Should I assume this will never get fixed due to summaries not sticking around long-term?

The hacks I have in to "pretend" to do inference are pretty scary, and I'd hate to have to rely on them longer than we have to.

[matanlurey]

Ping @stereotype441 @devoncarew

[stereotype441]

Yeah, I think it's safe to assume this won't get fixed until we switch over to using the common front end. :(

[matanlurey]

Thanks!

[leonsenft]

Yeah, I think it's safe to assume this won't get fixed until we switch over to using the common front end. :(

@bwilkerson @stereotype441 This issue recently came up for a customer during the null safety migration. Considering that the analyzer didn't end up using the CFE, could we look into this again?

[stereotype441]

@scheglov, who has taken over the analyzer summary work.

[scheglov]

The test case in the first message already works, probably since about 2020.
I will add a couple of tests for it https://dart-review.googlesource.com/c/sdk/+/323228, mostly to show that it works.

[leonsenft]

Perhaps the test case here was fixed, but I believe we're still able to reproduce the issue with a more complex example. Please see the internal issue for a reproduction.

[scheglov]

What makes you think that there is still an issue in the analyzer?

[leonsenft]

I think I figured out the issue.

The code in question looked like:

const tokenA = OpaqueToken<List<A>>();
const tokenB = OpaqueToken<List<B>>();

List<A> createListA() => ...;
List<B> createListB() => ...;

const provider = [
  FactoryProvider.forToken(tokenA, createListA),
  FactoryProvider.forToken(tokenB, createListB),
];

It's important to note the type signature of FactoryProvider.forToken:

class FactoryProvider<T extends Object> implements Provider<T> {
  FactoryProvider.forToken(OpaqueToken<T> token, Function useFactory) { ... }
}

In our compiler, we were first trying to infer the type of each provider its type T. The type we were getting back in this case is List<Object>. I think this makes sense, because the inferred type of providers would be List<FactoryProvider<List<Object>>().

In practice we should probably be preferring the type of the token itself, over the type returned from inference.

If this sounds like it's working as expected then feel free to close.