From 4bc4c60663a7ef68711cbf1f1536bb2b2857e7bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Capucho?= Date: Sat, 19 Jun 2021 15:50:28 +0100 Subject: [PATCH] [glsl-in] Implicit casts for function calls --- src/front/glsl/ast.rs | 45 +--- src/front/glsl/functions.rs | 250 ++++++++++++------ src/front/glsl/parser.rs | 40 +-- src/front/glsl/parser_tests.rs | 48 ++++ src/front/mod.rs | 24 -- tests/in/glsl/900-implicit-conversions.vert | 15 ++ .../wgsl/900-implicit-conversions-vert.wgsl | 39 +++ 7 files changed, 307 insertions(+), 154 deletions(-) create mode 100644 tests/in/glsl/900-implicit-conversions.vert create mode 100644 tests/out/wgsl/900-implicit-conversions-vert.wgsl diff --git a/src/front/glsl/ast.rs b/src/front/glsl/ast.rs index aff74e9f23..f7e920888d 100644 --- a/src/front/glsl/ast.rs +++ b/src/front/glsl/ast.rs @@ -26,14 +26,10 @@ pub struct GlobalLookup { pub mutable: bool, } -#[derive(Debug, PartialEq, Eq, Hash)] -pub struct FunctionSignature { - pub name: String, - pub parameters: Vec>, -} - #[derive(Debug, Clone)] pub struct FunctionDeclaration { + /// Normalized function parameters, modifiers are not applied + pub parameters: Vec>, pub qualifiers: Vec, pub handle: Handle, /// Wheter this function was already defined or is just a prototype @@ -84,7 +80,7 @@ pub struct Program<'a> { pub workgroup_size: [u32; 3], pub early_fragment_tests: bool, - pub lookup_function: FastHashMap, + pub lookup_function: FastHashMap>, pub lookup_type: FastHashMap>, pub global_variables: Vec<(String, GlobalLookup)>, @@ -120,7 +116,7 @@ impl<'a> Program<'a> { } pub fn resolve_type<'b>( - &'b mut self, + &'b self, context: &'b mut Context, handle: Handle, meta: SourceMetadata, @@ -146,33 +142,6 @@ impl<'a> Program<'a> { } } - pub fn resolve_handle( - &mut self, - context: &mut Context, - handle: Handle, - meta: SourceMetadata, - ) -> Result, ErrorKind> { - let resolve_ctx = ResolveContext { - constants: &self.module.constants, - types: &self.module.types, - global_vars: &self.module.global_variables, - local_vars: context.locals, - functions: &self.module.functions, - arguments: context.arguments, - }; - match context - .typifier - .grow(handle, context.expressions, &resolve_ctx) - { - //TODO: better error report - Err(error) => Err(ErrorKind::SemanticError( - meta, - format!("Can't resolve type: {:?}", error).into(), - )), - Ok(()) => Ok(context.typifier.get_handle(handle, &mut self.module.types)), - } - } - pub fn solve_constant( &mut self, ctx: &Context, @@ -362,7 +331,7 @@ impl<'function> Context<'function> { pub fn add_function_arg( &mut self, program: &mut Program, - sig: &mut FunctionSignature, + parameters: &mut Vec>, body: &mut Block, name: Option, ty: Handle, @@ -374,7 +343,7 @@ impl<'function> Context<'function> { ty, binding: None, }; - sig.parameters.push(ty); + parameters.push(ty); if qualifier.is_lhs() { arg.ty = program.module.types.fetch_or_append(Type { @@ -880,7 +849,7 @@ impl<'function> Context<'function> { } } -fn type_power(kind: ScalarKind) -> Option { +pub fn type_power(kind: ScalarKind) -> Option { Some(match kind { ScalarKind::Sint => 0, ScalarKind::Uint => 1, diff --git a/src/front/glsl/functions.rs b/src/front/glsl/functions.rs index 9095f4c4b1..ecadf0ac06 100644 --- a/src/front/glsl/functions.rs +++ b/src/front/glsl/functions.rs @@ -384,67 +384,126 @@ impl Program<'_> { )) } _ => { - let mut parameters = Vec::new(); + let declarations = self.lookup_function.get(&name).ok_or_else(|| { + ErrorKind::SemanticError( + meta, + format!("Unknown function '{}'", name).into(), + ) + })?; - for (e, meta) in args { - let handle = self.resolve_handle(ctx, e, meta)?; + let mut maybe_decl = None; - parameters.push(handle) + 'outer: for decl in declarations { + if args.len() != decl.parameters.len() { + continue; + } + + let mut exact = true; + + for (decl_arg, call_arg) in decl.parameters.iter().zip(args.iter()) { + let decl_inner = &self.module.types[*decl_arg].inner; + let call_inner = self.resolve_type(ctx, call_arg.0, call_arg.1)?; + + if decl_inner != call_inner { + exact = false; + + match ( + decl_inner.scalar_kind().and_then(type_power), + call_inner.scalar_kind().and_then(type_power), + ) { + (Some(decl_power), Some(call_power)) => { + if decl_power < call_power { + continue 'outer; + } + } + _ => continue 'outer, + } + } + } + + if exact { + maybe_decl = Some(decl); + break; + } else if maybe_decl.is_some() { + return Err(ErrorKind::SemanticError( + meta, + format!("Ambiguous best function for '{}'", name).into(), + )); + } else { + maybe_decl = Some(decl) + } } - let sig = FunctionSignature { name, parameters }; + let decl = maybe_decl.ok_or_else(|| { + ErrorKind::SemanticError( + meta, + format!("Unknown function '{}'", name).into(), + ) + })?; - let fun = self - .lookup_function - .get(&sig) - .ok_or_else(|| { - ErrorKind::SemanticError( - meta, - // FIXME: Proper signature display - format!("Unknown function: {:?}", sig).into(), - ) - })? - .clone(); + let qualifiers = decl.qualifiers.clone(); + let parameters = decl.parameters.clone(); + let function = decl.handle; + let is_void = decl.void; - let mut arguments = Vec::with_capacity(raw_args.len()); + let mut arguments = Vec::with_capacity(args.len()); let mut proxy_writes = Vec::new(); - for (qualifier, expr) in fun.qualifiers.iter().zip(raw_args.iter()) { - let handle = ctx.lower_expect(self, *expr, qualifier.is_lhs(), body)?.0; - if qualifier.is_lhs() - && matches! { ctx.get_expression(handle), &Expression::Swizzle { .. } } + for (qualifier, (expr, parameter)) in qualifiers + .iter() + .zip(raw_args.iter().zip(parameters.iter())) + { + let (mut handle, meta) = + ctx.lower_expect(self, *expr, qualifier.is_lhs(), body)?; + + if let TypeInner::Vector { size, kind, width } = + *self.resolve_type(ctx, handle, meta)? { - let meta = ctx.hir_exprs[*expr].meta; - let ty = self.resolve_handle(ctx, handle, meta)?; - let temp_var = ctx.locals.append(LocalVariable { - name: None, - ty, - init: None, - }); - let temp_expr = - ctx.add_expression(Expression::LocalVariable(temp_var), body); + if qualifier.is_lhs() + && matches!( + *ctx.get_expression(handle), + Expression::Swizzle { .. } + ) + { + let ty = self.module.types.append(Type { + name: None, + inner: TypeInner::Vector { size, kind, width }, + }); + let temp_var = ctx.locals.append(LocalVariable { + name: None, + ty, + init: None, + }); + let temp_expr = ctx + .add_expression(Expression::LocalVariable(temp_var), body); - body.push(Statement::Store { - pointer: temp_expr, - value: handle, - }); + body.push(Statement::Store { + pointer: temp_expr, + value: handle, + }); - arguments.push(temp_expr); - proxy_writes.push((*expr, temp_expr)); - } else { - arguments.push(handle); + arguments.push(temp_expr); + proxy_writes.push((*expr, temp_expr)); + continue; + } } + + if let Some(kind) = self.module.types[*parameter].inner.scalar_kind() { + ctx.implicit_conversion(self, &mut handle, meta, kind)?; + } + + arguments.push(handle) } ctx.emit_flush(body); - let result = if !fun.void { - Some(ctx.add_expression(Expression::Call(fun.handle), body)) + let result = if !is_void { + Some(ctx.add_expression(Expression::Call(function), body)) } else { None }; body.push(crate::Statement::Call { - function: fun.handle, + function, arguments, result, }); @@ -505,22 +564,46 @@ impl Program<'_> { pub fn add_function( &mut self, mut function: Function, - sig: FunctionSignature, + name: String, + // Normalized function parameters, modifiers are not applied + parameters: Vec>, qualifiers: Vec, meta: SourceMetadata, ) -> Result, ErrorKind> { ensure_block_returns(&mut function.body); - let stage = self.entry_points.get(&sig.name); + let stage = self.entry_points.get(&name); Ok(if let Some(&stage) = stage { let handle = self.module.functions.append(function); - self.entries.push((sig.name, stage, handle)); + self.entries.push((name, stage, handle)); self.function_arg_use.push(Vec::new()); handle } else { let void = function.result.is_none(); - if let Some(decl) = self.lookup_function.get_mut(&sig) { + let &mut Program { + ref mut lookup_function, + ref mut module, + .. + } = self; + + let declarations = lookup_function.entry(name).or_default(); + + 'outer: for decl in declarations.iter_mut() { + if parameters.len() != decl.parameters.len() { + continue; + } + + for (new_parameter, old_parameter) in parameters.iter().zip(decl.parameters.iter()) + { + let new_inner = &module.types[*new_parameter].inner; + let old_inner = &module.types[*old_parameter].inner; + + if new_inner != old_inner { + continue 'outer; + } + } + if decl.defined { return Err(ErrorKind::SemanticError( meta, @@ -529,56 +612,73 @@ impl Program<'_> { } decl.defined = true; + decl.qualifiers = qualifiers; *self.module.functions.get_mut(decl.handle) = function; - decl.handle - } else { - self.function_arg_use.push(Vec::new()); - let handle = self.module.functions.append(function); - self.lookup_function.insert( - sig, - FunctionDeclaration { - qualifiers, - handle, - defined: true, - void, - }, - ); - handle + return Ok(decl.handle); } + + self.function_arg_use.push(Vec::new()); + let handle = module.functions.append(function); + declarations.push(FunctionDeclaration { + parameters, + qualifiers, + handle, + defined: true, + void, + }); + handle }) } pub fn add_prototype( &mut self, function: Function, - sig: FunctionSignature, + name: String, + // Normalized function parameters, modifiers are not applied + parameters: Vec>, qualifiers: Vec, meta: SourceMetadata, ) -> Result<(), ErrorKind> { let void = function.result.is_none(); - self.function_arg_use.push(Vec::new()); - let handle = self.module.functions.append(function); + let &mut Program { + ref mut lookup_function, + ref mut module, + .. + } = self; + + let declarations = lookup_function.entry(name).or_default(); + + 'outer: for decl in declarations.iter_mut() { + if parameters.len() != decl.parameters.len() { + continue; + } + + for (new_parameter, old_parameter) in parameters.iter().zip(decl.parameters.iter()) { + let new_inner = &module.types[*new_parameter].inner; + let old_inner = &module.types[*old_parameter].inner; + + if new_inner != old_inner { + continue 'outer; + } + } - if self - .lookup_function - .insert( - sig, - FunctionDeclaration { - qualifiers, - handle, - defined: false, - void, - }, - ) - .is_some() - { return Err(ErrorKind::SemanticError( meta, "Prototype already defined".into(), )); } + self.function_arg_use.push(Vec::new()); + let handle = module.functions.append(function); + declarations.push(FunctionDeclaration { + parameters, + qualifiers, + handle, + defined: false, + void, + }); + Ok(()) } diff --git a/src/front/glsl/parser.rs b/src/front/glsl/parser.rs index 9cc881cb13..b1f933e100 100644 --- a/src/front/glsl/parser.rs +++ b/src/front/glsl/parser.rs @@ -1,8 +1,7 @@ use super::{ ast::{ - Context, FunctionCall, FunctionCallKind, FunctionSignature, GlobalLookup, GlobalLookupKind, - HirExpr, HirExprKind, ParameterQualifier, Profile, StorageQualifier, StructLayout, - TypeQualifier, + Context, FunctionCall, FunctionCallKind, GlobalLookup, GlobalLookupKind, HirExpr, + HirExprKind, ParameterQualifier, Profile, StorageQualifier, StructLayout, TypeQualifier, }, error::ErrorKind, lex::Lexer, @@ -667,12 +666,10 @@ impl<'source, 'program, 'options> Parser<'source, 'program, 'options> { let mut expressions = Arena::new(); let mut local_variables = Arena::new(); let mut arguments = Vec::new(); + // Normalized function parameters, modifiers are not applied let mut parameters = Vec::new(); + let mut qualifiers = Vec::new(); let mut body = Block::new(); - let mut sig = FunctionSignature { - name: name.clone(), - parameters: Vec::new(), - }; let mut context = Context::new( self.program, @@ -685,8 +682,8 @@ impl<'source, 'program, 'options> Parser<'source, 'program, 'options> { self.parse_function_args( &mut context, &mut body, + &mut qualifiers, &mut parameters, - &mut sig, )?; let end_meta = self.expect(TokenValue::RightParen)?.meta; @@ -698,13 +695,14 @@ impl<'source, 'program, 'options> Parser<'source, 'program, 'options> { // This branch handles function prototypes self.program.add_prototype( Function { - name: Some(name), + name: Some(name.clone()), result, arguments, ..Default::default() }, - sig, + name, parameters, + qualifiers, meta, )?; @@ -721,7 +719,7 @@ impl<'source, 'program, 'options> Parser<'source, 'program, 'options> { let Context { arg_use, .. } = context; let handle = self.program.add_function( Function { - name: Some(name), + name: Some(name.clone()), result, expressions, named_expressions: crate::FastHashMap::default(), @@ -729,8 +727,9 @@ impl<'source, 'program, 'options> Parser<'source, 'program, 'options> { arguments, body, }, - sig, + name, parameters, + qualifiers, meta, )?; @@ -1737,19 +1736,26 @@ impl<'source, 'program, 'options> Parser<'source, 'program, 'options> { &mut self, context: &mut Context, body: &mut Block, - parameters: &mut Vec, - sig: &mut FunctionSignature, + qualifiers: &mut Vec, + parameters: &mut Vec>, ) -> Result<()> { loop { if self.peek_type_name() || self.peek_parameter_qualifier() { let qualifier = self.parse_parameter_qualifier(); - parameters.push(qualifier); + qualifiers.push(qualifier); let ty = self.parse_type_non_void()?.0; match self.expect_peek()?.value { TokenValue::Comma => { self.bump()?; - context.add_function_arg(&mut self.program, sig, body, None, ty, qualifier); + context.add_function_arg( + &mut self.program, + parameters, + body, + None, + ty, + qualifier, + ); continue; } TokenValue::Identifier(_) => { @@ -1760,7 +1766,7 @@ impl<'source, 'program, 'options> Parser<'source, 'program, 'options> { context.add_function_arg( &mut self.program, - sig, + parameters, body, Some(name), ty, diff --git a/src/front/glsl/parser_tests.rs b/src/front/glsl/parser_tests.rs index 40281ed368..8ff6aa1fe3 100644 --- a/src/front/glsl/parser_tests.rs +++ b/src/front/glsl/parser_tests.rs @@ -471,6 +471,54 @@ fn implicit_conversions() { &entry_points, ) .unwrap(); + + assert_eq!( + parse_program( + r#" + # version 450 + void test(int a) {} + void test(uint a) {} + + void main() { + test(1.0); + } + "#, + &entry_points + ) + .err() + .unwrap(), + ErrorKind::SemanticError( + SourceMetadata { + start: 156, + end: 165 + }, + "Unknown function \'test\'".into() + ) + ); + + assert_eq!( + parse_program( + r#" + # version 450 + void test(float a) {} + void test(uint a) {} + + void main() { + test(1); + } + "#, + &entry_points + ) + .err() + .unwrap(), + ErrorKind::SemanticError( + SourceMetadata { + start: 158, + end: 165 + }, + "Ambiguous best function for \'test\'".into() + ) + ); } #[test] diff --git a/src/front/mod.rs b/src/front/mod.rs index 45c93143ec..8181592922 100644 --- a/src/front/mod.rs +++ b/src/front/mod.rs @@ -69,30 +69,6 @@ impl Typifier { self.resolutions[expr_handle.index()].inner_with(types) } - pub fn get_handle( - &mut self, - expr_handle: Handle, - types: &mut Arena, - ) -> Handle { - let mut dummy = TypeResolution::Value(crate::TypeInner::Sampler { comparison: false }); - let res = &mut self.resolutions[expr_handle.index()]; - - std::mem::swap(&mut dummy, res); - - let v = match dummy { - TypeResolution::Handle(h) => h, - TypeResolution::Value(inner) => { - let h = types.fetch_or_append(crate::Type { name: None, inner }); - dummy = TypeResolution::Handle(h); - h - } - }; - - std::mem::swap(&mut dummy, res); - - v - } - pub fn grow( &mut self, expr_handle: Handle, diff --git a/tests/in/glsl/900-implicit-conversions.vert b/tests/in/glsl/900-implicit-conversions.vert new file mode 100644 index 0000000000..d3908bb112 --- /dev/null +++ b/tests/in/glsl/900-implicit-conversions.vert @@ -0,0 +1,15 @@ +// ISSUE: #900 +#version 450 + +// Signature match call the second overload +void exact(float a) {} +void exact(int a) {} + +// No signature match but one overload satisfies the cast rules +void implicit(float a) {} +void implicit(int a) {} + +void main() { + exact(1); + implicit(1u); +} diff --git a/tests/out/wgsl/900-implicit-conversions-vert.wgsl b/tests/out/wgsl/900-implicit-conversions-vert.wgsl new file mode 100644 index 0000000000..49e4a57044 --- /dev/null +++ b/tests/out/wgsl/900-implicit-conversions-vert.wgsl @@ -0,0 +1,39 @@ +fn exact(a: f32) { + var a1: f32; + + a1 = a; + return; +} + +fn exact1(a2: i32) { + var a3: i32; + + a3 = a2; + return; +} + +fn implicit(a4: f32) { + var a5: f32; + + a5 = a4; + return; +} + +fn implicit1(a6: i32) { + var a7: i32; + + a7 = a6; + return; +} + +fn main1() { + exact1(1); + implicit(f32(1u)); + return; +} + +[[stage(vertex)]] +fn main() { + main1(); + return; +}