parse parenthesized expressions #171

vemoo · 2022-05-07T16:49:29Z

As per doc comments tuples are of len != 1

dada/components/dada-ir/src/code/syntax.rs

Lines 135 to 136 in 0793cdf

    
               /// `(expr)` of len != 1 
        
               Tuple(Vec<Expr>),

so I fixed parsing to match that.

fixes #141

netlify · 2022-05-07T16:49:34Z

✅ Deploy Preview for dada-lang ready!

Name	Link
🔨 Latest commit	`d46cd45`
🔍 Latest deploy log	https://app.netlify.com/sites/dada-lang/deploys/627a93c8d71e5c00081065df
😎 Deploy Preview	https://deploy-preview-171--dada-lang.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

vemoo · 2022-05-07T17:44:18Z

print((1))

is causing this assert to fail:

dada/components/dada-validate/src/validate/validator.rs

Lines 205 to 207 in 0793cdf

    
           // Check that the validated expression always has the same 
        
           // origin as the expression we started with. 
        
           assert_eq!(result.origin_in(self.origins).syntax_expr, expr);

I have fixed it by doing:

syntax::ExprData::Parenthesized(parenthesized_expr) => {
    let data = self
        .validate_expr_in_mode(*parenthesized_expr, mode)
        .data(self.tables)
        .clone();
    self.add(data, expr)
}

in

dada/components/dada-validate/src/validate/validator.rs

Lines 400 to 402 in 0793cdf

    
           syntax::ExprData::Parenthesized(parenthesized_expr) => { 
        
               self.validate_expr_in_mode(*parenthesized_expr, mode) 
        
           }

but I don't know if that's the proper fix.

vemoo · 2022-05-08T09:49:11Z

Ok, I think I found the proper fix, by adding validated::ExprData::Parenthesized

xffxff · 2022-05-09T10:34:32Z

components/dada-ir/src/code/syntax.rs

