Escape control chars in source when printing diagnostics (#8049)

Author: andyleiserson

CHANGELOG.md

@@ -76,6 +76,7 @@ By @Vecvec in [#7913](https://github.com/gfx-rs/wgpu/pull/7913).
 #### Naga
 
 - Naga now requires that no type be larger than 1 GB. This limit may be lowered in the future; feedback on an appropriate value for the limit is welcome. By @andyleiserson in [#7950](https://github.com/gfx-rs/wgpu/pull/7950).
+- If the shader source contains control characters, Naga now replaces them with U+FFFD ("replacement character") in diagnostic output. By @andyleiserson in [#8049](https://github.com/gfx-rs/wgpu/pull/8049).
 
 #### DX12
 
@@ -182,8 +183,6 @@ let (device, queue) = adapter
     .unwrap();
 ```
 
-More examples of this
-
 By @Vecvec in [#7829](https://github.com/gfx-rs/wgpu/pull/7829).
 
 ### Naga

naga/src/error.rs

@@ -1,4 +1,4 @@
-use alloc::{boxed::Box, string::String};
+use alloc::{borrow::Cow, boxed::Box, string::String};
 use core::{error::Error, fmt};
 
 #[derive(Clone, Debug)]
@@ -41,7 +41,7 @@ impl fmt::Display for ShaderError<crate::WithSpan<crate::valid::ValidationError>
         use codespan_reporting::{files::SimpleFile, term};
 
         let label = self.label.as_deref().unwrap_or_default();
-        let files = SimpleFile::new(label, &self.source);
+        let files = SimpleFile::new(label, replace_control_chars(&self.source));
         let config = term::Config::default();
 
         let writer = {
@@ -137,3 +137,32 @@ where
         Some(&self.inner)
     }
 }
+
+pub(crate) fn replace_control_chars(s: &str) -> Cow<'_, str> {
+    const REPLACEMENT_CHAR: &str = "\u{FFFD}";
+    debug_assert_eq!(
+        REPLACEMENT_CHAR.chars().next().unwrap(),
+        char::REPLACEMENT_CHARACTER
+    );
+
+    let mut res = Cow::Borrowed(s);
+    let mut offset = 0;
+
+    while let Some(found_pos) = res[offset..].find(|c: char| c.is_control() && !c.is_whitespace()) {
+        offset += found_pos;
+        let found_len = res[offset..].chars().next().unwrap().len_utf8();
+        res.to_mut()
+            .replace_range(offset..offset + found_len, REPLACEMENT_CHAR);
+        offset += REPLACEMENT_CHAR.len();
+    }
+
+    res
+}
+
+#[test]
+fn test_replace_control_chars() {
+    // The UTF-8 encoding of \u{0080} is multiple bytes.
+    let input = "Foo\u{0080}Bar\u{0001}Baz\n";
+    let expected = "Foo\u{FFFD}Bar\u{FFFD}Baz\n";
+    assert_eq!(replace_control_chars(input), expected);
+}

naga/src/front/glsl/error.rs

@@ -12,7 +12,7 @@ use pp_rs::token::PreprocessorError;
 use thiserror::Error;
 
 use super::token::TokenValue;
-use crate::SourceLocation;
+use crate::{error::replace_control_chars, SourceLocation};
 use crate::{error::ErrorWrite, proc::ConstantEvaluatorError, Span};
 
 fn join_with_comma(list: &[ExpectedToken]) -> String {
@@ -171,7 +171,7 @@ impl ParseErrors {
 
     pub fn emit_to_writer_with_path(&self, writer: &mut impl ErrorWrite, source: &str, path: &str) {
         let path = path.to_string();
-        let files = SimpleFile::new(path, source);
+        let files = SimpleFile::new(path, replace_control_chars(source));
         let config = term::Config::default();
 
         for err in &self.errors {

naga/src/front/spv/error.rs

@@ -8,7 +8,11 @@ use codespan_reporting::files::SimpleFile;
 use codespan_reporting::term;
 
 use super::ModuleState;
-use crate::{arena::Handle, error::ErrorWrite, front::atomic_upgrade};
+use crate::{
+    arena::Handle,
+    error::{replace_control_chars, ErrorWrite},
+    front::atomic_upgrade,
+};
 
 #[derive(Clone, Debug, thiserror::Error)]
 pub enum Error {
@@ -157,7 +161,7 @@ impl Error {
 
     pub fn emit_to_writer_with_path(&self, writer: &mut impl ErrorWrite, source: &str, path: &str) {
         let path = path.to_string();
-        let files = SimpleFile::new(path, source);
+        let files = SimpleFile::new(path, replace_control_chars(source));
         let config = term::Config::default();
         let diagnostic = Diagnostic::error().with_message(format!("{self:?}"));
 

naga/src/front/wgsl/error.rs

@@ -2,6 +2,7 @@
 
 use crate::common::wgsl::TryToWgsl;
 use crate::diagnostic_filter::ConflictingDiagnosticRuleError;
+use crate::error::replace_control_chars;
 use crate::proc::{Alignment, ConstantEvaluatorError, ResolveError};
 use crate::{Scalar, SourceLocation, Span};
 
@@ -79,7 +80,7 @@ impl ParseError {
         P: AsRef<std::path::Path>,
     {
         let path = path.as_ref().display().to_string();
-        let files = SimpleFile::new(path, source);
+        let files = SimpleFile::new(path, replace_control_chars(source));
         let config = term::Config::default();
 
         cfg_if::cfg_if! {
@@ -105,7 +106,7 @@ impl ParseError {
         P: AsRef<std::path::Path>,
     {
         let path = path.as_ref().display().to_string();
-        let files = SimpleFile::new(path, source);
+        let files = SimpleFile::new(path, replace_control_chars(source));
         let config = term::Config::default();
 
         let mut writer = crate::error::DiagnosticBuffer::new();

naga/src/span.rs

@@ -6,7 +6,7 @@ use alloc::{
 };
 use core::{error::Error, fmt, ops::Range};
 
-use crate::{path_like::PathLike, Arena, Handle, UniqueArena};
+use crate::{error::replace_control_chars, path_like::PathLike, Arena, Handle, UniqueArena};
 
 /// A source code span, used for error reporting.
 #[derive(Clone, Copy, Debug, PartialEq, Default)]
@@ -291,7 +291,7 @@ impl<E> WithSpan<E> {
         use codespan_reporting::{files, term};
 
         let path = path.to_string_lossy();
-        let files = files::SimpleFile::new(path, source);
+        let files = files::SimpleFile::new(path, replace_control_chars(source));
         let config = term::Config::default();
 
         cfg_if::cfg_if! {
@@ -323,7 +323,7 @@ impl<E> WithSpan<E> {
         use codespan_reporting::{files, term};
 
         let path = path.to_string_lossy();
-        let files = files::SimpleFile::new(path, source);
+        let files = files::SimpleFile::new(path, replace_control_chars(source));
         let config = term::Config::default();
 
         let mut writer = crate::error::DiagnosticBuffer::new();

naga/tests/naga/wgsl_errors.rs

@@ -3887,3 +3887,18 @@ fn max_type_size_array_of_structs() {
         ))
     }
 }
+
+#[cfg(feature = "wgsl-in")]
+#[test]
+fn source_with_control_char() {
+    check(
+        "\x07",
+        "error: expected global item (`struct`, `const`, `var`, `alias`, `fn`, `diagnostic`, `enable`, `requires`, `;`) or the end of the file, found \"\\u{7}\"
+  ┌─ wgsl:1:1
+  │
+1 │ �
+  │ ^ expected global item (`struct`, `const`, `var`, `alias`, `fn`, `diagnostic`, `enable`, `requires`, `;`) or the end of the file
+
+",
+    );
+}