WGSL front end rejects negative constant literal subscripts, but spec says they're permitted

[jimblandy]

The WGSL specification says that a[-1] for some array a is permitted, and accesses some unspecified element of a, but our front end rejects this:

error: expected non-negative integer constant expression, found `-1`
  ┌─ wgsl:4:26
  │
4 │                 return a[-1];
  │                          ^^ expected non-negative integer

This is hardly the biggest fish we have to fry, but it's important for web compatibility that implementations agree on when errors are signaled and when they are tolerated. This particular case, of indexes that are easy recognized at compile time to be invalid, has been discussed in the committee meetings, and the decision was that they should be permitted to run.

[jimblandy]

Seems to have been introduced in 3370147.
cc: @Frizi

[jimblandy]

To be clear, I 100% agree that it's a better developer experience to have this signaled at compile time, and that for uses outside the web the current behavior is preferable. But Naga also needs to be able to provide strict compliance when used in WebGPU. So if we want to retain the ability to flag the program statically, it needs to be configurable. And I think we should consider carefully whether we want the complexity of that configuration.

[Frizi]

The reason this happens is that AccessIndex expression allows only u32 index. This can be "fixed" by either falling back onto Access with the whole expression being treated as dynamic, or modify the IR to allow negative indices. I say "fixed", because both will likely result in hard errors on the backend side anyway.

naga/src/front/wgsl/mod.rs

Lines 1756 to 1780 in a7d3b60

	if let crate::Expression::Constant(constant) = ctx.expressions[index] {
	let expr_span = open_brace_span.end..close_brace_span.start;
	let index = match ctx.constants[constant].inner {
	ConstantInner::Scalar {
	value: ScalarValue::Uint(int),
	..
	} => u32::try_from(int).map_err(|_| Error::BadU32Constant(expr_span)),
	ConstantInner::Scalar {
	value: ScalarValue::Sint(int),
	..
	} => u32::try_from(int).map_err(|_| Error::BadU32Constant(expr_span)),
	_ => Err(Error::BadU32Constant(expr_span)),
	}?;
	crate::Expression::AccessIndex {
	base: handle,
	index,
	}
	} else {
	crate::Expression::Access {
	base: handle,
	index,
	}
	}

[jimblandy]

Yes - I came across that and was really happy to see that you'd used try_from; so many people will just use as and not worry about the dropped bits.

[teoxoy]

Closing as this is obsolete. Since const-expressions have been added to the spec we are now required to raise OOB errors.