Skip to content
This repository has been archived by the owner on Jan 29, 2025. It is now read-only.

Commit

Permalink
wgsl-in: Implement lexical scopes
Browse files Browse the repository at this point in the history 
Previously the wgsl frontend wasn't aware of lexical scopes causing all
variables and named expressions to share a single function scope, this
meant that if a variable was defined in a block with the same name as a
variable in the function body, the variable in the function body would
be lost and exiting the block all references to the variable in the
function body would be replaced with the variable of the block.

This commit fixes that by using the previously introduced `SymbolTable`
to track the lexical and perform the variable lookups, scopes are pushed
and popped as defined in the wgsl specification.
  • Loading branch information
@JCapucho @jimblandy
JCapucho authored and jimblandy committed Sep 2, 2022
1 parent fc9d222 commit 0a3a4eb
Show file tree
Hide file tree
Showing 5 changed files with 189 additions and 35 deletions.
42 changes: 21 additions & 21 deletions src/front/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ type Scope<Name, Var> = FastHashMap<Name, Var>;
/// Structure responsible for managing variable lookups and keeping track of
/// lexical scopes
///
/// The symbol table is generic over the variable representation and it's name
/// The symbol table is generic over the variable representation and its name
/// to allow larger flexibility on the frontends on how they might represent them.
///
/// ```
Expand All @@ -161,13 +161,13 @@ type Scope<Name, Var> = FastHashMap<Name, Var>;
/// // Check that `var1` exists and is `0`
/// assert_eq!(symbol_table.lookup("var1"), Some(&0));
///
/// // Push a new scope and add a variable to it with name `var1` shadowing the
/// // Push a new scope and add a variable to it named `var1` shadowing the
/// // variable of our previous scope
/// symbol_table.push_scope();
/// symbol_table.add("var1", 1);
///
/// // Check that `var1` now points to the new value of `1` and `var2` still
/// // exists with it's value of `2`
/// // exists with its value of `2`
/// assert_eq!(symbol_table.lookup("var1"), Some(&1));
/// assert_eq!(symbol_table.lookup("var2"), Some(&2));
///
Expand All @@ -178,15 +178,15 @@ type Scope<Name, Var> = FastHashMap<Name, Var>;
/// assert_eq!(symbol_table.lookup("var1"), Some(&0));
/// ```
///
/// Scopes are ordered as LIFO stack so a variable defined in a later scope
/// Scopes are ordered as a LIFO stack so a variable defined in a later scope
/// with the same name as another variable defined in a earlier scope will take
/// precedence in the lookup. Scopes can be added with [`push_scope`] and
/// removed with [`pop_scope`].
///
/// A root scope is added when the symbol table is created and must always be
/// present, trying to pop it will result in a panic.
/// present. Trying to pop it will result in a panic.
///
/// Variables can be added with [`add`] and looked up with [`lookup`], adding a
/// Variables can be added with [`add`] and looked up with [`lookup`]. Adding a
/// variable will do so in the currently active scope and as mentioned
/// previously a lookup will search from the current scope to the root scope.
///
Expand All @@ -195,29 +195,29 @@ type Scope<Name, Var> = FastHashMap<Name, Var>;
/// [`add`]: Self::add
/// [`lookup`]: Self::lookup
pub struct SymbolTable<Name, Var> {
/// Stack of lexical scopes, not all scopes are active see [`cursor`]
/// Stack of lexical scopes. Not all scopes are active; see [`cursor`].
///
/// [`cursor`]: Self::cursor
scopes: Vec<Scope<Name, Var>>,
/// Limit of the [`scopes`] stack (exclusive), by using a separate value for
/// the stack length instead of `Vec`'s own internal length the scopes can
/// be reused to cache memory allocations
/// Limit of the [`scopes`] stack (exclusive). By using a separate value for
/// the stack length instead of `Vec`'s own internal length, the scopes can
/// be reused to cache memory allocations.
///
/// [`scopes`]: Self::scopes
cursor: usize,
}

