Fix nested filename variables.

T-Pau · May 3, 2024 · 4f1d577 · 4f1d577
1 parent a8c4367
commit 4f1d577
Show file tree

Hide file tree

Showing 6 changed files with 40 additions and 39 deletions.
diff --git a/src/FilenameList.cc b/src/FilenameList.cc
@@ -44,9 +44,6 @@ FilenameList::FilenameList(Tokenizer& tokenizer, Type type) {
         auto word = FilenameWord{tokenizer, force_build};
         if (!word.empty()) {
             words.emplace_back(word);
-            if (!word.is_resolved()) {
-                resolved = false;
-            }
         }
 
         if (scoped) {
@@ -55,6 +52,9 @@ FilenameList::FilenameList(Tokenizer& tokenizer, Type type) {
                 if (token.type == Tokenizer::TokenType::END_SCOPE) {
                     return;
                 }
+                else if (word.empty()) {
+                    throw Exception("unterminated scope");
+                }
                 else {
                     tokenizer.unget(token);
                 }

diff --git a/src/FilenameList.h b/src/FilenameList.h
@@ -63,7 +63,7 @@ class FilenameList {
     std::vector<FilenameWord> words;
     std::vector<Filename> filenames;
     bool force_build{false};
-    bool resolved{true};
+    bool resolved{false};
 };
 
 std::ostream& operator<<(std::ostream& stream, const FilenameList& filename_list);

diff --git a/src/FilenameWord.cc b/src/FilenameWord.cc
@@ -106,22 +106,24 @@ void FilenameWord::resolve(const ResolveContext& context) {
         if (std::holds_alternative<VariableReference>(element)) {
             auto& variable_reference = std::get<VariableReference>(element);
             if (auto variable = variable_reference.resolve(context)) {
-                if (variable->is_filename()) {
-                    contains_filename_variable = true;
-                }
-                if (!variable->is_resolved()) {
-                    context.result.add_unresolved_variable_use(variable->name);
-                    resolved = false;
-                }
                 element = variable;
             }
             else {
                 throw Exception("unknown variable %s", variable_reference.name.c_str());
             }
         }
+        if (std::holds_alternative<const Variable*>(element)) {
+            auto variable = std::get<const Variable*>(element);
+            if (variable->is_filename()) {
+                contains_filename_variable = true;
+            }
+            if (!variable->is_resolved()) {
+                context.result.add_unresolved_variable_use(variable->name);
+                resolved = false;
+            }
+        }
     }
 
-    // TODO: this shouldn't be neccessary
     if (resolved && !contains_filename_variable) {
         auto name = std::string();
         for (const auto& element: elements) {
@@ -186,7 +188,7 @@ void FilenameWord::collect_filenames(std::vector<Filename>& filenames) const {
             }
         }
         else {
-            filenames.emplace_back(prefix);
+            throw Exception("internal error: file name never resolved");
         }
     }
 }
diff --git a/src/Tokenizer.cc b/src/Tokenizer.cc
@@ -187,6 +187,17 @@ Tokenizer::Token Tokenizer::get_next() {
     }
 
     while (true) {
+        if (beggining_of_line) {
+            beggining_of_line = false;
+            auto new_indent = count_space();
+            // TODO: doesn't return space token for indent, is that okay?
+            if (new_indent != indent) {
+                const auto type = new_indent > indent ? TokenType::BEGIN_SCOPE : TokenType::END_SCOPE;
+                indent = new_indent;
+                return Token{ type };
+            }
+        }
+
         switch (auto c = source.get()) {
             case '#':
                 while ((c = source.get()) != '\n' && c != EOF) {
@@ -216,10 +227,7 @@ Tokenizer::Token Tokenizer::get_next() {
                 return Token{ TokenType::NEWLINE };
 
             case ' ':
-                if (auto token = tokenize_space()) {
-                    return *token;
-                }
-                break;
+                return tokenize_space();
 
             case ':':
                 beggining_of_line = false;
@@ -368,30 +376,22 @@ Tokenizer::Token Tokenizer::tokenize_dollar() {
     }
 }
 
-std::optional<Tokenizer::Token> Tokenizer::tokenize_space() {
-    auto length = 1;
+int Tokenizer::count_space() {
+    auto length = 0;
 
     while (source.get() == ' ') {
         length += 1;
     }
     source.unget();
 
-    if (beggining_of_line) {
-        if (length == indent) {
-            return {};
-        }
-        else {
-            const auto type = length > indent ? TokenType::BEGIN_SCOPE : TokenType::END_SCOPE;
-            beggining_of_line = false;
-            indent = length;
-            return Token{ type };
-        }
-    }
-    else {
-        return Token{ TokenType::SPACE, std::string(length, ' ') };
-    }
+    return length;
+}
+
+Tokenizer::Token Tokenizer::tokenize_space() {
+    return Token{ TokenType::SPACE, std::string(count_space() + 1, ' ') };
 }
 
+
 Tokenizer::Token Tokenizer::tokenize_word(int first_character) {
     std::string value{ static_cast<char>(first_character) };
 

diff --git a/src/Tokenizer.h b/src/Tokenizer.h
@@ -115,9 +115,10 @@ class Tokenizer {
     static bool is_word(int c) {const auto ctype = type(c); return ctype == CharacterType::BRACED_VARIABLE || ctype == CharacterType::SIMPLE_VARIABLE || ctype == CharacterType::OTHER;}
 
     [[nodiscard]] Token get_next();
+    [[nodiscard]] int count_space();
     [[nodiscard]] Token tokenize_braced_variable();
     [[nodiscard]] Token tokenize_dollar();
-    [[nodiscard]] std::optional<Token> tokenize_space();
+    [[nodiscard]] Token tokenize_space();
     [[nodiscard]] Token tokenize_variable(int first_character);
     [[nodiscard]] Token tokenize_word(int first_character);
 

diff --git a/tests/nested-file-variable.test-not → tests/nested-file-variable.test b/tests/nested-file-variable.test-not → tests/nested-file-variable.test
@@ -23,11 +23,11 @@ file build/build.ninja {} <>
 # Do not edit.
 
 build_directory = .
+common_sources = ../input-2 ../input-3
 source_directory = ..
-sources = ../input ../input-2 output
+sources = ../input-1 ../input-2 ../input-3 ../input-4
 top_build_directory = .
 top_source_directory = ..
-version = 2
 
 rule a
     command = a $in $out
@@ -37,9 +37,7 @@ rule fast-ninja
     command = fast-ninja ..
     generator = 1
 
-build output : a ../input
-
-build final : a ../input ../input-2 output
+build output : a ../input-1 ../input-2 ../input-3 ../input-4
 
 build build.ninja : fast-ninja ../build.fninja
 end-of-inline-data