mirror of
https://github.com/carbon-language/carbon-lang.git
synced 2026-10-05 22:02:55 +01:00
Trailing comments (#7441)
Carbon currently requires a comment to be the only non-whitespace on its line. A `//` comment that follows other content on a line, called a _trailing comment_, is a lexer error. This proposal removes that restriction, allowing a comment to follow other content on a line. Everything else about comments is unchanged: a comment still begins with `//`, still requires whitespace after the `//`, and still runs to the end of the line. Carbon continues to provide only line comments; no block or intra-line comments are added. Three observations motivate the change. First, trailing comments are well suited to short _annotations_ attached to a specific entity or value on a line. Second, the lexer design now makes it trivial to lex trailing comments, and in fact requires extra logic and potentially cost to reject them. Third, C++ code routinely uses trailing comments, so allowing them lets Carbon carry the layout of migrated code over directly, rather than reworking each comment to read well in a different structure. Implementation notes (beyond the proposal's design): Keeping trailing comments cheap to lex required a few supporting changes, all of which keep the cost off the lexer's hot path: - The lexer already dispatches `//` to comment lexing wherever it appears, so classifying a comment as trailing is a single O(1) check of whether the `//` is the line's first non-whitespace (`start + indent`). The hot comment path is otherwise unchanged. - That check relies on each line's recorded indentation being its real leading whitespace. Multi-line string literals previously recorded the column where the literal opened for the lines they span; they now record the true (closing-delimiter) indentation instead. - Parser error recovery (`SkipPastLikelyEnd`) had relied on that opening-column indentation to keep tokens following a multi-line string literal attached to the same construct. It now reconstructs that relationship directly by consulting the line on which the literal opened, including when other tokens follow the closing delimiter (such as `''' + "more"`). This is on the cold recovery path. - `CommentData` records the trailing bit in the high bit of its length field, keeping it at 8 bytes. Assisted-by: Claude Code
This commit is contained in:
@@ -30,6 +30,10 @@ struct StringLiteral::Introducer {
|
||||
// The length of the introducer, including the file type indicator and
|
||||
// newline for a multi-line string literal.
|
||||
int prefix_size;
|
||||
// Whether the introducer is valid. Only a `'''` introducer with a malformed
|
||||
// introducer line is invalid; `prefix_size` then covers that line without
|
||||
// its newline.
|
||||
bool is_valid = true;
|
||||
|
||||
// Lex the introducer for a string literal, after any '#'s.
|
||||
static auto Lex(llvm::StringRef source_text) -> std::optional<Introducer>;
|
||||
@@ -52,15 +56,56 @@ auto StringLiteral::Introducer::Lex(llvm::StringRef source_text)
|
||||
}
|
||||
|
||||
if (kind != Kind::SingleLine) {
|
||||
// The rest of the line must be a valid file type indicator: a sequence of
|
||||
// characters containing neither '#' nor '"' followed by a newline.
|
||||
auto prefix_end = source_text.find_first_of("#\n\"", indicator.size());
|
||||
if (prefix_end != llvm::StringRef::npos &&
|
||||
source_text[prefix_end] == '\n') {
|
||||
// Include the newline in the prefix size.
|
||||
return Introducer{.kind = kind,
|
||||
.terminator = indicator,
|
||||
.prefix_size = static_cast<int>(prefix_end + 1)};
|
||||
// The rest of the opening line is an optional file type indicator, which
|
||||
// may be followed by a trailing comment. The line must be terminated by a
|
||||
// newline; the string literal's content begins on the following line.
|
||||
size_t line_end = source_text.find('\n', indicator.size());
|
||||
if (line_end != llvm::StringRef::npos) {
|
||||
llvm::StringRef rest = source_text.slice(indicator.size(), line_end);
|
||||
// Strip a trailing comment, if present. A `//` followed by whitespace or
|
||||
// the end of the line begins one; it is treated like trailing whitespace
|
||||
// and is not part of the file type indicator. Because it is removed
|
||||
// here, it may contain `'`, `#`, or `"`, which the indicator itself may
|
||||
// not.
|
||||
// TODO: Surface this comment through the lexer's comment records rather
|
||||
// than only carrying it within the string literal token's spelling.
|
||||
for (size_t slashes = rest.find("//"); slashes != llvm::StringRef::npos;
|
||||
slashes = rest.find("//", slashes + 1)) {
|
||||
llvm::StringRef after_slashes = rest.drop_front(slashes + 2);
|
||||
if (after_slashes.empty() || after_slashes.starts_with(' ') ||
|
||||
after_slashes.starts_with('\t')) {
|
||||
rest = rest.take_front(slashes);
|
||||
break;
|
||||
}
|
||||
}
|
||||
// The file type indicator is the remaining text with surrounding
|
||||
// whitespace trimmed. It must not contain `'`, `#`, or `"`, which would
|
||||
// be ambiguous with the closing delimiter and the hash and double-quoted
|
||||
// string introducers.
|
||||
// TODO: Diagnose a `//` within the indicator: `//` not followed by
|
||||
// whitespace is reserved here as everywhere, rather than being valid
|
||||
// indicator text.
|
||||
llvm::StringRef file_type = rest.trim(" \t");
|
||||
if (file_type.find_first_of("'#\"") == llvm::StringRef::npos) {
|
||||
// Include the newline in the prefix size.
|
||||
return Introducer{.kind = kind,
|
||||
.terminator = indicator,
|
||||
.prefix_size = static_cast<int>(line_end + 1)};
|
||||
}
|
||||
}
|
||||
if (kind == Kind::MultiLine) {
|
||||
// The introducer line is malformed. A character literal is never empty,
|
||||
// so the leading `''` cannot begin one and there is no other way to lex
|
||||
// this text; return an invalid introducer for diagnosis. A `"""`
|
||||
// introducer falls through instead: `""` is a valid empty string
|
||||
// literal.
|
||||
return Introducer{
|
||||
.kind = kind,
|
||||
.terminator = indicator,
|
||||
.prefix_size = static_cast<int>(line_end == llvm::StringRef::npos
|
||||
? source_text.size()
|
||||
: line_end),
|
||||
.is_valid = false};
|
||||
}
|
||||
}
|
||||
|
||||
@@ -119,6 +164,17 @@ auto StringLiteral::Lex(llvm::StringRef source_text)
|
||||
cursor += introducer->prefix_size;
|
||||
const int prefix_len = cursor;
|
||||
|
||||
if (!introducer->is_valid) {
|
||||
// A malformed `'''` introducer line: return an invalid literal covering
|
||||
// the introducer line so the caller can diagnose it.
|
||||
llvm::StringRef text = source_text.take_front(prefix_len);
|
||||
return StringLiteral(text, /*content=*/llvm::StringRef(),
|
||||
/*content_needs_validation=*/false, hash_level,
|
||||
introducer->kind,
|
||||
/*is_terminated=*/false,
|
||||
/*has_invalid_introducer=*/true);
|
||||
}
|
||||
|
||||
llvm::SmallString<16> terminator(introducer->terminator);
|
||||
llvm::SmallString<16> escape("\\");
|
||||
|
||||
|
||||
Reference in New Issue
Block a user