Remove nodiscard uses. (#3418)

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.

common/enum_base.h

@@ -133,7 +133,7 @@ class EnumBase : public Printable<DerivedT> {
   // This method will be automatically defined using the static `names` string
   // table in the base class, which is in turn will be populated for each
   // derived type using the macro helpers in this file.
-  [[nodiscard]] auto name() const -> llvm::StringRef { return Names[AsInt()]; }
+  auto name() const -> llvm::StringRef { return Names[AsInt()]; }
 
   // Prints this value using its name.
   auto Print(llvm::raw_ostream& out) const -> void { out << name(); }

common/error.h

@@ -20,6 +20,8 @@ namespace Carbon {
 struct Success {};
 
 // Tracks an error message.
+//
+// This is nodiscard to enforce error handling prior to destruction.
 class [[nodiscard]] Error : public Printable<Error> {
  public:
   // Represents an error state.
@@ -66,6 +68,8 @@ class [[nodiscard]] Error : public Printable<Error> {
 
 // Holds a value of type `T`, or an Error explaining why the value is
 // unavailable.
+//
+// This is nodiscard to enforce error handling prior to destruction.
 template <typename T>
 class [[nodiscard]] ErrorOr {
  public:
@@ -137,7 +141,7 @@ class ErrorBuilder {
   // Accumulates string message to a temporary `ErrorBuilder`. After streaming,
   // the builder must be converted to an `Error` or `ErrorOr`.
   template <typename T>
-  [[nodiscard]] auto operator<<(const T& message) && -> ErrorBuilder&& {
+  auto operator<<(const T& message) && -> ErrorBuilder&& {
     *out_ << message;
     return std::move(*this);
   }

toolchain/diagnostics/diagnostic_emitter.h

@@ -147,8 +147,7 @@ class DiagnosticLocationTranslator {
  public:
   virtual ~DiagnosticLocationTranslator() = default;
 
-  [[nodiscard]] virtual auto GetLocation(LocationT loc)
-      -> DiagnosticLocation = 0;
+  virtual auto GetLocation(LocationT loc) -> DiagnosticLocation = 0;
 };
 
 namespace Internal {

toolchain/lex/numeric_literal.h

@@ -49,7 +49,7 @@ class NumericLiteral {
   auto ComputeValue(DiagnosticEmitter<const char*>& emitter) const -> Value;
 
   // Get the text corresponding to this literal.
-  [[nodiscard]] auto text() const -> llvm::StringRef { return text_; }
+  auto text() const -> llvm::StringRef { return text_; }
 
  private:
   class Parser;

toolchain/lex/string_literal.h

@@ -33,13 +33,13 @@ class StringLiteral {
       -> llvm::StringRef;
 
   // Get the text corresponding to this literal.
-  [[nodiscard]] auto text() const -> llvm::StringRef { return text_; }
+  auto text() const -> llvm::StringRef { return text_; }
 
   // Determine whether this is a multi-line string literal.
-  [[nodiscard]] auto is_multi_line() const -> bool { return multi_line_; }
+  auto is_multi_line() const -> bool { return multi_line_; }
 
   // Returns true if the string has a valid terminator.
-  [[nodiscard]] auto is_terminated() const -> bool { return is_terminated_; }
+  auto is_terminated() const -> bool { return is_terminated_; }
 
  private:
   enum MultiLineKind : int8_t {

toolchain/lex/token_kind.h

@@ -36,37 +36,31 @@ class TokenKind : public CARBON_ENUM_BASE(TokenKind) {
   // inside of other tokens, outside of the contents of other tokens they
   // don't require any specific characters before or after to distinguish them
   // in the source. Returns false otherwise.
-  [[nodiscard]] auto is_symbol() const -> bool { return IsSymbol[AsInt()]; }
+  auto is_symbol() const -> bool { return IsSymbol[AsInt()]; }
 
   // Test whether this kind of token is a grouping symbol (part of an opening
   // and closing pair that must always be matched in the token stream).
-  [[nodiscard]] auto is_grouping_symbol() const -> bool {
-    return IsGroupingSymbol[AsInt()];
-  }
+  auto is_grouping_symbol() const -> bool { return IsGroupingSymbol[AsInt()]; }
 
   // Test whether this kind of token is an opening symbol for a group.
-  [[nodiscard]] auto is_opening_symbol() const -> bool {
-    return IsOpeningSymbol[AsInt()];
-  }
+  auto is_opening_symbol() const -> bool { return IsOpeningSymbol[AsInt()]; }
 
   // Returns the associated closing symbol for an opening symbol.
   //
   // The token kind must be an opening symbol.
-  [[nodiscard]] auto closing_symbol() const -> TokenKind {
+  auto closing_symbol() const -> TokenKind {
     auto result = ClosingSymbol[AsInt()];
     CARBON_DCHECK(result != Error) << "Only opening symbols are valid!";
     return result;
   }
 
   // Test whether this kind of token is a closing symbol for a group.
-  [[nodiscard]] auto is_closing_symbol() const -> bool {
-    return IsClosingSymbol[AsInt()];
-  }
+  auto is_closing_symbol() const -> bool { return IsClosingSymbol[AsInt()]; }
 
   // Returns the associated opening symbol for a closing symbol.
   //
   // The token kind must be a closing symbol.
-  [[nodiscard]] auto opening_symbol() const -> TokenKind {
+  auto opening_symbol() const -> TokenKind {
     auto result = OpeningSymbol[AsInt()];
     CARBON_DCHECK(result != Error) << "Only closing symbols are valid!";
     return result;
@@ -74,15 +68,13 @@ class TokenKind : public CARBON_ENUM_BASE(TokenKind) {
 
   // Test whether this kind of token is a one-character symbol whose character
   // is not part of any other symbol.
-  [[nodiscard]] auto is_one_char_symbol() const -> bool {
-    return IsOneCharSymbol[AsInt()];
-  };
+  auto is_one_char_symbol() const -> bool { return IsOneCharSymbol[AsInt()]; };
 
   // Test whether this kind of token is a keyword.
-  [[nodiscard]] auto is_keyword() const -> bool { return IsKeyword[AsInt()]; };
+  auto is_keyword() const -> bool { return IsKeyword[AsInt()]; };
 
   // Test whether this kind of token is a sized type literal.
-  [[nodiscard]] auto is_sized_type_literal() const -> bool {
+  auto is_sized_type_literal() const -> bool {
     return *this == TokenKind::IntegerTypeLiteral ||
            *this == TokenKind::UnsignedIntegerTypeLiteral ||
            *this == TokenKind::FloatingPointTypeLiteral;
@@ -90,19 +82,18 @@ class TokenKind : public CARBON_ENUM_BASE(TokenKind) {
 
   // If this token kind has a fixed spelling when in source code, returns it.
   // Otherwise returns an empty string.
-  [[nodiscard]] auto fixed_spelling() const -> llvm::StringRef {
+  auto fixed_spelling() const -> llvm::StringRef {
     return FixedSpelling[AsInt()];
   };
 
   // Get the expected number of parse tree nodes that will be created for this
   // token.
-  [[nodiscard]] auto expected_parse_tree_size() const -> int {
+  auto expected_parse_tree_size() const -> int {
     return ExpectedParseTreeSize[AsInt()];
   }
 
   // Test whether this token kind is in the provided list.
-  [[nodiscard]] auto IsOneOf(std::initializer_list<TokenKind> kinds) const
-      -> bool {
+  auto IsOneOf(std::initializer_list<TokenKind> kinds) const -> bool {
     for (TokenKind kind : kinds) {
       if (*this == kind) {
         return true;

toolchain/lex/tokenized_buffer.h

@@ -133,66 +133,65 @@ class TokenLocationTranslator : public DiagnosticLocationTranslator<Token> {
 // `HasError` returning true.
 class TokenizedBuffer : public Printable<TokenizedBuffer> {
  public:
-  [[nodiscard]] auto GetKind(Token token) const -> TokenKind;
-  [[nodiscard]] auto GetLine(Token token) const -> Line;
+  auto GetKind(Token token) const -> TokenKind;
+  auto GetLine(Token token) const -> Line;
 
   // Returns the 1-based line number.
-  [[nodiscard]] auto GetLineNumber(Token token) const -> int;
+  auto GetLineNumber(Token token) const -> int;
 
   // Returns the 1-based column number.
-  [[nodiscard]] auto GetColumnNumber(Token token) const -> int;
+  auto GetColumnNumber(Token token) const -> int;
 
   // Returns the source text lexed into this token.
-  [[nodiscard]] auto GetTokenText(Token token) const -> llvm::StringRef;
+  auto GetTokenText(Token token) const -> llvm::StringRef;
 
   // Returns the identifier associated with this token. The token kind must be
   // an `Identifier`.
-  [[nodiscard]] auto GetIdentifier(Token token) const -> IdentifierId;
+  auto GetIdentifier(Token token) const -> IdentifierId;
 
   // Returns the value of an `IntegerLiteral()` token.
-  [[nodiscard]] auto GetIntegerLiteral(Token token) const -> IntegerId;
+  auto GetIntegerLiteral(Token token) const -> IntegerId;
 
   // Returns the value of an `RealLiteral()` token.
-  [[nodiscard]] auto GetRealLiteral(Token token) const -> RealId;
+  auto GetRealLiteral(Token token) const -> RealId;
 
   // Returns the value of a `StringLiteral()` token.
-  [[nodiscard]] auto GetStringLiteral(Token token) const -> StringLiteralId;
+  auto GetStringLiteral(Token token) const -> StringLiteralId;
 
   // Returns the size specified in a `*TypeLiteral()` token.
-  [[nodiscard]] auto GetTypeLiteralSize(Token token) const
-      -> const llvm::APInt&;
+  auto GetTypeLiteralSize(Token token) const -> const llvm::APInt&;
 
   // Returns the closing token matched with the given opening token.
   //
   // The given token must be an opening token kind.
-  [[nodiscard]] auto GetMatchedClosingToken(Token opening_token) const -> Token;
+  auto GetMatchedClosingToken(Token opening_token) const -> Token;
 
   // Returns the opening token matched with the given closing token.
   //
   // The given token must be a closing token kind.
-  [[nodiscard]] auto GetMatchedOpeningToken(Token closing_token) const -> Token;
+  auto GetMatchedOpeningToken(Token closing_token) const -> Token;
 
   // Returns whether the given token has leading whitespace.
-  [[nodiscard]] auto HasLeadingWhitespace(Token token) const -> bool;
+  auto HasLeadingWhitespace(Token token) const -> bool;
   // Returns whether the given token has trailing whitespace.
-  [[nodiscard]] auto HasTrailingWhitespace(Token token) const -> bool;
+  auto HasTrailingWhitespace(Token token) const -> bool;
 
   // Returns whether the token was created as part of an error recovery effort.
   //
   // For example, a closing paren inserted to match an unmatched paren.
-  [[nodiscard]] auto IsRecoveryToken(Token token) const -> bool;
+  auto IsRecoveryToken(Token token) const -> bool;
 
   // Returns the 1-based line number.
-  [[nodiscard]] auto GetLineNumber(Line line) const -> int;
+  auto GetLineNumber(Line line) const -> int;
 
   // Returns the 1-based indentation column number.
-  [[nodiscard]] auto GetIndentColumnNumber(Line line) const -> int;
+  auto GetIndentColumnNumber(Line line) const -> int;
 
   // Returns the next line handle.
-  [[nodiscard]] auto GetNextLine(Line line) const -> Line;
+  auto GetNextLine(Line line) const -> Line;
 
   // Returns the previous line handle.
-  [[nodiscard]] auto GetPrevLine(Line line) const -> Line;
+  auto GetPrevLine(Line line) const -> Line;
 
   // Prints a description of the tokenized stream to the provided `raw_ostream`.
   //
@@ -220,16 +219,16 @@ class TokenizedBuffer : public Printable<TokenizedBuffer> {
   auto PrintToken(llvm::raw_ostream& output_stream, Token token) const -> void;
 
   // Returns true if the buffer has errors that were detected at lexing time.
-  [[nodiscard]] auto has_errors() const -> bool { return has_errors_; }
+  auto has_errors() const -> bool { return has_errors_; }
 
-  [[nodiscard]] auto tokens() const -> llvm::iterator_range<TokenIterator> {
+  auto tokens() const -> llvm::iterator_range<TokenIterator> {
     return llvm::make_range(TokenIterator(Token(0)),
                             TokenIterator(Token(token_infos_.size())));
   }
 
-  [[nodiscard]] auto size() const -> int { return token_infos_.size(); }
+  auto size() const -> int { return token_infos_.size(); }
 
-  [[nodiscard]] auto expected_parse_tree_size() const -> int {
+  auto expected_parse_tree_size() const -> int {
     return expected_parse_tree_size_;
   }
 
@@ -332,12 +331,12 @@ class TokenizedBuffer : public Printable<TokenizedBuffer> {
       : value_stores_(&value_stores), source_(&source) {}
 
   auto GetLineInfo(Line line) -> LineInfo&;
-  [[nodiscard]] auto GetLineInfo(Line line) const -> const LineInfo&;
+  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 GetTokenInfo(Token token) const -> const TokenInfo&;
   auto AddToken(TokenInfo info) -> Token;
-  [[nodiscard]] auto GetTokenPrintWidths(Token token) const -> PrintWidths;
+  auto GetTokenPrintWidths(Token token) const -> PrintWidths;
   auto PrintToken(llvm::raw_ostream& output_stream, Token token,
                   PrintWidths widths) const -> void;
 

toolchain/parse/precedence.h

@@ -78,7 +78,7 @@ class PrecedenceGroup {
       -> OperatorPriority;
 
   // Get the associativity of this precedence group.
-  [[nodiscard]] auto GetAssociativity() const -> Associativity {
+  auto GetAssociativity() const -> Associativity {
     return static_cast<Associativity>(GetPriority(*this, *this));
   }
 

toolchain/parse/tree.h

@@ -88,43 +88,40 @@ class Tree : public Printable<Tree> {
                     llvm::raw_ostream* vlog_stream) -> Tree;
 
   // Tests whether there are any errors in the parse tree.
-  [[nodiscard]] auto has_errors() const -> bool { return has_errors_; }
+  auto has_errors() const -> bool { return has_errors_; }
 
   // Returns the number of nodes in this parse tree.
-  [[nodiscard]] auto size() const -> int { return node_impls_.size(); }
+  auto size() const -> int { return node_impls_.size(); }
 
   // Returns an iterable range over the parse tree nodes in depth-first
   // postorder.
-  [[nodiscard]] auto postorder() const
-      -> llvm::iterator_range<PostorderIterator>;
+  auto postorder() const -> llvm::iterator_range<PostorderIterator>;
 
   // Returns an iterable range over the parse tree node and all of its
   // descendants in depth-first postorder.
-  [[nodiscard]] auto postorder(Node n) const
-      -> llvm::iterator_range<PostorderIterator>;
+  auto postorder(Node n) const -> llvm::iterator_range<PostorderIterator>;
 
   // Returns an iterable range over the direct children of a node in the parse
   // tree. This is a forward range, but is constant time to increment. The order
   // of children is the same as would be found in a reverse postorder traversal.
-  [[nodiscard]] auto children(Node n) const
-      -> llvm::iterator_range<SiblingIterator>;
+  auto children(Node n) const -> llvm::iterator_range<SiblingIterator>;
 
   // Returns an iterable range over the roots of the parse tree. This is a
   // forward range, but is constant time to increment. The order of roots is the
   // same as would be found in a reverse postorder traversal.
-  [[nodiscard]] auto roots() const -> llvm::iterator_range<SiblingIterator>;
+  auto roots() const -> llvm::iterator_range<SiblingIterator>;
 
   // Tests whether a particular node contains an error and may not match the
   // full expected structure of the grammar.
-  [[nodiscard]] auto node_has_error(Node n) const -> bool;
+  auto node_has_error(Node n) const -> bool;
 
   // Returns the kind of the given parse tree node.
-  [[nodiscard]] auto node_kind(Node n) const -> NodeKind;
+  auto node_kind(Node n) const -> NodeKind;
 
   // Returns the token the given parse tree node models.
-  [[nodiscard]] auto node_token(Node n) const -> Lex::Token;
+  auto node_token(Node n) const -> Lex::Token;
 
-  [[nodiscard]] auto node_subtree_size(Node n) const -> int32_t;
+  auto node_subtree_size(Node n) const -> int32_t;
 
   auto packaging_directive() const -> const std::optional<PackagingDirective>& {
     return packaging_directive_;
@@ -180,7 +177,7 @@ class Tree : public Printable<Tree> {
   // This is primarily intended to be used as a
   // debugging aid. This routine doesn't directly CHECK so that it can be used
   // within a debugger.
-  [[nodiscard]] auto Verify() const -> ErrorOr<Success>;
+  auto Verify() const -> ErrorOr<Success>;
 
  private:
   friend class Context;

toolchain/sem_ir/inst_kind.h

@@ -51,17 +51,17 @@ class InstKind : public CARBON_ENUM_BASE(InstKind) {
   using EnumBase::Create;
 
   // Returns the name to use for this instruction kind in Semantics IR.
-  [[nodiscard]] auto ir_name() const -> llvm::StringLiteral;
+  auto ir_name() const -> llvm::StringLiteral;
 
   // Returns whether this kind of instruction is expected to produce a value.
-  [[nodiscard]] auto value_kind() const -> InstValueKind;
+  auto value_kind() const -> InstValueKind;
 
   // Returns whether this instruction kind is a code block terminator, such as
   // an unconditional branch instruction, or part of the termination sequence,
   // such as a conditional branch instruction. The termination sequence of a
   // code block appears after all other instructions, and ends with a
   // terminator instruction.
-  [[nodiscard]] auto terminator_kind() const -> TerminatorKind;
+  auto terminator_kind() const -> TerminatorKind;
 
   // Compute a fingerprint for this instruction kind, allowing its use as part
   // of the key in a `FoldingSet`.
@@ -77,7 +77,7 @@ class InstKind : public CARBON_ENUM_BASE(InstKind) {
 
  private:
   // Looks up the definition for this instruction kind.
-  [[nodiscard]] auto definition() const -> const Definition&;
+  auto definition() const -> const Definition&;
 };
 
 #define CARBON_SEM_IR_INST_KIND(Name) \
@@ -95,13 +95,11 @@ static_assert(sizeof(InstKind) == 1, "Kind objects include padding!");
 class InstKind::Definition : public InstKind {
  public:
   // Returns the name to use for this instruction kind in Semantics IR.
-  [[nodiscard]] constexpr auto ir_name() const -> llvm::StringLiteral {
-    return ir_name_;
-  }
+  constexpr auto ir_name() const -> llvm::StringLiteral { return ir_name_; }
 
   // Returns whether this instruction kind is a code block terminator. See
   // InstKind::terminator_kind().
-  [[nodiscard]] constexpr auto terminator_kind() const -> TerminatorKind {
+  constexpr auto terminator_kind() const -> TerminatorKind {
     return terminator_kind_;
   }
 

toolchain/source/source_buffer.h

@@ -49,11 +49,9 @@ class SourceBuffer {
   // Use one of the factory functions above to create a source buffer.
   SourceBuffer() = delete;
 
-  [[nodiscard]] auto filename() const -> llvm::StringRef { return filename_; }
+  auto filename() const -> llvm::StringRef { return filename_; }
 
-  [[nodiscard]] auto text() const -> llvm::StringRef {
-    return text_->getBuffer();
-  }
+  auto text() const -> llvm::StringRef { return text_->getBuffer(); }
 
   [[nodiscard]] auto is_regular_file() const -> bool {
     return is_regular_file_;