Fix and optimize visit_target usage #510
Conversation
By now only non-nested tuples are accepted by the parser, but this will change. This change makes visit_target() call itself for items in a tuple. So enable the function to call itself, I needed to fix the lifetime annotation, because the references inside a Target instance may outlife a reference to instance itself.
askama_shared/src/generator.rs
Outdated
| } | ||
| Target::Tuple(targets) => { | ||
| let shadowed = targets.iter().any(|name| { | ||
| let (shadowed, _) = self.shadows_or_declares_variable(&Target::Name(name)); |
There was a problem hiding this comment.
self.shadows_or_declares_variable(&Target::Name(name)).0?
askama_shared/src/generator.rs
Outdated
| if shadowed { | ||
| (true, true) | ||
| } else { | ||
| (false, true) | ||
| } |
askama_shared/src/generator.rs
Outdated
| if shadowed { | ||
| // Need to flush the buffer if the variable is being shadowed, | ||
| // to ensure the old variable is used. | ||
| self.write_buf_writable(buf)?; | ||
| } | ||
| if declared { | ||
| buf.write("let "); | ||
| } |
There was a problem hiding this comment.
Would it make sense to inline these effects into shadows_or_declares_variable() (I suppose it would need yet another name)? Splitting it this way doesn't make for a great abstraction.
askama_shared/src/generator.rs
Outdated
| if initialized { | ||
| self.locals.insert(name, LocalMeta::initialized()); | ||
| } else { | ||
| self.locals.insert_with_default(name); | ||
| } |
There was a problem hiding this comment.
Here's an example where I'd probably write a match instead. What do you think?
This change also fixes a bug in the loop generator, which failed for shadowed variables.
|
I addressed you comments. Actually I'm not sure if I like matching bools. I'm quite new to rust and as a Python / C++ programmer it just feels wrong. You'd get publicly stoned if you shared C++ code that contains |
Can you articulate what you dislike about them? |
There was a bit of code duplication in generator.rs with respect to the output of targets.
Also there was a bug which caused broken code for
which tried to write to
valinstead of shadowing it.