Skip to content

Commit ca405e3

Browse files
JCapuchokvark
authored andcommitted
[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 gfx-rs#1237
1 parent fc47008 commit ca405e3

File tree

7 files changed

+103
-48
lines changed

7 files changed

+103
-48
lines changed

src/front/glsl/ast.rs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use std::fmt;
22

3-
use super::{builtins::MacroCall, SourceMetadata};
3+
use super::{builtins::MacroCall, context::ExprPos, SourceMetadata};
44
use crate::{
55
BinaryOperator, Binding, Constant, Expression, Function, GlobalVariable, Handle, Interpolation,
66
Sampling, StorageAccess, StorageClass, Type, UnaryOperator,
@@ -214,6 +214,14 @@ impl ParameterQualifier {
214214
_ => false,
215215
}
216216
}
217+
218+
/// Converts from a parameter qualifier into a [`ExprPos`](ExprPos)
219+
pub fn as_pos(&self) -> ExprPos {
220+
match *self {
221+
ParameterQualifier::Out | ParameterQualifier::InOut => ExprPos::Lhs,
222+
_ => ExprPos::Rhs,
223+
}
224+
}
217225
}
218226

219227
/// The glsl profile used by a shader

src/front/glsl/context.rs

Lines changed: 56 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,28 @@ use crate::{
1515
};
1616
use std::{convert::TryFrom, ops::Index};
1717

