FrontendTool: fix escaping of filenames in Makefile

The dependency file that is being generated should not escape : and # in
the filename.  This makes the behaviour of the filename escaping similar
to clang and GCC and fixes incorrect quoting of paths on Windows.

Author: compnerd

include/swift/FrontendTool/FrontendTool.h

@@ -44,6 +44,12 @@ class FrontendObserver {
   virtual void configuredCompiler(CompilerInstance &instance);
 };
 
+namespace frontend {
+namespace utils {
+StringRef escapeForMake(StringRef raw, llvm::SmallVectorImpl<char> &buffer);
+}
+}
+
 /// Perform all the operations of the frontend, exactly as if invoked
 /// with -frontend.
 ///

lib/FrontendTool/FrontendTool.cpp

@@ -99,6 +99,40 @@ static std::string displayName(StringRef MainExecutablePath) {
   return Name;
 }
 
+StringRef
+swift::frontend::utils::escapeForMake(StringRef raw,
+                                      llvm::SmallVectorImpl<char> &buffer) {
+  buffer.clear();
+
+  // The escaping rules for GNU make are complicated due to the various
+  // subsitutions and use of the tab in the leading position for recipes.
+  // Various symbols have significance in different contexts.  It is not
+  // possible to correctly quote all characters in Make (as of 3.7).  Match
+  // gcc and clang's behaviour for the escaping which covers only a subset of
+  // characters.
+  for (unsigned I = 0, E = raw.size(); I != E; ++I) {
+    switch (raw[I]) {
+    case '#': // Handle '#' the broken GCC way
+      buffer.push_back('\\');
+      break;
+
+    case ' ':
+      for (unsigned J = I; J && raw[J - 1] == '\\'; --J)
+        buffer.push_back('\\');
+      buffer.push_back('\\');
+      break;
+
+    case '$': // $ is escaped by $
+      buffer.push_back('$');
+      break;
+    }
+    buffer.push_back(raw[I]);
+  }
+  buffer.push_back('\0');
+
+  return buffer.data();
+}
+
 /// Emits a Make-style dependencies file.
 static bool emitMakeDependenciesIfNeeded(DiagnosticEngine &diags,
                                          DependencyTracker *depTracker,
@@ -118,39 +152,21 @@ static bool emitMakeDependenciesIfNeeded(DiagnosticEngine &diags,
     return true;
   }
 
-  // Declare a helper for escaping file names for use in Makefiles.
-  llvm::SmallString<256> pathBuf;
-  auto escape = [&](StringRef raw) -> StringRef {
-    pathBuf.clear();
-
-    static const char badChars[] = " $#:\n";
-    size_t prev = 0;
-    for (auto index = raw.find_first_of(badChars); index != StringRef::npos;
-         index = raw.find_first_of(badChars, index+1)) {
-      pathBuf.append(raw.slice(prev, index));
-      if (raw[index] == '$')
-        pathBuf.push_back('$');
-      else
-        pathBuf.push_back('\\');
-      prev = index;
-    }
-    pathBuf.append(raw.substr(prev));
-    return pathBuf;
-  };
+  llvm::SmallString<256> buffer;
 
   // FIXME: Xcode can't currently handle multiple targets in a single
   // dependency line.
   opts.forAllOutputPaths(input, [&](const StringRef targetName) {
-    out << escape(targetName) << " :";
+    out << swift::frontend::utils::escapeForMake(targetName, buffer) << " :";
     // First include all other files in the module. Make-style dependencies
     // need to be conservative!
     for (auto const &path :
          reversePathSortedFilenames(opts.InputsAndOutputs.getInputFilenames()))
-      out << ' ' << escape(path);
+      out << ' ' << swift::frontend::utils::escapeForMake(path, buffer);
     // Then print dependencies we've picked up during compilation.
     for (auto const &path :
            reversePathSortedFilenames(depTracker->getDependencies()))
-      out << ' ' << escape(path);
+      out << ' ' << swift::frontend::utils::escapeForMake(path, buffer);
     out << '\n';
   });
 

unittests/CMakeLists.txt

@@ -7,6 +7,7 @@ if(SWIFT_INCLUDE_TOOLS)
   add_subdirectory(AST)
   add_subdirectory(Basic)
   add_subdirectory(Driver)
+  add_subdirectory(FrontendTool)
   add_subdirectory(IDE)
   add_subdirectory(Parse)
   add_subdirectory(SwiftDemangle)

unittests/FrontendTool/CMakeLists.txt

@@ -0,0 +1,3 @@
+add_swift_unittest(SwiftFrontendTests
+  FrontendToolTests.cpp)
+target_link_libraries(SwiftFrontendTests PRIVATE swiftFrontendTool)

unittests/FrontendTool/FrontendToolTests.cpp

@@ -0,0 +1,34 @@
+//===--- FrontendTests.cpp ------------------------------------------------===//
+//
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2014 - 2017 Apple Inc. and the Swift project authors
+// Licensed under Apache License v2.0 with Runtime Library Exception
+//
+// See https://swift.org/LICENSE.txt for license information
+// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
+//
+//===----------------------------------------------------------------------===//
+
+#include "gtest/gtest.h"
+#include "swift/FrontendTool/FrontendTool.h"
+#include "llvm/ADT/SmallString.h"
+
+using namespace swift;
+
+namespace {
+
+TEST(FrontendTests, MakefileQuotingTest) {
+  using namespace frontend::utils;
+
+  llvm::SmallString<256> buffer;
+  EXPECT_EQ(escapeForMake("S:\\b\\llvm", buffer), "S:\\b\\llvm");
+  EXPECT_EQ(escapeForMake("S:\\b\\swift\\$sBoWV", buffer),
+            "S:\\b\\swift\\$$sBoWV");
+  EXPECT_EQ(escapeForMake("S:\\b\\swift\\#hashtag", buffer),
+            "S:\\b\\swift\\\\#hashtag");
+
+}
+
+} // end anonymous namespace
+