From 9464a81ae216a4224bbf52d45e4f119ed2cc7914 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Capucho?= Date: Sat, 22 May 2021 20:26:25 +0100 Subject: [PATCH] [glsl-in] Fix parameter qualifiers Previously the types where changed in a later phase now they are changed right when the argument is added, this makes the implementation slightly less spec compliant because the following code will compile whilst it shouldn't ```glsl void test(float a) {} void test(out float b) {} ``` --- src/front/glsl/ast.rs | 27 ++++++++--- src/front/glsl/functions.rs | 88 +++++++++++----------------------- src/front/glsl/parser.rs | 29 +++++++++-- src/front/glsl/parser_tests.rs | 16 +++++++ 4 files changed, 90 insertions(+), 70 deletions(-) diff --git a/src/front/glsl/ast.rs b/src/front/glsl/ast.rs index f43f69cfe5..9d8d5217ee 100644 --- a/src/front/glsl/ast.rs +++ b/src/front/glsl/ast.rs @@ -25,7 +25,7 @@ pub struct FunctionSignature { #[derive(Debug, Clone)] pub struct FunctionDeclaration { - pub parameters: Vec, + pub qualifiers: Vec, pub handle: Handle, /// Wheter this function was already defined or is just a prototype pub defined: bool, @@ -295,22 +295,37 @@ impl<'function> Context<'function> { /// Add function argument to current scope pub fn add_function_arg( &mut self, + program: &mut Program, + sig: &mut FunctionSignature, body: &mut Block, name: Option, ty: Handle, - parameter: ParameterQualifier, + qualifier: ParameterQualifier, ) { let index = self.arguments.len(); - self.arguments.push(FunctionArgument { + let mut arg = FunctionArgument { name: name.clone(), ty, binding: None, - }); + }; + sig.parameters.push(ty); + + if qualifier.is_lhs() { + arg.ty = program.module.types.fetch_or_append(Type { + name: None, + inner: TypeInner::Pointer { + base: arg.ty, + class: StorageClass::Function, + }, + }) + } + + self.arguments.push(arg); if let Some(name) = name { let expr = self.add_expression(Expression::FunctionArgument(index as u32), body); - let mutable = parameter != ParameterQualifier::Const; - let load = if parameter.is_lhs() { + let mutable = qualifier != ParameterQualifier::Const; + let load = if qualifier.is_lhs() { Some(self.add_expression(Expression::Load { pointer: expr }, body)) } else { None diff --git a/src/front/glsl/functions.rs b/src/front/glsl/functions.rs index a5ffa082da..8bb5d04a73 100644 --- a/src/front/glsl/functions.rs +++ b/src/front/glsl/functions.rs @@ -1,8 +1,8 @@ use crate::{ proc::ensure_block_returns, Arena, BinaryOperator, Binding, Block, BuiltIn, EntryPoint, Expression, Function, FunctionArgument, FunctionResult, Handle, MathFunction, - RelationalFunction, SampleLevel, ShaderStage, Statement, StorageClass, StructMember, - SwizzleComponent, Type, TypeInner, + RelationalFunction, SampleLevel, ShaderStage, Statement, StructMember, SwizzleComponent, Type, + TypeInner, }; use super::{ast::*, error::ErrorKind, SourceMetadata}; @@ -337,7 +337,7 @@ impl Program<'_> { .clone(); let mut arguments = Vec::with_capacity(raw_args.len()); - for (qualifier, expr) in fun.parameters.iter().zip(raw_args.iter()) { + for (qualifier, expr) in fun.qualifiers.iter().zip(raw_args.iter()) { let handle = ctx.lower_expect(self, *expr, qualifier.is_lhs(), body)?.0; arguments.push(handle) } @@ -390,37 +390,17 @@ impl Program<'_> { pub fn add_function( &mut self, mut function: Function, - parameters: Vec, + sig: FunctionSignature, + qualifiers: Vec, meta: SourceMetadata, ) -> Result<(), ErrorKind> { ensure_block_returns(&mut function.body); - let name = function - .name - .clone() - .ok_or_else(|| ErrorKind::SemanticError(meta, "Unnamed function".into()))?; - let stage = self.entry_points.get(&name); + let stage = self.entry_points.get(&sig.name); if let Some(&stage) = stage { let handle = self.module.functions.append(function); - self.entries.push((name, stage, handle)); + self.entries.push((sig.name, stage, handle)); } else { - let sig = FunctionSignature { - name, - parameters: function.arguments.iter().map(|p| p.ty).collect(), - }; - - for (arg, qualifier) in function.arguments.iter_mut().zip(parameters.iter()) { - if qualifier.is_lhs() { - arg.ty = self.module.types.fetch_or_append(Type { - name: None, - inner: TypeInner::Pointer { - base: arg.ty, - class: StorageClass::Function, - }, - }) - } - } - let void = function.result.is_none(); if let Some(decl) = self.lookup_function.get_mut(&sig) { @@ -438,7 +418,7 @@ impl Program<'_> { self.lookup_function.insert( sig, FunctionDeclaration { - parameters, + qualifiers, handle, defined: true, void, @@ -452,43 +432,33 @@ impl Program<'_> { pub fn add_prototype( &mut self, - mut function: Function, - parameters: Vec, + function: Function, + sig: FunctionSignature, + qualifiers: Vec, meta: SourceMetadata, ) -> Result<(), ErrorKind> { - let name = function - .name - .clone() - .ok_or_else(|| ErrorKind::SemanticError(meta, "Unnamed function".into()))?; - let sig = FunctionSignature { - name, - parameters: function.arguments.iter().map(|p| p.ty).collect(), - }; let void = function.result.is_none(); - for (arg, qualifier) in function.arguments.iter_mut().zip(parameters.iter()) { - if qualifier.is_lhs() { - arg.ty = self.module.types.fetch_or_append(Type { - name: None, - inner: TypeInner::Pointer { - base: arg.ty, - class: StorageClass::Function, - }, - }) - } - } - let handle = self.module.functions.append(function); - self.lookup_function.insert( - sig, - FunctionDeclaration { - parameters, - handle, - defined: false, - void, - }, - ); + if self + .lookup_function + .insert( + sig, + FunctionDeclaration { + qualifiers, + handle, + defined: false, + void, + }, + ) + .is_some() + { + return Err(ErrorKind::SemanticError( + meta, + "Prototype already defined".into(), + )); + } Ok(()) } diff --git a/src/front/glsl/parser.rs b/src/front/glsl/parser.rs index a3552ea169..cd7370631d 100644 --- a/src/front/glsl/parser.rs +++ b/src/front/glsl/parser.rs @@ -1,7 +1,7 @@ use super::{ ast::{ - Context, FunctionCall, FunctionCallKind, GlobalLookup, HirExpr, HirExprKind, - ParameterQualifier, Profile, StorageQualifier, StructLayout, TypeQualifier, + Context, FunctionCall, FunctionCallKind, FunctionSignature, GlobalLookup, HirExpr, + HirExprKind, ParameterQualifier, Profile, StorageQualifier, StructLayout, TypeQualifier, }, error::ErrorKind, lex::Lexer, @@ -574,6 +574,10 @@ impl<'source, 'program, 'options> Parser<'source, 'program, 'options> { let mut arguments = Vec::new(); let mut parameters = Vec::new(); let mut body = Block::new(); + let mut sig = FunctionSignature { + name: name.clone(), + parameters: Vec::new(), + }; let mut context = Context::new( self.program, @@ -583,7 +587,12 @@ impl<'source, 'program, 'options> Parser<'source, 'program, 'options> { &mut arguments, ); - self.parse_function_args(&mut context, &mut body, &mut parameters)?; + self.parse_function_args( + &mut context, + &mut body, + &mut parameters, + &mut sig, + )?; let end_meta = self.expect(TokenValue::RightParen)?.meta; meta = meta.union(&end_meta); @@ -599,6 +608,7 @@ impl<'source, 'program, 'options> Parser<'source, 'program, 'options> { arguments, ..Default::default() }, + sig, parameters, meta, )?; @@ -622,6 +632,7 @@ impl<'source, 'program, 'options> Parser<'source, 'program, 'options> { arguments, body, }, + sig, parameters, meta, )?; @@ -1555,6 +1566,7 @@ impl<'source, 'program, 'options> Parser<'source, 'program, 'options> { context: &mut Context, body: &mut Block, parameters: &mut Vec, + sig: &mut FunctionSignature, ) -> Result<()> { loop { if self.peek_type_name() || self.peek_parameter_qualifier() { @@ -1565,7 +1577,7 @@ impl<'source, 'program, 'options> Parser<'source, 'program, 'options> { match self.expect_peek()?.value { TokenValue::Comma => { self.bump()?; - context.add_function_arg(body, None, ty, qualifier); + context.add_function_arg(&mut self.program, sig, body, None, ty, qualifier); continue; } TokenValue::Identifier(_) => { @@ -1574,7 +1586,14 @@ impl<'source, 'program, 'options> Parser<'source, 'program, 'options> { let size = self.parse_array_specifier()?; let ty = self.maybe_array(ty, size); - context.add_function_arg(body, Some(name), ty, qualifier); + context.add_function_arg( + &mut self.program, + sig, + body, + Some(name), + ty, + qualifier, + ); if self.bump_if(TokenValue::Comma).is_some() { continue; diff --git a/src/front/glsl/parser_tests.rs b/src/front/glsl/parser_tests.rs index 91c0e22fc4..fda0183888 100644 --- a/src/front/glsl/parser_tests.rs +++ b/src/front/glsl/parser_tests.rs @@ -397,6 +397,22 @@ fn functions() { &entry_points, ) .unwrap(); + + parse_program( + r#" + # version 450 + void fun(vec2 in_parameter, out float out_parameter) { + ivec2 _ = ivec2(in_parameter); + } + + void main() { + float a; + fun(vec2(1.0), a); + } + "#, + &entry_points, + ) + .unwrap(); } #[test]