This reflects how we're naming classes that derive from these classes,
and matches usage for each existing `Id` and `Index` type, except:
- `Parse::NodeId` previously inherited from `ComparableIndexBase`, and
is no longer comparable.
- `SemIR::MemberIndex` previously inherited from `IndexBase`, and is now
comparable.
Making `Parse::NodeId` non-comparable reflects that it's intended to be
an opaque identifier for a node and that the ordering is an
implementation detail rather than part of the intended public interface.
`PostorderIterator` and `SiblingIterator` still rely on the numerical
meaning of `NodeId`s, but that's OK since they're part of the node
implementation.
Per [#toolchain
discussion](https://discord.com/channels/655572317891461132/655578254970716160/1176632520834560211)
We'd at one point been trying to put `[[nodiscard]]` everywhere, but
then we stopped because it had felt verbose without finding many issues
(plus, people plain forgot to add it). Some history in #888.
Since newer code gets added without it, we now have code like:
```
auto GetLineInfo(Line line) -> LineInfo&;
[[nodiscard]] auto GetLineInfo(Line line) const -> const LineInfo&;
auto AddLine(LineInfo info) -> Line;
auto GetTokenInfo(Token token) -> TokenInfo&;
[[nodiscard]] auto GetTokenInfo(Token token) const -> const TokenInfo&;
auto AddToken(TokenInfo info) -> Token;
[[nodiscard]] auto GetTokenPrintWidths(Token token) const -> PrintWidths;
```
Here, the lack of `[[nodiscard]]` doesn't mean anything: for example,
`GetLineInfo` should not have its result discarded if it's called. But
the mix could be confusing for readers.
As a resolution, remove the attribute. `[[nodiscard]]` should be treated
like other attributes going forward, which essentially means "avoid in
general, add a comment to explain why the attribute is needed" rather
than use-as-default.
As I was working on this, I noticed `import` and `library` syntax needs
to be fixed for how it imports the current package, and for `Main`
libraries. This mostly reflects the current state in its testing.
Otherwise, this should handle most of the errors I could think of:
dependency cycles, redundant imports, etc.
It does not actually deal with the nuances of cross-IR references.
---------
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
Building on #3311, change SemIR to use the SharedValueStore. Since this
removes hermeticity, raw output no longer prints ints, reals, and
strings. TokenizedBuffer accessors are modified to return IDs because
values are often passed through in semantics without needing to read
them.
I would've put SharedValueStores on Context, except for the
GetArrayBoundValue convenience method. I felt awkward removing that, so
it's on File, at least for now. That's then used by the formatter and
Lower too. The flipside of this is that TokenizedBuffer has a
SharedValueStores only for printing, so maybe that's similar enough to
what File is doing.
This doesn't start shifting other SemIR members to ValueStore, but that
seems like a next step.
Continuing with #3070. Just a dir and file rename (only prefix change is
lexer_file_test). Everything in the lex dir should be marked as a move.
Note, I think this closes#3070. There may still be further cleanup
later, but the organizational changes suggested there are being
completed.
---------
Co-authored-by: Chandler Carruth <chandlerc@gmail.com>
Continuing with #3070. Just a dir and file rename (mostly removing
prefixes, although for parse_tree_fuzzer and parse_tree_file_test I'm
dropping "tree" instead of "parse"). Everything in the parse dir should
be marked as a move.
Richard Smith
Jon Ross-Perkins
