comment clarification and typo fix

RalfJung · RalfJung · commit 5067dd21b6e8 · 2024-04-18T22:27:07.000+02:00
diff --git a/src/tools/miri/src/shims/os_str.rs b/src/tools/miri/src/shims/os_str.rs
@@ -260,6 +260,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
         let this = self.eval_context_ref();
         let target_os = &this.tcx.sess.target.os;
 
+        // Below we assume that everything non-Windows works like Unix, at least
+        // when it comes to file system path conventions.
         #[cfg(windows)]
         return if target_os == "windows" {
             // Windows-on-Windows, all fine.
@@ -297,6 +299,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                     {
                         converted.remove(0);
                     }
+                    // If the path starts with `\\`, it is a magic Windows path. Conveniently, paths
+                    // starting with `//` on Unix are also magic where the first component can have
+                    // "application-specific" meaning, which is reflected e.g. by `path::absolute`
+                    // leaving leading `//` alone (but normalizing leading `///` to `/`). So we
+                    // don't have to do anything, the magic Windows path should work mostly fine as
+                    // a magic Unix path.
                 }
             }
             Cow::Owned(OsString::from_wide(&converted))
@@ -324,13 +332,15 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                     {
                         converted.remove(0);
                     }
-                    // If this start withs a `\` but not a `\\`, then for Windows this is a relative
-                    // path. But the host path is absolute as it started with `/`. We add `\\?` so
-                    // it starts with `\\?\` which is some magic path on Windows that *is*
-                    // considered absolute.
+                    // If this starts withs a `\` but not a `\\`, then for Windows this is a
+                    // relative path (relative to "the root of the current directory", e.g. the
+                    // drive letter). But the host path on Unix is absolute as it starts with `/`.
                     else if converted.get(0).copied() == Some(b'\\')
                         && converted.get(1).copied() != Some(b'\\')
                     {
+                        // We add `\\?` so it starts with `\\?\` which is some magic path on Windows
+                        // that *is* considered absolute. This way we store the absolute host path
+                        // in something that looks like an absolute path to the (Windows) target.
                         converted.splice(0..0, b"\\\\?".iter().copied());
                     }
                 }

Original file line number	Diff line number	Diff line change
`@@ -260,6 +260,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {`
`260`	`260`	`let this = self.eval_context_ref();`
`261`	`261`	`let target_os = &this.tcx.sess.target.os;`
`262`	`262`
	`263`	`+ // Below we assume that everything non-Windows works like Unix, at least`
	`264`	`+ // when it comes to file system path conventions.`
`263`	`265`	`#[cfg(windows)]`
`264`	`266`	`return if target_os == "windows" {`
`265`	`267`	`// Windows-on-Windows, all fine.`
`@@ -297,6 +299,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {`
`297`	`299`	`{`
`298`	`300`	`converted.remove(0);`
`299`	`301`	`}`
	`302`	+ // If the path starts with `\\`, it is a magic Windows path. Conveniently, paths
	`303`	+ // starting with `//` on Unix are also magic where the first component can have
	`304`	+ // "application-specific" meaning, which is reflected e.g. by `path::absolute`
	`305`	+ // leaving leading `//` alone (but normalizing leading `///` to `/`). So we
	`306`	`+ // don't have to do anything, the magic Windows path should work mostly fine as`
	`307`	`+ // a magic Unix path.`
`300`	`308`	`}`
`301`	`309`	`}`
`302`	`310`	`Cow::Owned(OsString::from_wide(&converted))`
`@@ -324,13 +332,15 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {`
`324`	`332`	`{`
`325`	`333`	`converted.remove(0);`
`326`	`334`	`}`
`327`		- // If this start withs a `\` but not a `\\`, then for Windows this is a relative
`328`		- // path. But the host path is absolute as it started with `/`. We add `\\?` so
`329`		- // it starts with `\\?\` which is some magic path on Windows that is
`330`		`- // considered absolute.`
	`335`	+ // If this starts withs a `\` but not a `\\`, then for Windows this is a
	`336`	`+ // relative path (relative to "the root of the current directory", e.g. the`
	`337`	+ // drive letter). But the host path on Unix is absolute as it starts with `/`.
`331`	`338`	`else if converted.get(0).copied() == Some(b'\\')`
`332`	`339`	`&& converted.get(1).copied() != Some(b'\\')`
`333`	`340`	`{`
	`341`	+ // We add `\\?` so it starts with `\\?\` which is some magic path on Windows
	`342`	`+ // that is considered absolute. This way we store the absolute host path`
	`343`	`+ // in something that looks like an absolute path to the (Windows) target.`
`334`	`344`	`converted.splice(0..0, b"\\\\?".iter().copied());`
`335`	`345`	`}`
`336`	`346`	`}`