diff --git a/docs/design/README.md b/docs/design/README.md index a81a414c543a..381d3f3783d8 100644 --- a/docs/design/README.md +++ b/docs/design/README.md @@ -209,8 +209,8 @@ accessible as members of `Math`, like `Math.Sqrt`. The `Core.Print` function comes from the `Core` package's `io` library. Unlike C++, the namespaces of different packages are kept separate, so there are no name conflicts. -Carbon [comments](#code-and-comments) must be on a line by themselves starting -with `//`: +Carbon [comments](#code-and-comments) start with `//` and run to the end of the +line, either on a line by themselves or following other content: ```carbon // Returns the smallest factor of `n` > 1, and @@ -353,11 +353,14 @@ allowed to have non-ASCII characters. var résultat: String = "Succès"; ``` -Comments start with two slashes `//` and go to the end of the line. They are -required to be the only non-whitespace on the line. +Comments start with two slashes `//` and go to the end of the line. A comment +may be the only content on its line, or it may follow other content as a trailing +comment. Full-line comments are preferred for documentation, while trailing +comments mark or annotate a specific line. ```carbon -// Compute an approximation of π +// Compute an approximation of π. +var pi: f64 = 3.14159; // Accurate enough for our purposes. ``` > References: @@ -368,6 +371,8 @@ required to be the only non-whitespace on the line. > [#142: Unicode source files](https://github.com/carbon-language/carbon-lang/pull/142) > - Proposal > [#198: Comments](https://github.com/carbon-language/carbon-lang/pull/198) +> - Proposal +> [#7441: Trailing comments](https://github.com/carbon-language/carbon-lang/pull/7441) ## Build modes diff --git a/docs/design/lexical_conventions/character_literals.md b/docs/design/lexical_conventions/character_literals.md index 377392df1e3c..19faf6c11212 100644 --- a/docs/design/lexical_conventions/character_literals.md +++ b/docs/design/lexical_conventions/character_literals.md @@ -33,6 +33,10 @@ A character literal consists of a sequence of characters enclosed in single quotes (`'`). - The contents must represent precisely one Unicode code point. +- A character literal is never empty. `''` does not begin a character + literal; it only occurs as part of the `'''` that begins or ends a + [block string literal](string_literals.md), which keeps `'''` + unambiguous. - Hex escape sequences (`\xHH`) are supported but limited to values up to `0x7F` (where the UTF-8 code unit and Unicode code point values are identical). Values `0x80` and above are disallowed in character literals to @@ -66,3 +70,5 @@ letters (for example, `\x0A`, not `\x0a`). [#1964: Character Literals](https://github.com/carbon-language/carbon-lang/pull/1964) - Proposal [#6710: `char` redesign](https://github.com/carbon-language/carbon-lang/pull/6710) +- Proposal + [#7441: Trailing comments](https://github.com/carbon-language/carbon-lang/pull/7441) diff --git a/docs/design/lexical_conventions/comments.md b/docs/design/lexical_conventions/comments.md index 91a3263d566d..a5092aa78164 100644 --- a/docs/design/lexical_conventions/comments.md +++ b/docs/design/lexical_conventions/comments.md @@ -12,6 +12,7 @@ SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception - [Overview](#overview) - [Details](#details) +- [Style](#style) - [Alternatives considered](#alternatives-considered) - [References](#references) @@ -23,13 +24,18 @@ A comment is a lexical element beginning with the characters `//` and running to the end of the line. We have no mechanism for physical line continuation, so a trailing `\` does not extend a comment to subsequent lines. +A comment may be the entire content of a line, or it may follow other content on +the line as a _trailing comment_. A full-line comment typically introduces the +code below it, while a trailing comment annotates the content on its own line. + ## Details -In the comments after the `//` a whitespace character is required to make the -comment valid. Newline is a whitespace character, so a line containing only `//` -is a valid comment. The end of the file also constitutes whitespace. +After the `//` a whitespace character is required to make the comment valid. +Newline is a whitespace character, so a line containing only `//` is a valid +comment. The end of the file also constitutes whitespace. -All comments are removed prior to formation of tokens. +All comments are removed prior to formation of tokens; a comment produces no +token. Example: @@ -37,13 +43,24 @@ Example: // This is a comment and is ignored. \ This is not a comment. -var Int: x; // error, trailing comments not allowed +var Int: x; // This is a trailing comment annotating `x`. ``` Currently no support for block comments is provided. Commenting out larger regions of human-readable text or code is accomplished by commenting out every line in the region. +## Style + +Full-line and trailing comments serve different purposes: + +- Prefer a full-line comment for documentation. It has less line-length + pressure and more easily stays attached to the code it describes as that + code changes. +- Use a trailing comment only to annotate or mark a specific line, in a + context where a comment on the preceding line would be awkward, verbose, or + imprecise. + ## Alternatives considered - [Intra-line comments](/proposals/p000198-comments.md#intra-line-comments) @@ -51,8 +68,11 @@ line in the region. - [Block comments](/proposals/p000198-comments.md#block-comments-2) - [Documentation comments](/proposals/p000198-comments.md#documentation-comments) - [Code folding comments](/proposals/p000198-comments.md#code-folding-comments) +- [Keep requiring comments to be alone on their line](/proposals/p007441-trailing-comments.md#keep-requiring-comments-to-be-alone-on-their-line) ## References - Proposal [#198: Comments](https://github.com/carbon-language/carbon-lang/pull/198) +- Proposal + [#7441: Trailing comments](https://github.com/carbon-language/carbon-lang/pull/7441) diff --git a/docs/design/lexical_conventions/string_literals.md b/docs/design/lexical_conventions/string_literals.md index dd376725ea1e..b8fb04adbb44 100644 --- a/docs/design/lexical_conventions/string_literals.md +++ b/docs/design/lexical_conventions/string_literals.md @@ -151,11 +151,21 @@ var String: invalid = ''' '''; ``` -A _file type indicator_ is any sequence of non-whitespace characters other than -`'` or `#`. The file type indicator has no semantic meaning to the Carbon -compiler, but some file type indicators are understood by the language tooling -(for example, syntax highlighter, code formatter) as indicating the structure of -the string literal's content. +A _file type indicator_ is the text following the `'''` on the introducer line, +with surrounding whitespace removed; it may not contain `'`, `#`, or `"`. A +[trailing comment](comments.md) may follow the file type indicator on the +introducer line. It is an ordinary comment, and tools treat it as one; like the +surrounding whitespace it is not part of the indicator, and so it may contain +characters that the indicator may not. The file type indicator has no semantic +meaning to the Carbon compiler, but some file type indicators are understood by +the language tooling (for example, syntax highlighter, code formatter) as +indicating the structure of the string literal's content. + +Because a [character literal](character_literals.md) is never empty, `''` can +only occur as the start of the `'''` that begins or ends a block string +literal. A `'''` whose introducer line does not have the required form, such +as the single-line `'''foo'''`, is an error; it is never interpreted as a +sequence of character literals. ```carbon // This is a block string literal. Its first two characters are spaces, and its @@ -346,3 +356,5 @@ string in the type system. In such string literals, we should consider rejecting [#199: String literals](https://github.com/carbon-language/carbon-lang/pull/199) - Proposal [#2040: Unicode escape code length](https://github.com/carbon-language/carbon-lang/pull/2040) +- Proposal + [#7441: Trailing comments](https://github.com/carbon-language/carbon-lang/pull/7441) diff --git a/proposals/p007441-trailing-comments.md b/proposals/p007441-trailing-comments.md new file mode 100644 index 000000000000..b2c98b56d605 --- /dev/null +++ b/proposals/p007441-trailing-comments.md @@ -0,0 +1,399 @@ +# Trailing comments + + + +[Pull request](https://github.com/carbon-language/carbon-lang/pull/7441) + + + +## Table of contents + +- [Abstract](#abstract) +- [Problem](#problem) +- [Background](#background) + - [The experimental restriction](#the-experimental-restriction) + - [The lexer](#the-lexer) +- [Proposal](#proposal) +- [Details](#details) + - [Lexical rule](#lexical-rule) + - [Empty character literals and `'''`](#empty-character-literals-and-) + - [Examples](#examples) + - [Style guidance](#style-guidance) + - [Tooling](#tooling) +- [Rationale](#rationale) +- [Alternatives considered](#alternatives-considered) + - [Keep requiring comments to be alone on their line](#keep-requiring-comments-to-be-alone-on-their-line) + - [Also allow intra-line and block comments](#also-allow-intra-line-and-block-comments) + - [Restrict trailing comments to specific positions](#restrict-trailing-comments-to-specific-positions) + - [Use a directionality marker](#use-a-directionality-marker) + - [Express annotations only through the grammar](#express-annotations-only-through-the-grammar) + + + +## Abstract + +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. + +## Problem + +The [original comments proposal](/proposals/p000198-comments.md) provided a +single kind of comment, `//` to end of line, and restricted it so that "no code +is permitted prior to a comment on the same line." That proposal explicitly +labeled the restriction **experimental** and said the decision "should be +revisited if we find there is a need for such comments in the context of the +complete language design." + +Several considerations now motivate revisiting it: + +- The need for short, free-form _annotations_ that attach directly to a line. + These are especially useful in examples, presentations, and when discussing + code, even though they may rarely come up in real-world code bases. +- In a reversal from the original lexer, it is now requiring _extra_ lexing + complexity and potentially performance to reject trailing comments. +- Migrating C++ code, a core Carbon goal, routinely encounters trailing + comments. The restriction forces each one to be reworked into a layout that + reads well without it, rather than carrying the existing layout over + directly. + +## Background + +### The experimental restriction + +The [comments proposal](/proposals/p000198-comments.md) made several decisions +that this proposal leaves untouched: + +- There is one kind of comment, introduced by `//` and running to the end of + the line; there is no physical line continuation. +- The `//` must be followed by whitespace (or end of file). Sequences where + `//` is _not_ followed by whitespace are + [reserved for future extension](/proposals/p000198-comments.md#reserved-comments) + such as documentation or fold markers. +- There are no block comments; a region is commented out by prefixing each + line. + +The one decision this proposal changes is the +[experimental](/proposals/p000198-comments.md#intra-line-comments) requirement +that a comment be the only non-whitespace on its line. The original proposal's +rationale for that requirement was primarily about _tools_: an intra-line or +trailing comment makes it harder for a formatter to know what program element +the comment "attaches to," and reflowing aligned trailing comments across edits +is genuinely complex. It grouped trailing comments together with intra-line +(C-style `/* ... */`) comments and declined both, pending experience. + +This proposal separates the two. It allows trailing line comments, which run to +the end of the line and so have an unambiguous extent, while continuing to +exclude intra-line and block comments, which are where most of the original +tooling complexity lives. + +### The lexer + +Carbon's lexer is a table-dispatch loop: the first byte of each lexical element +selects a handler, and `/` is wired to a handler that decides between a `/` +operator and a `//` comment with a max-munch rule. That dispatch does not depend +on _where_ in the line the `/` occurs. A `//` reaching the comment handler in +the middle of a line takes the same path as one at the start of a line. + +A comment, once recognized, is consumed by advancing to the end of the line, +which the lexer already tracks. The only work the lexer does today that is +specific to trailing comments is the explicit check that the `//` begins at the +line's first non-whitespace position, which exists solely to _reject_ trailing +comments. Removing that rejection and recording the comment instead is less +code, not more, and adds no new scanning. In this sense, supporting trailing +comments is nearly free in the lexer, while continuing to reject them is what +now costs extra logic in the comment lexer's hot path. + +## Proposal + +Allow a `//` comment to follow other content on a line. Such a comment is called +a _trailing comment_. It is lexically identical to a full-line comment: it +begins with `//`, requires whitespace after the `//`, and runs to the end of the +line. The sole change is that a comment is no longer required to be the only +non-whitespace on its line. + +Carbon still provides only line comments. This proposal does not add intra-line +or block comments, and does not change the reserved status of `//` sequences +that are not followed by whitespace. + +## Details + +### Lexical rule + +A comment is a lexical element beginning with `//` and running to the end of the +line. A comment may appear either as the entire content of a line (after +optional leading whitespace) or following other content on the line. In both +cases the character after `//` must be whitespace (newline and end of file count +as whitespace), and the comment is removed prior to formation of tokens; it +produces no token. + +The `//@...` tooling directives (such as `//@dump-sem-ir-begin`) remain +recognized only as full-line comments, since they are line-oriented markers; in +trailing position `//@` is simply a `//` not followed by whitespace, and so is a +reserved (currently invalid) comment as before. + +A block string literal's introducer line, consisting of the `'''` and an +optional file type indicator, may also carry a trailing comment. This is an +ordinary trailing comment, and it behaves as a comment everywhere: syntax +highlighting renders it as a comment, `carbon format` preserves it, tooling +that inspects comments sees it, and `//` not followed by whitespace remains +reserved on this line just as elsewhere, so it is not valid within a file type +indicator. The one special rule is needed because the file type indicator +would otherwise run to the end of the line: a trailing comment ends the file +type indicator, just as trailing whitespace does. Because the comment is not +part of the indicator, it may contain `'`, `#`, and `"` even though the +indicator may not. + +### Empty character literals and `'''` + +Allowing a comment on the introducer line, and allowing it to contain `'`, +creates a lexical ambiguity. Consider: + +```carbon +'''foo // ' +``` + +This could be a block string literal introducer with the file type `foo` and a +comment, or an empty character literal `''` followed by the character literal +`'foo // '`. + +This proposal resolves the ambiguity by specifying that a +[character literal](/docs/design/lexical_conventions/character_literals.md) is +never empty: `''` never begins a character literal, and so `'''` unambiguously +begins (or ends) a block string literal. An empty character literal has no +meaning or use case, and the toolchain already rejects `''` as an error, so +this only codifies the existing behavior as the disambiguation rule; it does +not change which programs are valid. It also lets a lexer classify `''` +without further lookahead. + +With this rule the file type indicator no longer needs a restricted character +set to be unambiguous, but this proposal keeps the restriction because it +improves error recovery: an introducer line whose file type indicator would +contain `'`, `#`, or `"` is diagnosed as an error and does not open a block +string literal, so text like `let s: String = '''single-line?''';` produces a +contained error rather than treating the remainder of the file as string +content or as a sequence of character literals. + +### Examples + +A trailing comment may follow any content, and the following code is now valid +rather than an error: + +```carbon +var count: i32 = 0; // a) A local variable, + +fn Render(frame: Frame) { + Draw(frame); // b) A function call, + Flush(); +} // c) And a closing brace. +``` + +Full-line and trailing comments coexist; a full-line comment still introduces +the code below it, while a trailing comment annotates the content on its own +line: + +```carbon +// Compute the smallest prime factor. +var factor: i32 = SmallestFactor(n); // TODO: i32 -> i64 +``` + +The reserved-comment rule is unchanged, so a trailing `//` that is not followed +by whitespace is still an error: + +```carbon +var x: i32 = 0; //rejected: whitespace is required after `//` +``` + +A trailing comment may also follow the file type indicator on a block string +literal's introducer line: + +```carbon +var query: String = '''sql // TODO: switch to a prepared statement + SELECT * FROM t + '''; +``` + +### Style guidance + +Trailing comments are an addition to full-line comments, not a replacement, and +the two suit different purposes: + +- Prefer a full-line comment for documentation. It has less line-length + pressure and more easily stays attached to the code it describes as that + code changes. +- Use a trailing comment only to annotate or mark a specific line, in a + context where a comment on the preceding line would be awkward, verbose, + or imprecise. + +### Tooling + +The lexer records each trailing comment as a comment, marked as trailing so that +tools can distinguish it from a comment that introduces the following code. A +trailing comment is never coalesced with the full-line comments adjacent to it, +even when they line up, because it belongs to the content on its own line. + +One exception in the toolchain today: a trailing comment on a block string +literal's introducer line is carried within the string literal token's spelling +rather than in the lexer's comment records, and `carbon format` preserves it +verbatim as part of the literal. Surfacing it through the comment records, and +diagnosing a reserved `//` sequence within a file type indicator, remain as +toolchain follow-ups. + +`carbon format` keeps a trailing comment on the line it annotates, separated +from the preceding content by a single space, rather than relocating it to its +own line. This is the behavior an author intends, and it lets code migrated from +C++ keep the comment layout it already had. + +Reflowing trailing comments under more aggressive reformatting, for example +maintaining a column of aligned trailing comments or moving an over-long +trailing comment to its own line, is a formatting-quality concern rather than a +language one, and can be improved over time without further language changes. + +## Rationale + +- [Code that is easy to read, understand, and write](/docs/project/goals.md#code-that-is-easy-to-read-understand-and-write): + Line-specific annotations are often needed when explaining or discussing + code in order to understand it. +- [Software and language evolution](/docs/project/goals.md#software-and-language-evolution): + This is the planned revisit of an explicitly experimental restriction, now + that the surrounding design has matured. The change is strictly relaxing: it + only makes previously-invalid programs valid, so it imposes no migration on + existing code. +- [Fast and scalable development](/docs/project/goals.md#fast-and-scalable-development) + and + [Language tools and ecosystem](/docs/project/goals.md#language-tools-and-ecosystem): + Supporting trailing comments costs the lexer essentially nothing; continuing + to reject them is what would cost more, so the feature does not compromise + performance or simplicity. +- [Interoperability with and migration from existing C++ code](/docs/project/goals.md#interoperability-with-and-migration-from-existing-c-code): + Trailing line comments are ubiquitous in C++ and are what programmers + expect. Allowing them lets migrated C++ code keep its existing layout + instead of reworking each trailing comment to read well in a different + structure, which avoids gratuitous churn and is consistent with the original + proposal's choice of `//` to match C++. + +## Alternatives considered + +### Keep requiring comments to be alone on their line + +We could leave the experimental restriction in place and continue to require +every comment to be the only non-whitespace on its line. + +- Advantages: it is the simplest possible rule; it sidesteps the formatter + questions about what a trailing comment attaches to and how to reflow + aligned trailing comments; and it gently pushes authors to promote per-line + notes into the grammar (such as named arguments) where tools can see them. +- Disadvantages: it leaves the _annotation_ use case unserved. A short note + that belongs beside a specific value or branch must move to its own line or + be omitted, which is awkward when teaching, presenting, or discussing code. + It also forces C++ code that uses trailing comments to be restructured on + migration rather than carried over as-is, and it keeps the lexer doing extra + work to reject a construct it can otherwise handle for free. +- Core of the decision: the original proposal deferred this precisely so it + could be revisited with experience. That experience is that trailing + comments matter for explaining and discussing code and are pervasive in the + C++ code Carbon aims to migrate, while the lexer cost that motivated the + restriction has reversed: rejecting trailing comments now takes more work + than accepting them. Connecting to + [code that is easy to read, understand, and write](/docs/project/goals.md#code-that-is-easy-to-read-understand-and-write), + the value for annotations and migration outweighs the simplicity of the + stricter rule. + +### Also allow intra-line and block comments + +We could go further and also allow comments that attach to something smaller +than a line, such as C-style intra-line comments like `f(/*size=*/5)` or block +comments that span or interrupt lines, as the +[original proposal discussed](/proposals/p000198-comments.md#block-comments-2). + +- Advantages: this would additionally cover the syntactic-disambiguation use + case (annotating an argument inline) and would make commenting out a region + or a fragment of a line more ergonomic. +- Disadvantages: intra-line comments are where most of the formatting + difficulty lives, since a tool must understand which token a `/*...*/` + attaches to and how to wrap it, and `/*...*/` with Carbon-specific semantics + risks confusing C++ readers, as the original proposal noted. Carbon also + intends to address inline argument annotation through the grammar (for + example, named arguments) so that such utterances are meaningful to tools + rather than being text. +- Core of the decision: trailing line comments capture most of the value, the + short annotations on a line, while keeping the lexical model simple (still + only `//` to end of line) and the comment's extent unambiguous. The + additional intra-line and block forms carry the costs the original proposal + identified without a corresponding need, so they remain out of scope and the + syntactic-disambiguation use case continues to be a matter for the grammar. + +### Restrict trailing comments to specific positions + +We could allow trailing comments only after particular constructs, for example +only after a `;`, an enumerator, or a struct field, to bound the formatter's +problem to a small set of well-understood layouts. + +- Advantages: it would limit where aligned-comment reflow can arise and would + let a formatter special-case each permitted position. +- Disadvantages: it makes the lexical and grammatical rules markedly more + complex and harder to remember, introduces surprising "why not here?" edges, + and requires the grammar to enumerate the allowed positions and keep that + list current as the language grows. +- Core of the decision: a uniform "a comment may follow any content on its + line" rule is simpler to specify, learn, and implement, and the formatter + handles the general case directly. A positional restriction trades a real + and pervasive cost in simplicity for a speculative tooling convenience, + which is a poor fit for Carbon's preference for one obvious way to do + things. + +### Use a directionality marker + +We could permit comments that attach intra-line but require a marker indicating +direction, such as the `//>` / `//<` forms +[sketched in the original proposal](/proposals/p000198-comments.md#intra-line-comments), +so a tool can tell what the comment attaches to. + +- Advantages: it gives tools an explicit attachment hint without full + intra-line comment parsing. +- Disadvantages: it invents novel syntax for a niche case, is unfamiliar to + C++ developers, and still leaves the hard problem of line-wrapping such + comments largely unsolved. +- Core of the decision: this was already set aside in the original proposal + and the same reasoning holds. End-of-line trailing comments need no + attachment marker because their extent and their subject, the content on + their line, are clear, so the marker adds complexity without solving a + problem this proposal has. + +### Express annotations only through the grammar + +We could decline trailing comments and instead require every per-line annotation +to be promoted into the language, for example as a named argument or a future +attribute or documentation syntax, so the information is structured and visible +to tools. + +- Advantages: structured annotations are meaningful to tools and can be + validated, and this keeps a single, uniform comment placement. +- Disadvantages: many annotations are free-form prose ("RFC 2324", "linear if + 1.0", "pixels") with no structured meaning to capture, and forcing them into + the grammar is heavyweight and often impossible. A declaration-level + documentation facility, were one added, would target declarations rather + than arbitrary values on a line, so it would not cover the annotation case + either. None of these options help carry trailing comments over from + migrated C++. +- Core of the decision: the grammar is the right tool when an annotation has + structure worth capturing, and such cases can use it as the language grows. + Free-form notes beside a line of code, including those already present in + migrated C++, are what a comment is for, and trailing comments let authors + keep them where they belong. diff --git a/toolchain/check/testdata/interop/cpp/basics/cpp_diagnostics.carbon b/toolchain/check/testdata/interop/cpp/basics/cpp_diagnostics.carbon index 7152fc5a9e33..ccaa70fce8a5 100644 --- a/toolchain/check/testdata/interop/cpp/basics/cpp_diagnostics.carbon +++ b/toolchain/check/testdata/interop/cpp/basics/cpp_diagnostics.carbon @@ -303,17 +303,19 @@ import Cpp library "indirect_warning.h"; // --- fail_import_cpp_library_lexer_error.carbon -library "[[@TEST_NAME]]"; // Trailing comment +library "[[@TEST_NAME]]"; -// TODO: Move this warning to be after the lexer trailing comment error. +//!Lexer error: whitespace is required after the comment introducer. + +// TODO: Move this warning to be after the lexer error. // CHECK:STDERR: fail_import_cpp_library_lexer_error.carbon:[[@LINE+9]]:10: in file included here [InCppInclude] // CHECK:STDERR: ./one_warning.h:2:2: warning: "warning1" [CppInteropParseWarning] // CHECK:STDERR: 2 | #warning "warning1" // CHECK:STDERR: | ^ // CHECK:STDERR: -// CHECK:STDERR: fail_import_cpp_library_lexer_error.carbon:[[@LINE-8]]:44: error: trailing comments are not permitted [TrailingComment] -// CHECK:STDERR: library "import_cpp_library_lexer_error"; // Trailing comment -// CHECK:STDERR: ^ +// CHECK:STDERR: fail_import_cpp_library_lexer_error.carbon:[[@LINE-8]]:3: error: whitespace is required after '//' [NoWhitespaceAfterCommentIntroducer] +// CHECK:STDERR: //!Lexer error: whitespace is required after the comment introducer. +// CHECK:STDERR: ^ // CHECK:STDERR: import Cpp library "one_warning.h"; diff --git a/toolchain/diagnostics/kind.def b/toolchain/diagnostics/kind.def index 02921d2667a0..d9a9bf2a825c 100644 --- a/toolchain/diagnostics/kind.def +++ b/toolchain/diagnostics/kind.def @@ -70,12 +70,12 @@ CARBON_DIAGNOSTIC_KIND(InvalidDigit) CARBON_DIAGNOSTIC_KIND(InvalidDigitSeparator) CARBON_DIAGNOSTIC_KIND(InvalidHorizontalWhitespaceInString) CARBON_DIAGNOSTIC_KIND(MismatchedIndentInString) +CARBON_DIAGNOSTIC_KIND(MultiLineStringInvalidIntroducer) CARBON_DIAGNOSTIC_KIND(MultiLineStringWithDoubleQuotes) CARBON_DIAGNOSTIC_KIND(NoWhitespaceAfterCommentIntroducer) CARBON_DIAGNOSTIC_KIND(TooManyDigits) CARBON_DIAGNOSTIC_KIND(TooManyTokens) CARBON_DIAGNOSTIC_KIND(TooManyTypeBitWidthDigits) -CARBON_DIAGNOSTIC_KIND(TrailingComment) CARBON_DIAGNOSTIC_KIND(UnicodeEscapeMissingBracedDigits) CARBON_DIAGNOSTIC_KIND(UnicodeEscapeSurrogate) CARBON_DIAGNOSTIC_KIND(UnicodeEscapeTooLarge) diff --git a/toolchain/format/formatter.cpp b/toolchain/format/formatter.cpp index 9503e53a16b5..287fdbae118d 100644 --- a/toolchain/format/formatter.cpp +++ b/toolchain/format/formatter.cpp @@ -12,11 +12,8 @@ auto Formatter::Run() -> bool { return false; } - auto comments = tokens_->comments(); - auto comment_it = comments.begin(); - // If there are no tokens or comments, format as empty. - if (tokens_->size() == 0 && comment_it == comments.end()) { + if (tokens_->size() == 0 && next_comment_ == comments_end_) { *out_ << "\n"; return true; } @@ -24,15 +21,12 @@ auto Formatter::Run() -> bool { for (auto token : tokens_->tokens()) { auto token_kind = tokens_->GetKind(token); - while (comment_it != comments.end() && - tokens_->IsAfterComment(token, *comment_it)) { - RequireEmptyLine(); - PrepareForSpacedContent(); - // TODO: We do need to adjust the indent of multi-line comments. - *out_ << tokens_->GetCommentText(*comment_it); - // Comment text includes a terminating newline, so just update the state. - line_state_ = LineState::Empty; - ++comment_it; + // Emit any comments that come before this token in the source. Trailing + // comments are attached to the still-open current line; full-line comments + // are emitted on their own line. + while (next_comment_ != comments_end_ && + tokens_->IsAfterComment(token, *next_comment_)) { + EmitComment(); } switch (token_kind) { @@ -81,10 +75,42 @@ auto Formatter::Run() -> bool { break; } } + + // Materialize any newline deferred by the final line. + if (line_state_ == LineState::EndOfLine) { + *out_ << "\n"; + line_state_ = LineState::Empty; + } return true; } +auto Formatter::EmitComment() -> void { + auto comment = *next_comment_; + ++next_comment_; + + if (tokens_->IsTrailingComment(comment) && line_state_ != LineState::Empty) { + // Keep the trailing comment on the current line, separated by a space. The + // line still has content because its newline was deferred (`EndOfLine`) or + // not yet required. + *out_ << " " << tokens_->GetCommentText(comment); + } else { + // A full-line comment (or a trailing comment with nothing left to attach + // to) is emitted on its own line. + RequireEmptyLine(); + PrepareForSpacedContent(); + // TODO: We do need to adjust the indent of multi-line comments. + *out_ << tokens_->GetCommentText(comment); + } + // Comment text includes a terminating newline, so just update the state. + line_state_ = LineState::Empty; +} + auto Formatter::PrepareForPackedContent() -> void { + // Materialize a deferred newline before starting to fill a fresh line. + if (line_state_ == LineState::EndOfLine) { + *out_ << "\n"; + line_state_ = LineState::Empty; + } if (line_state_ == LineState::Empty) { out_->indent(indent_); line_state_ = LineState::HasSeparator; @@ -92,9 +118,10 @@ auto Formatter::PrepareForPackedContent() -> void { } auto Formatter::RequireEmptyLine() -> void { + // Defer the newline so a trailing comment can still attach to this line; it + // is materialized by the next content or at end of file. if (line_state_ != LineState::Empty) { - *out_ << "\n"; - line_state_ = LineState::Empty; + line_state_ = LineState::EndOfLine; } } diff --git a/toolchain/format/formatter.h b/toolchain/format/formatter.h index 89f3c9419774..9de39feb10f6 100644 --- a/toolchain/format/formatter.h +++ b/toolchain/format/formatter.h @@ -26,7 +26,10 @@ namespace Carbon::Format { class Formatter { public: explicit Formatter(const Lex::TokenizedBuffer* tokens, llvm::raw_ostream* out) - : tokens_(tokens), out_(out) {} + : tokens_(tokens), + out_(out), + next_comment_(tokens->comments().begin()), + comments_end_(tokens->comments().end()) {} // See class comments. auto Run() -> bool; @@ -42,12 +45,26 @@ class Formatter { // The current line has content, and will need a separator, typically a // single space or newline. NeedsSeparator, + // The current line has content and is complete; a newline is pending but + // has not yet been emitted. We defer the newline so that a trailing comment + // can still be attached to this line before it is broken. The newline is + // materialized by the next content emitted (see `PrepareForPackedContent`) + // or when the file ends. + EndOfLine, }; - // Ensure output is on an empty line, setting line_state_ to Empty. May output - // a newline, dependent on line state. Does not indent, allowing blank lines. + // Marks the current line as complete, so the next content starts a new line. + // The newline is deferred rather than emitted immediately, allowing a + // trailing comment to be attached first. Does not indent, allowing blank + // lines. auto RequireEmptyLine() -> void; + // Emits the comment at `next_comment_` and advances past it. A trailing + // comment is kept on the current line (separated by a space) when there is + // still content to attach it to; otherwise the comment is emitted on its own + // line. + auto EmitComment() -> void; + // Ensures there is a separator before adding new content. May do // `PrepareForPackedContent` or output a separator space, dependent on line // state. Always results in line_state_ being HasSeparator; the caller is @@ -71,6 +88,10 @@ class Formatter { // The output stream for formatted content. llvm::raw_ostream* out_; + // The next comment to emit, and one past the last comment. + Lex::CommentIterator next_comment_; + Lex::CommentIterator comments_end_; + // The state of the line currently written to output. LineState line_state_ = LineState::Empty; diff --git a/toolchain/format/testdata/basics/comments.carbon b/toolchain/format/testdata/basics/comments.carbon index f12706a0f90a..e8aabf1ba217 100644 --- a/toolchain/format/testdata/basics/comments.carbon +++ b/toolchain/format/testdata/basics/comments.carbon @@ -29,6 +29,27 @@ class C { // // Comment +// A trailing comment may follow a variable, a function call, or a closing +// brace, and the formatter keeps it on the line it annotates. +var count: i32 = 0; // a) A local variable, + +fn Render(frame: Frame) { + Draw(frame); // b) A function call, + Flush(); +} // c) And a closing brace. + +// A trailing comment on a block string literal's introducer line is carried +// within the literal token and is preserved, with or without a file type +// indicator. A trailing comment after the literal's closing line stays +// attached to that line. +var query: String = '''sql // TODO: switch to a prepared statement + SELECT * FROM t + '''; + +var poem: String = ''' // No file type indicator, just a comment. + Roses are red. + '''; // A trailing comment after the closing line. + // --- AUTOUPDATE-SPLIT // CHECK:STDOUT: // A comment @@ -43,3 +64,20 @@ class C { // CHECK:STDOUT: // Block // CHECK:STDOUT: // // CHECK:STDOUT: // Comment +// CHECK:STDOUT: // A trailing comment may follow a variable, a function call, or a closing +// CHECK:STDOUT: // brace, and the formatter keeps it on the line it annotates. +// CHECK:STDOUT: var count: i32 = 0; // a) A local variable, +// CHECK:STDOUT: fn Render (frame: Frame) { +// CHECK:STDOUT: Draw (frame); // b) A function call, +// CHECK:STDOUT: Flush (); +// CHECK:STDOUT: } // c) And a closing brace. +// CHECK:STDOUT: // A trailing comment on a block string literal's introducer line is carried +// CHECK:STDOUT: // within the literal token and is preserved, with or without a file type +// CHECK:STDOUT: // indicator. A trailing comment after the literal's closing line stays +// CHECK:STDOUT: // attached to that line. +// CHECK:STDOUT: var query: String = '''sql // TODO: switch to a prepared statement +// CHECK:STDOUT: SELECT * FROM t +// CHECK:STDOUT: '''; +// CHECK:STDOUT: var poem: String = ''' // No file type indicator, just a comment. +// CHECK:STDOUT: Roses are red. +// CHECK:STDOUT: '''; // A trailing comment after the closing line. diff --git a/toolchain/lex/lex.cpp b/toolchain/lex/lex.cpp index 91a91617bf9d..025d7a5b6dff 100644 --- a/toolchain/lex/lex.cpp +++ b/toolchain/lex/lex.cpp @@ -951,43 +951,42 @@ auto Lexer::EndDumpSemIRRangeIfIncomplete(const char* diag_loc) -> void { auto Lexer::LexComment(llvm::StringRef source_text, ssize_t& position) -> void { CARBON_DCHECK(source_text.substr(position).starts_with("//")); int32_t comment_start = position; - - // Any comment must be the only non-whitespace on the line. const auto line_info = current_line_info(); - if (LLVM_UNLIKELY(position != line_info.start + line_info.indent)) { - CARBON_DIAGNOSTIC(TrailingComment, Error, - "trailing comments are not permitted"); - emitter_.Emit(source_text.begin() + position, TrailingComment); - - // Note that we cannot fall-through here as the logic below doesn't handle - // trailing comments. Instead, we treat trailing comments as vertical - // whitespace, which already is designed to skip over any erroneous text at - // the end of the line. - LexVerticalWhitespace(source_text, position); - buffer_.AddComment(line_info.indent, comment_start, position); - return; - } + // A comment is _trailing_ when it follows other content on its line, rather + // than being the only non-whitespace on the line. Both kinds of comment are + // lexed identically -- they run from `//` to the end of the line -- but we + // record the distinction so that tooling can tell a comment annotating the + // code on its line apart from one introducing the code below it. + // + // `line_info.indent` is the width of the line's leading whitespace, so + // `line_info.start + line_info.indent` is the line's first non-whitespace + // byte. + const bool is_trailing = position != line_info.start + line_info.indent; // The introducer '//' must be followed by whitespace or EOF. bool is_valid_after_slashes = true; if (position + 2 < static_cast(source_text.size()) && LLVM_UNLIKELY(!IsSpace(source_text[position + 2]))) { llvm::StringRef comment_text = source_text.substr(position); - if (comment_text.starts_with("//@include-in-dumps\n")) { - buffer_.has_include_in_dumps_ = true; - AdvanceToLine(source_text, position, next_line()); - return; - } - if (comment_text.starts_with("//@dump-sem-ir-begin\n")) { - BeginDumpSemIRRange(comment_text.begin()); - AdvanceToLine(source_text, position, next_line()); - return; - } - if (comment_text.starts_with("//@dump-sem-ir-end\n")) { - EndDumpSemIRRange(comment_text.begin()); - AdvanceToLine(source_text, position, next_line()); - return; + // The `//@...` directives are tooling markers that are only meaningful as + // full-line comments, so we only recognize them when not trailing. + if (!is_trailing) { + if (comment_text.starts_with("//@include-in-dumps\n")) { + buffer_.has_include_in_dumps_ = true; + AdvanceToLine(source_text, position, next_line()); + return; + } + if (comment_text.starts_with("//@dump-sem-ir-begin\n")) { + BeginDumpSemIRRange(comment_text.begin()); + AdvanceToLine(source_text, position, next_line()); + return; + } + if (comment_text.starts_with("//@dump-sem-ir-end\n")) { + EndDumpSemIRRange(comment_text.begin()); + AdvanceToLine(source_text, position, next_line()); + return; + } } CARBON_DIAGNOSTIC(NoWhitespaceAfterCommentIntroducer, Error, "whitespace is required after '//'"); @@ -1001,6 +1000,22 @@ auto Lexer::LexComment(llvm::StringRef source_text, ssize_t& position) -> void { LineIndex line_index = next_line(); position = buffer_.line_infos_.Get(line_index).start; + // A trailing comment runs to the end of its line. Unlike a full-line comment, + // it can never be part of a block of identical comment lines, so we skip the + // block-skipping optimization below and simply advance past this one line. We + // also don't optimize for the case of a trailing comment as we expect them to + // be relatively rare compared to other comment structures. + if (LLVM_UNLIKELY(is_trailing)) { + buffer_.AddComment(line_info.indent, comment_start, position, + /*is_trailing=*/true); + // Unlike a full-line comment, a trailing comment can directly follow a + // token that cleared the leading-whitespace flag (`x;// y`), so restore it + // here for the next line's first token. + NoteWhitespace(); + AdvanceToLine(source_text, position, line_index); + return; + } + // A very common pattern is a long block of comment lines all with the same // indent and comment start. We skip these comment blocks in bulk both for // speed and to reduce redundant diagnostics if each line has the same @@ -1083,7 +1098,7 @@ auto Lexer::LexComment(llvm::StringRef source_text, ssize_t& position) -> void { } } - buffer_.AddComment(indent, comment_start, position); + buffer_.AddComment(indent, comment_start, position, /*is_trailing=*/false); AdvanceToLine(source_text, position, line_index); } @@ -1151,7 +1166,6 @@ auto Lexer::LexStringLiteral(llvm::StringRef source_text, ssize_t& position) // Capture the position before we step past the token. int32_t byte_offset = position; - int string_column = byte_offset - current_line_info().start; position += literal->text().size(); // Helper for error paths. @@ -1172,15 +1186,40 @@ auto Lexer::LexStringLiteral(llvm::StringRef source_text, ssize_t& position) return lex_as_error(); } + if (literal->has_invalid_introducer()) { + // The literal covers only the malformed introducer line, so it spans no + // lines and needs no line updates. + CARBON_DIAGNOSTIC(MultiLineStringInvalidIntroducer, Error, + "invalid multi-line string literal introducer; a file " + "type indicator may not contain `'`, `#`, or `\"`, and " + "the content must begin on a new line"); + emitter_.Emit(literal->text().begin(), MultiLineStringInvalidIntroducer); + return lex_as_error(); + } + // Update line and column information. if (literal->kind() != StringLiteral::Kind::SingleLine) { + // A block string literal's content is indented to match its closing + // delimiter: leading whitespace up to the delimiter's column is + // indentation, and anything past it is part of the content. Each line the + // literal spans is given the closing delimiter's column as its indentation. + // The closing line's indentation must be correct because tokens and + // comments can follow the closing delimiter and rely on it, for example to + // detect a trailing comment. Multi-line literals are rare, so this cold + // path need not be fast. + LineIndex first_spanned_line(line_index_.index + 1); while (next_line_info().start < position) { ++line_index_.index; - current_line_info().indent = string_column; } - // Note that we've updated the current line at this point, but - // `set_indent_` is already true from above. That remains correct as the - // last line of the multi-line literal *also* has its indent set. + // The closing delimiter is the first non-whitespace on the closing line, so + // its column is that line's leading-whitespace width. + LineInfo& closing_line_info = current_line_info(); + ssize_t indent_end = closing_line_info.start; + SkipHorizontalWhitespace(source_text, indent_end); + int32_t indent = indent_end - closing_line_info.start; + for (int32_t i = first_spanned_line.index; i <= line_index_.index; ++i) { + buffer_.line_infos_.Get(LineIndex(i)).indent = indent; + } } if (!literal->is_terminated()) { diff --git a/toolchain/lex/string_literal.cpp b/toolchain/lex/string_literal.cpp index 3c1d32adafac..7974d4a79551 100644 --- a/toolchain/lex/string_literal.cpp +++ b/toolchain/lex/string_literal.cpp @@ -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; @@ -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(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(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(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("\\"); diff --git a/toolchain/lex/string_literal.h b/toolchain/lex/string_literal.h index 30a5905c0848..00a6169a6191 100644 --- a/toolchain/lex/string_literal.h +++ b/toolchain/lex/string_literal.h @@ -63,18 +63,27 @@ class StringLiteral { // Returns true if the string has a valid terminator. auto is_terminated() const -> bool { return is_terminated_; } + // Returns true if this is a multi-line string literal whose introducer line + // is malformed. Such a literal covers just the introducer line and is never + // terminated. + auto has_invalid_introducer() const -> bool { + return has_invalid_introducer_; + } + private: struct Introducer; explicit StringLiteral(llvm::StringRef text, llvm::StringRef content, bool content_needs_validation, int hash_level, - Kind kind, bool is_terminated) + Kind kind, bool is_terminated, + bool has_invalid_introducer = false) : text_(text), content_(content), content_needs_validation_(content_needs_validation), hash_level_(hash_level), kind_(kind), - is_terminated_(is_terminated) {} + is_terminated_(is_terminated), + has_invalid_introducer_(has_invalid_introducer) {} // The complete text of the string literal. llvm::StringRef text_; @@ -98,6 +107,9 @@ class StringLiteral { // Whether the literal is valid, or should only be used for errors. bool is_terminated_; + + // Whether this is a multi-line literal whose introducer line is malformed. + bool has_invalid_introducer_; }; } // namespace Carbon::Lex diff --git a/toolchain/lex/string_literal_test.cpp b/toolchain/lex/string_literal_test.cpp index c5641756b7e6..20996e0395e9 100644 --- a/toolchain/lex/string_literal_test.cpp +++ b/toolchain/lex/string_literal_test.cpp @@ -91,6 +91,17 @@ TEST_F(StringLiteralTest, StringLiteralBounds) { // #"""# does not start a multiline string literal. R"(#"""#)", R"(##"""##)", + + // A trailing comment may follow the file type indicator on the opening + // line, and unlike the indicator it may contain '\'', '#', and '"'. + R"('''py // a #2 "trailing" comment isn't part of the indicator + content + ''')", + // TODO: `//` not followed by whitespace is reserved; diagnose it rather + // than lexing it as part of the file type indicator. + R"('''//no-space-is-not-a-comment + content + ''')", }; for (llvm::StringLiteral test : valid) { @@ -115,6 +126,13 @@ TEST_F(StringLiteralTest, StringLiteralBounds) { "#'''\n'''", R"(" \ ")", + // A malformed `'''` introducer line is an invalid literal covering that + // line; it is never re-lexed as character literals, because a character + // literal is not allowed to be empty. + "'''not multi-line'''", + "'''bad#type\ncontent\n'''", + "'''no newline", + "#'''bad#type\ncontent\n'''#", // clang-format on }; @@ -151,6 +169,11 @@ TEST_F(StringLiteralTest, StringLiteralContents) { // contain tabs. {"'''\n\t \t\n'''", "\n"}, + // A trailing comment after the file type indicator is ignored and does + // not affect the content, even when it contains '\'', '#', or '"'. + {"'''py // a #2 \"trailing\" comment isn't an indicator\nhello\n'''", + "hello\n"}, + // Indent removal. {R"( '''file type indicator diff --git a/toolchain/lex/testdata/fail_multiline_string_introducer.carbon b/toolchain/lex/testdata/fail_multiline_string_introducer.carbon new file mode 100644 index 000000000000..c8b75d98cdbc --- /dev/null +++ b/toolchain/lex/testdata/fail_multiline_string_introducer.carbon @@ -0,0 +1,51 @@ +// Part of the Carbon Language project, under the Apache License v2.0 with LLVM +// Exceptions. See /LICENSE for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +// A malformed `'''` introducer line is diagnosed and skipped as an error. It +// is never re-lexed as character literals: a character literal is not allowed +// to be empty, so `''` can only begin a block string literal. +// +// AUTOUPDATE +// TIP: To test this file alone, run: +// TIP: bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/lex/testdata/fail_multiline_string_introducer.carbon +// TIP: To dump output, run: +// TIP: bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/lex/testdata/fail_multiline_string_introducer.carbon +// CHECK:STDOUT: - filename: fail_multiline_string_introducer.carbon +// CHECK:STDOUT: tokens: + +// A `'''` in the middle of a line cannot close a block string literal, so a +// single-line use is a malformed introducer. +// CHECK:STDERR: fail_multiline_string_introducer.carbon:[[@LINE+4]]:17: error: invalid multi-line string literal introducer; a file type indicator may not contain `'`, `#`, or `"`, and the content must begin on a new line [MultiLineStringInvalidIntroducer] +// CHECK:STDERR: var s: String = '''not multi-line'''; +// CHECK:STDERR: ^ +// CHECK:STDERR: +var s: String = '''not multi-line'''; +// CHECK:STDOUT: - { index: 1, kind: "Var", line: {{ *}}[[@LINE-1]], column: 1, indent: 1, spelling: "var", has_leading_space: true } +// CHECK:STDOUT: - { index: 2, kind: "Identifier", line: {{ *}}[[@LINE-2]], column: 5, indent: 1, spelling: "s", identifier: 0, has_leading_space: true } +// CHECK:STDOUT: - { index: 3, kind: "Colon", line: {{ *}}[[@LINE-3]], column: 6, indent: 1, spelling: ":" } +// CHECK:STDOUT: - { index: 4, kind: "Identifier", line: {{ *}}[[@LINE-4]], column: 8, indent: 1, spelling: "String", identifier: 1, has_leading_space: true } +// CHECK:STDOUT: - { index: 5, kind: "Equal", line: {{ *}}[[@LINE-5]], column: 15, indent: 1, spelling: "=", has_leading_space: true } +// CHECK:STDOUT: - { index: 6, kind: "Error", line: {{ *}}[[@LINE-6]], column: 17, indent: 1, spelling: "'''not multi-line''';", has_leading_space: true } + +// A file type indicator may not contain `#`. The error covers the introducer +// line and lexing continues on the next line. +// CHECK:STDERR: fail_multiline_string_introducer.carbon:[[@LINE+4]]:17: error: invalid multi-line string literal introducer; a file type indicator may not contain `'`, `#`, or `"`, and the content must begin on a new line [MultiLineStringInvalidIntroducer] +// CHECK:STDERR: var t: String = '''bad#type +// CHECK:STDERR: ^ +// CHECK:STDERR: +var t: String = '''bad#type +// CHECK:STDOUT: - { index: 7, kind: "Var", line: {{ *}}[[@LINE-1]], column: 1, indent: 1, spelling: "var", has_leading_space: true } +// CHECK:STDOUT: - { index: 8, kind: "Identifier", line: {{ *}}[[@LINE-2]], column: 5, indent: 1, spelling: "t", identifier: 2, has_leading_space: true } +// CHECK:STDOUT: - { index: 9, kind: "Colon", line: {{ *}}[[@LINE-3]], column: 6, indent: 1, spelling: ":" } +// CHECK:STDOUT: - { index: 10, kind: "Identifier", line: {{ *}}[[@LINE-4]], column: 8, indent: 1, spelling: "String", identifier: 1, has_leading_space: true } +// CHECK:STDOUT: - { index: 11, kind: "Equal", line: {{ *}}[[@LINE-5]], column: 15, indent: 1, spelling: "=", has_leading_space: true } +// CHECK:STDOUT: - { index: 12, kind: "Error", line: {{ *}}[[@LINE-6]], column: 17, indent: 1, spelling: "'''bad#type", has_leading_space: true } +var u: i32 = 1; +// CHECK:STDOUT: - { index: 13, kind: "Var", line: {{ *}}[[@LINE-1]], column: 1, indent: 1, spelling: "var", has_leading_space: true } +// CHECK:STDOUT: - { index: 14, kind: "Identifier", line: {{ *}}[[@LINE-2]], column: 5, indent: 1, spelling: "u", identifier: 3, has_leading_space: true } +// CHECK:STDOUT: - { index: 15, kind: "Colon", line: {{ *}}[[@LINE-3]], column: 6, indent: 1, spelling: ":" } +// CHECK:STDOUT: - { index: 16, kind: "IntTypeLiteral", line: {{ *}}[[@LINE-4]], column: 8, indent: 1, spelling: "i32", has_leading_space: true } +// CHECK:STDOUT: - { index: 17, kind: "Equal", line: {{ *}}[[@LINE-5]], column: 12, indent: 1, spelling: "=", has_leading_space: true } +// CHECK:STDOUT: - { index: 18, kind: "IntLiteral", line: {{ *}}[[@LINE-6]], column: 14, indent: 1, spelling: "1", value: "1", has_leading_space: true } +// CHECK:STDOUT: - { index: 19, kind: "Semi", line: {{ *}}[[@LINE-7]], column: 15, indent: 1, spelling: ";" } diff --git a/toolchain/lex/testdata/fail_trailing_comments.carbon b/toolchain/lex/testdata/fail_trailing_comments.carbon deleted file mode 100644 index d3d947b14841..000000000000 --- a/toolchain/lex/testdata/fail_trailing_comments.carbon +++ /dev/null @@ -1,53 +0,0 @@ -// Part of the Carbon Language project, under the Apache License v2.0 with LLVM -// Exceptions. See /LICENSE for license information. -// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -// -// Trailing comments should produce errors, but we should continue to parse (and -// diagnose) code errors beyond them. - -var a: i32 = 1; // trailing comment -var b: 32 = 13; // more trailing comment -var c: i32 = 0.4; // still more trailing comment - -// We keep the auto-update marker below the above so check lines don't disrupt -// the trailing comment lines with similar prefixes. -// AUTOUPDATE -// TIP: To test this file alone, run: -// TIP: bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/lex/testdata/fail_trailing_comments.carbon -// TIP: To dump output, run: -// TIP: bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/lex/testdata/fail_trailing_comments.carbon -// CHECK:STDERR: fail_trailing_comments.carbon:[[@LINE-11]]:19: error: trailing comments are not permitted [TrailingComment] -// CHECK:STDERR: var a: i32 = 1; // trailing comment -// CHECK:STDERR: ^ -// CHECK:STDERR: -// CHECK:STDERR: fail_trailing_comments.carbon:[[@LINE-14]]:19: error: trailing comments are not permitted [TrailingComment] -// CHECK:STDERR: var b: 32 = 13; // more trailing comment -// CHECK:STDERR: ^ -// CHECK:STDERR: -// CHECK:STDERR: fail_trailing_comments.carbon:[[@LINE-17]]:19: error: trailing comments are not permitted [TrailingComment] -// CHECK:STDERR: var c: i32 = 0.4; // still more trailing comment -// CHECK:STDERR: ^ -// CHECK:STDERR: -// CHECK:STDOUT: - filename: fail_trailing_comments.carbon -// CHECK:STDOUT: tokens: -// CHECK:STDOUT: - { index: 1, kind: "Var", line: {{ *}}[[@LINE-25]], column: 1, indent: 1, spelling: "var", has_leading_space: true } -// CHECK:STDOUT: - { index: 2, kind: "Identifier", line: {{ *}}[[@LINE-26]], column: 5, indent: 1, spelling: "a", identifier: 0, has_leading_space: true } -// CHECK:STDOUT: - { index: 3, kind: "Colon", line: {{ *}}[[@LINE-27]], column: 6, indent: 1, spelling: ":" } -// CHECK:STDOUT: - { index: 4, kind: "IntTypeLiteral", line: {{ *}}[[@LINE-28]], column: 8, indent: 1, spelling: "i32", has_leading_space: true } -// CHECK:STDOUT: - { index: 5, kind: "Equal", line: {{ *}}[[@LINE-29]], column: 12, indent: 1, spelling: "=", has_leading_space: true } -// CHECK:STDOUT: - { index: 6, kind: "IntLiteral", line: {{ *}}[[@LINE-30]], column: 14, indent: 1, spelling: "1", value: "1", has_leading_space: true } -// CHECK:STDOUT: - { index: 7, kind: "Semi", line: {{ *}}[[@LINE-31]], column: 15, indent: 1, spelling: ";" } -// CHECK:STDOUT: - { index: 8, kind: "Var", line: {{ *}}[[@LINE-31]], column: 1, indent: 1, spelling: "var", has_leading_space: true } -// CHECK:STDOUT: - { index: 9, kind: "Identifier", line: {{ *}}[[@LINE-32]], column: 5, indent: 1, spelling: "b", identifier: 1, has_leading_space: true } -// CHECK:STDOUT: - { index: 10, kind: "Colon", line: {{ *}}[[@LINE-33]], column: 6, indent: 1, spelling: ":" } -// CHECK:STDOUT: - { index: 11, kind: "IntLiteral", line: {{ *}}[[@LINE-34]], column: 8, indent: 1, spelling: "32", value: "32", has_leading_space: true } -// CHECK:STDOUT: - { index: 12, kind: "Equal", line: {{ *}}[[@LINE-35]], column: 11, indent: 1, spelling: "=", has_leading_space: true } -// CHECK:STDOUT: - { index: 13, kind: "IntLiteral", line: {{ *}}[[@LINE-36]], column: 13, indent: 1, spelling: "13", value: "13", has_leading_space: true } -// CHECK:STDOUT: - { index: 14, kind: "Semi", line: {{ *}}[[@LINE-37]], column: 15, indent: 1, spelling: ";" } -// CHECK:STDOUT: - { index: 15, kind: "Var", line: {{ *}}[[@LINE-37]], column: 1, indent: 1, spelling: "var", has_leading_space: true } -// CHECK:STDOUT: - { index: 16, kind: "Identifier", line: {{ *}}[[@LINE-38]], column: 5, indent: 1, spelling: "c", identifier: 2, has_leading_space: true } -// CHECK:STDOUT: - { index: 17, kind: "Colon", line: {{ *}}[[@LINE-39]], column: 6, indent: 1, spelling: ":" } -// CHECK:STDOUT: - { index: 18, kind: "IntTypeLiteral", line: {{ *}}[[@LINE-40]], column: 8, indent: 1, spelling: "i32", has_leading_space: true } -// CHECK:STDOUT: - { index: 19, kind: "Equal", line: {{ *}}[[@LINE-41]], column: 12, indent: 1, spelling: "=", has_leading_space: true } -// CHECK:STDOUT: - { index: 20, kind: "RealLiteral", line: {{ *}}[[@LINE-42]], column: 14, indent: 1, spelling: "0.4", value: "4*10^-1", has_leading_space: true } -// CHECK:STDOUT: - { index: 21, kind: "Semi", line: {{ *}}[[@LINE-43]], column: 17, indent: 1, spelling: ";" } diff --git a/toolchain/lex/testdata/multiline_string_literals.carbon b/toolchain/lex/testdata/multiline_string_literals.carbon index fc70ce7c8f37..0157281bfd4f 100644 --- a/toolchain/lex/testdata/multiline_string_literals.carbon +++ b/toolchain/lex/testdata/multiline_string_literals.carbon @@ -23,6 +23,18 @@ a a """ +// --- introducer_trailing_comment.carbon + +// A trailing comment may follow the file type indicator on the introducer line. +// It is treated like trailing whitespace: it is part of the literal's introducer +// and produces no comment token, and it may contain `'`, `#`, and `"` even +// though the file type indicator may not. +// +// TODO: Surface this comment through the lexer's comment records. +var s: String = '''py // A trailing comment with ', #, and " in it. + content + '''; + // --- AUTOUPDATE-SPLIT // CHECK:STDERR: fail_indent_mismatch.carbon:3:1: error: indentation does not match that of the closing `'''` in multi-line string literal [MismatchedIndentInString] @@ -39,3 +51,12 @@ a // CHECK:STDOUT: - filename: fail_quotes.carbon // CHECK:STDOUT: tokens: // CHECK:STDOUT: - { index: 1, kind: "StringLiteral", line: {{ *}}2, column: 1, indent: 1, spelling: "\"\"\"\na\n\"\"\"", value: "a\n", has_leading_space: true } +// CHECK:STDOUT: - filename: introducer_trailing_comment.carbon +// CHECK:STDOUT: tokens: +// CHECK:STDOUT: - { index: 1, kind: "Var", line: {{ *}}8, column: 1, indent: 1, spelling: "var", has_leading_space: true } +// CHECK:STDOUT: - { index: 2, kind: "Identifier", line: {{ *}}8, column: 5, indent: 1, spelling: "s", identifier: 0, has_leading_space: true } +// CHECK:STDOUT: - { index: 3, kind: "Colon", line: {{ *}}8, column: 6, indent: 1, spelling: ":" } +// CHECK:STDOUT: - { index: 4, kind: "Identifier", line: {{ *}}8, column: 8, indent: 1, spelling: "String", identifier: 1, has_leading_space: true } +// CHECK:STDOUT: - { index: 5, kind: "Equal", line: {{ *}}8, column: 15, indent: 1, spelling: "=", has_leading_space: true } +// CHECK:STDOUT: - { index: 6, kind: "StringLiteral", line: {{ *}}8, column: 17, indent: 1, spelling: "'''py // A trailing comment with ', #, and \" in it.\n content\n '''", value: "content\n", has_leading_space: true } +// CHECK:STDOUT: - { index: 7, kind: "Semi", line: {{ *}}10, column: 6, indent: 3, spelling: ";" } diff --git a/toolchain/lex/testdata/trailing_comments.carbon b/toolchain/lex/testdata/trailing_comments.carbon new file mode 100644 index 000000000000..c4635233f079 --- /dev/null +++ b/toolchain/lex/testdata/trailing_comments.carbon @@ -0,0 +1,66 @@ +// Part of the Carbon Language project, under the Apache License v2.0 with LLVM +// Exceptions. See /LICENSE for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +// A trailing comment follows other content on a line. It produces no token and +// no error; the next token simply has leading whitespace. A trailing comment is +// never coalesced with the leading comments around it. +// +// AUTOUPDATE +// TIP: To test this file alone, run: +// TIP: bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/lex/testdata/trailing_comments.carbon +// TIP: To dump output, run: +// TIP: bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/lex/testdata/trailing_comments.carbon +// CHECK:STDOUT: - filename: trailing_comments.carbon +// CHECK:STDOUT: tokens: + +// A leading comment introduces the code below it. +var a: i32 = 1; // A trailing comment annotates the code on its line. +// CHECK:STDOUT: - { index: 1, kind: "Var", line: {{ *}}[[@LINE-1]], column: 1, indent: 1, spelling: "var", has_leading_space: true } +// CHECK:STDOUT: - { index: 2, kind: "Identifier", line: {{ *}}[[@LINE-2]], column: 5, indent: 1, spelling: "a", identifier: 0, has_leading_space: true } +// CHECK:STDOUT: - { index: 3, kind: "Colon", line: {{ *}}[[@LINE-3]], column: 6, indent: 1, spelling: ":" } +// CHECK:STDOUT: - { index: 4, kind: "IntTypeLiteral", line: {{ *}}[[@LINE-4]], column: 8, indent: 1, spelling: "i32", has_leading_space: true } +// CHECK:STDOUT: - { index: 5, kind: "Equal", line: {{ *}}[[@LINE-5]], column: 12, indent: 1, spelling: "=", has_leading_space: true } +// CHECK:STDOUT: - { index: 6, kind: "IntLiteral", line: {{ *}}[[@LINE-6]], column: 14, indent: 1, spelling: "1", value: "1", has_leading_space: true } +// CHECK:STDOUT: - { index: 7, kind: "Semi", line: {{ *}}[[@LINE-7]], column: 15, indent: 1, spelling: ";" } +var b: i32 = 2; // Another trailing comment. +// CHECK:STDOUT: - { index: 8, kind: "Var", line: {{ *}}[[@LINE-1]], column: 1, indent: 1, spelling: "var", has_leading_space: true } +// CHECK:STDOUT: - { index: 9, kind: "Identifier", line: {{ *}}[[@LINE-2]], column: 5, indent: 1, spelling: "b", identifier: 1, has_leading_space: true } +// CHECK:STDOUT: - { index: 10, kind: "Colon", line: {{ *}}[[@LINE-3]], column: 6, indent: 1, spelling: ":" } +// CHECK:STDOUT: - { index: 11, kind: "IntTypeLiteral", line: {{ *}}[[@LINE-4]], column: 8, indent: 1, spelling: "i32", has_leading_space: true } +// CHECK:STDOUT: - { index: 12, kind: "Equal", line: {{ *}}[[@LINE-5]], column: 12, indent: 1, spelling: "=", has_leading_space: true } +// CHECK:STDOUT: - { index: 13, kind: "IntLiteral", line: {{ *}}[[@LINE-6]], column: 14, indent: 1, spelling: "2", value: "2", has_leading_space: true } +// CHECK:STDOUT: - { index: 14, kind: "Semi", line: {{ *}}[[@LINE-7]], column: 15, indent: 1, spelling: ";" } + +// A trailing comment may directly follow a token with no space before `//`. The +// next line's first token (`var`) is still marked as having leading whitespace. +var c: i32 = 3;// no space before this comment +// CHECK:STDOUT: - { index: 15, kind: "Var", line: {{ *}}[[@LINE-1]], column: 1, indent: 1, spelling: "var", has_leading_space: true } +// CHECK:STDOUT: - { index: 16, kind: "Identifier", line: {{ *}}[[@LINE-2]], column: 5, indent: 1, spelling: "c", identifier: 2, has_leading_space: true } +// CHECK:STDOUT: - { index: 17, kind: "Colon", line: {{ *}}[[@LINE-3]], column: 6, indent: 1, spelling: ":" } +// CHECK:STDOUT: - { index: 18, kind: "IntTypeLiteral", line: {{ *}}[[@LINE-4]], column: 8, indent: 1, spelling: "i32", has_leading_space: true } +// CHECK:STDOUT: - { index: 19, kind: "Equal", line: {{ *}}[[@LINE-5]], column: 12, indent: 1, spelling: "=", has_leading_space: true } +// CHECK:STDOUT: - { index: 20, kind: "IntLiteral", line: {{ *}}[[@LINE-6]], column: 14, indent: 1, spelling: "3", value: "3", has_leading_space: true } +// CHECK:STDOUT: - { index: 21, kind: "Semi", line: {{ *}}[[@LINE-7]], column: 15, indent: 1, spelling: ";" } +var d: i32 = 4; +// CHECK:STDOUT: - { index: 22, kind: "Var", line: {{ *}}[[@LINE-1]], column: 1, indent: 1, spelling: "var", has_leading_space: true } +// CHECK:STDOUT: - { index: 23, kind: "Identifier", line: {{ *}}[[@LINE-2]], column: 5, indent: 1, spelling: "d", identifier: 3, has_leading_space: true } +// CHECK:STDOUT: - { index: 24, kind: "Colon", line: {{ *}}[[@LINE-3]], column: 6, indent: 1, spelling: ":" } +// CHECK:STDOUT: - { index: 25, kind: "IntTypeLiteral", line: {{ *}}[[@LINE-4]], column: 8, indent: 1, spelling: "i32", has_leading_space: true } +// CHECK:STDOUT: - { index: 26, kind: "Equal", line: {{ *}}[[@LINE-5]], column: 12, indent: 1, spelling: "=", has_leading_space: true } +// CHECK:STDOUT: - { index: 27, kind: "IntLiteral", line: {{ *}}[[@LINE-6]], column: 14, indent: 1, spelling: "4", value: "4", has_leading_space: true } +// CHECK:STDOUT: - { index: 28, kind: "Semi", line: {{ *}}[[@LINE-7]], column: 15, indent: 1, spelling: ";" } + +fn F() { +// CHECK:STDOUT: - { index: 29, kind: "Fn", line: {{ *}}[[@LINE-1]], column: 1, indent: 1, spelling: "fn", has_leading_space: true } +// CHECK:STDOUT: - { index: 30, kind: "Identifier", line: {{ *}}[[@LINE-2]], column: 4, indent: 1, spelling: "F", identifier: 4, has_leading_space: true } +// CHECK:STDOUT: - { index: 31, kind: "OpenParen", line: {{ *}}[[@LINE-3]], column: 5, indent: 1, spelling: "(", closing_token: 32 } +// CHECK:STDOUT: - { index: 32, kind: "CloseParen", line: {{ *}}[[@LINE-4]], column: 6, indent: 1, spelling: ")", opening_token: 31 } +// CHECK:STDOUT: - { index: 33, kind: "OpenCurlyBrace", line: {{ *}}[[@LINE-5]], column: 8, indent: 1, spelling: "{", closing_token: 38, has_leading_space: true } + G(); // Trailing comments may follow any content, including a `;`. + // CHECK:STDOUT: - { index: 34, kind: "Identifier", line: {{ *}}[[@LINE-1]], column: 3, indent: 3, spelling: "G", identifier: 5, has_leading_space: true } + // CHECK:STDOUT: - { index: 35, kind: "OpenParen", line: {{ *}}[[@LINE-2]], column: 4, indent: 3, spelling: "(", closing_token: 36 } + // CHECK:STDOUT: - { index: 36, kind: "CloseParen", line: {{ *}}[[@LINE-3]], column: 5, indent: 3, spelling: ")", opening_token: 35 } + // CHECK:STDOUT: - { index: 37, kind: "Semi", line: {{ *}}[[@LINE-4]], column: 6, indent: 3, spelling: ";" } +} +// CHECK:STDOUT: - { index: 38, kind: "CloseCurlyBrace", line: {{ *}}[[@LINE-1]], column: 1, indent: 1, spelling: "}", opening_token: 33, has_leading_space: true } diff --git a/toolchain/lex/tokenized_buffer.cpp b/toolchain/lex/tokenized_buffer.cpp index c3509d0c7d6c..0d20a3bad2dc 100644 --- a/toolchain/lex/tokenized_buffer.cpp +++ b/toolchain/lex/tokenized_buffer.cpp @@ -413,16 +413,33 @@ auto TokenizedBuffer::GetCommentText(CommentIndex comment_index) const return source_->text().substr(comment_data.start, comment_data.length); } -auto TokenizedBuffer::AddComment(int32_t indent, int32_t start, int32_t end) - -> void { - if (comments_.size() > 0) { +auto TokenizedBuffer::IsTrailingComment(CommentIndex comment_index) const + -> bool { + return comments_.Get(comment_index).is_trailing; +} + +auto TokenizedBuffer::AddComment(int32_t indent, int32_t start, int32_t end, + bool is_trailing) -> void { + // A comment runs forward from its start, and its length is stored in 31 bits + // (the high bit holds `is_trailing`). A non-negative byte offset always fits + // in 31 bits because the source size is bounded by `INT32_MAX`. + CARBON_DCHECK(start <= end); + + // A block of identical full-line comments is coalesced into a single comment. + // A trailing comment is always standalone: it never extends a preceding + // comment, nor is it extended by a following one. + if (!is_trailing && comments_.size() > 0) { auto& comment = comments_.Get(CommentIndex(comments_.size() - 1)); - if (comment.start + comment.length + indent == start) { + if (!comment.is_trailing && + comment.start + comment.length + indent == start) { + CARBON_DCHECK(comment.start <= end); comment.length = end - comment.start; return; } } - comments_.Add({.start = start, .length = end - start}); + comments_.Add({.start = start, + .length = static_cast(end - start), + .is_trailing = is_trailing}); } auto TokenizedBuffer::CollectMemUsage(MemUsage& mem_usage, diff --git a/toolchain/lex/tokenized_buffer.h b/toolchain/lex/tokenized_buffer.h index 780ce9f3b586..fe02d6bd1ab5 100644 --- a/toolchain/lex/tokenized_buffer.h +++ b/toolchain/lex/tokenized_buffer.h @@ -68,9 +68,18 @@ struct CommentData { // buffer provided. int32_t start; - // The comment's length. - int32_t length; + // The comment's length, in the low 31 bits. The high bit is stolen for + // `is_trailing` below to keep `CommentData` at 8 bytes. A byte length is + // non-negative and the source size is required to be less than `INT32_MAX` + // (see `Lex`), so the length always fits in 31 bits. + uint32_t length : 31; + + // Whether this is a _trailing_ comment: one that follows other content on its + // line, rather than being the only non-whitespace on the line. Trailing + // comments are never coalesced with adjacent comments. + uint32_t is_trailing : 1; }; +static_assert(sizeof(CommentData) == 8, "CommentData should pack to 8 bytes"); // Indices for `CommentData` within the buffer. struct CommentIndex : public IndexBase { @@ -196,6 +205,10 @@ class TokenizedBuffer : public Printable { // Returns the comment's full text range. auto GetCommentText(CommentIndex comment_index) const -> llvm::StringRef; + // Returns whether the comment is a trailing comment: one that follows other + // content on its line. + auto IsTrailingComment(CommentIndex comment_index) const -> bool; + // Returns tokens as YAML. This prints the tracked token information on a // single line for each token. We use the single-line format so that output is // compact, and so that tools like `grep` are compatible. @@ -335,9 +348,12 @@ class TokenizedBuffer : public Printable { auto PrintToken(llvm::raw_ostream& output_stream, TokenIndex token, PrintWidths widths) const -> void; - // Adds a comment. This uses the indent to potentially stitch together two - // adjacent comments. - auto AddComment(int32_t indent, int32_t start, int32_t end) -> void; + // Adds a comment. For full-line comments (`is_trailing` is false), this uses + // the indent to potentially stitch together two adjacent comments into a + // single block. Trailing comments are never stitched, either onto a preceding + // comment or as the target of a following one. + auto AddComment(int32_t indent, int32_t start, int32_t end, bool is_trailing) + -> void; // Used to allocate computed string literals. llvm::BumpPtrAllocator allocator_; diff --git a/toolchain/lex/tokenized_buffer_test.cpp b/toolchain/lex/tokenized_buffer_test.cpp index e7c824de6603..051031959865 100644 --- a/toolchain/lex/tokenized_buffer_test.cpp +++ b/toolchain/lex/tokenized_buffer_test.cpp @@ -90,7 +90,7 @@ TEST_F(LexerTest, TracksLinesAndColumns) { {.kind = TokenKind::Identifier, .line = 6, .column = 6, - .indent_column = 11, + .indent_column = 2, .text = "y"}, {.kind = TokenKind::FileEnd, .line = 6, .column = 7}, })); @@ -128,7 +128,7 @@ TEST_F(LexerTest, TracksLinesAndColumnsCrLf) { {.kind = TokenKind::Identifier, .line = 6, .column = 6, - .indent_column = 11, + .indent_column = 2, .text = "y"}, {.kind = TokenKind::FileEnd, .line = 6, .column = 7}, })); @@ -722,7 +722,6 @@ TEST_F(LexerTest, Comments) { TEST_F(LexerTest, InvalidComments) { llvm::StringLiteral testcases[] = { " /// foo\n", - "foo // bar\n", "//! hello", " //world", }; @@ -859,7 +858,7 @@ TEST_F(LexerTest, StringLiterals) { {.kind = TokenKind::Identifier, .line = 7, .column = 10, - .indent_column = 5, + .indent_column = 6, .text = "trailing"}, {.kind = TokenKind::StringLiteral, .line = 9, @@ -1098,17 +1097,32 @@ TEST_F(LexerTest, TypeLiteralTooManyDigits) { })); } -TEST_F(LexerTest, DiagnosticTrailingComment) { - llvm::StringLiteral testcase = R"( - // Hello! - var String x; // trailing comment - )"; +TEST_F(LexerTest, TrailingComment) { + // A comment that follows other content on a line is a valid trailing comment. + auto& buffer = compile_helper_.GetTokenizedBuffer( + "// leading\nvar x: i32 = 0; // trailing\n// trailing's neighbor\n"); + EXPECT_FALSE(buffer.has_errors()); - Testing::MockDiagnosticConsumer consumer; - EXPECT_CALL(consumer, HandleDiagnostic(IsSingleDiagnostic( - Diagnostics::Kind::TrailingComment, - Diagnostics::Level::Error, 3, 19, _))); - compile_helper_.GetTokenizedBuffer(testcase, &consumer); + // The trailing comment is recorded but never coalesced with an adjacent + // full-line comment, so the leading comment, the trailing comment, and the + // following full-line comment remain three separate comments. + EXPECT_THAT(buffer.comments_size(), Eq(3)); +} + +TEST_F(LexerTest, TrailingCommentAfterMultiLineString) { + // A multi-line string literal records the real indentation of each line it + // spans. Here the trailing `//` is deliberately aligned to the column where + // the literal opened (both at column 17); if the literal instead recorded + // that opening column as the final line's indent (as it once did), the + // trailing-comment check in `Lexer::LexComment` would misclassify this as a + // full-line comment. + auto& buffer = compile_helper_.GetTokenizedBuffer( + "var x: String = '''\n" + " text\n" + " '''; // trailing\n"); + EXPECT_FALSE(buffer.has_errors()); + ASSERT_THAT(buffer.comments_size(), Eq(1)); + EXPECT_TRUE(buffer.IsTrailingComment(CommentIndex(0))); } TEST_F(LexerTest, DiagnosticWhitespace) { diff --git a/toolchain/parse/context.cpp b/toolchain/parse/context.cpp index 2e1e201f88e0..c71316bdca07 100644 --- a/toolchain/parse/context.cpp +++ b/toolchain/parse/context.cpp @@ -145,18 +145,18 @@ auto Context::SkipPastLikelyEnd(Lex::TokenIndex skip_root) -> Lex::TokenIndex { return *(position_ - 1); } - Lex::LineIndex root_line = tokens().GetLine(skip_root); - int root_line_indent = tokens().GetIndentColumnNumber(root_line); + int root_line_indent = + tokens().GetIndentColumnNumber(tokens().GetLine(skip_root)); - // We will keep scanning through tokens on the same line as the root or - // lines with greater indentation than root's line. - auto is_same_line_or_indent_greater_than_root = [&](Lex::TokenIndex t) { - Lex::LineIndex l = tokens().GetLine(t); - if (l == root_line) { - return true; - } - - return tokens().GetIndentColumnNumber(l) > root_line_indent; + // We keep scanning through tokens that don't start their own line and + // through lines indented more than the root's line. Tokens that don't start + // their line include the rest of the root's line and, because comments run + // to the end of the line, tokens after a multi-line string literal's + // closing delimiter (as in `''' + "more"`), both of which continue the + // construct regardless of indentation. + auto keep_scanning = [&](Lex::TokenIndex t) { + int indent = tokens().GetIndentColumnNumber(tokens().GetLine(t)); + return tokens().GetColumnNumber(t) > indent || indent > root_line_indent; }; do { @@ -179,8 +179,7 @@ auto Context::SkipPastLikelyEnd(Lex::TokenIndex skip_root) -> Lex::TokenIndex { // Otherwise just step forward one token. ++position_; - } while (position_ != end_ && - is_same_line_or_indent_greater_than_root(*position_)); + } while (position_ != end_ && keep_scanning(*position_)); return *(position_ - 1); } diff --git a/toolchain/parse/testdata/basics/multiline_string_recovery.carbon b/toolchain/parse/testdata/basics/multiline_string_recovery.carbon new file mode 100644 index 000000000000..2c57f51c5df1 --- /dev/null +++ b/toolchain/parse/testdata/basics/multiline_string_recovery.carbon @@ -0,0 +1,57 @@ +// Part of the Carbon Language project, under the Apache License v2.0 with LLVM +// Exceptions. See /LICENSE for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +// Error recovery scans across the lines a multi-line string literal spans, +// even when the literal's lines are less indented than the erroneous construct +// and the literal is inside a skipped bracketed group. +// +// AUTOUPDATE +// TIP: To test this file alone, run: +// TIP: bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/parse/testdata/basics/multiline_string_recovery.carbon +// TIP: To dump output, run: +// TIP: bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/parse/testdata/basics/multiline_string_recovery.carbon + +// --- fail_string_in_skipped_group.carbon + +fn F() { + // The error skips past the string literal's group and the tokens after the + // closing delimiter, ending the declaration at the `;` so that the following + // declaration parses cleanly. + // CHECK:STDERR: fail_string_in_skipped_group.carbon:[[@LINE+4]]:20: error: `var` declarations must end with a `;` [ExpectedDeclSemi] + // CHECK:STDERR: var x: i32 = bad bad (''' + // CHECK:STDERR: ^~~ + // CHECK:STDERR: + var x: i32 = bad bad (''' +str +''') and_more; + var y: i32 = 0; +} + +// CHECK:STDOUT: - filename: fail_string_in_skipped_group.carbon +// CHECK:STDOUT: parse_tree: [ +// CHECK:STDOUT: {kind: 'FileStart', text: ''}, +// CHECK:STDOUT: {kind: 'FunctionIntroducer', text: 'fn'}, +// CHECK:STDOUT: {kind: 'IdentifierNameMaybeBeforeSignature', text: 'F'}, +// CHECK:STDOUT: {kind: 'ExplicitParamListStart', text: '('}, +// CHECK:STDOUT: {kind: 'ExplicitParamList', text: ')', subtree_size: 2}, +// CHECK:STDOUT: {kind: 'FunctionDefinitionStart', text: '{', subtree_size: 5}, +// CHECK:STDOUT: {kind: 'VariableIntroducer', text: 'var'}, +// CHECK:STDOUT: {kind: 'IdentifierNameNotBeforeSignature', text: 'x'}, +// CHECK:STDOUT: {kind: 'IntTypeLiteral', text: 'i32'}, +// CHECK:STDOUT: {kind: 'VarBindingPattern', text: ':', subtree_size: 3}, +// CHECK:STDOUT: {kind: 'VariablePattern', text: 'var', subtree_size: 4}, +// CHECK:STDOUT: {kind: 'VariableInitializer', text: '='}, +// CHECK:STDOUT: {kind: 'IdentifierNameExpr', text: 'bad'}, +// CHECK:STDOUT: {kind: 'VariableDecl', text: ';', has_error: yes, subtree_size: 8}, +// CHECK:STDOUT: {kind: 'VariableIntroducer', text: 'var'}, +// CHECK:STDOUT: {kind: 'IdentifierNameNotBeforeSignature', text: 'y'}, +// CHECK:STDOUT: {kind: 'IntTypeLiteral', text: 'i32'}, +// CHECK:STDOUT: {kind: 'VarBindingPattern', text: ':', subtree_size: 3}, +// CHECK:STDOUT: {kind: 'VariablePattern', text: 'var', subtree_size: 4}, +// CHECK:STDOUT: {kind: 'VariableInitializer', text: '='}, +// CHECK:STDOUT: {kind: 'IntLiteral', text: '0'}, +// CHECK:STDOUT: {kind: 'VariableDecl', text: ';', subtree_size: 8}, +// CHECK:STDOUT: {kind: 'FunctionDefinition', text: '}', subtree_size: 22}, +// CHECK:STDOUT: {kind: 'FileEnd', text: ''}, +// CHECK:STDOUT: ] diff --git a/toolchain/parse/testdata/class/fail_base.carbon b/toolchain/parse/testdata/class/fail_base.carbon index d607f47aedf8..486eb0e80e46 100644 --- a/toolchain/parse/testdata/class/fail_base.carbon +++ b/toolchain/parse/testdata/class/fail_base.carbon @@ -29,14 +29,10 @@ class A { base: { } // We should resume parsing here after the previous error. - // CHECK:STDERR: fail_base.carbon:[[@LINE+8]]:3: error: `base` declarations must end with a `;` [ExpectedDeclSemi] + // CHECK:STDERR: fail_base.carbon:[[@LINE+4]]:3: error: `base` declarations must end with a `;` [ExpectedDeclSemi] // CHECK:STDERR: var n: i32; // CHECK:STDERR: ^~~ // CHECK:STDERR: - // CHECK:STDERR: fail_base.carbon:[[@LINE+4]]:7: error: unrecognized declaration introducer [UnrecognizedDecl] - // CHECK:STDERR: var n: i32; - // CHECK:STDERR: ^ - // CHECK:STDERR: var n: i32; } @@ -68,10 +64,8 @@ class B { // CHECK:STDOUT: {kind: 'BaseColon', text: ':'}, // CHECK:STDOUT: {kind: 'StructLiteralStart', text: '{'}, // CHECK:STDOUT: {kind: 'StructLiteral', text: '}', subtree_size: 2}, -// CHECK:STDOUT: {kind: 'BaseDecl', text: 'var', has_error: yes, subtree_size: 5}, -// CHECK:STDOUT: {kind: 'InvalidParseStart', text: 'n', has_error: yes}, -// CHECK:STDOUT: {kind: 'InvalidParseSubtree', text: ';', has_error: yes, subtree_size: 2}, -// CHECK:STDOUT: {kind: 'ClassDefinition', text: '}', subtree_size: 19}, +// CHECK:STDOUT: {kind: 'BaseDecl', text: ';', has_error: yes, subtree_size: 5}, +// CHECK:STDOUT: {kind: 'ClassDefinition', text: '}', subtree_size: 17}, // CHECK:STDOUT: {kind: 'ClassIntroducer', text: 'class'}, // CHECK:STDOUT: {kind: 'BaseModifier', text: 'base'}, // CHECK:STDOUT: {kind: 'IdentifierNameNotBeforeSignature', text: 'Foo'}, diff --git a/toolchain/parse/testdata/namespace/fail_modifiers.carbon b/toolchain/parse/testdata/namespace/fail_modifiers.carbon index fb9e53703c18..989c1a2c6abb 100644 --- a/toolchain/parse/testdata/namespace/fail_modifiers.carbon +++ b/toolchain/parse/testdata/namespace/fail_modifiers.carbon @@ -10,14 +10,10 @@ virtual namespace B -// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+8]]:1: error: `namespace` declarations must end with a `;` [ExpectedDeclSemi] +// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+4]]:1: error: `namespace` declarations must end with a `;` [ExpectedDeclSemi] // CHECK:STDERR: impl namespace // CHECK:STDERR: ^~~~ // CHECK:STDERR: -// CHECK:STDERR: fail_modifiers.carbon:[[@LINE+4]]:6: error: `namespace` introducer should be followed by a name [ExpectedDeclName] -// CHECK:STDERR: impl namespace -// CHECK:STDERR: ^~~~~~~~~ -// CHECK:STDERR: impl namespace // CHECK:STDOUT: - filename: fail_modifiers.carbon @@ -26,9 +22,6 @@ impl namespace // CHECK:STDOUT: {kind: 'NamespaceStart', text: 'namespace'}, // CHECK:STDOUT: {kind: 'VirtualModifier', text: 'virtual'}, // CHECK:STDOUT: {kind: 'IdentifierNameNotBeforeSignature', text: 'B'}, -// CHECK:STDOUT: {kind: 'Namespace', text: 'impl', has_error: yes, subtree_size: 4}, -// CHECK:STDOUT: {kind: 'NamespaceStart', text: 'namespace'}, -// CHECK:STDOUT: {kind: 'InvalidParse', text: '', has_error: yes}, -// CHECK:STDOUT: {kind: 'Namespace', text: 'namespace', has_error: yes, subtree_size: 3}, +// CHECK:STDOUT: {kind: 'Namespace', text: 'namespace', has_error: yes, subtree_size: 4}, // CHECK:STDOUT: {kind: 'FileEnd', text: ''}, // CHECK:STDOUT: ] diff --git a/toolchain/parse/testdata/packages/import/cpp_inline.carbon b/toolchain/parse/testdata/packages/import/cpp_inline.carbon index 2251b54b9036..90d1fd6ba7a3 100644 --- a/toolchain/parse/testdata/packages/import/cpp_inline.carbon +++ b/toolchain/parse/testdata/packages/import/cpp_inline.carbon @@ -94,6 +94,16 @@ inline ''' int n; '''; +// Recovery skips to the `;` even when tokens follow the closing `'''` on its +// line, so the `;` ends the declaration rather than starting an empty one. +// CHECK:STDERR: fail_inline_cpp_syntax_errors.carbon:[[@LINE+4]]:8: error: expected `Cpp` after `inline` [ExpectedCppAfterInline] +// CHECK:STDERR: inline ''' +// CHECK:STDERR: ^~~ +// CHECK:STDERR: +inline ''' +int n; +''' + "more"; + // CHECK:STDERR: fail_inline_cpp_syntax_errors.carbon:[[@LINE+4]]:11: error: expected string literal after `inline Cpp` [ExpectedStringAfterInlineCpp] // CHECK:STDERR: inline Cpp; // CHECK:STDERR: ^ @@ -215,6 +225,8 @@ int n; // CHECK:STDOUT: {kind: 'InlineIntroducer', text: 'inline'}, // CHECK:STDOUT: {kind: 'InlineCppDecl', text: ';', has_error: yes, subtree_size: 2}, // CHECK:STDOUT: {kind: 'InlineIntroducer', text: 'inline'}, +// CHECK:STDOUT: {kind: 'InlineCppDecl', text: ';', has_error: yes, subtree_size: 2}, +// CHECK:STDOUT: {kind: 'InlineIntroducer', text: 'inline'}, // CHECK:STDOUT: {kind: 'CppNameExpr', text: 'Cpp'}, // CHECK:STDOUT: {kind: 'InlineCppDecl', text: ';', has_error: yes, subtree_size: 3}, // CHECK:STDOUT: {kind: 'InlineIntroducer', text: 'inline'},