Lookup of identifier is now done while resolving expressions

api/sixtyfps-rs/sixtyfps-rs-macro/lib.rs

@@ -141,13 +141,11 @@ pub fn sixtyfps(stream: TokenStream) -> TokenStream {
         for (k, v) in &item.init_properties {
             let k = quote::format_ident!("{}", k);
             let v = match v {
-                Expression::Invalid | Expression::Uncompiled(_) => quote!(),
-                // That's an error
-                Expression::Identifier(_) => quote!(),
                 Expression::StringLiteral(s) => {
                     quote!(sixtyfps::re_exports::SharedString::from(#s))
                 }
                 Expression::NumberLiteral(n) => quote!(#n),
+                _ => quote!(compile_error! {"unsupported expression"}),
             };
             init.push(quote!(self_.#field_name.#k.set(#v as _);));
         }

sixtyfps_compiler/expressions.rs

@@ -1,27 +1,47 @@
+#![allow(dead_code)] // FIXME: remove when warnings are gone
 use crate::diagnostics::Diagnostics;
 use crate::object_tree::*;
-use crate::parser::{SyntaxKind, SyntaxNode, SyntaxNodeEx};
+use crate::parser::{Spanned, SyntaxKind, SyntaxNode, SyntaxNodeEx};
 use crate::typeregister::{Type, TypeRegister};
 use core::str::FromStr;
 use std::cell::RefCell;
-use std::rc::Rc;
+use std::rc::{Rc, Weak};
 
+/// Contains information which allow to lookup identifier in expressions
+struct LookupCtx<'a> {
+    /// The type register
+    tr: &'a TypeRegister,
+    /// the type of the property for which this expression refers.
+    /// (some property come in the scope)
+    property_type: Type,
+
+    /// document_root
+    document_root: Rc<Component>,
+
+    /// Somewhere to report diagnostics
+    diag: &'a mut Diagnostics,
+}
+
+/// The Expression is hold by properties, so it should not hold any strong references to node from the object_tree
 #[derive(Debug, Clone)]
 pub enum Expression {
     /// Something went wrong (and an error will be reported)
     Invalid,
     /// We haven't done the lookup yet
     Uncompiled(SyntaxNode),
-    /// A simple identifier, for example `something`, the .0 is then "something"
-    Identifier(String),
     /// A string literal. The .0 is the content of the string, without the quotes
     StringLiteral(String),
     /// Number
     NumberLiteral(f64),
+
+    /// Reference to the signal <name> in the <element> within the <Component>
+    ///
+    /// Note: if we are to separate expression and statement, we probably do not need to have signal reference within expressions
+    SignalReference { component: Weak<Component>, element: Weak<RefCell<Element>>, name: String },
 }
 
 impl Expression {
-    pub fn from_code_statement_node(node: SyntaxNode, diag: &mut Diagnostics) -> Self {
+    fn from_code_statement_node(node: SyntaxNode, ctx: &mut LookupCtx) -> Self {
         debug_assert_eq!(node.kind(), SyntaxKind::CodeStatement);
 
         node.child_node(SyntaxKind::Expression)
@@ -29,21 +49,24 @@ impl Expression {
                 node.child_node(SyntaxKind::CodeBlock)
                     .and_then(|c| c.child_node(SyntaxKind::Expression))
             })
-            .map_or(Self::Invalid, |n| Self::from_expression_node(n, diag))
+            .map_or(Self::Invalid, |n| Self::from_expression_node(n, ctx))
     }
 
-    pub fn from_expression_node(node: SyntaxNode, diag: &mut Diagnostics) -> Self {
+    fn from_expression_node(node: SyntaxNode, ctx: &mut LookupCtx) -> Self {
         node.child_node(SyntaxKind::Expression)
-            .map(|n| Self::from_expression_node(n, diag))
+            .map(|n| Self::from_expression_node(n, ctx))
             .or_else(|| {
                 node.child_node(SyntaxKind::BangExpression)
-                    .map(|n| Self::from_bang_expresion_node(n, diag))
+                    .map(|n| Self::from_bang_expresion_node(n, ctx))
+            })
+            .or_else(|| {
+                node.child_token(SyntaxKind::Identifier)
+                    .map(|s| Self::from_unqualified_identifier(s, ctx))
             })
-            .or_else(|| node.child_text(SyntaxKind::Identifier).map(|s| Self::Identifier(s)))
             .or_else(|| {
                 node.child_text(SyntaxKind::StringLiteral).map(|s| {
                     unescape_string(&s).map(Self::StringLiteral).unwrap_or_else(|| {
-                        diag.push_error("Cannot parse string literal".into(), node.span());
+                        ctx.diag.push_error("Cannot parse string literal".into(), node.span());
                         Self::Invalid
                     })
                 })
@@ -51,7 +74,7 @@ impl Expression {
             .or_else(|| {
                 node.child_text(SyntaxKind::NumberLiteral).map(|s| {
                     f64::from_str(&s).ok().map(Self::NumberLiteral).unwrap_or_else(|| {
-                        diag.push_error("Cannot parse number literal".into(), node.span());
+                        ctx.diag.push_error("Cannot parse number literal".into(), node.span());
                         Self::Invalid
                     })
                 })
@@ -59,22 +82,22 @@ impl Expression {
             .unwrap_or(Self::Invalid)
     }
 
-    fn from_bang_expresion_node(node: SyntaxNode, diag: &mut Diagnostics) -> Self {
+    fn from_bang_expresion_node(node: SyntaxNode, ctx: &mut LookupCtx) -> Self {
         match node.child_text(SyntaxKind::Identifier).as_ref().map(|x| x.as_str()) {
             None => {
                 debug_assert!(false, "the parser should not allow that");
-                diag.push_error("Missing bang keyword".into(), node.span());
+                ctx.diag.push_error("Missing bang keyword".into(), node.span());
                 return Self::Invalid;
             }
             Some("img") => {
                 // FIXME: we probably need a better syntax and make this at another level.
                 let s = match node
                     .child_node(SyntaxKind::Expression)
-                    .map_or(Self::Invalid, |n| Self::from_expression_node(n, diag))
+                    .map_or(Self::Invalid, |n| Self::from_expression_node(n, ctx))
                 {
                     Expression::StringLiteral(p) => p,
                     _ => {
-                        diag.push_error(
+                        ctx.diag.push_error(
                             "img! Must be followed by a valid path".into(),
                             node.span(),
                         );
@@ -86,7 +109,7 @@ impl Expression {
                 if path.is_absolute() {
                     return Expression::StringLiteral(s);
                 }
-                let path = diag.path(node.span()).parent().unwrap().join(path);
+                let path = ctx.diag.path(node.span()).parent().unwrap().join(path);
                 if path.is_absolute() {
                     return Expression::StringLiteral(path.to_string_lossy().to_string());
                 }
@@ -95,11 +118,45 @@ impl Expression {
                 )
             }
             Some(x) => {
-                diag.push_error(format!("Unkown bang keyword `{}`", x), node.span());
+                ctx.diag.push_error(format!("Unkown bang keyword `{}`", x), node.span());
                 return Self::Invalid;
             }
         }
     }
+
+    fn from_unqualified_identifier(
+        identifier: crate::parser::SyntaxToken,
+        ctx: &mut LookupCtx,
+    ) -> Self {
+        // Perform the lookup
+        let s = identifier.text().as_str();
+
+        if matches!(ctx.property_type, Type::Color) {
+            let value: Option<u32> = match s {
+                "blue" => Some(0xff0000ff),
+                "red" => Some(0xffff0000),
+                "green" => Some(0xff00ff00),
+                "yellow" => Some(0xffffff00),
+                "black" => Some(0xff000000),
+                "white" => Some(0xffffffff),
+                _ => None,
+            };
+            if let Some(value) = value {
+                // FIXME: there should be a ColorLiteral
+                return Self::NumberLiteral(value as f64);
+            }
+        } else if matches!(ctx.property_type, Type::Signal) {
+            return Self::SignalReference {
+                component: Rc::downgrade(&ctx.document_root),
+                element: Rc::downgrade(&ctx.document_root.root_element),
+                name: s.to_string(),
+            };
+        }
+
+        ctx.diag.push_error(format!("Unkown unqualified identifier '{}'", s), identifier.span());
+
+        Self::Invalid
+    }
 }
 
 fn unescape_string(string: &str) -> Option<String> {
@@ -114,29 +171,37 @@ fn unescape_string(string: &str) -> Option<String> {
 pub fn resolve_expressions(doc: &Document, diag: &mut Diagnostics, tr: &mut TypeRegister) {
     fn resolve_expressions_in_element_recursively(
         elem: &Rc<RefCell<Element>>,
+        doc: &Document,
         diag: &mut Diagnostics,
         tr: &mut TypeRegister,
     ) {
         let base = elem.borrow().base_type.clone();
         for (prop, expr) in &mut elem.borrow_mut().bindings {
             if let Expression::Uncompiled(node) = expr {
-                let new_expr = if matches!(base.lookup_property(&*prop), Type::Signal) {
+                let mut lookup_ctx = LookupCtx {
+                    tr,
+                    property_type: base.lookup_property(&*prop),
+                    document_root: doc.root_component.clone(),
+                    diag,
+                };
+
+                let new_expr = if matches!(lookup_ctx.property_type, Type::Signal) {
                     //FIXME: proper signal suport (node is a codeblock)
                     node.child_node(SyntaxKind::Expression)
-                        .map(|en| Expression::from_expression_node(en, diag))
+                        .map(|en| Expression::from_expression_node(en, &mut lookup_ctx))
                         .unwrap_or(Expression::Invalid)
                 } else {
-                    Expression::from_code_statement_node(node.clone(), diag)
+                    Expression::from_code_statement_node(node.clone(), &mut lookup_ctx)
                 };
                 *expr = new_expr;
             }
         }
 
         for child in &elem.borrow().children {
-            resolve_expressions_in_element_recursively(child, diag, tr);
+            resolve_expressions_in_element_recursively(child, doc, diag, tr);
         }
     }
     for x in &doc.inner_components {
-        resolve_expressions_in_element_recursively(&x.root_element, diag, tr)
+        resolve_expressions_in_element_recursively(&x.root_element, doc, diag, tr)
     }
 }

sixtyfps_compiler/generator/cpp.rs

@@ -137,10 +137,9 @@ fn handle_item(item: &LoweredItem, main_struct: &mut Struct, init: &mut Vec<Stri
     init.extend(item.init_properties.iter().map(|(s, i)| {
         use crate::expressions::Expression::*;
         let init = match &i {
-            Invalid | Uncompiled(_) => "".into(),
-            Identifier(i) => i.clone(),
             StringLiteral(s) => format!(r#"sixtyfps::SharedString("{}")"#, s.escape_default()),
             NumberLiteral(n) => n.to_string(),
+            _ => format!("\n#error: unsupported expression {:?}\n", i),
         };
         format!("{id}.{prop}.set({init});", id = id, prop = s, init = init)
     }));

sixtyfps_compiler/lower.rs

@@ -1,5 +1,5 @@
 //! This module contains the code that lower the tree to the datastructure that that the runtime understand
-use crate::typeregister::Type;
+use crate::{expressions::Expression, typeregister::Type};
 use std::collections::HashMap;
 use std::rc::Rc;
 
@@ -19,7 +19,7 @@ pub struct NativeItemType {
 pub struct LoweredItem {
     pub id: String,
     pub native_type: Rc<NativeItemType>,
-    pub init_properties: HashMap<String, crate::expressions::Expression>,
+    pub init_properties: HashMap<String, Expression>,
     /// Right now we only allow forwarding and this connect with the signal in the root
     pub connect_signals: HashMap<String, String>,
     pub children: Vec<LoweredItem>,
@@ -72,31 +72,12 @@ impl LoweredComponent {
         let current_component_id = state.current_component_id.clone();
         let format_signal = |name| format!("{}_{}", current_component_id, name);
         state.signals.extend(element.signals_declaration.iter().map(format_signal));
-
         for (k, e) in element.bindings.iter() {
-            if let crate::expressions::Expression::Identifier(x) = e {
+            if let Expression::SignalReference { name, .. } = e {
-                let value: u32 = match &**x {
+                lowered.connect_signals.insert(
-                    "blue" => 0xff0000ff,
+                    if is_builtin { format!("{}.{}", id, k) } else { format!("{}_{}", id, k) },
-                    "red" => 0xffff0000,
+                    format_signal(name),
-                    "green" => 0xff00ff00,
+                );
-                    "yellow" => 0xffffff00,
-                    "black" => 0xff000000,
-                    "white" => 0xffffffff,
-                    _ => {
-                        lowered.connect_signals.insert(
-                            if is_builtin {
-                                format!("{}.{}", id, k)
-                            } else {
-                                format!("{}_{}", id, k)
-                            },
-                            format_signal(x),
-                        );
-                        continue;
-                    }
-                };
-                lowered
-                    .init_properties
-                    .insert(k.clone(), crate::expressions::Expression::NumberLiteral(value.into()));
             } else {
                 lowered.init_properties.insert(k.clone(), e.clone());
             }

sixtyfps_compiler/object_tree.rs

@@ -4,7 +4,7 @@
 
 use crate::diagnostics::Diagnostics;
 use crate::expressions::Expression;
-use crate::parser::{SyntaxKind, SyntaxNode, SyntaxNodeEx};
+use crate::parser::{Spanned, SyntaxKind, SyntaxNode, SyntaxNodeEx};
 use crate::typeregister::{Type, TypeRegister};
 use std::cell::RefCell;
 use std::collections::HashMap;
@@ -65,8 +65,10 @@ impl Component {
 /// An Element is an instentation of a Component
 #[derive(Default, Debug)]
 pub struct Element {
-    //     node: SyntaxNode,
+    /* node: SyntaxNode, */
+    /// The id as named in the original .60 file
     pub id: String,
+    pub lowered_id: String,
     pub base: QualifiedTypeName,
     pub base_type: crate::typeregister::Type,
     /// Currently contains also the signals. FIXME: should that be changed?

sixtyfps_compiler/parser.rs

@@ -312,7 +312,6 @@ pub trait SyntaxNodeEx {
     fn child_node(&self, kind: SyntaxKind) -> Option<SyntaxNode>;
     fn child_token(&self, kind: SyntaxKind) -> Option<SyntaxToken>;
     fn child_text(&self, kind: SyntaxKind) -> Option<String>;
-    fn span(&self) -> crate::diagnostics::Span;
 }
 
 impl SyntaxNodeEx for SyntaxNode {
@@ -327,6 +326,21 @@ impl SyntaxNodeEx for SyntaxNode {
             .find(|n| n.kind() == kind)
             .and_then(|x| x.as_token().map(|x| x.text().to_string()))
     }
+}
+
+/// Returns a span.  This is implemented for tokens and nodes
+pub trait Spanned {
+    fn span(&self) -> crate::diagnostics::Span;
+}
+
+impl Spanned for SyntaxNode {
+    fn span(&self) -> crate::diagnostics::Span {
+        // FIXME!  this does not work with proc_macro span
+        crate::diagnostics::Span::new(self.text_range().start().into())
+    }
+}
+
+impl Spanned for SyntaxToken {
     fn span(&self) -> crate::diagnostics::Span {
         // FIXME!  this does not work with proc_macro span
         crate::diagnostics::Span::new(self.text_range().start().into())