Skip to content

Conversation

ssyram
Copy link
Contributor

@ssyram ssyram commented Jul 8, 2025

Now the dyn Trait Support should be quite ready with a few TODOs.

Features Implemented

  • Supporting associated types, now the vtable structs & instances all additionally take additional generic types corresponding to and taken from &dyn Trait's associated type restrictions. The basic support of the associated types are from Translate vtables with associated types #796.
  • The vtable instances are now properly generated for the concrete impls, i.e., those with explicit impl Trait for Ty environments.
  • The vtable method calls are propertly generated that, calling methods of &dyn Trait will be translated to finding the corresponding function pointer from the vtable (fetched by ptr_metadata operation) and then call the function pointer.
  • A new pass is added to compute the correct vtable instance metadata when possible, i.e., the size, align and, the most complex case, the drop fields. Naturally, the content will not be computable if the stuff is behind a generics, this is inevitable and the fields will remain unknown. But everything should be explicit in the monomorphized case in the future. Specifically, the size and align subjects to whether the layout of the type is properly computable. The drop field, as the most complex case, now also handles the cases when the concrete impl types are Arrays & Tuples -- they should be the only composite cases.
  • The vtable instances support for closures are now added. All tests passed. But there are identified issues, see Bug: Closures with drop glue translated ssyram/charon#20 for the closure itself. But it should work properly in most of the cases -- as dyn FnOnce is really strange, and rare.

TODOs

  • Monomorphization support for dyn Trait.
  • Vtable instances for closures.

@Nadrieril
Copy link
Member

Nadrieril commented Jul 8, 2025

Haven't reviewed everything but from what I've seen: could you change how we represent the existential predicates? The current encoding will cause issues with generics (see inline comments).

@ssyram
Copy link
Contributor Author

ssyram commented Jul 9, 2025

Thanks for all the suggestions from both of you! I'm still not sure about how to refractor the Predicate form. So I will leave it to the discussion in Zulip. Let me fetch & implement the vtable struct & instances first.

@ssyram
Copy link
Contributor Author

ssyram commented Jul 9, 2025

Many fixes applied. Thanks for the advice. Mainly involving the fix for the three kinds of existential predicates. But I'm still not sure about if they work as expected...

@Nadrieril
Copy link
Member

Nadrieril commented Jul 9, 2025

