Revert "Allow newline in string literal"
This reverts commit acfe5fde8c5475a9a83bd55f3f222a6dbbcbd0ae.
Reason for revert: This change is being reverted due to concerns about
introducing confusing and less helpful error messages.
Allowing newlines in regular string literals can lead to misleading
error messages when, for example, a closing quote is forgotten. This
would make debugging more difficult.
The utility of multi-line strings is also limited without additional
features, such as special handling for leading whitespace. While the
idea has merit, the change in its current state is not robust enough and
would require better error handling to be a net positive.
See discussion in
https://gn-review.git.corp.google.com/c/gn/+/19361/comments/f029b489_c14ec6de
for more details.
Original change's description:
> Allow newline in string literal
>
> It seems fine to have newline in string literal and it is convinient if
> we want to have multiline string literal in someplaces e.g. print,
> assert.
>
> Change-Id: Ied7845ad6b6f6548035bd545e5c21dc837769ee7
> Reviewed-on: https://gn-review.googlesource.com/c/gn/+/19361
> Commit-Queue: David Turner <digit@google.com>
> Reviewed-by: David Turner <digit@google.com>
> Reviewed-by: Dirk Pranke <dpranke@chromium.org>
# Not skipping CQ checks because original CL landed > 1 day ago.
Change-Id: Ic28abb3a3c005ca83de4805a1a9fc79b0b84fba9
Reviewed-on: https://gn-review.googlesource.com/c/gn/+/19540
Reviewed-by: Nico Weber <thakis@google.com>
Commit-Queue: Takuto Ikuta <tikuta@google.com>
diff --git a/docs/language.md b/docs/language.md
index 1ef533e..629125e 100644
--- a/docs/language.md
+++ b/docs/language.md
@@ -63,8 +63,7 @@
### Strings
Strings are enclosed in double-quotes and use backslash as the escape
-character. They can span multiple lines. The only escape sequences supported
-are:
+character. The only escape sequences supported are:
* `\"` (for literal quote)
* `\$` (for literal dollars sign)
diff --git a/src/gn/tokenizer.cc b/src/gn/tokenizer.cc
index 52e1c75..aeeb8b0 100644
--- a/src/gn/tokenizer.cc
+++ b/src/gn/tokenizer.cc
@@ -287,6 +287,22 @@
if (IsCurrentStringTerminator(initial)) {
Advance(); // Skip past last "
break;
+ } else if (IsCurrentNewline()) {
+ // We don't support newlines in string literals. This is a deliberate
+ // choice to avoid a class of bugs and confusing error messages that
+ // can arise from accidentally unclosed string literals. For example,
+ // a missing quote could cause the lexer to consume subsequent lines
+ // as part of the string, leading to a perplexing error message far
+ // from the actual mistake.
+ //
+ // While supporting multi-line strings could be convenient (e.g., for
+ // asserts), its utility is limited without features like leading
+ // whitespace trimming. The consensus is that the potential for
+ // hard-to-debug errors outweighs the benefits of this feature in its
+ // current simple form. For more context, see the discussion in
+ // https://gn-review.git.corp.google.com/c/gn/+/19361/comments/f029b489_c14ec6de.
+ *err_ = Err(LocationRange(location, GetCurrentLocation()),
+ "Newline in string constant.");
}
Advance();
}
diff --git a/src/gn/tokenizer_unittest.cc b/src/gn/tokenizer_unittest.cc
index 5decc68..dfec895 100644
--- a/src/gn/tokenizer_unittest.cc
+++ b/src/gn/tokenizer_unittest.cc
@@ -70,14 +70,11 @@
}
TEST(Tokenizer, String) {
- TokenExpectation strings[] = {
- {Token::STRING, "\"foo\""},
- {Token::STRING, "\"bar\\\"baz\""},
- {Token::STRING, "\"asdf\\\\\""},
- {Token::STRING, "\"new\nline\""},
- };
- EXPECT_TRUE(CheckTokenizer(
- " \"foo\" \"bar\\\"baz\" \"asdf\\\\\" \"new\nline\"", strings));
+ TokenExpectation strings[] = {{Token::STRING, "\"foo\""},
+ {Token::STRING, "\"bar\\\"baz\""},
+ {Token::STRING, "\"asdf\\\\\""}};
+ EXPECT_TRUE(
+ CheckTokenizer(" \"foo\" \"bar\\\"baz\" \"asdf\\\\\" ", strings));
}
TEST(Tokenizer, Operator) {
