Fix quoting preprocessor defines for compilation database
If a preprocessor define has a space character, which usually happens
with double-quoted text, current compilation database writer
cannot handle correctly. These spaces should be escaped.
Tested that gn is generating correctly compile_commands.json on WebRTC
with Linux.
Bug: gn:109
Change-Id: I4ba2ed4c807e4cb9e078720df3b64834a4f9ec8e
Reviewed-on: https://gn-review.googlesource.com/c/gn/+/6780
Reviewed-by: Brett Wilson <brettw@chromium.org>
Commit-Queue: Brett Wilson <brettw@chromium.org>
diff --git a/AUTHORS b/AUTHORS
index b78d6a5..0640d6c 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -9,6 +9,7 @@
# See python fnmatch module documentation for more information.
Google Inc. <*@google.com>
+HyperConnect Inc. <*@hpcnt.com>
IBM Inc. <*@*.ibm.com>
Loongson Technology Corporation Limited. <*@loongson.cn>
MIPS Technologies, Inc. <*@mips.com>
diff --git a/src/gn/compile_commands_writer.cc b/src/gn/compile_commands_writer.cc
index 10984cc..9ff67ca 100644
--- a/src/gn/compile_commands_writer.cc
+++ b/src/gn/compile_commands_writer.cc
@@ -57,9 +57,9 @@
target->config_values().has_precompiled_headers();
std::ostringstream defines_out;
- RecursiveTargetConfigToStream<std::string>(
- target, &ConfigValues::defines,
- DefineWriter(ESCAPE_NINJA_PREFORMATTED_COMMAND, true), defines_out);
+ RecursiveTargetConfigToStream<std::string>(target, &ConfigValues::defines,
+ DefineWriter(ESCAPE_SPACE, true),
+ defines_out);
base::EscapeJSONString(defines_out.str(), false, &flags.defines);
std::ostringstream includes_out;
diff --git a/src/gn/compile_commands_writer_unittest.cc b/src/gn/compile_commands_writer_unittest.cc
index 96ef9eb..99dc55d 100644
--- a/src/gn/compile_commands_writer_unittest.cc
+++ b/src/gn/compile_commands_writer_unittest.cc
@@ -239,7 +239,7 @@
target.sources().push_back(SourceFile("//foo/input.cc"));
target.config_values().defines().push_back("BOOL_DEF");
target.config_values().defines().push_back("INT_DEF=123");
- target.config_values().defines().push_back("STR_DEF=\"ABCD-1\"");
+ target.config_values().defines().push_back("STR_DEF=\"ABCD 1\"");
ASSERT_TRUE(target.OnResolved(&err));
targets.push_back(&target);
@@ -248,7 +248,7 @@
writer.RenderJSON(build_settings(), targets, &out);
const char expected[] =
- "-DBOOL_DEF -DINT_DEF=123 -DSTR_DEF=\\\\\\\"ABCD-1\\\\\\\"";
+ "-DBOOL_DEF -DINT_DEF=123 -DSTR_DEF=\\\\\\\"ABCD\\\\ 1\\\\\\\"";
EXPECT_TRUE(out.find(expected) != std::string::npos);
}
diff --git a/src/gn/escape.cc b/src/gn/escape.cc
index 9f567fb..98f77d3 100644
--- a/src/gn/escape.cc
+++ b/src/gn/escape.cc
@@ -41,6 +41,19 @@
1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 0, 0, 0, 0};
// clang-format on
+size_t EscapeStringToString_Space(const std::string_view& str,
+ const EscapeOptions& options,
+ char* dest,
+ bool* needed_quoting) {
+ size_t i = 0;
+ for (const auto& elem : str) {
+ if (elem == ' ')
+ dest[i++] = '\\';
+ dest[i++] = elem;
+ }
+ return i;
+}
+
// Uses the stack if the space needed is small and the heap otherwise.
class StackOrHeapBuffer {
public:
@@ -204,6 +217,8 @@
case ESCAPE_NONE:
strncpy(dest, str.data(), str.size());
return str.size();
+ case ESCAPE_SPACE:
+ return EscapeStringToString_Space(str, options, dest, needed_quoting);
case ESCAPE_NINJA:
return EscapeStringToString_Ninja(str, options, dest, needed_quoting);
case ESCAPE_DEPFILE:
diff --git a/src/gn/escape.h b/src/gn/escape.h
index 36a9c5c..28f31bf 100644
--- a/src/gn/escape.h
+++ b/src/gn/escape.h
@@ -12,6 +12,9 @@
// No escaping.
ESCAPE_NONE,
+ // Space only.
+ ESCAPE_SPACE,
+
// Ninja string escaping.
ESCAPE_NINJA,
diff --git a/src/gn/escape_unittest.cc b/src/gn/escape_unittest.cc
index 48f6941..ca42ac9 100644
--- a/src/gn/escape_unittest.cc
+++ b/src/gn/escape_unittest.cc
@@ -69,3 +69,12 @@
// Only $ is escaped.
EXPECT_EQ("a: \"$$\\b<;", EscapeString("a: \"$\\b<;", opts, nullptr));
}
+
+TEST(Escape, Space) {
+ EscapeOptions opts;
+ opts.mode = ESCAPE_SPACE;
+
+ // ' ' is escaped.
+ EXPECT_EQ("-VERSION=\"libsrtp2\\ 2.1.0-pre\"",
+ EscapeString("-VERSION=\"libsrtp2 2.1.0-pre\"", opts, nullptr));
+}