impl<Name, Var> SymbolTable<Name, Var> {
/// Adds a new lexical scope
/// Adds a new lexical scope.
///
/// All variables declared after this points will be added to this scope
/// until another scope is pushed or [`pop_scope`] is called causing this
/// All variables declared after this point will be added to this scope
/// until another scope is pushed or [`pop_scope`] is called, causing this
/// scope to be removed along with all variables added to it.
///
/// [`pop_scope`]: Self::pop_scope
pub fn push_scope(&mut self) {
// If the cursor is equal to the scopes stack length then we need to
// push another empty scope, otherwise we can reuse the already existing
// If the cursor is equal to the scope's stack length then we need to
// push another empty scope. Otherwise we can reuse the already existing
// scope.
if self.scopes.len() == self.cursor {
self.scopes.push(FastHashMap::default())
Expand All @@ -228,13 +228,13 @@ impl<Name, Var> SymbolTable<Name, Var> {
self.cursor += 1;
}

/// Removes the current lexical scope and all it's variables
/// Removes the current lexical scope and all its variables
///
/// # PANICS
/// - If the current lexical scope is the root scope
pub fn pop_scope(&mut self) {
// Despite the method title, the variables are only deleted when the
// scope is reused, this is because while a clear is inevitable if the
// scope is reused. This is because while a clear is inevitable if the
// scope needs to be reused, there are cases where the scope might be
// popped and not reused, i.e. if another scope with the same nesting
// level is never pushed again.
Expand All @@ -248,7 +248,7 @@ impl<Name, Var> SymbolTable<Name, Var>
where
Name: std::hash::Hash + Eq,
{
/// Perform a lookup for a variable named `name`
/// Perform a lookup for a variable named `name`.
///
/// As stated in the struct level documentation the lookup will proceed from
/// the current scope to the root scope, returning `Some` when a variable is
Expand All @@ -269,11 +269,11 @@ where
None
}

/// Adds a new variable to the current scope
/// Adds a new variable to the current scope.
///
/// Returns the previous variable with the same name in this scope if it
/// exists so that the frontend might handle it in case variable shadowing
/// is disallowed
/// exists, so that the frontend might handle it in case variable shadowing
/// is disallowed.
pub fn add(&mut self, name: Name, var: Var) -> Option<Var> {
self.scopes[self.cursor - 1].insert(name, var)
}
Expand Down
63 changes: 49 additions & 14 deletions src/front/wgsl/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -749,7 +749,7 @@ impl<'a> StringValueLookup<'a> for FastHashMap<&'a str, TypedExpression> {
}

struct StatementContext<'input, 'temp, 'out> {
lookup_ident: &'temp mut FastHashMap<&'input str, TypedExpression>,
symbol_table: &'temp mut super::SymbolTable<&'input str, TypedExpression>,
typifier: &'temp mut super::Typifier,
variables: &'out mut Arena<crate::LocalVariable>,
expressions: &'out mut Arena<crate::Expression>,
Expand All @@ -764,7 +764,7 @@ struct StatementContext<'input, 'temp, 'out> {
impl<'a, 'temp> StatementContext<'a, 'temp, '_> {
fn reborrow(&mut self) -> StatementContext<'a, '_, '_> {
StatementContext {
lookup_ident: self.lookup_ident,
symbol_table: self.symbol_table,
typifier: self.typifier,
variables: self.variables,
expressions: self.expressions,
Expand All @@ -786,7 +786,7 @@ impl<'a, 'temp> StatementContext<'a, 'temp, '_> {
'temp: 't,
{
ExpressionContext {
lookup_ident: self.lookup_ident,
symbol_table: self.symbol_table,
typifier: self.typifier,
expressions: self.expressions,
types: self.types,
Expand All @@ -807,7 +807,7 @@ struct SamplingContext {
}

struct ExpressionContext<'input, 'temp, 'out> {
lookup_ident: &'temp FastHashMap<&'input str, TypedExpression>,
symbol_table: &'temp mut super::SymbolTable<&'input str, TypedExpression>,
typifier: &'temp mut super::Typifier,
expressions: &'out mut Arena<crate::Expression>,
types: &'out mut UniqueArena<crate::Type>,
Expand All @@ -823,7 +823,7 @@ struct ExpressionContext<'input, 'temp, 'out> {
impl<'a> ExpressionContext<'a, '_, '_> {
fn reborrow(&mut self) -> ExpressionContext<'a, '_, '_> {
ExpressionContext {
lookup_ident: self.lookup_ident,
symbol_table: self.symbol_table,
typifier: self.typifier,
expressions: self.expressions,
types: self.types,
Expand Down Expand Up @@ -2286,7 +2286,7 @@ impl Parser {
)
}
(Token::Word(word), span) => {
if let Some(definition) = ctx.lookup_ident.get(word) {
if let Some(definition) = ctx.symbol_table.lookup(word) {
let _ = lexer.next();
self.pop_scope(lexer);

Expand Down Expand Up @@ -3432,6 +3432,9 @@ impl Parser {
mut context: StatementContext<'a, '_, 'out>,
) -> Result<(bool, crate::Block), Error<'a>> {
let mut body = crate::Block::new();
// Push a new lexical scope for the switch case body
context.symbol_table.push_scope();

lexer.expect(Token::Paren('{'))?;
let fall_through = loop {
// default statements
Expand All @@ -3445,6 +3448,8 @@ impl Parser {
}
self.parse_statement(lexer, context.reborrow(), &mut body, false)?;
};
// Pop the switch case body lexical scope
context.symbol_table.pop_scope();

Ok((fall_through, body))
}
Expand All @@ -3465,6 +3470,9 @@ impl Parser {
}
(Token::Paren('{'), _) => {
self.push_scope(Scope::Block, lexer);
// Push a new lexical scope for the block statement
context.symbol_table.push_scope();

let _ = lexer.next();
let mut statements = crate::Block::new();
while !lexer.skip(Token::Paren('}')) {
Expand All @@ -3475,6 +3483,9 @@ impl Parser {
is_uniform_control_flow,
)?;
}
// Pop the block statement lexical scope
context.symbol_table.pop_scope();

self.pop_scope(lexer);
let span = NagaSpan::from(self.pop_scope(lexer));
block.push(crate::Statement::Block(statements), span);
Expand Down Expand Up @@ -3539,7 +3550,7 @@ impl Parser {
}
}
block.extend(emitter.finish(context.expressions));
context.lookup_ident.insert(
context.symbol_table.add(
name,
TypedExpression {
handle: expr_id,
Expand Down Expand Up @@ -3655,7 +3666,7 @@ impl Parser {
let expr_id = context
.expressions
.append(crate::Expression::LocalVariable(var_id), Default::default());
context.lookup_ident.insert(
context.symbol_table.add(
name,
TypedExpression {
handle: expr_id,
Expand Down Expand Up @@ -3843,10 +3854,14 @@ impl Parser {
},
NagaSpan::from(span),
);
// Push a lexical scope for the while loop body
context.symbol_table.push_scope();

while !lexer.skip(Token::Paren('}')) {
self.parse_statement(lexer, context.reborrow(), &mut body, false)?;
}
// Pop the while loop body lexical scope
context.symbol_table.pop_scope();

Some(crate::Statement::Loop {
body,
Expand All @@ -3857,6 +3872,9 @@ impl Parser {
"for" => {
let _ = lexer.next();
lexer.expect(Token::Paren('('))?;
// Push a lexical scope for the for loop
context.symbol_table.push_scope();

if !lexer.skip(Token::Separator(';')) {
let num_statements = block.len();
let (_, span) = lexer.capture_span(|lexer| {
Expand Down Expand Up @@ -3904,7 +3922,9 @@ impl Parser {
let mut continuing = crate::Block::new();
if !lexer.skip(Token::Paren(')')) {
match lexer.peek().0 {
Token::Word(ident) if context.lookup_ident.get(ident).is_none() => {
Token::Word(ident)
if context.symbol_table.lookup(ident).is_none() =>
{
self.parse_function_statement(
lexer,
ident,
Expand All @@ -3923,6 +3943,8 @@ impl Parser {
while !lexer.skip(Token::Paren('}')) {
self.parse_statement(lexer, context.reborrow(), &mut body, false)?;
}
// Pop the for loop lexical scope
context.symbol_table.pop_scope();

Some(crate::Statement::Loop {
body,
Expand Down Expand Up @@ -4013,7 +4035,7 @@ impl Parser {
}
// assignment or a function call
ident => {
match context.lookup_ident.get(ident) {
match context.symbol_table.lookup(ident) {
Some(_) => self.parse_assignment_statement(
lexer,
context.as_expression(block, &mut emitter),
Expand Down Expand Up @@ -4052,6 +4074,10 @@ impl Parser {
let mut body = crate::Block::new();
let mut continuing = crate::Block::new();
let mut break_if = None;

// Push a lexical scope for the loop body
context.symbol_table.push_scope();

lexer.expect(Token::Paren('{'))?;

loop {
Expand Down Expand Up @@ -4113,6 +4139,9 @@ impl Parser {
self.parse_statement(lexer, context.reborrow(), &mut body, false)?;
}

// Pop the loop body lexical scope
context.symbol_table.pop_scope();

Ok(crate::Statement::Loop {
body,
continuing,
Expand All @@ -4127,6 +4156,9 @@ impl Parser {
is_uniform_control_flow: bool,
) -> Result<crate::Block, Error<'a>> {
self.push_scope(Scope::Block, lexer);
// Push a lexical scope for the block
context.symbol_table.push_scope();

lexer.expect(Token::Paren('{'))?;
let mut block = crate::Block::new();
while !lexer.skip(Token::Paren('}')) {
Expand All @@ -4137,6 +4169,9 @@ impl Parser {
is_uniform_control_flow,
)?;
}
//Pop the block lexical scope
context.symbol_table.pop_scope();

self.pop_scope(lexer);
Ok(block)
}
Expand Down Expand Up @@ -4165,7 +4200,7 @@ impl Parser {
) -> Result<(crate::Function, &'a str), Error<'a>> {
self.push_scope(Scope::FunctionDecl, lexer);
// read function name
let mut lookup_ident = FastHashMap::default();
let mut symbol_table = super::SymbolTable::default();
let (fun_name, span) = lexer.next_ident_with_span()?;
if crate::keywords::wgsl::RESERVED.contains(&fun_name) {
return Err(Error::ReservedKeyword(span));
Expand All @@ -4191,7 +4226,7 @@ impl Parser {
_ => unreachable!(),
};
let expression = expressions.append(expression.clone(), span);
lookup_ident.insert(
symbol_table.add(
name,
TypedExpression {
handle: expression,
Expand Down Expand Up @@ -4221,7 +4256,7 @@ impl Parser {
crate::Expression::FunctionArgument(param_index),
NagaSpan::from(param_name_span),
);
lookup_ident.insert(
symbol_table.add(
param_name,
TypedExpression {
handle: expression,
Expand Down Expand Up @@ -4266,7 +4301,7 @@ impl Parser {
fun.body = self.parse_block(
lexer,
StatementContext {
lookup_ident: &mut lookup_ident,
symbol_table: &mut symbol_table,
typifier: &mut typifier,
variables: &mut fun.local_variables,
expressions: &mut fun.expressions,
Expand Down
Loading

0 comments on commit 0a3a4eb

Please sign in to comment.