4 files changed, 24 insertions, 28 deletions

diff --git a/src/makefile/input.rs b/src/makefile/input.rs
index 40720c2..6333a66 100644
--- a/src/makefile/input.rs
+++ b/src/makefile/input.rs
@@ -212,7 +212,8 @@ impl<'a, 'parent> MakefileReader<'a, 'parent, BufReader<File>> {
     ) -> Result<Self> {
         let mut macros = MacroSet::new();
         #[cfg(feature = "full")]
-        if let Some(mut old_makefile_list) = macros.pop("MAKEFILE_LIST") {
+        if let Some(old_makefile_list) = stack.get("MAKEFILE_LIST") {
+            let mut old_makefile_list = old_makefile_list.into_owned();
             old_makefile_list.text.extend(TokenString::text(format!(
                 " {}",
                 path.as_ref().to_string_lossy()
@@ -454,7 +455,7 @@ impl<'a, 'parent, R: BufRead> MakefileReader<'a, 'parent, R> {
                     let action = line
                         .action(
                             self.conditional_stack.last(),
-                            |name| self.macros.is_defined(name),
+                            |name| self.stack.with_scope(&self.macros).is_defined(name),
                             |t| self.expand_macros_deferred_eval(t, &mut deferred_eval_context),
                         )
                         .wrap_err_with(|| {
@@ -820,17 +821,16 @@ impl<'a, 'parent, R: BufRead> MakefileReader<'a, 'parent, R> {
             value
         };
 
-        let skipped = match self.macros.get(name) {
+        let skipped = match self
+            .stack
+            .with_scope(&self.macros)
+            .get(name)
+            .map(|x| x.source.clone())
+        {
             // We always let command line or MAKEFLAGS macros override macros from the file.
-            Some(Macro {
-                source: ItemSource::CommandLineOrMakeflags,
-                ..
-            }) => true,
+            Some(ItemSource::CommandLineOrMakeflags) => true,
             // We let environment variables override macros from the file only if the command-line argument to do that was given
-            Some(Macro {
-                source: ItemSource::Environment,
-                ..
-            }) => self.args.environment_overrides,
+            Some(ItemSource::Environment) => self.args.environment_overrides,
             Some(_) => skip_if_defined,
             None => false,
         };
@@ -846,8 +846,9 @@ impl<'a, 'parent, R: BufRead> MakefileReader<'a, 'parent, R> {
             &value
         );
 
-        let value = match self.macros.pop(name) {
-            Some(mut old_value) if append => {
+        let value = match self.stack.with_scope(&self.macros).get(name) {
+            Some(old_value) if append => {
+                let mut old_value = old_value.into_owned();
                 #[cfg(feature = "full")]
                 let value = if old_value.eagerly_expanded {
                     TokenString::text(self.expand_macros(&value).wrap_err_with(|| {
diff --git a/src/makefile/macro.rs b/src/makefile/macro.rs
index 301173a..b2a1510 100644
--- a/src/makefile/macro.rs
+++ b/src/makefile/macro.rs
@@ -119,7 +119,8 @@ impl Set {
         }
     }
 
-    pub fn get(&self, name: &str) -> Option<&Macro> {
+    /// To properly process inherited macros, use [MacroScopeStack::get].
+    pub fn get_non_recursive(&self, name: &str) -> Option<&Macro> {
         self.data.get(name)
     }
 
@@ -127,17 +128,6 @@ impl Set {
         self.data.insert(name, r#macro);
     }
 
-    #[cfg(feature = "full")]
-    pub fn is_defined(&self, name: &str) -> bool {
-        self.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<Macro> {
-        // TODO figure out a better way to handle inheritance
-        self.data.remove(name)
-    }
-
     pub fn extend(
         &mut self,
         other: HashMap<String, Macro>,
diff --git a/src/makefile/macro_scope.rs b/src/makefile/macro_scope.rs
index bee603e..ba9b6cf 100644
--- a/src/makefile/macro_scope.rs
+++ b/src/makefile/macro_scope.rs
@@ -26,7 +26,7 @@ pub trait MacroScope {
 
 impl MacroScope for MacroSet {
     fn get(&self, name: &str) -> Option<Cow<Macro>> {
-        self.get(name).map(Cow::Borrowed)
+        self.get_non_recursive(name).map(Cow::Borrowed)
     }
 }
 
@@ -82,7 +82,7 @@ impl<'a> MacroScopeStack<'a> {
         }
     }
 
-    fn get(&self, name: &str) -> Option<Cow<Macro>> {
+    pub fn get(&self, name: &str) -> Option<Cow<Macro>> {
         for scope in &self.scopes {
             if let Some(r#macro) = scope.get(name) {
                 return Some(r#macro);
@@ -91,6 +91,11 @@ impl<'a> MacroScopeStack<'a> {
         None
     }
 
+    #[cfg(feature = "full")]
+    pub fn is_defined(&self, name: &str) -> bool {
+        self.get(name).map_or(false, |x| !x.text.is_empty())
+    }
+
     pub fn expand<#[cfg(feature = "full")] R: BufRead>(
         &self,
         text: &TokenString,
diff --git a/src/makefile/mod.rs b/src/makefile/mod.rs
index ddfac6f..9b2f626 100644
--- a/src/makefile/mod.rs
+++ b/src/makefile/mod.rs
@@ -194,7 +194,7 @@ impl<'a> Makefile<'a> {
 
         let follow_gnu = cfg!(feature = "full");
 
-        let vpath_options = match self.macros.get("VPATH") {
+        let vpath_options = match self.macros.get_non_recursive("VPATH") {
             Some(Macro { text, .. }) if follow_gnu => {
                 let vpath = self.expand_macros(text, None)?;
                 env::split_paths(&vpath).collect()