Skip to content

fix(namer): escape, rather than strip, non-ASCII ident. characters #7995

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.

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

Open
wants to merge 2 commits into
base: trunk
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 6 additions & 3 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,10 @@ Fix `STATUS_HEAP_CORRUPTION` crash when concurrently calling `create_sampler`. B
- Fixed build error occurring when the `profiling` dependency is configured to have profiling active. By @kpreid in [#7916](https://github.com/gfx-rs/wgpu/pull/7916).
- Emit a validation error instead of panicking when a query set index is OOB. By @ErichDonGubler in [#7908](https://github.com/gfx-rs/wgpu/pull/7908).

#### Naga

- Escape, rather than strip, identifiers with Unicode. By @ErichDonGubler in [7995](https://github.com/gfx-rs/wgpu/pull/7995).

## v26.0.0 (2025-07-09)

### Major Features
Expand Down Expand Up @@ -145,9 +149,9 @@ let (device, queue) = adapter
.unwrap();
```

More examples of this
More examples of this

By @Vecvec in [#7829](https://github.com/gfx-rs/wgpu/pull/7829).
By @Vecvec in [#7829](https://github.com/gfx-rs/wgpu/pull/7829).

### Naga

Expand Down Expand Up @@ -231,7 +235,6 @@ By @Vecvec in [#7829](https://github.com/gfx-rs/wgpu/pull/7829).
- The definition of `enum CommandEncoderError` has changed significantly, to reflect which errors can be raised by `CommandEncoder.finish()`. There are also some errors that no longer appear directly in `CommandEncoderError`, and instead appear nested within the `RenderPass` or `ComputePass` variants.
- `CopyError` has been removed. Errors that were previously a `CopyError` are now a `CommandEncoderError` returned by `finish()`. (The detailed reasons for copies to fail were and still are described by `TransferError`, which was previously a variant of `CopyError`, and is now a variant of `CommandEncoderError`).


#### Naga

- Mark `readonly_and_readwrite_storage_textures` & `packed_4x8_integer_dot_product` language extensions as implemented. By @teoxoy in [#7543](https://github.com/gfx-rs/wgpu/pull/7543)
Expand Down
29 changes: 20 additions & 9 deletions naga/src/proc/namer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,20 +131,31 @@ impl Namer {
{
Cow::Borrowed(string)
} else {
let mut filtered = string
.chars()
.filter(|&c| c.is_ascii_alphanumeric() || c == '_')
.fold(String::new(), |mut s, c| {
if s.ends_with('_') && c == '_' {
return s;
}
let mut filtered = string.chars().fold(String::new(), |mut s, c| {
let had_underscore_at_end = s.ends_with('_');
if had_underscore_at_end && c == '_' {
return s;
}
if c.is_ascii_alphanumeric() || c == '_' {
s.push(c);
s
});
} else {
use core::fmt::Write as _;
if !s.is_empty() && !had_underscore_at_end {
s.push('_');
}
write!(s, "u{:04x}_", c as u32).unwrap();
Copy link
Contributor

Choose a reason for hiding this comment

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

Not important, since you've covered it with snapshot tests, but one thing I've noticed about naga/wgpu is that we don't have a lot of unit tests, and this behavior seems like a good candidate for unit testing. (But as I said, not important, I don't think it's worth going back and changing/adding the tests, this is more a reminder to be thinking about unit tests in general.)

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed on the scope; I also think it would be nice to use several snapshot-ish unit tests in follow-up work, just to make it easier to reason about some changes.

}
s
});
let stripped_len = filtered.trim_end_matches(SEPARATOR).len();
filtered.truncate(stripped_len);
if filtered.is_empty() {
filtered.push_str("unnamed");
} else if filtered.starts_with(|c: char| c.is_ascii_digit()) {
unreachable!(
"internal error: invalid identifier starting with ASCII digit {:?}",
filtered.chars().nth(0)
)
}
Cow::Owned(filtered)
};
Expand Down
14 changes: 14 additions & 0 deletions naga/tests/in/wgsl/7995-unicode-idents.wgsl
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// NOTE: This allows us to suppress compaction below, to force the handling of identifiers
// containing Unicode.
@group(0) @binding(0)
var<storage> asdf: f32;

fn compute() -> f32 {
let θ2 = asdf + 9001.0;
return θ2;
}

@compute @workgroup_size(1, 1)
fn main() {
compute();
}
4 changes: 2 additions & 2 deletions naga/tests/out/glsl/spv-do-while.main.Fragment.glsl
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ precision highp float;
precision highp int;


void fb1_(inout bool cond) {
void f_u0028_b1_u003b(inout bool cond) {
bool loop_init = true;
while(true) {
if (!loop_init) {
Expand All @@ -22,7 +22,7 @@ void fb1_(inout bool cond) {
void main_1() {
bool param = false;
param = false;
fb1_(param);
f_u0028_b1_u003b(param);
return;
}

Expand Down
21 changes: 21 additions & 0 deletions naga/tests/out/glsl/wgsl-7995-unicode-idents.main.Compute.glsl
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
#version 310 es

precision highp float;
precision highp int;

layout(local_size_x = 1, local_size_y = 1, local_size_z = 1) in;

layout(std430) readonly buffer type_block_0Compute { float _group_0_binding_0_cs; };


float compute() {
float _e1 = _group_0_binding_0_cs;
float u03b8_2_ = (_e1 + 9001.0);
return u03b8_2_;
}

void main() {
float _e0 = compute();
return;
}

Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@ precision highp int;

layout(local_size_x = 1, local_size_y = 1, local_size_z = 1) in;

struct _atomic_compare_exchange_resultSint4_ {
struct _atomic_compare_exchange_result_u003c_Sint_u002c_4_u003e {
Copy link
Contributor

Choose a reason for hiding this comment

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

It's slightly unfortunate to use so many characters for an internally-generated type name. Maybe there could be a rule like "any number of consecutive :<>, characters are mapped to a single underscore"?

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe something like 3a4c931?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, that looks great to me.

int old_value;
bool exchanged;
};
struct _atomic_compare_exchange_resultUint4_ {
struct _atomic_compare_exchange_result_u003c_Uint_u002c_4_u003e {
uint old_value;
bool exchanged;
};
Expand Down Expand Up @@ -50,7 +50,7 @@ void main() {
int new = floatBitsToInt((intBitsToFloat(_e14) + 1.0));
uint _e20 = i;
int _e22 = old;
_atomic_compare_exchange_resultSint4_ _e23; _e23.old_value = atomicCompSwap(_group_0_binding_0_cs[_e20], _e22, new);
_atomic_compare_exchange_result_u003c_Sint_u002c_4_u003e _e23; _e23.old_value = atomicCompSwap(_group_0_binding_0_cs[_e20], _e22, new);
_e23.exchanged = (_e23.old_value == _e22);
old = _e23.old_value;
exchanged = _e23.exchanged;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@ precision highp int;

layout(local_size_x = 1, local_size_y = 1, local_size_z = 1) in;

struct _atomic_compare_exchange_resultSint4_ {
struct _atomic_compare_exchange_result_u003c_Sint_u002c_4_u003e {
int old_value;
bool exchanged;
};
struct _atomic_compare_exchange_resultUint4_ {
struct _atomic_compare_exchange_result_u003c_Uint_u002c_4_u003e {
uint old_value;
bool exchanged;
};
Expand Down Expand Up @@ -50,7 +50,7 @@ void main() {
uint new = floatBitsToUint((uintBitsToFloat(_e14) + 1.0));
uint _e20 = i_1;
uint _e22 = old_1;
_atomic_compare_exchange_resultUint4_ _e23; _e23.old_value = atomicCompSwap(_group_0_binding_1_cs[_e20], _e22, new);
_atomic_compare_exchange_result_u003c_Uint_u002c_4_u003e _e23; _e23.old_value = atomicCompSwap(_group_0_binding_1_cs[_e20], _e22, new);
_e23.exchanged = (_e23.old_value == _e22);
old_1 = _e23.old_value;
exchanged_1 = _e23.exchanged;
Expand Down
20 changes: 10 additions & 10 deletions naga/tests/out/glsl/wgsl-atomicOps.cs_main.Compute.glsl
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@ struct Struct {
uint atomic_scalar;
int atomic_arr[2];
};
struct _atomic_compare_exchange_resultUint4_ {
struct _atomic_compare_exchange_result_u003c_Uint_u002c_4_u003e {
uint old_value;
bool exchanged;
};
struct _atomic_compare_exchange_resultSint4_ {
struct _atomic_compare_exchange_result_u003c_Sint_u002c_4_u003e {
int old_value;
bool exchanged;
};
Expand Down Expand Up @@ -135,21 +135,21 @@ void main() {
int _e295 = atomicExchange(workgroup_atomic_arr[1], 1);
uint _e299 = atomicExchange(workgroup_struct.atomic_scalar, 1u);
int _e304 = atomicExchange(workgroup_struct.atomic_arr[1], 1);
_atomic_compare_exchange_resultUint4_ _e308; _e308.old_value = atomicCompSwap(_group_0_binding_0_cs, 1u, 2u);
_atomic_compare_exchange_result_u003c_Uint_u002c_4_u003e _e308; _e308.old_value = atomicCompSwap(_group_0_binding_0_cs, 1u, 2u);
_e308.exchanged = (_e308.old_value == 1u);
_atomic_compare_exchange_resultSint4_ _e313; _e313.old_value = atomicCompSwap(_group_0_binding_1_cs[1], 1, 2);
_atomic_compare_exchange_result_u003c_Sint_u002c_4_u003e _e313; _e313.old_value = atomicCompSwap(_group_0_binding_1_cs[1], 1, 2);
_e313.exchanged = (_e313.old_value == 1);
_atomic_compare_exchange_resultUint4_ _e318; _e318.old_value = atomicCompSwap(_group_0_binding_2_cs.atomic_scalar, 1u, 2u);
_atomic_compare_exchange_result_u003c_Uint_u002c_4_u003e _e318; _e318.old_value = atomicCompSwap(_group_0_binding_2_cs.atomic_scalar, 1u, 2u);
_e318.exchanged = (_e318.old_value == 1u);
_atomic_compare_exchange_resultSint4_ _e324; _e324.old_value = atomicCompSwap(_group_0_binding_2_cs.atomic_arr[1], 1, 2);
_atomic_compare_exchange_result_u003c_Sint_u002c_4_u003e _e324; _e324.old_value = atomicCompSwap(_group_0_binding_2_cs.atomic_arr[1], 1, 2);
_e324.exchanged = (_e324.old_value == 1);
_atomic_compare_exchange_resultUint4_ _e328; _e328.old_value = atomicCompSwap(workgroup_atomic_scalar, 1u, 2u);
_atomic_compare_exchange_result_u003c_Uint_u002c_4_u003e _e328; _e328.old_value = atomicCompSwap(workgroup_atomic_scalar, 1u, 2u);
_e328.exchanged = (_e328.old_value == 1u);
_atomic_compare_exchange_resultSint4_ _e333; _e333.old_value = atomicCompSwap(workgroup_atomic_arr[1], 1, 2);
_atomic_compare_exchange_result_u003c_Sint_u002c_4_u003e _e333; _e333.old_value = atomicCompSwap(workgroup_atomic_arr[1], 1, 2);
_e333.exchanged = (_e333.old_value == 1);
_atomic_compare_exchange_resultUint4_ _e338; _e338.old_value = atomicCompSwap(workgroup_struct.atomic_scalar, 1u, 2u);
_atomic_compare_exchange_result_u003c_Uint_u002c_4_u003e _e338; _e338.old_value = atomicCompSwap(workgroup_struct.atomic_scalar, 1u, 2u);
_e338.exchanged = (_e338.old_value == 1u);
_atomic_compare_exchange_resultSint4_ _e344; _e344.old_value = atomicCompSwap(workgroup_struct.atomic_arr[1], 1, 2);
_atomic_compare_exchange_result_u003c_Sint_u002c_4_u003e _e344; _e344.old_value = atomicCompSwap(workgroup_struct.atomic_arr[1], 1, 2);
_e344.exchanged = (_e344.old_value == 1);
return;
}
Expand Down
4 changes: 2 additions & 2 deletions naga/tests/out/hlsl/spv-do-while.hlsl
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
void fb1_(inout bool cond)
void f_u0028_b1_u003b(inout bool cond)
{
uint2 loop_bound = uint2(4294967295u, 4294967295u);
bool loop_init = true;
Expand All @@ -22,7 +22,7 @@ void main_1()
bool param = (bool)0;

param = false;
fb1_(param);
f_u0028_b1_u003b(param);
return;
}

Expand Down
2 changes: 1 addition & 1 deletion naga/tests/out/hlsl/spv-fetch_depth.hlsl
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ void function()
}

[numthreads(32, 1, 1)]
void cullfetch_depth()
void cull_u003a_u003a_fetch_depth()
{
function();
}
2 changes: 1 addition & 1 deletion naga/tests/out/hlsl/spv-fetch_depth.ron
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
],
compute:[
(
entry_point:"cullfetch_depth",
entry_point:"cull_u003a_u003a_fetch_depth",
target_profile:"cs_5_1",
),
],
Expand Down
15 changes: 15 additions & 0 deletions naga/tests/out/hlsl/wgsl-7995-unicode-idents.hlsl
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
ByteAddressBuffer asdf : register(t0);

float compute()
{
float _e1 = asfloat(asdf.Load(0));
float u03b8_2_ = (_e1 + 9001.0);
return u03b8_2_;
}

[numthreads(1, 1, 1)]
void main()
{
const float _e0 = compute();
return;
}
12 changes: 12 additions & 0 deletions naga/tests/out/hlsl/wgsl-7995-unicode-idents.ron
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
(
vertex:[
],
fragment:[
],
compute:[
(
entry_point:"main",
target_profile:"cs_5_1",
),
],
)
8 changes: 4 additions & 4 deletions naga/tests/out/hlsl/wgsl-atomicCompareExchange-int64.hlsl
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@ struct NagaConstants {
};
ConstantBuffer<NagaConstants> _NagaConstants: register(b0, space1);

struct _atomic_compare_exchange_resultSint8_ {
struct _atomic_compare_exchange_result_u003c_Sint_u002c_8_u003e {
int64_t old_value;
bool exchanged;
int _end_pad_0;
};

struct _atomic_compare_exchange_resultUint8_ {
struct _atomic_compare_exchange_result_u003c_Uint_u002c_8_u003e {
uint64_t old_value;
bool exchanged;
int _end_pad_0;
Expand Down Expand Up @@ -63,7 +63,7 @@ void test_atomic_compare_exchange_i64_()
int64_t new_ = (_e14 + 10L);
uint _e19 = i;
int64_t _e21 = old;
_atomic_compare_exchange_resultSint8_ _e22; arr_i64_.InterlockedCompareExchange64(_e19*8, _e21, new_, _e22.old_value);
_atomic_compare_exchange_result_u003c_Sint_u002c_8_u003e _e22; arr_i64_.InterlockedCompareExchange64(_e19*8, _e21, new_, _e22.old_value);
_e22.exchanged = (_e22.old_value == _e21);
old = _e22.old_value;
exchanged = _e22.exchanged;
Expand Down Expand Up @@ -115,7 +115,7 @@ void test_atomic_compare_exchange_u64_()
uint64_t new_1 = (_e14 + 10uL);
uint _e19 = i_1;
uint64_t _e21 = old_1;
_atomic_compare_exchange_resultUint8_ _e22; arr_u64_.InterlockedCompareExchange64(_e19*8, _e21, new_1, _e22.old_value);
_atomic_compare_exchange_result_u003c_Uint_u002c_8_u003e _e22; arr_u64_.InterlockedCompareExchange64(_e19*8, _e21, new_1, _e22.old_value);
_e22.exchanged = (_e22.old_value == _e21);
old_1 = _e22.old_value;
exchanged_1 = _e22.exchanged;
Expand Down
8 changes: 4 additions & 4 deletions naga/tests/out/hlsl/wgsl-atomicCompareExchange.hlsl
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
struct _atomic_compare_exchange_resultSint4_ {
struct _atomic_compare_exchange_result_u003c_Sint_u002c_4_u003e {
int old_value;
bool exchanged;
};

struct _atomic_compare_exchange_resultUint4_ {
struct _atomic_compare_exchange_result_u003c_Uint_u002c_4_u003e {
uint old_value;
bool exchanged;
};
Expand Down Expand Up @@ -54,7 +54,7 @@ void test_atomic_compare_exchange_i32_()
int new_ = asint((asfloat(_e14) + 1.0));
uint _e20 = i;
int _e22 = old;
_atomic_compare_exchange_resultSint4_ _e23; arr_i32_.InterlockedCompareExchange(_e20*4, _e22, new_, _e23.old_value);
_atomic_compare_exchange_result_u003c_Sint_u002c_4_u003e _e23; arr_i32_.InterlockedCompareExchange(_e20*4, _e22, new_, _e23.old_value);
_e23.exchanged = (_e23.old_value == _e22);
old = _e23.old_value;
exchanged = _e23.exchanged;
Expand Down Expand Up @@ -106,7 +106,7 @@ void test_atomic_compare_exchange_u32_()
uint new_1 = asuint((asfloat(_e14) + 1.0));
uint _e20 = i_1;
uint _e22 = old_1;
_atomic_compare_exchange_resultUint4_ _e23; arr_u32_.InterlockedCompareExchange(_e20*4, _e22, new_1, _e23.old_value);
_atomic_compare_exchange_result_u003c_Uint_u002c_4_u003e _e23; arr_u32_.InterlockedCompareExchange(_e20*4, _e22, new_1, _e23.old_value);
_e23.exchanged = (_e23.old_value == _e22);
old_1 = _e23.old_value;
exchanged_1 = _e23.exchanged;
Expand Down
20 changes: 10 additions & 10 deletions naga/tests/out/hlsl/wgsl-atomicOps-int64.hlsl
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,13 @@ struct Struct {
int64_t atomic_arr[2];
};

struct _atomic_compare_exchange_resultUint8_ {
struct _atomic_compare_exchange_result_u003c_Uint_u002c_8_u003e {
uint64_t old_value;
bool exchanged;
int _end_pad_0;
};

struct _atomic_compare_exchange_resultSint8_ {
struct _atomic_compare_exchange_result_u003c_Sint_u002c_8_u003e {
int64_t old_value;
bool exchanged;
int _end_pad_0;
Expand Down Expand Up @@ -126,21 +126,21 @@ void cs_main(uint3 id : SV_GroupThreadID, uint3 __local_invocation_id : SV_Group
int64_t _e279; InterlockedExchange(workgroup_atomic_arr[1], 1L, _e279);
uint64_t _e283; InterlockedExchange(workgroup_struct.atomic_scalar, 1uL, _e283);
int64_t _e288; InterlockedExchange(workgroup_struct.atomic_arr[1], 1L, _e288);
_atomic_compare_exchange_resultUint8_ _e292; storage_atomic_scalar.InterlockedCompareExchange64(0, 1uL, 2uL, _e292.old_value);
_atomic_compare_exchange_result_u003c_Uint_u002c_8_u003e _e292; storage_atomic_scalar.InterlockedCompareExchange64(0, 1uL, 2uL, _e292.old_value);
_e292.exchanged = (_e292.old_value == 1uL);
_atomic_compare_exchange_resultSint8_ _e297; storage_atomic_arr.InterlockedCompareExchange64(8, 1L, 2L, _e297.old_value);
_atomic_compare_exchange_result_u003c_Sint_u002c_8_u003e _e297; storage_atomic_arr.InterlockedCompareExchange64(8, 1L, 2L, _e297.old_value);
_e297.exchanged = (_e297.old_value == 1L);
_atomic_compare_exchange_resultUint8_ _e302; storage_struct.InterlockedCompareExchange64(0, 1uL, 2uL, _e302.old_value);
_atomic_compare_exchange_result_u003c_Uint_u002c_8_u003e _e302; storage_struct.InterlockedCompareExchange64(0, 1uL, 2uL, _e302.old_value);
_e302.exchanged = (_e302.old_value == 1uL);
_atomic_compare_exchange_resultSint8_ _e308; storage_struct.InterlockedCompareExchange64(8+8, 1L, 2L, _e308.old_value);
_atomic_compare_exchange_result_u003c_Sint_u002c_8_u003e _e308; storage_struct.InterlockedCompareExchange64(8+8, 1L, 2L, _e308.old_value);
_e308.exchanged = (_e308.old_value == 1L);
_atomic_compare_exchange_resultUint8_ _e312; InterlockedCompareExchange(workgroup_atomic_scalar, 1uL, 2uL, _e312.old_value);
_atomic_compare_exchange_result_u003c_Uint_u002c_8_u003e _e312; InterlockedCompareExchange(workgroup_atomic_scalar, 1uL, 2uL, _e312.old_value);
_e312.exchanged = (_e312.old_value == 1uL);
_atomic_compare_exchange_resultSint8_ _e317; InterlockedCompareExchange(workgroup_atomic_arr[1], 1L, 2L, _e317.old_value);
_atomic_compare_exchange_result_u003c_Sint_u002c_8_u003e _e317; InterlockedCompareExchange(workgroup_atomic_arr[1], 1L, 2L, _e317.old_value);
_e317.exchanged = (_e317.old_value == 1L);
_atomic_compare_exchange_resultUint8_ _e322; InterlockedCompareExchange(workgroup_struct.atomic_scalar, 1uL, 2uL, _e322.old_value);
_atomic_compare_exchange_result_u003c_Uint_u002c_8_u003e _e322; InterlockedCompareExchange(workgroup_struct.atomic_scalar, 1uL, 2uL, _e322.old_value);
_e322.exchanged = (_e322.old_value == 1uL);
_atomic_compare_exchange_resultSint8_ _e328; InterlockedCompareExchange(workgroup_struct.atomic_arr[1], 1L, 2L, _e328.old_value);
_atomic_compare_exchange_result_u003c_Sint_u002c_8_u003e _e328; InterlockedCompareExchange(workgroup_struct.atomic_arr[1], 1L, 2L, _e328.old_value);
_e328.exchanged = (_e328.old_value == 1L);
return;
}
Loading
Loading