From 1562001d8cf0b5a68198ebf3b7db115f288cbc1a Mon Sep 17 00:00:00 2001 From: Alexei Barnes Date: Wed, 16 Aug 2023 09:38:09 +0100 Subject: [PATCH] Modify response file module to work with gcc/clang * Remove anything specific about MSVC from the response file code (which is just the name and a comment) * Add unit tests to the module * Enable use of the response file code in gcc/clang (via the gcc module, clang defaults to this behaviour) --- src/compiler/gcc.rs | 11 +++---- src/compiler/msvc.rs | 4 +-- src/compiler/response_file.rs | 54 +++++++++++++++++++++++++++++++---- 3 files changed, 56 insertions(+), 13 deletions(-) diff --git a/src/compiler/gcc.rs b/src/compiler/gcc.rs index 99a112646..eb1f9be05 100644 --- a/src/compiler/gcc.rs +++ b/src/compiler/gcc.rs @@ -16,6 +16,7 @@ use crate::compiler::args::*; use crate::compiler::c::{ ArtifactDescriptor, CCompilerImpl, CCompilerKind, Language, ParsedArguments, }; +use crate::compiler::response_file::SplitResponseFileArgs; use crate::compiler::{clang, Cacheable, ColorMode, CompileCommand, CompilerArguments}; use crate::mock_command::{CommandCreatorSync, RunCommand}; use crate::util::{run_input_output, OsStrExt}; @@ -887,11 +888,11 @@ impl<'a> Iterator for ExpandIncludeFile<'a> { debug!("failed to read @-file `{}`: {}", file.display(), e); return Some(arg); } - if contents.contains('"') || contents.contains('\'') { - return Some(arg); - } - let new_args = contents.split_whitespace().collect::>(); - self.stack.extend(new_args.iter().rev().map(|s| s.into())); + // Parse the response file contents, taking into account quote-wrapped strings and new-line separators. + let resp_file_args = SplitResponseFileArgs::from(&contents).collect::>(); + // Pump arguments back to the stack, in reverse order so we can `Vec::pop` and visit in original front-to-back order. + let rev_args = resp_file_args.iter().rev().map(|s| s.into()); + self.stack.extend(rev_args); } } } diff --git a/src/compiler/msvc.rs b/src/compiler/msvc.rs index a7e3b951a..e1c39c94b 100644 --- a/src/compiler/msvc.rs +++ b/src/compiler/msvc.rs @@ -16,7 +16,7 @@ use crate::compiler::args::*; use crate::compiler::c::{ ArtifactDescriptor, CCompilerImpl, CCompilerKind, Language, ParsedArguments, }; -use crate::compiler::response_file::SplitMsvcResponseFileArgs; +use crate::compiler::response_file::SplitResponseFileArgs; use crate::compiler::{ clang, gcc, write_temp_file, Cacheable, ColorMode, CompileCommand, CompilerArguments, }; @@ -1222,7 +1222,7 @@ impl<'a> Iterator for ExpandIncludeFile<'a> { trace!("Expanded response file {:?} to {:?}", file_path, content); // Parse the response file contents, taking into account quote-wrapped strings and new-line separators. - let resp_file_args = SplitMsvcResponseFileArgs::from(&content).collect::>(); + let resp_file_args = SplitResponseFileArgs::from(&content).collect::>(); // Pump arguments back to the stack, in reverse order so we can `Vec::pop` and visit in original front-to-back order. let rev_args = resp_file_args.iter().rev().map(|s| s.into()); self.stack.extend(rev_args); diff --git a/src/compiler/response_file.rs b/src/compiler/response_file.rs index 57eca4c4f..5891aa5ba 100644 --- a/src/compiler/response_file.rs +++ b/src/compiler/response_file.rs @@ -1,5 +1,4 @@ - -/// An iterator over the arguments in a Windows command line. +/// An iterator over the arguments in a response file. /// /// This produces results identical to `CommandLineToArgvW` except in the /// following cases: @@ -24,13 +23,13 @@ /// - https://msdn.microsoft.com/en-us/library/windows/desktop/bb776391(v=vs.85).aspx /// - https://msdn.microsoft.com/en-us/library/windows/desktop/17w5ykft(v=vs.85).aspx #[derive(Clone, Debug)] -pub struct SplitMsvcResponseFileArgs<'a> { +pub struct SplitResponseFileArgs<'a> { /// String slice of the file content that is being parsed. /// Slice is mutated as this iterator is executed. file_content: &'a str, } -impl<'a, T> From<&'a T> for SplitMsvcResponseFileArgs<'a> +impl<'a, T> From<&'a T> for SplitResponseFileArgs<'a> where T: AsRef + 'static, { @@ -41,7 +40,7 @@ where } } -impl<'a> SplitMsvcResponseFileArgs<'a> { +impl<'a> SplitResponseFileArgs<'a> { /// Appends backslashes to `target` by decrementing `count`. /// If `step` is >1, then `count` is decremented by `step`, resulting in 1 backslash appended for every `step`. fn append_backslashes_to(target: &mut String, count: &mut usize, step: usize) { @@ -52,7 +51,7 @@ impl<'a> SplitMsvcResponseFileArgs<'a> { } } -impl<'a> Iterator for SplitMsvcResponseFileArgs<'a> { +impl<'a> Iterator for SplitResponseFileArgs<'a> { type Item = String; fn next(&mut self) -> Option { @@ -121,3 +120,46 @@ impl<'a> Iterator for SplitMsvcResponseFileArgs<'a> { Some(arg) } } + +#[cfg(test)] +mod test { + use super::*; + + #[test] + fn parse_simple_args() { + let content = "-A1 -A2 -A3 -I ../includes"; + let args = SplitResponseFileArgs::from(&content).collect::>(); + assert_eq!(args[0], "-A1"); + assert_eq!(args[1], "-A2"); + assert_eq!(args[2], "-A3"); + assert_eq!(args[3], "-I"); + assert_eq!(args[4], "../includes"); + } + + #[test] + fn parse_quoted_path_arg() { + let content = "-I \"../included headers\""; + let args = SplitResponseFileArgs::from(&content).collect::>(); + assert_eq!(args[0], "-I"); + assert_eq!(args[1], "../included headers"); + } + + #[test] + fn parse_escaped_quoted_path_arg() { + let content = "-I \"../included \\\"headers\\\"\""; + let args = SplitResponseFileArgs::from(&content).collect::>(); + assert_eq!(args[0], "-I"); + assert_eq!(args[1], "../included \"headers\""); + } + + #[test] + fn parse_various_whitespace_characters() { + let content = "-A1 -A2\n-A3\n\r-A4\r-A5\n "; + let args = SplitResponseFileArgs::from(&content).collect::>(); + assert_eq!(args[0], "-A1"); + assert_eq!(args[1], "-A2"); + assert_eq!(args[2], "-A3"); + assert_eq!(args[3], "-A4"); + assert_eq!(args[4], "-A5"); + } +}