Could you add a lot of tests please? We need to cover edge cases even if we don't support them yet (write //@ known-failure at the top for these cases). We should have tests for supertraits, multiple supertraits, casting in a generic context, associated type projections, + 'a bounds, multiple trait bounds (dyn Any + Send), just builtins (dyn Send), traits with methods that are generic and have a Self: Sized bound (which makes them dyn-compatible but means the vtable doesn't have all the methods), etc.

@ssyram
Copy link
Contributor Author

ssyram commented Jul 9, 2025

Yes, of course. Many tests are coming when the infrastructure is about to get done... Maybe we can focus on the implementation for now?

@Nadrieril
Copy link
Member

Nadrieril commented Jul 9, 2025

Maybe we can focus on the implementation for now?

The tests will help me understand what the implementation is doing, I can't just be running the code in my head. It's imo crucial to add the tests while doing the implementation because that's when we think of edge cases. When we're done implementing it's too late.

ssyram and others added 7 commits August 22, 2025 11:25
* Initial plan

* Changes before error encountered

Co-authored-by: ssyram <[email protected]>

* Add basic transform_dyn_trait_calls pass framework

Co-authored-by: ssyram <[email protected]>

* Implement detection framework for dyn trait calls transformation pass

Co-authored-by: ssyram <[email protected]>

* Add detailed tracing and structure analysis for dyn trait transformation

Co-authored-by: ssyram <[email protected]>

* Remove temporary files and fix documentation test failures

Co-authored-by: ssyram <[email protected]>

* Implement vtable dispatch transformation for dyn trait method calls

This implements the core transformation that converts method calls on `&dyn Trait` objects from direct trait method calls to vtable function pointer dispatch.

The transformation detects calls with dyn trait arguments and generates:
1. vtable := ptr_metadata(dyn_trait_receiver) - extract vtable pointer
2. method_ptr := (*vtable).method_field - get method from vtable
3. Converts call to use function pointer: call(method_ptr)(args)

Key changes:
- Added transform_dyn_trait_calls pass to ULLBC pipeline
- Detects regular function calls with dyn trait arguments
- Creates new locals for vtable and method pointer
- Generates vtable access statements with proper AST structures
- Successfully transforms the call terminator to use function pointer

The transformation is working as evidenced by the ULLBC output showing the new vtable/method_ptr locals and the transformed call structure.

Co-authored-by: ssyram <[email protected]>

* Implement vtable dispatch transformation for dyn trait method calls

Co-authored-by: ssyram <[email protected]>

* Refactor transform_dyn_trait_calls for better modularity and readability

Co-authored-by: ssyram <[email protected]>

* Changes before error encountered

Co-authored-by: ssyram <[email protected]>

---------

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: ssyram <[email protected]>
…etter code organization (#9)

* Initial plan

* Initial plan for transform-dyn-trait-call refactoring

Co-authored-by: ssyram <[email protected]>

* Refactor transform-dyn-trait-call functions into struct methods

Co-authored-by: ssyram <[email protected]>

---------

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: ssyram <[email protected]>
Drop shims are correctly implemented with recursion to handle both Array & Tuple cases.

* Initial plan

* Initial analysis and planning for vtable metadata computation

Co-authored-by: ssyram <[email protected]>

* Initial analysis and plan for vtable metadata computation

Co-authored-by: ssyram <[email protected]>

* Implement basic drop case analysis in vtable metadata computation

Co-authored-by: ssyram <[email protected]>

* Improve drop case analysis with proper Drop trait detection

Co-authored-by: ssyram <[email protected]>

* Successfully implement null drop operands for types that don't need drop

Co-authored-by: ssyram <[email protected]>

* Refactor vtable metadata computation to use struct-based approach

Co-authored-by: ssyram <[email protected]>

* Changes before error encountered

Co-authored-by: ssyram <[email protected]>

* Changes before error encountered

Co-authored-by: ssyram <[email protected]>

* Improve drop shim generation with better structured naming and case distinction

Co-authored-by: ssyram <[email protected]>

* Implement actual drop shim function generation for vtable metadata

Co-authored-by: ssyram <[email protected]>

* Fix drop shim function generation with proper lifetime handling

Co-authored-by: ssyram <[email protected]>

* Implement proper dyn trait types and Concretize cast for drop shims

Co-authored-by: ssyram <[email protected]>

* Set drop shim functions as transparent for better naming display

Co-authored-by: ssyram <[email protected]>

* Implement generics handling for drop shim functions - first iteration

Co-authored-by: ssyram <[email protected]>

* Fix drop shim parameter types and implement proper dyn trait handling

Co-authored-by: ssyram <[email protected]>

* Implement generics handling for drop shim function references

Co-authored-by: ssyram <[email protected]>

* Refactor code organization and fix drop shim parameter types from pointers to references

Co-authored-by: ssyram <[email protected]>

* Fix generics handling with proper region variables and minimal generic parameters

Co-authored-by: ssyram <[email protected]>

* Complete vtable drop shim implementation with proper reference types and simplified generics

Co-authored-by: ssyram <[email protected]>

* re-implemented layout info

* Delete vtables

* minor adjust

* Implement create_dyn_trait_param_type by extracting from drop field type and handle Unknown case

Co-authored-by: ssyram <[email protected]>

* fixd drop receiver

* Add drop_glue field to TypeDecl for tracking drop implementations (#14)

* Initial plan

* Add drop_glue field to TypeDecl and implement translation logic

Co-authored-by: ssyram <[email protected]>

* Fix test case ordering after adding drop_glue field

Co-authored-by: ssyram <[email protected]>

* Fix panic in combine_span_iter when handling empty iterators

Co-authored-by: ssyram <[email protected]>

* new ml & no-enqueue

---------

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: ssyram <[email protected]>
Co-authored-by: ssyram <[email protected]>

* types fixed

* Implement proper generics handling for drop shim functions

Co-authored-by: ssyram <[email protected]>

* revised drop case

* Implement comprehensive drop case analysis with array, tuple, and Box support

Co-authored-by: ssyram <[email protected]>

* minor

* minor

* Implement concretize casting for array, tuple, and Box drop shims

Co-authored-by: ssyram <[email protected]>

* Enhance drop shim generation with structured drop case analysis

Co-authored-by: ssyram <[email protected]>

* Implement tuple field access with projections for drop shims

Co-authored-by: ssyram <[email protected]>

* Complete comprehensive drop shim logic implementation with detailed case analysis

Co-authored-by: ssyram <[email protected]>

* Simplify and combine DropShimKind and DropCase enums, simplify Box handling

Co-authored-by: ssyram <[email protected]>

* fixed drop field

* Implement complete tuple and array drop shim logic with CFG blocks and function calls

Co-authored-by: ssyram <[email protected]>

* Fix unused block_id assignment and add tuple/array drop test cases

Co-authored-by: ssyram <[email protected]>

* new example added

* Complete Box drop shim generics fix with proper trait clauses

Fix Box drop shim generics - provide T and A type parameters correctly

Co-Authored-By: ssyram <[email protected]>

* Revert "Complete Box drop shim generics fix with proper trait clauses"

This reverts commit b889ebb.

* Initial analysis and understanding of Box drop generics issue

Co-authored-by: ssyram <[email protected]>

* fixed the case

* Initial analysis and planning for Box drop generics and region binder fixes

Co-authored-by: ssyram <[email protected]>

* Fix region binders and partial Box generics handling

Co-authored-by: ssyram <[email protected]>

* Changes before error encountered

Co-authored-by: ssyram <[email protected]>

* Fix compilation errors and add missing Box detection methods

Co-authored-by: ssyram <[email protected]>

* Major progress on Box generics extraction and region variable fixes

Co-authored-by: ssyram <[email protected]>

* Initial analysis and planning for Box drop generics and region binder fixes

Fix compilation errors and add missing Box detection methods

Changes before error encountered

Fix region binders and partial Box generics handling

Co-Authored-By: ssyram <[email protected]>

* Revert "Initial analysis and planning for Box drop generics and region binder fixes"

This reverts commit 2802a90.

* Revert "Merge branch 'copilot/fix-95d3f204-c7e2-4956-8a71-c4a3988443ca' of https://github.com/ssyram/charon into copilot/fix-95d3f204-c7e2-4956-8a71-c4a3988443ca"

This reverts commit 048ec8f, reversing
changes made to eb84c07.

* name & generic some fix

* Implement region binder fix and Box built-in detection for drop shim generation

Co-authored-by: ssyram <[email protected]>

* Fix generics mismatch and improve Box built-in detection for drop shims

Co-authored-by: ssyram <[email protected]>

* fixed test region mismatch

* Fix tuple drop shim generation to create proper field-by-field drop logic

Co-authored-by: ssyram <[email protected]>

* Complete array and tuple drop shim implementation with proper traversal logic

Co-authored-by: ssyram <[email protected]>

* init-reform

* Refactor drop shim generation with proper constant expressions and utility methods

- Use `new_var()` and `place_for_var()` utilities instead of manual local creation
- Use `ConstantExpr` directly instead of casting to primitive types
- Fix scoping issues with concrete place by using `clone()` in loops
- Clean up code organization and use `return_place()` utility
- Remove unused variables to eliminate warnings
- All vtables tests now pass

Co-authored-by: ssyram <[email protected]>

* Fix array drop shim counter types to match array length expression type

Co-authored-by: ssyram <[email protected]>

* example changed

* Implement comprehensive drop shim generation with proper array/tuple traversal and drop calls

Co-authored-by: ssyram <[email protected]>

* Fix Vec<T> drop detection by bypassing needs_drop for ADT types

Co-authored-by: ssyram <[email protected]>

* Fix recursive drop case analysis to handle primitive types correctly

Co-authored-by: ssyram <[email protected]>

* Simplify analyze_drop_case and fix projection issues in drop shim generation

Co-authored-by: ssyram <[email protected]>

* fixed box issue & new box example

* Extract helper structures and simplify constant creation

Co-authored-by: ssyram <[email protected]>

* Refactor analyze_drop_case into smaller focused functions and organize code sections

Co-authored-by: ssyram <[email protected]>

* Clean up verbose comments, remove unused code, and simplify function structure

Co-authored-by: ssyram <[email protected]>

* new example

* more cleanup

* new framework

* debug done with new complex example

---------

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: ssyram <[email protected]>
Co-authored-by: ssyram <[email protected]>
* registration template

* Initial plan

* Implement Phase 1: Add closure vtable registration in translate_bodies.rs

Co-authored-by: ssyram <[email protected]>

* Implement Phase 2: Add closure support in translate_trait_objects.rs

Co-authored-by: ssyram <[email protected]>

* WIP Phase 3: Simplify closure vtable generation to avoid type resolution issues

Co-authored-by: ssyram <[email protected]>

* fixed panick from double translation

* format

* partial debug

* fixed ML issue

Now all tests passed, but the methods & parent pointers are still placeholders to be generated.

* impl done

* Translating Super Traits

* Initial plan

* Initial analysis: Identify missing supertrait vtable registration issue

Co-authored-by: ssyram <[email protected]>

* Partial fix: Successfully handle closure supertrait vtables

Co-authored-by: ssyram <[email protected]>

* Complete fix: Handle both closure and marker trait supertrait vtables

Co-authored-by: ssyram <[email protected]>

* parent traits done

---------

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: ssyram <[email protected]>
Co-authored-by: ssyram <[email protected]>

* Debug drop shim

---------

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: ssyram <[email protected]>
@ssyram ssyram marked this pull request as ready for review September 5, 2025 15:49
@Nadrieril
Copy link
Member

For the CI failures, unfortunately I don't know of a good way to normalize the paths, so just add //@ no-check-output to those test files.

Comment on lines +356 to +357
// When Box is encountered, enqueue the translation of its drop implementation
self.enqueue_box_drop_impl(span, item)?;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You shouldn't need a hack like that, what caused this?

Comment on lines -309 to +349
/// drop: fn(*mut dyn Trait<...>),
/// drop: fn(&mut dyn Trait<...>),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pointer here was not a mistake, creating &mut references has semantic side-effects which I don't fully understand, but I know the &mut in Drop::drop is a source of trickiness in relationship to Pin so we should not create more references than rustc does. Hence the pointer (it's meant to call drop_in_place anyway, which takes a pointer).

@Nadrieril
Copy link
Member

Nadrieril commented Sep 9, 2025

Could you please split this PR into several for the many features it implements? I see at least:

  • Remove the redundant TraitDeclRefs in TraitRefKind (that one could just be its own commit, doesn't need a full PR);
  • Handle dyn Trait with associated types;
  • Translate dyn Trait method calls;
  • Support dyn Fn* for closures;
  • There's probably more.

I can't review it like this with a messy commit history and a bunch of different features implemented at once. What I do see looks good though!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants