carbon-lang icon indicating copy to clipboard operation
carbon-lang copied to clipboard

AST does not reflect implicit conversions to `type` after call to `TypeCheckTypeExp`

Open zygoloid opened this issue 2 years ago • 8 comments

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

zygoloid avatar Feb 15 '23 22:02 zygoloid

can i get assigned to this issue?

jatinsahijwani avatar Feb 24 '23 06:02 jatinsahijwani

If the issue is still available can you please assign it to me.

ShivanshKhankriyal avatar Feb 24 '23 09:02 ShivanshKhankriyal

@zygoloid I want to contribute in this issue

AbhishekCS3459 avatar Mar 09 '23 05:03 AbhishekCS3459

@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.

jonmeow avatar Mar 10 '23 16:03 jonmeow

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.

github-actions[bot] avatar Jun 09 '23 02:06 github-actions[bot]

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 avatar Oct 24 '23 19:10 officialhemant511

@officialhemant511 If you want to offer a fix for this issue, please submit it as a pull request.

geoffromer avatar Oct 24 '23 20:10 geoffromer

@officialhemant511 If you want to offer a fix for this issue, please submit it as a pull request.

ohk sir :) i understand..

officialhemant511 avatar Oct 25 '23 20:10 officialhemant511

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

chandlerc avatar Jan 20 '24 00:01 chandlerc