18+
/// The position at which an expression is, used while lowering
19+
#[derive(Clone, Copy, PartialEq, Eq)]
20+
pub enum ExprPos {
21+
/// The expression is in the left hand side of an assignment
22+
Lhs,
23+
/// The expression is in the right hand side of an assignment
24+
Rhs,
25+
/// The expression is an array being indexed, needed to allow constant
26+
/// arrays to be dinamically indexed
27+
ArrayBase,
28+
}
29+
30+
impl ExprPos {
31+
/// Returns an lhs position if the current position is lhs otherwise ArrayBase
32+
fn maybe_array_base(&self) -> Self {
33+
match *self {
34+
ExprPos::Lhs => *self,
35+
_ => ExprPos::ArrayBase,
36+
}
37+
}
38+
}
39+
1840
#[derive(Debug)]
1941
pub struct Context {
2042
pub expressions: Arena<Expression>,
@@ -323,10 +345,10 @@ impl Context {
323345
mut stmt: StmtContext,
324346
parser: &mut Parser,
325347
expr: Handle<HirExpr>,
326-
lhs: bool,
348+
pos: ExprPos,
327349
body: &mut Block,
328350
) -> Result<(Option<Handle<Expression>>, SourceMetadata)> {
329-
let res = self.lower_inner(&stmt, parser, expr, lhs, body);
351+
let res = self.lower_inner(&stmt, parser, expr, pos, body);
330352

331353
stmt.hir_exprs.clear();
332354
self.stmt_ctx = Some(stmt);
@@ -344,10 +366,10 @@ impl Context {
344366
mut stmt: StmtContext,
345367
parser: &mut Parser,
346368
expr: Handle<HirExpr>,
347-
lhs: bool,
369+
pos: ExprPos,
348370
body: &mut Block,
349371
) -> Result<(Handle<Expression>, SourceMetadata)> {
350-
let res = self.lower_expect_inner(&stmt, parser, expr, lhs, body);
372+
let res = self.lower_expect_inner(&stmt, parser, expr, pos, body);
351373

352374
stmt.hir_exprs.clear();
353375
self.stmt_ctx = Some(stmt);
@@ -365,10 +387,10 @@ impl Context {
365387
stmt: &StmtContext,
366388
parser: &mut Parser,
367389
expr: Handle<HirExpr>,
368-
lhs: bool,
390+
pos: ExprPos,
369391
body: &mut Block,
370392
) -> Result<(Handle<Expression>, SourceMetadata)> {
371-
let (maybe_expr, meta) = self.lower_inner(stmt, parser, expr, lhs, body)?;
393+
let (maybe_expr, meta) = self.lower_inner(stmt, parser, expr, pos, body)?;
372394

373395
let expr = match maybe_expr {
374396
Some(e) => e,
@@ -389,16 +411,18 @@ impl Context {
389411
stmt: &StmtContext,
390412
parser: &mut Parser,
391413
expr: Handle<HirExpr>,
392-
lhs: bool,
414+
pos: ExprPos,
393415
body: &mut Block,
394416
) -> Result<(Option<Handle<Expression>>, SourceMetadata)> {
395417
let HirExpr { ref kind, meta } = stmt.hir_exprs[expr];
396418

397419
let handle = match *kind {
398420
HirExprKind::Access { base, index } => {
399-
let base = self.lower_expect_inner(stmt, parser, base, true, body)?.0;
421+
let base = self
422+
.lower_expect_inner(stmt, parser, base, pos.maybe_array_base(), body)?
423+
.0;
400424
let (index, index_meta) =
401-
self.lower_expect_inner(stmt, parser, index, false, body)?;
425+
self.lower_expect_inner(stmt, parser, index, ExprPos::Rhs, body)?;
402426

403427
let pointer = parser
404428
.solve_constant(self, index, index_meta)
@@ -427,7 +451,7 @@ impl Context {
427451
self.add_expression(Expression::Access { base, index }, meta, body)
428452
});
429453

430-
if !lhs {
454+
if ExprPos::Rhs == pos {
431455
let resolved = parser.resolve_type(self, pointer, meta)?;
432456
if resolved.pointer_class().is_some() {
433457
return Ok((
@@ -440,18 +464,18 @@ impl Context {
440464
pointer
441465
}
442466
HirExprKind::Select { base, ref field } => {
443-
let base = self.lower_expect_inner(stmt, parser, base, lhs, body)?.0;
467+
let base = self.lower_expect_inner(stmt, parser, base, pos, body)?.0;
444468

445-
parser.field_selection(self, lhs, body, base, field, meta)?
469+
parser.field_selection(self, ExprPos::Lhs == pos, body, base, field, meta)?
446470
}
447-
HirExprKind::Constant(constant) if !lhs => {
471+
HirExprKind::Constant(constant) if pos == ExprPos::Rhs => {
448472
self.add_expression(Expression::Constant(constant), meta, body)
449473
}
450-
HirExprKind::Binary { left, op, right } if !lhs => {
474+
HirExprKind::Binary { left, op, right } if pos == ExprPos::Rhs => {
451475
let (mut left, left_meta) =
452-
self.lower_expect_inner(stmt, parser, left, false, body)?;
476+
self.lower_expect_inner(stmt, parser, left, pos, body)?;
453477
let (mut right, right_meta) =
454-
self.lower_expect_inner(stmt, parser, right, false, body)?;
478+
self.lower_expect_inner(stmt, parser, right, pos, body)?;
455479

456480
match op {
457481
BinaryOperator::ShiftLeft | BinaryOperator::ShiftRight => self
@@ -543,14 +567,14 @@ impl Context {
543567
_ => self.add_expression(Expression::Binary { left, op, right }, meta, body),
544568
}
545569
}
546-
HirExprKind::Unary { op, expr } if !lhs => {
547-
let expr = self.lower_expect_inner(stmt, parser, expr, false, body)?.0;
570+
HirExprKind::Unary { op, expr } if pos == ExprPos::Rhs => {
571+
let expr = self.lower_expect_inner(stmt, parser, expr, pos, body)?.0;
548572

549573
self.add_expression(Expression::Unary { op, expr }, meta, body)
550574
}
551575
HirExprKind::Variable(ref var) => {
552-
if lhs {
553-
if !var.mutable {
576+
if pos != ExprPos::Rhs {
577+
if !var.mutable && ExprPos::Lhs == pos {
554578
parser.errors.push(Error {
555579
kind: ErrorKind::SemanticError(
556580
"Variable cannot be used in LHS position".into(),
@@ -566,7 +590,7 @@ impl Context {
566590
var.expr
567591
}
568592
}
569-
HirExprKind::Call(ref call) if !lhs => {
593+
HirExprKind::Call(ref call) if pos != ExprPos::Lhs => {
570594
let maybe_expr = parser.function_or_constructor_call(
571595
self,
572596
stmt,
@@ -581,14 +605,14 @@ impl Context {
581605
condition,
582606
accept,
583607
reject,
584-
} if !lhs => {
608+
} if ExprPos::Lhs != pos => {
585609
let condition = self
586-
.lower_expect_inner(stmt, parser, condition, false, body)?
610+
.lower_expect_inner(stmt, parser, condition, ExprPos::Rhs, body)?
587611
.0;
588612
let (mut accept, accept_meta) =
589-
self.lower_expect_inner(stmt, parser, accept, false, body)?;
613+
self.lower_expect_inner(stmt, parser, accept, pos, body)?;
590614
let (mut reject, reject_meta) =
591-
self.lower_expect_inner(stmt, parser, reject, false, body)?;
615+
self.lower_expect_inner(stmt, parser, reject, pos, body)?;
592616

593617
self.binary_implicit_conversion(
594618
parser,
@@ -608,10 +632,11 @@ impl Context {
608632
body,
609633
)
610634
}
611-
HirExprKind::Assign { tgt, value } if !lhs => {
612-
let (pointer, ptr_meta) = self.lower_expect_inner(stmt, parser, tgt, true, body)?;
635+
HirExprKind::Assign { tgt, value } if ExprPos::Lhs != pos => {
636+
let (pointer, ptr_meta) =
637+
self.lower_expect_inner(stmt, parser, tgt, ExprPos::Lhs, body)?;
613638
let (mut value, value_meta) =
614-
self.lower_expect_inner(stmt, parser, value, false, body)?;
639+
self.lower_expect_inner(stmt, parser, value, ExprPos::Rhs, body)?;
615640

616641
let scalar_components = self.expr_scalar_components(parser, pointer, ptr_meta)?;
617642

@@ -686,7 +711,9 @@ impl Context {
686711
false => BinaryOperator::Subtract,
687712
};
688713

689-
let pointer = self.lower_expect_inner(stmt, parser, expr, true, body)?.0;
714+
let pointer = self
715+
.lower_expect_inner(stmt, parser, expr, ExprPos::Lhs, body)?
716+
.0;
690717
let left = self.add_expression(Expression::Load { pointer }, meta, body);
691718

692719
let uint = match parser.resolve_type(self, left, meta)?.scalar_kind() {

src/front/glsl/functions.rs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use super::{
22
ast::*,
33
builtins::{inject_builtin, inject_double_builtin, sampled_to_depth},
4-
context::{Context, StmtContext},
4+
context::{Context, ExprPos, StmtContext},
55
error::{Error, ErrorKind},
66
types::scalar_components,
77
Parser, Result, SourceMetadata,
@@ -48,7 +48,7 @@ impl Parser {
4848
) -> Result<Option<Handle<Expression>>> {
4949
let args: Vec<_> = raw_args
5050
.iter()
51-
.map(|e| ctx.lower_expect_inner(stmt, self, *e, false, body))
51+
.map(|e| ctx.lower_expect_inner(stmt, self, *e, ExprPos::Rhs, body))
5252
.collect::<Result<_>>()?;
5353

5454
match fc {
@@ -565,7 +565,7 @@ impl Parser {
565565
.zip(raw_args.iter().zip(parameters.iter()))
566566
{
567567
let (mut handle, meta) =
568-
ctx.lower_expect_inner(stmt, self, *expr, parameter_info.qualifier.is_lhs(), body)?;
568+
ctx.lower_expect_inner(stmt, self, *expr, parameter_info.qualifier.as_pos(), body)?;
569569

570570
if let TypeInner::Vector { size, kind, width } =
571571
*self.resolve_type(ctx, handle, meta)?
@@ -638,7 +638,9 @@ impl Parser {
638638
ctx.emit_start();
639639
for (tgt, pointer) in proxy_writes {
640640
let value = ctx.add_expression(Expression::Load { pointer }, meta, body);
641-
let target = ctx.lower_expect_inner(stmt, self, tgt, true, body)?.0;
641+
let target = ctx
642+
.lower_expect_inner(stmt, self, tgt, ExprPos::Rhs, body)?
643+
.0;
642644

643645
ctx.emit_flush(body);
644646
body.push(

src/front/glsl/parser.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use super::{
22
ast::{FunctionKind, Profile, TypeQualifier},
3-
context::Context,
3+
context::{Context, ExprPos},
44
error::ExpectedToken,
55
error::{Error, ErrorKind},
66
lex::{Lexer, LexerResultKind},
@@ -199,7 +199,7 @@ impl<'source> ParsingContext<'source> {
199199

200200
let mut stmt_ctx = ctx.stmt_ctx();
201201
let expr = self.parse_conditional(parser, &mut ctx, &mut stmt_ctx, &mut block, None)?;
202-
let (root, meta) = ctx.lower_expect(stmt_ctx, parser, expr, false, &mut block)?;
202+
let (root, meta) = ctx.lower_expect(stmt_ctx, parser, expr, ExprPos::Rhs, &mut block)?;
203203

204204
Ok((parser.solve_constant(&ctx, root, meta)?, meta))
205205
}

src/front/glsl/parser/declarations.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use crate::{
44
GlobalLookup, GlobalLookupKind, Precision, StorageQualifier, StructLayout,
55
TypeQualifier,
66
},
7-
context::Context,
7+
context::{Context, ExprPos},
88
error::ExpectedToken,
99
offset,
1010
token::{Token, TokenValue},
@@ -103,7 +103,7 @@ impl<'source> ParsingContext<'source> {
103103
} else {
104104
let mut stmt = ctx.stmt_ctx();
105105
let expr = self.parse_assignment(parser, ctx, &mut stmt, body)?;
106-
let (mut init, init_meta) = ctx.lower_expect(stmt, parser, expr, false, body)?;
106+
let (mut init, init_meta) = ctx.lower_expect(stmt, parser, expr, ExprPos::Rhs, body)?;
107107

108108
let scalar_components = scalar_components(&parser.module.types[ty].inner);
109109
if let Some((kind, width)) = scalar_components {

src/front/glsl/parser/functions.rs

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
use crate::front::glsl::context::ExprPos;
12
use crate::front::glsl::SourceMetadata;
23
use crate::{
34
front::glsl::{
@@ -77,7 +78,8 @@ impl<'source> ParsingContext<'source> {
7778
let mut stmt = ctx.stmt_ctx();
7879
let expr = self.parse_expression(parser, ctx, &mut stmt, body)?;
7980
self.expect(parser, TokenValue::Semicolon)?;
80-
let (handle, meta) = ctx.lower_expect(stmt, parser, expr, false, body)?;
81+
let (handle, meta) =
82+
ctx.lower_expect(stmt, parser, expr, ExprPos::Rhs, body)?;
8183
(Some(handle), meta)
8284
}
8385
};
@@ -99,7 +101,8 @@ impl<'source> ParsingContext<'source> {
99101
let condition = {
100102
let mut stmt = ctx.stmt_ctx();
101103
let expr = self.parse_expression(parser, ctx, &mut stmt, body)?;
102-
let (handle, more_meta) = ctx.lower_expect(stmt, parser, expr, false, body)?;
104+
let (handle, more_meta) =
105+
ctx.lower_expect(stmt, parser, expr, ExprPos::Rhs, body)?;
103106
meta = meta.union(&more_meta);
104107
handle
105108
};
@@ -139,7 +142,7 @@ impl<'source> ParsingContext<'source> {
139142
let selector = {
140143
let mut stmt = ctx.stmt_ctx();
141144
let expr = self.parse_expression(parser, ctx, &mut stmt, body)?;
142-
ctx.lower_expect(stmt, parser, expr, false, body)?.0
145+
ctx.lower_expect(stmt, parser, expr, ExprPos::Rhs, body)?.0
143146
};
144147
self.expect(parser, TokenValue::RightParen)?;
145148

@@ -158,7 +161,7 @@ impl<'source> ParsingContext<'source> {
158161
let mut stmt = ctx.stmt_ctx();
159162
let expr = self.parse_expression(parser, ctx, &mut stmt, body)?;
160163
let (root, meta) =
161-
ctx.lower_expect(stmt, parser, expr, false, body)?;
164+
ctx.lower_expect(stmt, parser, expr, ExprPos::Rhs, body)?;
162165
let constant = parser.solve_constant(ctx, root, meta)?;
163166

164167
match parser.module.constants[constant].inner {
@@ -287,7 +290,7 @@ impl<'source> ParsingContext<'source> {
287290
let meta = meta.union(&self.expect(parser, TokenValue::RightParen)?.meta);
288291

289292
let (expr, expr_meta) =
290-
ctx.lower_expect(stmt, parser, root, false, &mut loop_body)?;
293+
ctx.lower_expect(stmt, parser, root, ExprPos::Rhs, &mut loop_body)?;
291294
let condition = ctx.add_expression(
292295
Expression::Unary {
293296
op: UnaryOperator::Not,
@@ -340,7 +343,7 @@ impl<'source> ParsingContext<'source> {
340343
let meta = start_meta.union(&end_meta);
341344

342345
let (expr, expr_meta) =
343-
ctx.lower_expect(stmt, parser, root, false, &mut loop_body)?;
346+
ctx.lower_expect(stmt, parser, root, ExprPos::Rhs, &mut loop_body)?;
344347
let condition = ctx.add_expression(
345348
Expression::Unary {
346349
op: UnaryOperator::Not,
@@ -383,7 +386,7 @@ impl<'source> ParsingContext<'source> {
383386
} else {
384387
let mut stmt = ctx.stmt_ctx();
385388
let expr = self.parse_expression(parser, ctx, &mut stmt, body)?;
386-
ctx.lower(stmt, parser, expr, false, body)?;
389+
ctx.lower(stmt, parser, expr, ExprPos::Rhs, body)?;
387390
self.expect(parser, TokenValue::Semicolon)?;
388391
}
389392
}
@@ -422,7 +425,7 @@ impl<'source> ParsingContext<'source> {
422425
} else {
423426
let mut stmt = ctx.stmt_ctx();
424427
let root = self.parse_expression(parser, ctx, &mut stmt, &mut block)?;
425-
ctx.lower_expect(stmt, parser, root, false, &mut block)?
428+
ctx.lower_expect(stmt, parser, root, ExprPos::Rhs, &mut block)?
426429
};
427430

428431
let condition = ctx.add_expression(
@@ -455,7 +458,7 @@ impl<'source> ParsingContext<'source> {
455458
let mut stmt = ctx.stmt_ctx();
456459
let rest =
457460
self.parse_expression(parser, ctx, &mut stmt, &mut continuing)?;
458-
ctx.lower(stmt, parser, rest, false, &mut continuing)?;
461+
ctx.lower(stmt, parser, rest, ExprPos::Rhs, &mut continuing)?;
459462
}
460463
}
461464

@@ -504,7 +507,7 @@ impl<'source> ParsingContext<'source> {
504507
| TokenValue::FloatConstant(_) => {
505508
let mut stmt = ctx.stmt_ctx();
506509
let expr = self.parse_expression(parser, ctx, &mut stmt, body)?;
507-
ctx.lower(stmt, parser, expr, false, body)?;
510+
ctx.lower(stmt, parser, expr, ExprPos::Rhs, body)?;
508511
self.expect(parser, TokenValue::Semicolon)?.meta
509512
}
510513
TokenValue::Semicolon => self.bump(parser)?.meta,

0 commit comments

Comments
 (0)