New syntax: New lookup rules for unqualified identifier

Instead of looking up any property in `self` and `root`, only resolve
the properties in scope declared in the current component.
This commit is contained in:
Olivier Goffart 2022-10-18 13:33:25 +02:00 committed by Olivier Goffart
parent bd412623ab
commit 0ba8f58076
10 changed files with 220 additions and 88 deletions

View file

@ -11,9 +11,10 @@ use crate::expression_tree::{
};
use crate::langtype::{Enumeration, EnumerationValue, Type};
use crate::namedreference::NamedReference;
use crate::object_tree::{find_parent_element, ElementRc, PropertyVisibility};
use crate::object_tree::{ElementRc, PropertyVisibility};
use crate::parser::NodeOrToken;
use crate::typeregister::TypeRegister;
use std::cell::RefCell;
/// Contains information which allow to lookup identifier in expressions
pub struct LookupCtx<'a> {
@ -199,11 +200,16 @@ impl LookupObject for SpecialIdLookup {
let last = ctx.component_scope.last();
None.or_else(|| f("self", Expression::ElementReference(Rc::downgrade(last?)).into()))
.or_else(|| {
f(
"parent",
Expression::ElementReference(Rc::downgrade(&find_parent_element(last?)?))
.into(),
)
let len = ctx.component_scope.len();
if len >= 2 {
f(
"parent",
Expression::ElementReference(Rc::downgrade(&ctx.component_scope[len - 2]))
.into(),
)
} else {
None
}
})
.or_else(|| f("true", Expression::BoolLiteral(true).into()))
.or_else(|| f("false", Expression::BoolLiteral(false).into()))
@ -219,44 +225,59 @@ impl LookupObject for IdLookup {
f: &mut impl FnMut(&str, LookupResult) -> Option<R>,
) -> Option<R> {
fn visit<R>(
roots: &[ElementRc],
root: &ElementRc,
f: &mut impl FnMut(&str, LookupResult) -> Option<R>,
) -> Option<R> {
for e in roots.iter().rev() {
if !e.borrow().id.is_empty() {
if let Some(r) =
f(&e.borrow().id, Expression::ElementReference(Rc::downgrade(e)).into())
{
return Some(r);
}
if !root.borrow().id.is_empty() {
if let Some(r) =
f(&root.borrow().id, Expression::ElementReference(Rc::downgrade(root)).into())
{
return Some(r);
}
for x in &e.borrow().children {
if x.borrow().repeated.is_some() {
continue;
}
if let Some(r) = visit(&[x.clone()], f) {
return Some(r);
}
}
for x in &root.borrow().children {
if x.borrow().repeated.is_some() {
continue;
}
if let Some(r) = visit(&x, f) {
return Some(r);
}
}
None
}
visit(ctx.component_scope, f)
for e in ctx.component_scope.iter().rev() {
if e.borrow().repeated.is_some() {
if let Some(r) = visit(e, f) {
return Some(r);
}
}
}
if let Some(root) = ctx.component_scope.first() {
if let Some(r) = visit(root, f) {
return Some(r);
}
}
None
}
// TODO: hash based lookup
}
struct InScopeLookup;
impl LookupObject for InScopeLookup {
fn for_each_entry<R>(
&self,
impl InScopeLookup {
fn visit_scope<R>(
ctx: &LookupCtx,
f: &mut impl FnMut(&str, LookupResult) -> Option<R>,
mut visit_entry: impl FnMut(&str, LookupResult) -> Option<R>,
mut visit_legacy_scope: impl FnMut(&ElementRc) -> Option<R>,
mut visit_scope: impl FnMut(&ElementRc) -> Option<R>,
) -> Option<R> {
for elem in ctx.component_scope.iter().rev() {
let is_legacy = ctx
.component_scope
.first()
.map_or(false, |e| e.borrow().enclosing_component.upgrade().unwrap().is_legacy_syntax);
for (idx, elem) in ctx.component_scope.iter().rev().enumerate() {
if let Some(repeated) = &elem.borrow().repeated {
if !repeated.index_id.is_empty() {
if let Some(r) = f(
if let Some(r) = visit_entry(
&repeated.index_id,
Expression::RepeaterIndexReference { element: Rc::downgrade(elem) }.into(),
) {
@ -264,45 +285,74 @@ impl LookupObject for InScopeLookup {
}
}
if !repeated.model_data_id.is_empty() {
if let Some(r) = f(
if let Some(r) = visit_entry(
&repeated.model_data_id,
Expression::RepeaterIndexReference { element: Rc::downgrade(elem) }.into(),
Expression::RepeaterModelReference { element: Rc::downgrade(elem) }.into(),
) {
return Some(r);
}
}
}
if let Some(r) = elem.for_each_entry(ctx, f) {
return Some(r);
if is_legacy {
if elem.borrow().repeated.is_some()
|| idx == 0
|| idx == ctx.component_scope.len() - 1
{
if let Some(r) = visit_legacy_scope(elem) {
return Some(r);
}
}
} else {
if let Some(r) = visit_scope(elem) {
return Some(r);
}
}
}
None
}
}
impl LookupObject for InScopeLookup {
fn for_each_entry<R>(
&self,
ctx: &LookupCtx,
f: &mut impl FnMut(&str, LookupResult) -> Option<R>,
) -> Option<R> {
let f = RefCell::new(f);
Self::visit_scope(
ctx,
|str, r| f.borrow_mut()(str, r),
|elem| elem.for_each_entry(ctx, *f.borrow_mut()),
|elem| {
for (name, prop) in &elem.borrow().property_declarations {
let e = expression_from_reference(
NamedReference::new(elem, name),
&prop.property_type,
);
if let Some(r) = f.borrow_mut()(name, e.into()) {
return Some(r);
}
}
None
},
)
}
fn lookup(&self, ctx: &LookupCtx, name: &str) -> Option<LookupResult> {
if name.is_empty() {
return None;
}
for elem in ctx.component_scope.iter().rev() {
if let Some(repeated) = &elem.borrow().repeated {
if repeated.index_id == name {
return Some(LookupResult::from(Expression::RepeaterIndexReference {
element: Rc::downgrade(elem),
}));
}
if repeated.model_data_id == name {
return Some(LookupResult::from(Expression::RepeaterModelReference {
element: Rc::downgrade(elem),
}));
}
}
if let Some(r) = elem.lookup(ctx, name) {
return Some(r);
}
}
None
Self::visit_scope(
ctx,
|str, r| (str == name).then(|| r),
|elem| elem.lookup(ctx, name),
|elem| {
elem.borrow().property_declarations.get(name).map(|prop| {
expression_from_reference(NamedReference::new(elem, name), &prop.property_type)
.into()
})
},
)
}
}

View file

@ -239,6 +239,9 @@ pub struct Component {
/// This is the main entry point for the code generators. Such a component
/// should have the full API, etc.
pub is_root_component: Cell<bool>,
/// True when this component was declared with the `:=` symbol instead of the `component` keyword
pub is_legacy_syntax: bool,
}
impl Component {
@ -259,6 +262,7 @@ impl Component {
tr,
),
child_insertion_point: RefCell::new(child_insertion_point),
is_legacy_syntax: node.child_token(SyntaxKind::ColonEqual).is_some(),
..Default::default()
};
let c = Rc::new(c);

View file

@ -14,21 +14,21 @@ use crate::lookup::LookupCtx;
use crate::object_tree::{Document, ElementRc};
use crate::parser::syntax_nodes;
use crate::typeregister::TypeRegister;
use std::rc::Rc;
#[derive(Clone)]
struct ComponentScope(Vec<ElementRc>);
pub fn resolve_aliases(doc: &Document, diag: &mut BuildDiagnostics) {
for component in doc.inner_components.iter() {
let scope = ComponentScope(vec![component.root_element.clone()]);
let scope = ComponentScope(vec![]);
crate::object_tree::recurse_elem_no_borrow(
&component.root_element,
&scope,
&mut |elem, scope| {
let mut new_scope = scope.clone();
if elem.borrow().repeated.is_some() {
new_scope.0.push(elem.clone())
}
new_scope.0.push(elem.clone());
let mut need_resolving = vec![];
for (prop, decl) in elem.borrow().property_declarations.iter() {
if matches!(decl.property_type, Type::InferredProperty | Type::InferredCallback)
@ -39,7 +39,7 @@ pub fn resolve_aliases(doc: &Document, diag: &mut BuildDiagnostics) {
// make it deterministic
need_resolving.sort();
for n in need_resolving {
resolve_alias(elem, &n, scope, &doc.local_registry, diag);
resolve_alias(elem, &n, &new_scope, &doc.local_registry, diag);
}
new_scope
},
@ -73,9 +73,7 @@ fn resolve_alias(
let mut lookup_ctx = LookupCtx::empty_context(type_register, diag);
lookup_ctx.property_name = Some(prop);
lookup_ctx.property_type = old_type.clone();
let mut scope = scope.0.clone();
scope.push(elem.clone());
lookup_ctx.component_scope = &scope;
lookup_ctx.component_scope = &scope.0;
crate::passes::resolving::resolve_two_way_binding(node, &mut lookup_ctx)
}
_ => panic!("There should be a Uncompiled expression at this point."),
@ -85,8 +83,12 @@ fn resolve_alias(
if let Some(nr) = &nr {
ty = nr.ty();
if matches!(ty, Type::InferredCallback | Type::InferredProperty) {
// Note that scope is might be too deep there, but it actually should work in most cases
resolve_alias(&nr.element(), nr.name(), scope, type_register, diag);
let element = nr.element();
if Rc::ptr_eq(&element, elem) {
resolve_alias(&element, nr.name(), scope, type_register, diag)
} else {
resolve_alias(&element, nr.name(), &recompute_scope(&element), type_register, diag)
};
ty = nr.ty();
}
}
@ -100,3 +102,31 @@ fn resolve_alias(
elem.borrow_mut().property_declarations.get_mut(prop).unwrap().property_type = ty;
}
}
/// Recompute the scope of element
///
/// (since there is no parent mapping, we need to recursively search for the element)
fn recompute_scope(element: &ElementRc) -> ComponentScope {
fn recurse(
base: &ElementRc,
needle: &ElementRc,
scope: &mut Vec<ElementRc>,
) -> std::ops::ControlFlow<()> {
scope.push(base.clone());
if Rc::ptr_eq(base, needle) {
return std::ops::ControlFlow::Break(());
}
for child in &base.borrow().children {
if recurse(child, needle, scope).is_break() {
return std::ops::ControlFlow::Break(());
}
}
scope.pop();
std::ops::ControlFlow::Continue(())
}
let root = element.borrow().enclosing_component.upgrade().unwrap().root_element.clone();
let mut scope = Vec::new();
recurse(&root, element, &mut scope);
ComponentScope(scope)
}

View file

@ -273,6 +273,7 @@ fn duplicate_sub_component(
popup_windows: Default::default(),
exported_global_names: component_to_duplicate.exported_global_names.clone(),
is_root_component: Default::default(),
is_legacy_syntax: component_to_duplicate.is_legacy_syntax,
};
let new_component = Rc::new(new_component);

View file

@ -84,29 +84,22 @@ pub fn resolve_expressions(
diag: &mut BuildDiagnostics,
) {
for component in doc.inner_components.iter() {
let scope = ComponentScope(vec![component.root_element.clone()]);
let scope = ComponentScope(vec![]);
recurse_elem(&component.root_element, &scope, &mut |elem, scope| {
let mut new_scope = scope.clone();
let mut is_repeated = elem.borrow().repeated.is_some();
if is_repeated {
new_scope.0.push(elem.clone())
}
new_scope.0.push(elem.clone());
let mut two_ways = vec![];
visit_element_expressions(elem, |expr, property_name, property_type| {
if is_repeated {
// The first expression is always the model and it needs to be resolved with the parent scope
debug_assert!(elem.borrow().repeated.as_ref().is_none()); // should be none because it is taken by the visit_element_expressions function
let mut parent_scope = scope.clone();
if let Some(parent) = find_parent_element(elem) {
parent_scope.0.push(parent)
};
resolve_expression(
expr,
property_name,
property_type(),
&parent_scope,
&scope,
&doc.local_registry,
type_loader,
&mut two_ways,
@ -129,7 +122,6 @@ pub fn resolve_expressions(
for (prop, nr) in two_ways {
elem.borrow().bindings.get(&prop).unwrap().borrow_mut().two_way_bindings.push(nr);
}
new_scope.0.pop();
new_scope
})
}
@ -541,6 +533,16 @@ impl Expression {
return Expression::Invalid;
}
}
for (prefix, e) in
[("self", ctx.component_scope.last()), ("root", ctx.component_scope.first())]
{
if let Some(e) = e {
if e.lookup(ctx, &first_str).is_some() {
ctx.diag.push_error(format!("Unknown unqualified identifier '{0}'. Did you mean '{prefix}.{0}'?", first.text()), &node);
return Expression::Invalid;
}
}
}
if it.next().is_some() {
ctx.diag.push_error(format!("Cannot access id '{}'", first.text()), &node);

View file

@ -0,0 +1,35 @@
// Copyright © SixtyFPS GmbH <info@slint-ui.com>
// SPDX-License-Identifier: GPL-3.0-only OR LicenseRef-Slint-commercial
component Compo inherits Text {
property <string> background: text;
// ^error{Unknown unqualified identifier 'text'. Did you mean 'self.text'?}
Rectangle {
background: background;
// ^error{Cannot convert string to brush}
}
Text {
text: background; // works: lookup in the root
}
if true : Rectangle {
property <color> text;
Text {
color: text; // works
text: background; // works
width: border-color;
// ^error{Unknown unqualified identifier 'border-color'$}
height: text;
// ^error{Cannot convert color to length}
}
}
Rectangle {
width: text;
// ^error{Unknown unqualified identifier 'text'. Did you mean 'root.text'?}
}
}

View file

@ -31,6 +31,9 @@ pub struct LookupChangeState {
/// Elements that should get an id
elements_id: HashMap<ByAddress<ElementRc>, String>,
/// the full lookup scope
pub scope: Vec<ElementRc>,
}
pub(crate) fn fold_node(
@ -135,9 +138,12 @@ fn fully_qualify_property_access(
.as_ref()
.map_or(false, |c| Rc::ptr_eq(&element, &c.root_element))
{
// We could decide to also prefix all root properties with root
//write!(file, "root.")?;
} else if state.current_elem.as_ref().map_or(false, |e| Rc::ptr_eq(&element, e))
write!(file, "root.")?;
} else if state
.lookup_change
.scope
.last()
.map_or(false, |e| Rc::ptr_eq(&element, e))
{
if let Some(replace_self) = &state.lookup_change.replace_self {
write!(file, "{replace_self}.")?;
@ -255,17 +261,9 @@ fn with_lookup_ctx<R>(state: &crate::State, f: impl FnOnce(&mut LookupCtx) -> R)
.zip(state.property_name.as_ref())
.map_or(Type::Invalid, |(e, n)| e.borrow().lookup_property(&n).property_type);
let scope = state
.current_component
.as_ref()
.map(|c| c.root_element.clone())
.into_iter()
.chain(state.current_elem.clone().into_iter())
.collect::<Vec<_>>();
lookup_context.property_name = state.property_name.as_ref().map(String::as_str);
lookup_context.property_type = ty;
lookup_context.component_scope = &scope;
lookup_context.component_scope = &state.lookup_change.scope;
Some(f(&mut lookup_context))
}
@ -319,3 +317,17 @@ fn ensure_element_has_id(
});
}
}
pub(crate) fn enter_element(state: &mut crate::State) {
if state
.lookup_change
.scope
.last()
.map_or(false, |e| e.borrow().base_type.to_string() == "Path")
{
// Path's sub-elements have strange lookup rules: They are considering self as the Path
state.lookup_change.replace_self = Some("parent".into());
} else {
state.lookup_change.scope.extend(state.current_elem.iter().cloned())
}
}

View file

@ -174,7 +174,7 @@ fn process_file(
let syntax_node = i_slint_compiler::parser::parse(source.clone(), Some(path), &mut diag);
let len = syntax_node.node.text_range().end().into();
let mut state = State::default();
if args.fully_qualify {
if args.fully_qualify || args.move_declaration {
let doc = syntax_node.clone().into();
let mut type_loader = TypeLoader::new(
i_slint_compiler::typeregister::TypeRegister::builtin(),
@ -254,10 +254,6 @@ fn visit_node(
}
}
SyntaxKind::Element => {
/*let element_node = syntax_nodes::Element::from(node.clone());
state.element_name = element_node
.QualifiedName()
.map(|qn| object_tree::QualifiedTypeName::from_node(qn).to_string());*/
if let Some(parent_el) = state.current_elem.take() {
state.current_elem = parent_el
.borrow()
@ -270,6 +266,8 @@ fn visit_node(
state.current_elem = Some(parent_co.root_element.clone())
}
}
experiments::lookup_changes::enter_element(&mut state);
}
_ => (),
}