Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

glsl-in: Allow multiple array specifiers #1780

Merged
merged 1 commit into from
Mar 27, 2022
Merged

glsl-in: Allow multiple array specifiers #1780

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 6 additions & 9 deletions src/front/glsl/parser/declarations.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ impl<'source> ParsingContext<'source> {
pub fn parse_init_declarator_list(
&mut self,
parser: &mut Parser,
ty: Handle<Type>,
mut ty: Handle<Type>,
ctx: &mut DeclarationContext,
) -> Result<()> {
// init_declarator_list:
Expand Down Expand Up @@ -169,8 +169,7 @@ impl<'source> ParsingContext<'source> {
// parse an array specifier if it exists
// NOTE: unlike other parse methods this one doesn't expect an array specifier and
// returns Ok(None) rather than an error if there is not one
let array_specifier = self.parse_array_specifier(parser)?;
let ty = parser.maybe_array(ty, meta, array_specifier);
self.parse_array_specifier(parser, &mut meta, &mut ty)?;

let init = self
.bump_if(parser, TokenValue::Assign)
Expand Down Expand Up @@ -463,7 +462,7 @@ impl<'source> ParsingContext<'source> {
body: &mut Block,
qualifiers: &mut TypeQualifiers,
ty_name: String,
meta: Span,
mut meta: Span,
) -> Result<Span> {
let layout = match qualifiers.layout_qualifiers.remove(&QualifierKey::Layout) {
Some((QualifierValue::Layout(l), _)) => l,
Expand Down Expand Up @@ -498,8 +497,7 @@ impl<'source> ParsingContext<'source> {
let name = match token.value {
TokenValue::Semicolon => None,
TokenValue::Identifier(name) => {
let array_specifier = self.parse_array_specifier(parser)?;
ty = parser.maybe_array(ty, token.meta, array_specifier);
self.parse_array_specifier(parser, &mut meta, &mut ty)?;

self.expect(parser, TokenValue::Semicolon)?;

Expand Down Expand Up @@ -561,13 +559,12 @@ impl<'source> ParsingContext<'source> {
loop {
// TODO: type_qualifier

let (ty, mut meta) = self.parse_type_non_void(parser)?;
let (mut ty, mut meta) = self.parse_type_non_void(parser)?;
let (name, end_meta) = self.expect_ident(parser)?;

meta.subsume(end_meta);

let array_specifier = self.parse_array_specifier(parser)?;
let ty = parser.maybe_array(ty, meta, array_specifier);
self.parse_array_specifier(parser, &mut meta, &mut ty)?;

self.expect(parser, TokenValue::Semicolon)?;

Expand Down
10 changes: 4 additions & 6 deletions src/front/glsl/parser/functions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -575,7 +575,7 @@ impl<'source> ParsingContext<'source> {
loop {
if self.peek_type_name(parser) || self.peek_parameter_qualifier(parser) {
let qualifier = self.parse_parameter_qualifier(parser);
let ty = self.parse_type_non_void(parser)?.0;
let mut ty = self.parse_type_non_void(parser)?.0;

match self.expect_peek(parser)?.value {
TokenValue::Comma => {
Expand All @@ -584,12 +584,10 @@ impl<'source> ParsingContext<'source> {
continue;
}
TokenValue::Identifier(_) => {
let name_meta = self.expect_ident(parser)?;
let mut name = self.expect_ident(parser)?;
self.parse_array_specifier(parser, &mut name.1, &mut ty)?;

let array_specifier = self.parse_array_specifier(parser)?;
let ty = parser.maybe_array(ty, name_meta.1, array_specifier);

context.add_function_arg(parser, body, Some(name_meta), ty, qualifier);
context.add_function_arg(parser, body, Some(name), ty, qualifier);

if self.bump_if(parser, TokenValue::Comma).is_some() {
continue;
Expand Down
94 changes: 61 additions & 33 deletions src/front/glsl/parser/types.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,45 +10,76 @@ use crate::{
};

impl<'source> ParsingContext<'source> {
/// Parses an optional array_specifier returning `Ok(None)` if there is no
/// LeftBracket
/// Parses an optional array_specifier returning wether or not it's present
/// and modifying the type handle if it exists
pub fn parse_array_specifier(
&mut self,
parser: &mut Parser,
) -> Result<Option<(ArraySize, Span)>> {
if let Some(Token { mut meta, .. }) = self.bump_if(parser, TokenValue::LeftBracket) {
if let Some(Token { meta: end_meta, .. }) =
self.bump_if(parser, TokenValue::RightBracket)
{
meta.subsume(end_meta);
return Ok(Some((ArraySize::Dynamic, meta)));
}
span: &mut Span,
ty: &mut Handle<Type>,
) -> Result<()> {
while self.parse_array_specifier_single(parser, span, ty)? {}
Ok(())
}

let (value, span) = self.parse_uint_constant(parser)?;
let constant = parser.module.constants.fetch_or_append(
crate::Constant {
/// Implementation of [`Self::parse_array_specifier`] for a single array_specifier
fn parse_array_specifier_single(
&mut self,
parser: &mut Parser,
span: &mut Span,
ty: &mut Handle<Type>,
) -> Result<bool> {
if self.bump_if(parser, TokenValue::LeftBracket).is_some() {
let size =
if let Some(Token { meta, .. }) = self.bump_if(parser, TokenValue::RightBracket) {
span.subsume(meta);
ArraySize::Dynamic
} else {
let (value, constant_span) = self.parse_uint_constant(parser)?;
let constant = parser.module.constants.fetch_or_append(
crate::Constant {
name: None,
specialization: None,
inner: crate::ConstantInner::Scalar {
width: 4,
value: crate::ScalarValue::Uint(value as u64),
},
},
constant_span,
);
let end_span = self.expect(parser, TokenValue::RightBracket)?.meta;
span.subsume(end_span);
ArraySize::Constant(constant)
};

parser
.layouter
.update(&parser.module.types, &parser.module.constants)
.unwrap();
let stride = parser.layouter[*ty].to_stride();
*ty = parser.module.types.insert(
Type {
name: None,
specialization: None,
inner: crate::ConstantInner::Scalar {
width: 4,
value: crate::ScalarValue::Uint(value as u64),
inner: TypeInner::Array {
base: *ty,
size,
stride,
},
},
span,
*span,
);
let end_meta = self.expect(parser, TokenValue::RightBracket)?.meta;
meta.subsume(end_meta);
Ok(Some((ArraySize::Constant(constant), meta)))

Ok(true)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand why we are mutating ty and returning Ok(bool) if we could instead return something like Result<Option<Handle<Type>>>. Do you think this would be a feasible thing to change here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We definitely could but I don't what the benefit would be since we would be mutating the type anyways.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where would we be mutating the type? This would just return a new type.
Or do you mean that the caller would likely reassign the type anyway? That's OK, I think.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes the caller always reassigns the type

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that would still be a better internal API.

} else {
Ok(None)
Ok(false)
}
}

pub fn parse_type(&mut self, parser: &mut Parser) -> Result<(Option<Handle<Type>>, Span)> {
let token = self.bump(parser)?;
let handle = match token.value {
TokenValue::Void => None,
TokenValue::TypeName(ty) => Some(parser.module.types.insert(ty, token.meta)),
let mut handle = match token.value {
TokenValue::Void => return Ok((None, token.meta)),
TokenValue::TypeName(ty) => parser.module.types.insert(ty, token.meta),
TokenValue::Struct => {
let mut meta = token.meta;
let ty_name = self.expect_ident(parser)?.0;
Expand All @@ -66,10 +97,10 @@ impl<'source> ParsingContext<'source> {
meta,
);
parser.lookup_type.insert(ty_name, ty);
Some(ty)
ty
}
TokenValue::Identifier(ident) => match parser.lookup_type.get(&ident) {
Some(ty) => Some(*ty),
Some(ty) => *ty,
None => {
return Err(Error {
kind: ErrorKind::UnknownType(ident),
Expand All @@ -92,12 +123,9 @@ impl<'source> ParsingContext<'source> {
}
};

let token_meta = token.meta;
let array_specifier = self.parse_array_specifier(parser)?;
let handle = handle.map(|ty| parser.maybe_array(ty, token_meta, array_specifier));
let mut meta = array_specifier.map_or(token_meta, |(_, meta)| meta);
meta.subsume(token_meta);
Ok((handle, meta))
let mut span = token.meta;
self.parse_array_specifier(parser, &mut span, &mut handle)?;
Ok((Some(handle), span))
}

pub fn parse_type_non_void(&mut self, parser: &mut Parser) -> Result<(Handle<Type>, Span)> {
Expand Down
29 changes: 2 additions & 27 deletions src/front/glsl/types.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use super::{constants::ConstantSolver, context::Context, Error, ErrorKind, Parser, Result, Span};
use crate::{
proc::ResolveContext, ArraySize, Bytes, Constant, Expression, Handle, ImageClass,
ImageDimension, ScalarKind, Type, TypeInner, VectorSize,
proc::ResolveContext, Bytes, Constant, Expression, Handle, ImageClass, ImageDimension,
ScalarKind, Type, TypeInner, VectorSize,
};

pub fn parse_type(type_name: &str) -> Option<Type> {
Expand Down Expand Up @@ -299,29 +299,4 @@ impl Parser {
meta,
})
}

pub(crate) fn maybe_array(
&mut self,
base: Handle<Type>,
mut meta: Span,
array_specifier: Option<(ArraySize, Span)>,
) -> Handle<Type> {
match array_specifier {
Some((size, size_meta)) => {
meta.subsume(size_meta);
self.layouter
.update(&self.module.types, &self.module.constants)
.unwrap();
let stride = self.layouter[base].to_stride();
self.module.types.insert(
Type {
name: None,
inner: TypeInner::Array { base, size, stride },
},
meta,
)
}
None => base,
}
}
}
4 changes: 4 additions & 0 deletions tests/in/glsl/declarations.vert
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ struct TestStruct {
float b;
};

float array_2d[2][2];
float array_toomanyd[2][2][2][2][2][2][2];

void main() {
const vec3 positions[2] = vec3[2](
Expand All @@ -26,5 +28,7 @@ void main() {
);
const TestStruct strct = TestStruct( 1, 2 );
const vec4 from_input_array = in_array[1];
const float a = array_2d[0][0];
const float b = array_toomanyd[0][0][0][0][0][0][0];
out_array[0] = vec4(2.0);
}
22 changes: 15 additions & 7 deletions tests/out/wgsl/declarations-vert.wgsl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,22 @@ var<private> vert: VertexData;
var<private> frag: FragmentData;
var<private> in_array_2: array<vec4<f32>,2u>;
var<private> out_array: array<vec4<f32>,2u>;
var<private> array_2d: array<array<f32,2u>,2u>;
var<private> array_toomanyd: array<array<array<array<array<array<array<f32,2u>,2u>,2u>,2u>,2u>,2u>,2u>;

fn main_1() {
var positions: array<vec3<f32>,2u> = array<vec3<f32>,2u>(vec3<f32>(-1.0, 1.0, 0.0), vec3<f32>(-1.0, -1.0, 0.0));
var strct: TestStruct = TestStruct(1.0, 2.0);
var from_input_array: vec4<f32>;
var a_1: f32;
var b: f32;

let _e32 = in_array_2;
from_input_array = _e32[1];
let _e34 = in_array_2;
from_input_array = _e34[1];
let _e39 = array_2d;
a_1 = _e39[0][0];
let _e50 = array_toomanyd;
b = _e50[0][0][0][0][0][0][0];
out_array[0] = vec4<f32>(2.0);
return;
}
Expand All @@ -43,9 +51,9 @@ fn main(@location(0) position: vec2<f32>, @location(1) a: vec2<f32>, @location(2
in_array_2[0] = in_array;
in_array_2[1] = in_array_1;
main_1();
let _e26 = frag.position;
let _e28 = frag.a;
let _e31 = out_array[0];
let _e33 = out_array[1];
return VertexOutput(_e26, _e28, _e31, _e33);
let _e30 = frag.position;
let _e32 = frag.a;
let _e35 = out_array[0];
let _e37 = out_array[1];
return VertexOutput(_e30, _e32, _e35, _e37);
}