Skip to content

Commit a54dec0

Browse files
authored
[wgsl-in] Avoid looking up newly introduced parameter names while parsing function parameters and its return type. (gfx-rs#8980)
1 parent c0f04c8 commit a54dec0

3 files changed

Lines changed: 48 additions & 1 deletion

File tree

cts_runner/test.lst

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -374,6 +374,7 @@ webgpu:shader,validation,extension,dual_source_blending:blend_src_stage_input_ou
374374
webgpu:shader,validation,extension,dual_source_blending:blend_src_syntax_validation:*
375375
webgpu:shader,validation,extension,dual_source_blending:use_blend_src_requires_extension_enabled:*
376376
webgpu:shader,validation,extension,pointer_composite_access:*
377+
webgpu:shader,validation,functions,restrictions:param_type_can_be_alias:*
377378
webgpu:shader,validation,parse,blankspace:blankspace:*
378379
webgpu:shader,validation,parse,blankspace:bom:*
379380
webgpu:shader,validation,parse,blankspace:null_characters:contains_null=false;*
@@ -383,6 +384,7 @@ webgpu:shader,validation,parse,enable:*
383384
webgpu:shader,validation,parse,identifiers:*
384385
webgpu:shader,validation,parse,requires:*
385386
webgpu:shader,validation,parse,semicolon:*
387+
webgpu:shader,validation,parse,shadow_builtins:function_param:*
386388
webgpu:shader,validation,parse,source:*
387389
webgpu:shader,validation,shader_io,entry_point:*
388390
webgpu:shader,validation,shader_io,invariant:*

naga/src/front/mod.rs

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,7 @@ pub struct SymbolTable<Name, Var> {
223223
///
224224
/// [`scopes`]: Self::scopes
225225
cursor: usize,
226+
lookup_cursor_is_one_behind: bool,
226227
}
227228

228229
impl<Name, Var> SymbolTable<Name, Var> {
@@ -232,8 +233,13 @@ impl<Name, Var> SymbolTable<Name, Var> {
232233
/// until another scope is pushed or [`pop_scope`] is called, causing this
233234
/// scope to be removed along with all variables added to it.
234235
///
236+
/// # PANICS
237+
/// - If the current lookup scope doesn't match the current scope
238+
///
235239
/// [`pop_scope`]: Self::pop_scope
236240
pub fn push_scope(&mut self) {
241+
self.check_lookup_scope_matches_current_scope();
242+
237243
// If the cursor is equal to the scope's stack length then we need to
238244
// push another empty scope. Otherwise we can reuse the already existing
239245
// scope.
@@ -250,16 +256,46 @@ impl<Name, Var> SymbolTable<Name, Var> {
250256
///
251257
/// # PANICS
252258
/// - If the current lexical scope is the root scope
259+
/// - If the current lookup scope doesn't match the current scope
253260
pub fn pop_scope(&mut self) {
254261
// Despite the method title, the variables are only deleted when the
255262
// scope is reused. This is because while a clear is inevitable if the
256263
// scope needs to be reused, there are cases where the scope might be
257264
// popped and not reused, i.e. if another scope with the same nesting
258265
// level is never pushed again.
259266
assert!(self.cursor != 1, "Tried to pop the root scope");
267+
self.check_lookup_scope_matches_current_scope();
260268

261269
self.cursor -= 1;
262270
}
271+
272+
/// Reduces the lookup scope by one level.
273+
///
274+
/// # PANICS
275+
/// - If the current lookup scope doesn't match the current scope
276+
pub fn reduce_lookup_scope(&mut self) {
277+
self.check_lookup_scope_matches_current_scope();
278+
self.lookup_cursor_is_one_behind = true;
279+
}
280+
281+
/// Resets the lookup scope to the current scope.
282+
///
283+
/// # PANICS
284+
/// - If the current lookup scope already matches the current scope
285+
pub fn reset_lookup_scope(&mut self) {
286+
assert!(
287+
self.lookup_cursor_is_one_behind,
288+
"current lookup scope already matches the current scope"
289+
);
290+
self.lookup_cursor_is_one_behind = false;
291+
}
292+
293+
fn check_lookup_scope_matches_current_scope(&self) {
294+
assert!(
295+
!self.lookup_cursor_is_one_behind,
296+
"current lookup scope doesn't match the current scope"
297+
);
298+
}
263299
}
264300

265301
impl<Name, Var> SymbolTable<Name, Var>
@@ -277,8 +313,11 @@ where
277313
Name: core::borrow::Borrow<Q>,
278314
Q: core::hash::Hash + Eq + ?Sized,
279315
{
316+
let cursor = self
317+
.cursor
318+
.saturating_sub(self.lookup_cursor_is_one_behind.into());
280319
// Iterate backwards through the scopes and try to find the variable
281-
for scope in self.scopes[..self.cursor].iter().rev() {
320+
for scope in self.scopes[..cursor].iter().rev() {
282321
if let Some(var) = scope.get(name) {
283322
return Some(var);
284323
}
@@ -316,6 +355,7 @@ impl<Name, Var> Default for SymbolTable<Name, Var> {
316355
Self {
317356
scopes: vec![FastHashMap::default()],
318357
cursor: 1,
358+
lookup_cursor_is_one_behind: false,
319359
}
320360
}
321361
}

naga/src/front/wgsl/parse/mod.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1712,6 +1712,9 @@ impl Parser {
17121712

17131713
// start a scope that contains arguments as well as the function body
17141714
ctx.local_table.push_scope();
1715+
// Reduce lookup scope to parse the parameter list and return type
1716+
// avoiding identifier lookup to match newly declared param names.
1717+
ctx.local_table.reduce_lookup_scope();
17151718

17161719
// read parameter list
17171720
let mut arguments = Vec::new();
@@ -1759,6 +1762,8 @@ impl Parser {
17591762
None
17601763
};
17611764

1765+
ctx.local_table.reset_lookup_scope();
1766+
17621767
// do not use `self.block` here, since we must not push a new scope
17631768
lexer.expect(Token::Paren('{'))?;
17641769
let brace_nesting_level = 1;

0 commit comments

Comments
 (0)