@@ -129,10 +129,10 @@ pub enum ExprData {
    /// `[shared|var|atomic] x = expr`
    Var(LocalVariableDecl, Expr),

-    /// `expr`
+    /// `(expr)`
    Parenthesized(Expr),


We don't seem to have anything parsed as a Parenthsized before 😅 .

nikomatsakis

Nice! I think that the assertion is probably just wrong, though.

nikomatsakis · 2022-05-10T09:17:54Z

components/dada-ir/src/code/validated.rs

@@ -258,6 +258,9 @@ pub enum ExprData {
    /// `expr.give`
    Give(Place),

+    /// `(expr)`
+    Parenthesized(Expr),


Hmm, I think we shouldn't have this in Validated....

...it belongs in Syntax, but Validated is supposed to be kind of only the things that have meaning to the compiler

If this is causing that assertion to fail, we should probably just remove the assertion.

nikomatsakis · 2022-05-10T09:19:15Z

components/dada-validate/src/validate/validator.rs

@@ -398,7 +398,8 @@ impl<'me> Validator<'me> {
            }

            syntax::ExprData::Parenthesized(parenthesized_expr) => {
-                self.validate_expr_in_mode(*parenthesized_expr, mode)
+                let validated_expr = self.validate_expr_in_mode(*parenthesized_expr, mode);
+                self.add(validated::ExprData::Parenthesized(validated_expr), expr)


...in which case this diff is unnecessary, right?

I had to change that because otherwise this assert was failing:

dada/components/dada-validate/src/validate/validator.rs

Lines 194 to 196 in 60986a3

// Check that the validated expression always has the same

// origin as the expression we started with.

assert_eq!(result.origin_in(self.origins).syntax_expr, expr);

Before adding Parenthesized to Validated I did

dada/components/dada-validate/src/validate/validator.rs

Lines 400 to 406 in e4c5bdc

syntax::ExprData::Parenthesized(parenthesized_expr) => {

let data = self

.validate_expr_in_mode(*parenthesized_expr, mode)

.data(self.tables)

.clone();

self.add(data, expr)

}

but that felt wrong. ¿Maybe that is a better fix?

Ah, you mean the assertion in validator.rs, not the one from the #141, right?
There's another one in give_validated_expr.

and use same commet as `validated::ExprData::Tuple` in `syntax::ExprData::Tuple`

nikomatsakis

Looked it over, this seems better, did have one thought...

nikomatsakis · 2022-05-10T22:01:58Z

components/dada-parse/src/parser/code.rs

@@ -449,7 +449,15 @@ impl CodeParser<'_, '_> {
        } else if let Some((span, token_tree)) = self.delimited('(') {
            let expr =
                self.with_sub_parser(token_tree, |subparser| subparser.parse_only_expr_seq());
-            Some(self.add(ExprData::Tuple(expr), span))
+
+            Some(self.add(


For better or worse, I think this change will mean that (a,) also parses as a parenthesized expression. We should check that. Not sure if we want that or not!

I will investigate the best way to emit an error for that case.

Probably we want an error? It's not entirely clear to me.

I'd be ok with just opening up an issue to follow-up on later.

created #177

nikomatsakis · 2022-05-12T08:12:21Z

bors r+

bors · 2022-05-12T08:17:47Z

Build succeeded:

parse parenthesized expressions

0793cdf

xffxff reviewed May 9, 2022

View reviewed changes

nikomatsakis requested changes May 10, 2022

View reviewed changes

vemoo added 2 commits May 10, 2022 18:27

fix syntax::ExprData::Parenthesized doc comment

2b45603

and use same commet as `validated::ExprData::Tuple` in `syntax::ExprData::Tuple`

remove wrong asserts that fail on print((1))

d46cd45

vemoo force-pushed the parenthesized_expr branch from 60986a3 to d46cd45 Compare May 10, 2022 16:33

nikomatsakis requested changes May 10, 2022

View reviewed changes

vemoo mentioned this pull request May 11, 2022

decide how handle (expr,) #177

Open

nikomatsakis approved these changes May 12, 2022

View reviewed changes

bors bot merged commit 4b516b9 into dada-lang:main May 12, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

parse parenthesized expressions #171

parse parenthesized expressions #171

vemoo commented May 7, 2022

netlify bot commented May 7, 2022 •

edited

Loading

vemoo commented May 7, 2022

vemoo commented May 8, 2022

xffxff May 9, 2022

nikomatsakis left a comment •

edited

Loading

nikomatsakis May 10, 2022

nikomatsakis May 10, 2022

nikomatsakis May 10, 2022

nikomatsakis May 10, 2022

vemoo May 10, 2022

vemoo May 10, 2022 •

edited

Loading

nikomatsakis left a comment

nikomatsakis May 10, 2022

vemoo May 11, 2022

nikomatsakis May 11, 2022

nikomatsakis May 11, 2022

vemoo May 11, 2022

nikomatsakis commented May 12, 2022

bors bot commented May 12, 2022

	// Check that the validated expression always has the same
	// origin as the expression we started with.
	assert_eq!(result.origin_in(self.origins).syntax_expr, expr);

	syntax::ExprData::Parenthesized(parenthesized_expr) => {
	let data = self
	.validate_expr_in_mode(*parenthesized_expr, mode)
	.data(self.tables)
	.clone();
	self.add(data, expr)
	}

parse parenthesized expressions #171

parse parenthesized expressions #171

Conversation

vemoo commented May 7, 2022

netlify bot commented May 7, 2022 • edited Loading

✅ Deploy Preview for dada-lang ready!

vemoo commented May 7, 2022

vemoo commented May 8, 2022

Choose a reason for hiding this comment

nikomatsakis left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vemoo May 10, 2022 • edited Loading

Choose a reason for hiding this comment

nikomatsakis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nikomatsakis commented May 12, 2022

bors bot commented May 12, 2022

netlify bot commented May 7, 2022 •

edited

Loading

nikomatsakis left a comment •

edited

Loading

vemoo May 10, 2022 •

edited

Loading