Skip to content

[IMP] propagate SELF evaluations#546

Open
mmahrouss wants to merge 1 commit intoalphafrom
alpha-propagate-self-eval-mdms
Open

[IMP] propagate SELF evaluations#546
mmahrouss wants to merge 1 commit intoalphafrom
alpha-propagate-self-eval-mdms

Conversation

@mmahrouss
Copy link
Collaborator

@mmahrouss mmahrouss commented Feb 11, 2026

Works by adding a weak pointer to the SELF EvaluationSymbolPtr variant,
which is used to propagate the SELF evaluation through method calls.
When a method with a SELF return type is called, under a subclass,
the weak pointer is updated to point to the new symbol,
allowing the evaluation to be correctly propagated.

Also SELF evaluations are now treated as weak evaluations when it is
final, like in features, or in any situation where it will not be
propagated any further

@mmahrouss mmahrouss added bug Something isn't working enhancement New feature or request labels Feb 11, 2026
@mmahrouss mmahrouss force-pushed the alpha-propagate-self-eval-mdms branch from 0d6f520 to 2177412 Compare February 13, 2026 16:42
}

pub fn inherits(&self, base: &Rc<RefCell<Symbol>>, checked: &mut Option<PtrWeakHashSet<Weak<RefCell<Symbol>>>>) -> bool {
pub fn inherits(&self, session: &mut SessionInfo, base: &Rc<RefCell<Symbol>>, checked: &mut Option<PtrWeakHashSet<Weak<RefCell<Symbol>>>>) -> bool {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I did this change, because I thought we needed it to check if Self is actually inheriting the base class, but I found out we do not need that check if everything is correct, and it was very time costly due to Model::inherits_from we can leave it for later for correctness. But, it needs to be optimized. Tell me your thoughts during review:)

@mmahrouss mmahrouss force-pushed the alpha-propagate-self-eval-mdms branch from 2177412 to a7c3fb8 Compare February 19, 2026 11:34
let inferred_type = inferred_type.borrow();
match inferred_type.typ() {
SymType::FUNCTION if !inferred_type.as_func().is_property => {
let call_parent = match eval.as_weak().context.get(&S!("base_attr")){
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is because in completion::add_model_attributes we add base_attr to the context , and we need that to correctly evaluate self, in the later call to get_symbol_weak_transformed to get the correct return type in the completion list

@mmahrouss mmahrouss force-pushed the alpha-propagate-self-eval-mdms branch from a7c3fb8 to 0be4d83 Compare February 19, 2026 13:46
Comment on lines +251 to +252
WEAK(EvaluationSymbolWeak),
SELF,
SELF(EvaluationSymbolWeak), // Weak symbol is the fall_back symbol, and used to check if symbol is a subclass
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Perhaps here WEAK should be renamed to something in contract to SELF? or is it okay like that? we can have FINAL and SELF so it shows the contrast?
FINAL is a final evaluation, SELF is an evaluation that could be overridden in subclasses.
What do you think?

}
let base_eval_ptrs: Vec<EvaluationSymbolPtr> = base_evals.iter().map(|base_eval| {
let base_sym_weak_eval_base = base_eval.symbol.get_symbol_weak_transformed(session, context, &mut diagnostics, None);
let base_sym_weak_eval_base = base_eval.symbol.get_symbol(session, context, &mut diagnostics, None);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Here get_symbol_weak_transformed was only doing get_symbol because base_call was never set.

evals.push(Evaluation{
symbol: EvaluationSymbol {
sym: EvaluationSymbolPtr::WEAK(EvaluationSymbolWeak{
sym: EvaluationSymbolPtr::SELF(EvaluationSymbolWeak{
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

super() carries a self(or cls) evaluation inherently

context.as_mut().unwrap().insert(S!("range"), old_range.unwrap());
}
if let EvaluationSymbolPtr::SELF = get_item_eval.symbol.get_symbol_ptr(){
if let EvaluationSymbolPtr::SELF(_) = get_item_eval.symbol.get_symbol_ptr(){
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In theory SELF should already carry base in it, I did not change it to avoid too many code changes.

}
}

pub fn inherits_from(&self, session: &mut SessionInfo, base: &Rc<RefCell<Model>>) -> bool {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is very inefficient, and it is not really used now, it may or not be useful in the future, but it will need some caching of inherit on a class to be efficient.


type PythonArchEvalHookFile = fn (odoo: &mut SessionInfo, entry: &Rc<RefCell<EntryPoint>>, file_symbol: Rc<RefCell<Symbol>>, symbol: Rc<RefCell<Symbol>>);

fn get_base_model_symbol(odoo: &mut SyncOdoo) -> Option<Rc<RefCell<Symbol>>> {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Is it worth caching that? it is only called a handful of times in the hooks? what do you think ?

@mmahrouss mmahrouss force-pushed the alpha-propagate-self-eval-mdms branch from 0be4d83 to cc6c451 Compare February 19, 2026 14:40
@mmahrouss mmahrouss marked this pull request as ready for review February 19, 2026 14:42
@mmahrouss mmahrouss requested a review from fda-odoo February 19, 2026 14:42
@mmahrouss mmahrouss self-assigned this Feb 19, 2026
Works by adding a weak pointer to the SELF EvaluationSymbolPtr variant,
which is used to propagate the SELF evaluation through method calls.
When a method with a SELF return type is called, under a subclass,
the weak pointer is updated to point to the new symbol,
allowing the evaluation to be correctly propagated.

Also SELF evaluations are now treated as weak evaluations when it is
final, like in features, or in any situation where it will not be
propagated any further
@mmahrouss mmahrouss force-pushed the alpha-propagate-self-eval-mdms branch from cc6c451 to 447c090 Compare February 19, 2026 14:45
@mmahrouss mmahrouss changed the title WIP: [IMP] propagate SELF evaluations [IMP] propagate SELF evaluations Feb 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant