From 5a101a96ada9ddbd4be54c46cd7d0125825c2283 Mon Sep 17 00:00:00 2001 From: Melody Horn Date: Tue, 6 Apr 2021 14:11:43 -0600 Subject: eagerly expand when appending to eagerly-expanded macros --- src/makefile/functions.rs | 56 +++++++++++++++++++++++++++------ src/makefile/input.rs | 56 ++++++++++++++++++++++----------- src/makefile/macro.rs | 80 ++++++++++++++++++++++++++++++++++++----------- src/makefile/mod.rs | 30 ++++++++++++------ 4 files changed, 165 insertions(+), 57 deletions(-) diff --git a/src/makefile/functions.rs b/src/makefile/functions.rs index 7bb0908..a16c268 100644 --- a/src/makefile/functions.rs +++ b/src/makefile/functions.rs @@ -6,7 +6,7 @@ use std::rc::Rc; use eyre::{bail, Result, WrapErr}; use super::pattern::r#match; -use super::r#macro::{Set as MacroSet, Source as MacroSource}; +use super::r#macro::{Macro, Set as MacroSet, Source as MacroSource}; use super::token::TokenString; pub fn expand_call( @@ -401,7 +401,15 @@ pub fn foreach( let mut macros = macros.with_overlay(); let results = words .map(|word| { - macros.set(var.clone(), MacroSource::File, TokenString::text(word)); + macros.set( + var.clone(), + Macro { + source: MacroSource::File, + text: TokenString::text(word), + #[cfg(feature = "full")] + eagerly_expanded: false, + }, + ); macros.expand(text) }) .collect::, _>>()?; @@ -417,7 +425,15 @@ pub fn call<'a>(macros: &MacroSet, args: impl Iterator) let mut macros = macros.with_overlay(); for (i, x) in args.into_iter().enumerate() { - macros.set(i.to_string(), MacroSource::File, TokenString::text(x)); + macros.set( + i.to_string(), + Macro { + source: MacroSource::File, + text: TokenString::text(x), + #[cfg(feature = "full")] + eagerly_expanded: false, + }, + ); } macros.expand(&TokenString::r#macro(function)) } @@ -579,10 +595,22 @@ mod test { let mut macros = MacroSet::new(); macros.set( "test1".to_owned(), - MacroSource::File, - TokenString::text("something"), + Macro { + source: MacroSource::File, + text: TokenString::text("something"), + #[cfg(feature = "full")] + eagerly_expanded: false, + }, + ); + macros.set( + "test2".to_owned(), + Macro { + source: MacroSource::File, + text: TokenString::text(""), + #[cfg(feature = "full")] + eagerly_expanded: false, + }, ); - macros.set("test2".to_owned(), MacroSource::File, TokenString::text("")); assert_eq!( call( @@ -668,8 +696,12 @@ mod test { let mut macros = MacroSet::new(); macros.set( "test".to_owned(), - MacroSource::File, - "worked for $(item).".parse()?, + Macro { + source: MacroSource::File, + text: "worked for $(item).".parse()?, + #[cfg(feature = "full")] + eagerly_expanded: false, + }, ); assert_eq!( call( @@ -691,8 +723,12 @@ mod test { let mut macros = MacroSet::new(); macros.set( "reverse".to_owned(), - MacroSource::File, - "$(2) $(1)".parse()?, + Macro { + source: MacroSource::File, + text: "$(2) $(1)".parse()?, + #[cfg(feature = "full")] + eagerly_expanded: false, + }, ); assert_eq!( call( diff --git a/src/makefile/input.rs b/src/makefile/input.rs index 5dfaed7..5883590 100644 --- a/src/makefile/input.rs +++ b/src/makefile/input.rs @@ -16,7 +16,7 @@ use super::command_line::CommandLine; #[cfg(feature = "full")] use super::conditional::{Line as ConditionalLine, State as ConditionalState}; use super::inference_rules::InferenceRule; -use super::r#macro::{Set as MacroSet, Source as MacroSource}; +use super::r#macro::{Macro, Set as MacroSet, Source as MacroSource}; use super::target::Target; use super::token::{tokenize, Token, TokenString}; @@ -162,21 +162,21 @@ impl<'a, 'parent> MakefileReader<'a, 'parent, BufReader> { path: impl AsRef, ) -> Result { #[cfg(feature = "full")] - if let Some((_, mut old_makefile_list)) = macros.pop("MAKEFILE_LIST") { - old_makefile_list.extend(TokenString::text(format!( + if let Some(mut old_makefile_list) = macros.pop("MAKEFILE_LIST") { + old_makefile_list.text.extend(TokenString::text(format!( " {}", path.as_ref().to_string_lossy() ))); - macros.set( - "MAKEFILE_LIST".to_owned(), - MacroSource::Builtin, - old_makefile_list, - ); + macros.set("MAKEFILE_LIST".to_owned(), old_makefile_list); } else { macros.set( "MAKEFILE_LIST".to_owned(), - MacroSource::Builtin, - TokenString::text(path.as_ref().to_string_lossy()), + Macro { + source: MacroSource::Builtin, + text: TokenString::text(path.as_ref().to_string_lossy()), + #[cfg(feature = "full")] + eagerly_expanded: false, + }, ); } let file = File::open(path); @@ -606,23 +606,41 @@ impl<'a, 'parent, R: BufRead> MakefileReader<'a, 'parent, R> { match self.macros.get(name) { // We always let command line or MAKEFLAGS macros override macros from the file. - Some((MacroSource::CommandLineOrMakeflags, _)) => return Ok(()), + Some(Macro { + source: MacroSource::CommandLineOrMakeflags, + .. + }) => return Ok(()), // We let environment variables override macros from the file only if the command-line argument to do that was given - Some((MacroSource::Environment, _)) if self.args.environment_overrides => return Ok(()), + Some(Macro { + source: MacroSource::Environment, + .. + }) if self.args.environment_overrides => return Ok(()), Some(_) if skip_if_defined => return Ok(()), _ => {} } let value = match self.macros.pop(name) { - Some((_, mut old_value)) if append => { - // TODO eagerly expand if appending to eagerly-expanded macro - old_value.extend(TokenString::text(" ")); - old_value.extend(value); + Some(mut old_value) if append => { + #[cfg(feature = "full")] + let value = if old_value.eagerly_expanded { + TokenString::text(self.expand_macros(&value).wrap_err_with(|| { + format!("while defining {} on line {}", name, line_number) + })?) + } else { + value + }; + old_value.text.extend(TokenString::text(" ")); + old_value.text.extend(value); old_value } - _ => value, + _ => Macro { + source: MacroSource::File, + text: value, + #[cfg(feature = "full")] + eagerly_expanded: expand_value, + }, }; - self.macros.set(name.into(), MacroSource::File, value); + self.macros.set(name.into(), value); Ok(()) } @@ -652,7 +670,7 @@ impl<'a, 'parent, R: BufRead> MakefileReader<'a, 'parent, R> { pub struct FinishedMakefileReader { pub inference_rules: Vec, - pub macros: HashMap, + pub macros: HashMap, pub targets: HashMap, pub first_non_special_target: Option, } diff --git a/src/makefile/macro.rs b/src/makefile/macro.rs index a6dcb74..1c8bd01 100644 --- a/src/makefile/macro.rs +++ b/src/makefile/macro.rs @@ -21,6 +21,14 @@ pub enum Source { Builtin, } +#[derive(Debug, Clone)] +pub struct Macro { + pub source: Source, + pub text: TokenString, + #[cfg(feature = "full")] + pub eagerly_expanded: bool, +} + pub trait LookupInternal: for<'a> Fn(&'a str) -> Result {} impl Fn(&'a str) -> Result> LookupInternal for F {} @@ -28,7 +36,7 @@ impl Fn(&'a str) -> Result> LookupInternal for F {} #[derive(Clone)] pub struct Set<'parent, 'lookup> { parent: Option<&'parent Set<'parent, 'lookup>>, - pub data: HashMap, + pub data: HashMap, lookup_internal: Option<&'lookup dyn LookupInternal>, #[cfg(feature = "full")] pub to_eval: Rc>>, @@ -47,15 +55,30 @@ impl<'parent, 'lookup> Set<'parent, 'lookup> { pub fn add_builtins(&mut self) { for (k, v) in builtins() { - self.data.insert(k.into(), (Source::Builtin, v)); + self.data.insert( + k.into(), + Macro { + source: Source::Builtin, + text: v, + #[cfg(feature = "full")] + eagerly_expanded: false, + }, + ); } } pub fn add_env(&mut self) { for (k, v) in env::vars() { if k != "MAKEFLAGS" && k != "SHELL" { - self.data - .insert(k, (Source::Environment, TokenString::text(v))); + self.data.insert( + k, + Macro { + source: Source::Environment, + text: TokenString::text(v), + #[cfg(feature = "full")] + eagerly_expanded: false, + }, + ); } } } @@ -73,27 +96,30 @@ impl<'parent, 'lookup> Set<'parent, 'lookup> { } } - pub fn get(&self, name: &str) -> Option<&(Source, TokenString)> { + pub fn get(&self, name: &str) -> Option<&Macro> { self.data .get(name) .or_else(|| self.parent.and_then(|parent| parent.get(name))) } - pub fn set(&mut self, name: String, source: Source, text: TokenString) { - self.data.insert(name, (source, text)); + pub fn set(&mut self, name: String, r#macro: Macro) { + self.data.insert(name, r#macro); } #[cfg(feature = "full")] pub fn is_defined(&self, name: &str) -> bool { - self.data.get(name).map_or(false, |(_, x)| !x.is_empty()) + self.data.get(name).map_or(false, |x| !x.text.is_empty()) } // `remove` is fine, but I think for "remove-and-return" `pop` is better - pub fn pop(&mut self, name: &str) -> Option<(Source, TokenString)> { - self.data.remove(name) + pub fn pop(&mut self, name: &str) -> Option { + // TODO figure out a better way to handle inheritance + self.data + .remove(name) + .or_else(|| self.parent.and_then(|p| p.get(name).cloned())) } - pub fn extend(&mut self, other: HashMap) { + pub fn extend(&mut self, other: HashMap) { self.data.extend(other); } @@ -118,7 +144,7 @@ impl<'parent, 'lookup> Set<'parent, 'lookup> { log::warn!("undefined macro {}", name); Ok(String::new()) }, - |(_, macro_value)| self.expand(macro_value), + |x| self.expand(&x.text), )? }; let macro_value = match replacement { @@ -155,11 +181,23 @@ impl<'parent, 'lookup> Set<'parent, 'lookup> { pub fn origin(&self, name: &str) -> &'static str { match self.data.get(name) { None => self.parent.map_or("undefined", |p| p.origin(name)), - Some((Source::Builtin, _)) => "default", - Some((Source::Environment, _)) => "environment", + Some(Macro { + source: Source::Builtin, + .. + }) => "default", + Some(Macro { + source: Source::Environment, + .. + }) => "environment", // TODO figure out when to return "environment override" - Some((Source::File, _)) => "file", - Some((Source::CommandLineOrMakeflags, _)) => "command line", + Some(Macro { + source: Source::File, + .. + }) => "file", + Some(Macro { + source: Source::CommandLineOrMakeflags, + .. + }) => "command line", // TODO handle override and automatic } } @@ -190,7 +228,7 @@ impl fmt::Display for Set<'_, '_> { let pieces = self .data .iter() - .map(|(k, (_, v))| format!("{}={}", k, v)) + .map(|(k, x)| format!("{}={}", k, &x.text)) .collect::>(); write!(f, "{}", pieces.join("\n")) } @@ -259,8 +297,12 @@ mod test { let mut macros = Set::new(); macros.set( "oof".to_owned(), - Source::File, - TokenString::text("bruh; swag; yeet;"), + Macro { + source: Source::File, + text: TokenString::text("bruh; swag; yeet;"), + #[cfg(feature = "full")] + eagerly_expanded: false, + }, ); assert_eq!( macros.expand(&"$(oof:;=?)".parse().unwrap())?, diff --git a/src/makefile/mod.rs b/src/makefile/mod.rs index 308d25d..aab5054 100644 --- a/src/makefile/mod.rs +++ b/src/makefile/mod.rs @@ -25,7 +25,7 @@ use command_line::CommandLine; use inference_rules::InferenceRule; use input::FinishedMakefileReader; pub use input::MakefileReader; -use r#macro::{Set as MacroSet, Source as MacroSource}; +use r#macro::{Macro, Set as MacroSet, Source as MacroSource}; use target::Target; use token::TokenString; @@ -62,8 +62,12 @@ impl<'a> Makefile<'a> { if let [name, value] = *r#macro.splitn(2, '=').collect::>() { macros.set( name.into(), - MacroSource::CommandLineOrMakeflags, - TokenString::text(value), + Macro { + source: MacroSource::CommandLineOrMakeflags, + text: TokenString::text(value), + #[cfg(feature = "full")] + eagerly_expanded: false, + }, ); } } @@ -73,15 +77,23 @@ impl<'a> Makefile<'a> { let make_cmd_goals = args.targets().collect::>(); macros.set( "MAKECMDGOALS".to_owned(), - MacroSource::Builtin, - TokenString::text(make_cmd_goals.join(" ")), + Macro { + source: MacroSource::Builtin, + text: TokenString::text(make_cmd_goals.join(" ")), + #[cfg(feature = "full")] + eagerly_expanded: false, + }, ); if let Ok(curdir) = env::current_dir() { macros.set( "CURDIR".to_owned(), - MacroSource::Builtin, - TokenString::text(curdir.to_string_lossy()), + Macro { + source: MacroSource::Builtin, + text: TokenString::text(curdir.to_string_lossy()), + #[cfg(feature = "full")] + eagerly_expanded: false, + }, ); } } @@ -135,8 +147,8 @@ impl<'a> Makefile<'a> { let follow_gnu = cfg!(feature = "full"); let vpath_options = match self.macros.get("VPATH") { - Some((_, vpath)) if follow_gnu => { - let vpath = self.expand_macros(vpath, None)?; + Some(Macro { text, .. }) if follow_gnu => { + let vpath = self.expand_macros(text, None)?; env::split_paths(&vpath).collect() } _ => vec![], -- cgit v1.2.3