carbon-lang
carbon-lang copied to clipboard
AST does not reflect implicit conversions to `type` after call to `TypeCheckTypeExp`
Description of the bug:
When explorer performs an implicit conversion, it usually annotates the result of that conversion back onto the AST. However, because TypeCheckTypeExp combines implicit conversion to type and interpretation of the resulting expression, it doesn't do this. That means that the AST doesn't properly reflect the actual operations that are performed. This would be a problem for anyone using explorer to understand the desugared meaning of a program.
What did you do, or what's a simple way to reproduce the bug?
One way to observe this is with tuples. Given:
var v: (i32, i32) = (1, 2);
... the conversion from the tuple value (i32, i32) to the corresponding tuple type is not reflected in the AST built by explorer.
What did you expect to happen?
Because explorer interprets its AST directly rather than forming an intermediate representation, the semantic behavior of the code should be visible in the AST after type-checking.
What actually happened?
The expression that performs conversion to type is generated, interpreted, then discarded within TypeCheckTypeExp.
Any other information, logs, or outputs that you want to share?
No response
can i get assigned to this issue?
If the issue is still available can you please assign it to me.
@zygoloid I want to contribute in this issue
@jatinsahijwani @ShivanshKhankriyal @AbhishekCS3459 We typically won't assign issues. I've added some documentation for why. Please feel to work on it if you have time, working on issues can be a good way to get a feel for the codebase regardless of whether your fix is the one merged.
We triage inactive PRs and issues in order to make it easier to find active work. If this issue should remain active or becomes active again, please comment or remove the inactive label. The long term label can also be added for issues which are expected to take time. \n\n\n This issue is labeled inactive because the last activity was over 90 days ago.
To address the issue where implicit conversions are not properly reflected in the AST built by the explorer, you can modify the implementation of the TypeCheckTypeExp function to annotate the result of the conversion back onto the AST. Here's a possible solution:
// Modify TypeCheckTypeExp function to include an annotation step fn TypeCheckTypeExp(exp: Expression) -> Expression { match exp { // Handle tuples explicitly Expression::Tuple(tuple) => { let annotated_tuple = tuple.iter().map(|item| { // Recursively type-check each item in the tuple let annotated_item = TypeCheckTypeExp(item.clone()); // Annotate the type of the item onto the AST node let annotated_type = item.get_type(); annotated_item.set_type(annotated_type.clone()); annotated_item }).collect(); // Return the annotated tuple Expression::Tuple(annotated_tuple) }, // Handle other expressions _ => { // Perform the regular type-checking for the expression // ... // Annotate the inferred type onto the AST node exp.set_type(inferred_type.clone()); // Return the annotated expression exp } } }
// Usage example let v: (i32, i32) = (1, 2); let annotated_v = TypeCheckTypeExp(Expression::Tuple(v.iter().map(|&x| Expression::Literal(x)).collect()));
In this solution, we modify the TypeCheckTypeExp function to handle tuples explicitly. When encountering a tuple expression, we recursively type-check each item in the tuple and annotate the type of the item onto the AST node. For other expressions, we perform the regular type-checking and annotate the inferred type onto the AST node.
This modification ensures that the AST properly reflects the operations performed during type-checking, including implicit conversions.
@officialhemant511 If you want to offer a fix for this issue, please submit it as a pull request.
@officialhemant511 If you want to offer a fix for this issue, please submit it as a pull request.
ohk sir :) i understand..
Closing explorer-specific issues as not-planned for now due to our decision to prioritize working on the toolchain over other implementation work in the near term: https://github.com/carbon-language/carbon-lang/blob/trunk/proposals/p3532.md