From ca405e3acf5c4785128c0a9ff5114f28bbef5289 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Capucho?= Date: Thu, 19 Aug 2021 22:07:57 +0100 Subject: [PATCH] [glsl-in] Allow dynamic indexing on constant variables Previously we always set the lhs flag when lowering to generate a pointer so that dynamic indexing would work, this would produce an error on constant variables since they can't be in a lhs context. Now we introduce an enum which distinguishes not only between lhs and rhs but also in array base, if lowering in a lhs context the base is also lowered in a lhs context but if lowering in rhs the base is lowered in a special array base context which bypasses the mutability check. Fixes #1237 --- src/front/glsl/ast.rs | 10 +++- src/front/glsl/context.rs | 85 ++++++++++++++++++--------- src/front/glsl/functions.rs | 10 ++-- src/front/glsl/parser.rs | 4 +- src/front/glsl/parser/declarations.rs | 4 +- src/front/glsl/parser/functions.rs | 23 ++++---- src/front/glsl/parser_tests.rs | 15 +++++ 7 files changed, 103 insertions(+), 48 deletions(-) diff --git a/src/front/glsl/ast.rs b/src/front/glsl/ast.rs index 8a8cdc0cde..5cadd03cb3 100644 --- a/src/front/glsl/ast.rs +++ b/src/front/glsl/ast.rs @@ -1,6 +1,6 @@ use std::fmt; -use super::{builtins::MacroCall, SourceMetadata}; +use super::{builtins::MacroCall, context::ExprPos, SourceMetadata}; use crate::{ BinaryOperator, Binding, Constant, Expression, Function, GlobalVariable, Handle, Interpolation, Sampling, StorageAccess, StorageClass, Type, UnaryOperator, @@ -214,6 +214,14 @@ impl ParameterQualifier { _ => false, } } + + /// Converts from a parameter qualifier into a [`ExprPos`](ExprPos) + pub fn as_pos(&self) -> ExprPos { + match *self { + ParameterQualifier::Out | ParameterQualifier::InOut => ExprPos::Lhs, + _ => ExprPos::Rhs, + } + } } /// The glsl profile used by a shader diff --git a/src/front/glsl/context.rs b/src/front/glsl/context.rs index 57787463c8..eb40a6a3fc 100644 --- a/src/front/glsl/context.rs +++ b/src/front/glsl/context.rs @@ -15,6 +15,28 @@ use crate::{ }; use std::{convert::TryFrom, ops::Index}; +/// The position at which an expression is, used while lowering +#[derive(Clone, Copy, PartialEq, Eq)] +pub enum ExprPos { + /// The expression is in the left hand side of an assignment + Lhs, + /// The expression is in the right hand side of an assignment + Rhs, + /// The expression is an array being indexed, needed to allow constant + /// arrays to be dinamically indexed + ArrayBase, +} + +impl ExprPos { + /// Returns an lhs position if the current position is lhs otherwise ArrayBase + fn maybe_array_base(&self) -> Self { + match *self { + ExprPos::Lhs => *self, + _ => ExprPos::ArrayBase, + } + } +} + #[derive(Debug)] pub struct Context { pub expressions: Arena, @@ -323,10 +345,10 @@ impl Context { mut stmt: StmtContext, parser: &mut Parser, expr: Handle, - lhs: bool, + pos: ExprPos, body: &mut Block, ) -> Result<(Option>, SourceMetadata)> { - let res = self.lower_inner(&stmt, parser, expr, lhs, body); + let res = self.lower_inner(&stmt, parser, expr, pos, body); stmt.hir_exprs.clear(); self.stmt_ctx = Some(stmt); @@ -344,10 +366,10 @@ impl Context { mut stmt: StmtContext, parser: &mut Parser, expr: Handle, - lhs: bool, + pos: ExprPos, body: &mut Block, ) -> Result<(Handle, SourceMetadata)> { - let res = self.lower_expect_inner(&stmt, parser, expr, lhs, body); + let res = self.lower_expect_inner(&stmt, parser, expr, pos, body); stmt.hir_exprs.clear(); self.stmt_ctx = Some(stmt); @@ -365,10 +387,10 @@ impl Context { stmt: &StmtContext, parser: &mut Parser, expr: Handle, - lhs: bool, + pos: ExprPos, body: &mut Block, ) -> Result<(Handle, SourceMetadata)> { - let (maybe_expr, meta) = self.lower_inner(stmt, parser, expr, lhs, body)?; + let (maybe_expr, meta) = self.lower_inner(stmt, parser, expr, pos, body)?; let expr = match maybe_expr { Some(e) => e, @@ -389,16 +411,18 @@ impl Context { stmt: &StmtContext, parser: &mut Parser, expr: Handle, - lhs: bool, + pos: ExprPos, body: &mut Block, ) -> Result<(Option>, SourceMetadata)> { let HirExpr { ref kind, meta } = stmt.hir_exprs[expr]; let handle = match *kind { HirExprKind::Access { base, index } => { - let base = self.lower_expect_inner(stmt, parser, base, true, body)?.0; + let base = self + .lower_expect_inner(stmt, parser, base, pos.maybe_array_base(), body)? + .0; let (index, index_meta) = - self.lower_expect_inner(stmt, parser, index, false, body)?; + self.lower_expect_inner(stmt, parser, index, ExprPos::Rhs, body)?; let pointer = parser .solve_constant(self, index, index_meta) @@ -427,7 +451,7 @@ impl Context { self.add_expression(Expression::Access { base, index }, meta, body) }); - if !lhs { + if ExprPos::Rhs == pos { let resolved = parser.resolve_type(self, pointer, meta)?; if resolved.pointer_class().is_some() { return Ok(( @@ -440,18 +464,18 @@ impl Context { pointer } HirExprKind::Select { base, ref field } => { - let base = self.lower_expect_inner(stmt, parser, base, lhs, body)?.0; + let base = self.lower_expect_inner(stmt, parser, base, pos, body)?.0; - parser.field_selection(self, lhs, body, base, field, meta)? + parser.field_selection(self, ExprPos::Lhs == pos, body, base, field, meta)? } - HirExprKind::Constant(constant) if !lhs => { + HirExprKind::Constant(constant) if pos == ExprPos::Rhs => { self.add_expression(Expression::Constant(constant), meta, body) } - HirExprKind::Binary { left, op, right } if !lhs => { + HirExprKind::Binary { left, op, right } if pos == ExprPos::Rhs => { let (mut left, left_meta) = - self.lower_expect_inner(stmt, parser, left, false, body)?; + self.lower_expect_inner(stmt, parser, left, pos, body)?; let (mut right, right_meta) = - self.lower_expect_inner(stmt, parser, right, false, body)?; + self.lower_expect_inner(stmt, parser, right, pos, body)?; match op { BinaryOperator::ShiftLeft | BinaryOperator::ShiftRight => self @@ -543,14 +567,14 @@ impl Context { _ => self.add_expression(Expression::Binary { left, op, right }, meta, body), } } - HirExprKind::Unary { op, expr } if !lhs => { - let expr = self.lower_expect_inner(stmt, parser, expr, false, body)?.0; + HirExprKind::Unary { op, expr } if pos == ExprPos::Rhs => { + let expr = self.lower_expect_inner(stmt, parser, expr, pos, body)?.0; self.add_expression(Expression::Unary { op, expr }, meta, body) } HirExprKind::Variable(ref var) => { - if lhs { - if !var.mutable { + if pos != ExprPos::Rhs { + if !var.mutable && ExprPos::Lhs == pos { parser.errors.push(Error { kind: ErrorKind::SemanticError( "Variable cannot be used in LHS position".into(), @@ -566,7 +590,7 @@ impl Context { var.expr } } - HirExprKind::Call(ref call) if !lhs => { + HirExprKind::Call(ref call) if pos != ExprPos::Lhs => { let maybe_expr = parser.function_or_constructor_call( self, stmt, @@ -581,14 +605,14 @@ impl Context { condition, accept, reject, - } if !lhs => { + } if ExprPos::Lhs != pos => { let condition = self - .lower_expect_inner(stmt, parser, condition, false, body)? + .lower_expect_inner(stmt, parser, condition, ExprPos::Rhs, body)? .0; let (mut accept, accept_meta) = - self.lower_expect_inner(stmt, parser, accept, false, body)?; + self.lower_expect_inner(stmt, parser, accept, pos, body)?; let (mut reject, reject_meta) = - self.lower_expect_inner(stmt, parser, reject, false, body)?; + self.lower_expect_inner(stmt, parser, reject, pos, body)?; self.binary_implicit_conversion( parser, @@ -608,10 +632,11 @@ impl Context { body, ) } - HirExprKind::Assign { tgt, value } if !lhs => { - let (pointer, ptr_meta) = self.lower_expect_inner(stmt, parser, tgt, true, body)?; + HirExprKind::Assign { tgt, value } if ExprPos::Lhs != pos => { + let (pointer, ptr_meta) = + self.lower_expect_inner(stmt, parser, tgt, ExprPos::Lhs, body)?; let (mut value, value_meta) = - self.lower_expect_inner(stmt, parser, value, false, body)?; + self.lower_expect_inner(stmt, parser, value, ExprPos::Rhs, body)?; let scalar_components = self.expr_scalar_components(parser, pointer, ptr_meta)?; @@ -686,7 +711,9 @@ impl Context { false => BinaryOperator::Subtract, }; - let pointer = self.lower_expect_inner(stmt, parser, expr, true, body)?.0; + let pointer = self + .lower_expect_inner(stmt, parser, expr, ExprPos::Lhs, body)? + .0; let left = self.add_expression(Expression::Load { pointer }, meta, body); let uint = match parser.resolve_type(self, left, meta)?.scalar_kind() { diff --git a/src/front/glsl/functions.rs b/src/front/glsl/functions.rs index 28190699aa..0b014aad32 100644 --- a/src/front/glsl/functions.rs +++ b/src/front/glsl/functions.rs @@ -1,7 +1,7 @@ use super::{ ast::*, builtins::{inject_builtin, inject_double_builtin, sampled_to_depth}, - context::{Context, StmtContext}, + context::{Context, ExprPos, StmtContext}, error::{Error, ErrorKind}, types::scalar_components, Parser, Result, SourceMetadata, @@ -48,7 +48,7 @@ impl Parser { ) -> Result>> { let args: Vec<_> = raw_args .iter() - .map(|e| ctx.lower_expect_inner(stmt, self, *e, false, body)) + .map(|e| ctx.lower_expect_inner(stmt, self, *e, ExprPos::Rhs, body)) .collect::>()?; match fc { @@ -565,7 +565,7 @@ impl Parser { .zip(raw_args.iter().zip(parameters.iter())) { let (mut handle, meta) = - ctx.lower_expect_inner(stmt, self, *expr, parameter_info.qualifier.is_lhs(), body)?; + ctx.lower_expect_inner(stmt, self, *expr, parameter_info.qualifier.as_pos(), body)?; if let TypeInner::Vector { size, kind, width } = *self.resolve_type(ctx, handle, meta)? @@ -638,7 +638,9 @@ impl Parser { ctx.emit_start(); for (tgt, pointer) in proxy_writes { let value = ctx.add_expression(Expression::Load { pointer }, meta, body); - let target = ctx.lower_expect_inner(stmt, self, tgt, true, body)?.0; + let target = ctx + .lower_expect_inner(stmt, self, tgt, ExprPos::Rhs, body)? + .0; ctx.emit_flush(body); body.push( diff --git a/src/front/glsl/parser.rs b/src/front/glsl/parser.rs index b30c9b65c0..d0e14b0041 100644 --- a/src/front/glsl/parser.rs +++ b/src/front/glsl/parser.rs @@ -1,6 +1,6 @@ use super::{ ast::{FunctionKind, Profile, TypeQualifier}, - context::Context, + context::{Context, ExprPos}, error::ExpectedToken, error::{Error, ErrorKind}, lex::{Lexer, LexerResultKind}, @@ -199,7 +199,7 @@ impl<'source> ParsingContext<'source> { let mut stmt_ctx = ctx.stmt_ctx(); let expr = self.parse_conditional(parser, &mut ctx, &mut stmt_ctx, &mut block, None)?; - let (root, meta) = ctx.lower_expect(stmt_ctx, parser, expr, false, &mut block)?; + let (root, meta) = ctx.lower_expect(stmt_ctx, parser, expr, ExprPos::Rhs, &mut block)?; Ok((parser.solve_constant(&ctx, root, meta)?, meta)) } diff --git a/src/front/glsl/parser/declarations.rs b/src/front/glsl/parser/declarations.rs index 806e98e357..546c366a29 100644 --- a/src/front/glsl/parser/declarations.rs +++ b/src/front/glsl/parser/declarations.rs @@ -4,7 +4,7 @@ use crate::{ GlobalLookup, GlobalLookupKind, Precision, StorageQualifier, StructLayout, TypeQualifier, }, - context::Context, + context::{Context, ExprPos}, error::ExpectedToken, offset, token::{Token, TokenValue}, @@ -103,7 +103,7 @@ impl<'source> ParsingContext<'source> { } else { let mut stmt = ctx.stmt_ctx(); let expr = self.parse_assignment(parser, ctx, &mut stmt, body)?; - let (mut init, init_meta) = ctx.lower_expect(stmt, parser, expr, false, body)?; + let (mut init, init_meta) = ctx.lower_expect(stmt, parser, expr, ExprPos::Rhs, body)?; let scalar_components = scalar_components(&parser.module.types[ty].inner); if let Some((kind, width)) = scalar_components { diff --git a/src/front/glsl/parser/functions.rs b/src/front/glsl/parser/functions.rs index c91e6aeb22..e1b41b1bc2 100644 --- a/src/front/glsl/parser/functions.rs +++ b/src/front/glsl/parser/functions.rs @@ -1,3 +1,4 @@ +use crate::front::glsl::context::ExprPos; use crate::front::glsl::SourceMetadata; use crate::{ front::glsl::{ @@ -77,7 +78,8 @@ impl<'source> ParsingContext<'source> { let mut stmt = ctx.stmt_ctx(); let expr = self.parse_expression(parser, ctx, &mut stmt, body)?; self.expect(parser, TokenValue::Semicolon)?; - let (handle, meta) = ctx.lower_expect(stmt, parser, expr, false, body)?; + let (handle, meta) = + ctx.lower_expect(stmt, parser, expr, ExprPos::Rhs, body)?; (Some(handle), meta) } }; @@ -99,7 +101,8 @@ impl<'source> ParsingContext<'source> { let condition = { let mut stmt = ctx.stmt_ctx(); let expr = self.parse_expression(parser, ctx, &mut stmt, body)?; - let (handle, more_meta) = ctx.lower_expect(stmt, parser, expr, false, body)?; + let (handle, more_meta) = + ctx.lower_expect(stmt, parser, expr, ExprPos::Rhs, body)?; meta = meta.union(&more_meta); handle }; @@ -139,7 +142,7 @@ impl<'source> ParsingContext<'source> { let selector = { let mut stmt = ctx.stmt_ctx(); let expr = self.parse_expression(parser, ctx, &mut stmt, body)?; - ctx.lower_expect(stmt, parser, expr, false, body)?.0 + ctx.lower_expect(stmt, parser, expr, ExprPos::Rhs, body)?.0 }; self.expect(parser, TokenValue::RightParen)?; @@ -158,7 +161,7 @@ impl<'source> ParsingContext<'source> { let mut stmt = ctx.stmt_ctx(); let expr = self.parse_expression(parser, ctx, &mut stmt, body)?; let (root, meta) = - ctx.lower_expect(stmt, parser, expr, false, body)?; + ctx.lower_expect(stmt, parser, expr, ExprPos::Rhs, body)?; let constant = parser.solve_constant(ctx, root, meta)?; match parser.module.constants[constant].inner { @@ -287,7 +290,7 @@ impl<'source> ParsingContext<'source> { let meta = meta.union(&self.expect(parser, TokenValue::RightParen)?.meta); let (expr, expr_meta) = - ctx.lower_expect(stmt, parser, root, false, &mut loop_body)?; + ctx.lower_expect(stmt, parser, root, ExprPos::Rhs, &mut loop_body)?; let condition = ctx.add_expression( Expression::Unary { op: UnaryOperator::Not, @@ -340,7 +343,7 @@ impl<'source> ParsingContext<'source> { let meta = start_meta.union(&end_meta); let (expr, expr_meta) = - ctx.lower_expect(stmt, parser, root, false, &mut loop_body)?; + ctx.lower_expect(stmt, parser, root, ExprPos::Rhs, &mut loop_body)?; let condition = ctx.add_expression( Expression::Unary { op: UnaryOperator::Not, @@ -383,7 +386,7 @@ impl<'source> ParsingContext<'source> { } else { let mut stmt = ctx.stmt_ctx(); let expr = self.parse_expression(parser, ctx, &mut stmt, body)?; - ctx.lower(stmt, parser, expr, false, body)?; + ctx.lower(stmt, parser, expr, ExprPos::Rhs, body)?; self.expect(parser, TokenValue::Semicolon)?; } } @@ -422,7 +425,7 @@ impl<'source> ParsingContext<'source> { } else { let mut stmt = ctx.stmt_ctx(); let root = self.parse_expression(parser, ctx, &mut stmt, &mut block)?; - ctx.lower_expect(stmt, parser, root, false, &mut block)? + ctx.lower_expect(stmt, parser, root, ExprPos::Rhs, &mut block)? }; let condition = ctx.add_expression( @@ -455,7 +458,7 @@ impl<'source> ParsingContext<'source> { let mut stmt = ctx.stmt_ctx(); let rest = self.parse_expression(parser, ctx, &mut stmt, &mut continuing)?; - ctx.lower(stmt, parser, rest, false, &mut continuing)?; + ctx.lower(stmt, parser, rest, ExprPos::Rhs, &mut continuing)?; } } @@ -504,7 +507,7 @@ impl<'source> ParsingContext<'source> { | TokenValue::FloatConstant(_) => { let mut stmt = ctx.stmt_ctx(); let expr = self.parse_expression(parser, ctx, &mut stmt, body)?; - ctx.lower(stmt, parser, expr, false, body)?; + ctx.lower(stmt, parser, expr, ExprPos::Rhs, body)?; self.expect(parser, TokenValue::Semicolon)?.meta } TokenValue::Semicolon => self.bump(parser)?.meta, diff --git a/src/front/glsl/parser_tests.rs b/src/front/glsl/parser_tests.rs index 717737bc59..58d4a7e71c 100644 --- a/src/front/glsl/parser_tests.rs +++ b/src/front/glsl/parser_tests.rs @@ -812,4 +812,19 @@ fn expressions() { "#, ) .unwrap(); + + // Dynamic indexing of array + parser + .parse( + &Options::from(ShaderStage::Vertex), + r#" + # version 450 + void main() { + const vec4 positions[1] = { vec4(0) }; + + gl_Position = positions[gl_VertexIndex]; + } + "#, + ) + .unwrap(); }