Style updates, mostly _ naming (#970)

There are some declaration order changes, and a few test classes switched from `struct` to `class`. However, this PR is mostly adopting `_` naming of private member variables due to the shift in naming style. None of what's here should have behavior impacts, it should just be style. Note, there are a lot of things that *look* like they could be accessor-named, but I'm not doing that in this change. Happy to do it separately if you want me to do another PR focused on it. Co-authored-by: Chandler Carruth <[email protected]>
carbon-language · Dec 7, 2021 · 8af2f7c · 8af2f7c
1 parent a34f2d7
commit 8af2f7c
Show file tree

Hide file tree

Showing 29 changed files with 488 additions and 478 deletions.
diff --git a/toolchain/common/yaml_test_helpers.cpp b/toolchain/common/yaml_test_helpers.cpp
@@ -72,7 +72,6 @@ auto operator<<(std::ostream& os, const Value& v) -> std::ostream& {
   // Variant visitor that prints the value in the form of code to recreate the
   // value.
   struct Printer {
-    std::ostream& out;
     auto operator()(NullValue) -> void { out << "Yaml::NullValue()"; }
     auto operator()(AliasValue) -> void { out << "Yaml::AliasValue()"; }
     auto operator()(ErrorValue) -> void { out << "Yaml::ErrorValue()"; }
@@ -103,6 +102,8 @@ auto operator<<(std::ostream& os, const Value& v) -> std::ostream& {
       }
       out << "}";
     }
+
+    std::ostream& out;
   };
   std::visit(Printer{os}, v);
   return os;

diff --git a/toolchain/diagnostics/diagnostic_emitter.h b/toolchain/diagnostics/diagnostic_emitter.h
@@ -154,22 +154,22 @@ struct SimpleDiagnostic {
 class ErrorTrackingDiagnosticConsumer : public DiagnosticConsumer {
  public:
   explicit ErrorTrackingDiagnosticConsumer(DiagnosticConsumer& next_consumer)
-      : next_consumer(&next_consumer) {}
+      : next_consumer_(&next_consumer) {}
 
   auto HandleDiagnostic(const Diagnostic& diagnostic) -> void override {
-    seen_error |= diagnostic.level == Diagnostic::Error;
-    next_consumer->HandleDiagnostic(diagnostic);
+    seen_error_ |= diagnostic.level == Diagnostic::Error;
+    next_consumer_->HandleDiagnostic(diagnostic);
   }
 
   // Returns whether we've seen an error since the last reset.
-  auto SeenError() const -> bool { return seen_error; }
+  auto SeenError() const -> bool { return seen_error_; }
 
   // Reset whether we've seen an error.
-  auto Reset() -> void { seen_error = false; }
+  auto Reset() -> void { seen_error_ = false; }
 
  private:
-  DiagnosticConsumer* next_consumer;
-  bool seen_error = false;
+  DiagnosticConsumer* next_consumer_;
+  bool seen_error_ = false;
 };
 
 }  // namespace Carbon

diff --git a/toolchain/diagnostics/diagnostic_emitter_test.cpp b/toolchain/diagnostics/diagnostic_emitter_test.cpp
@@ -25,15 +25,15 @@ struct FakeDiagnostic {
   // selection of the message.
   static constexpr llvm::StringLiteral Message = "{0}";
 
-  std::string message;
-
   auto Format() -> std::string {
     // Work around a bug in Clang's unused const variable warning by marking it
     // used here with a no-op.
     static_cast<void>(ShortName);
 
     return llvm::formatv(Message.data(), message).str();
   }
+
+  std::string message;
 };
 
 struct FakeDiagnosticLocationTranslator : DiagnosticLocationTranslator<int> {

diff --git a/toolchain/driver/driver.cpp b/toolchain/driver/driver.cpp
@@ -37,7 +37,7 @@ auto GetSubcommand(llvm::StringRef name) -> Subcommand {
 
 auto Driver::RunFullCommand(llvm::ArrayRef<llvm::StringRef> args) -> bool {
   if (args.empty()) {
-    error_stream << "ERROR: No subcommand specified.\n";
+    error_stream_ << "ERROR: No subcommand specified.\n";
     return false;
   }
 
@@ -46,8 +46,8 @@ auto Driver::RunFullCommand(llvm::ArrayRef<llvm::StringRef> args) -> bool {
       std::next(args.begin()), args.end());
   switch (GetSubcommand(subcommand_text)) {
     case Subcommand::Unknown:
-      error_stream << "ERROR: Unknown subcommand '" << subcommand_text
-                   << "'.\n";
+      error_stream_ << "ERROR: Unknown subcommand '" << subcommand_text
+                    << "'.\n";
       return false;
 
 #define CARBON_SUBCOMMAND(Name, ...) \
@@ -66,7 +66,7 @@ auto Driver::RunHelpSubcommand(llvm::ArrayRef<llvm::StringRef> args) -> bool {
     return false;
   }
 
-  output_stream << "List of subcommands:\n\n";
+  output_stream_ << "List of subcommands:\n\n";
 
   constexpr llvm::StringLiteral SubcommandsAndHelp[][2] = {
 #define CARBON_SUBCOMMAND(Name, Spelling, HelpText) {Spelling, HelpText},
@@ -84,19 +84,19 @@ auto Driver::RunHelpSubcommand(llvm::ArrayRef<llvm::StringRef> args) -> bool {
     // FIXME: We should wrap this to the number of columns left after the
     // subcommand on the terminal, and using a hanging indent.
     llvm::StringRef help_text = subcommand_and_help[1];
-    output_stream << "  "
-                  << llvm::left_justify(subcommand_text, max_subcommand_width)
-                  << " - " << help_text << "\n";
+    output_stream_ << "  "
+                   << llvm::left_justify(subcommand_text, max_subcommand_width)
+                   << " - " << help_text << "\n";
   }
 
-  output_stream << "\n";
+  output_stream_ << "\n";
   return true;
 }
 
 auto Driver::RunDumpTokensSubcommand(llvm::ArrayRef<llvm::StringRef> args)
     -> bool {
   if (args.empty()) {
-    error_stream << "ERROR: No input file specified.\n";
+    error_stream_ << "ERROR: No input file specified.\n";
     return false;
   }
 
@@ -109,24 +109,24 @@ auto Driver::RunDumpTokensSubcommand(llvm::ArrayRef<llvm::StringRef> args)
 
   auto source = SourceBuffer::CreateFromFile(input_file_name);
   if (!source) {
-    error_stream << "ERROR: Unable to open input source file: ";
+    error_stream_ << "ERROR: Unable to open input source file: ";
     llvm::handleAllErrors(source.takeError(),
                           [&](const llvm::ErrorInfoBase& ei) {
-                            ei.log(error_stream);
-                            error_stream << "\n";
+                            ei.log(error_stream_);
+                            error_stream_ << "\n";
                           });
     return false;
   }
   auto tokenized_source =
       TokenizedBuffer::Lex(*source, ConsoleDiagnosticConsumer());
-  tokenized_source.Print(output_stream);
+  tokenized_source.Print(output_stream_);
   return !tokenized_source.HasErrors();
 }
 
 auto Driver::RunDumpParseTreeSubcommand(llvm::ArrayRef<llvm::StringRef> args)
     -> bool {
   if (args.empty()) {
-    error_stream << "ERROR: No input file specified.\n";
+    error_stream_ << "ERROR: No input file specified.\n";
     return false;
   }
 
@@ -139,31 +139,31 @@ auto Driver::RunDumpParseTreeSubcommand(llvm::ArrayRef<llvm::StringRef> args)
 
   auto source = SourceBuffer::CreateFromFile(input_file_name);
   if (!source) {
-    error_stream << "ERROR: Unable to open input source file: ";
+    error_stream_ << "ERROR: Unable to open input source file: ";
     llvm::handleAllErrors(source.takeError(),
                           [&](const llvm::ErrorInfoBase& ei) {
-                            ei.log(error_stream);
-                            error_stream << "\n";
+                            ei.log(error_stream_);
+                            error_stream_ << "\n";
                           });
     return false;
   }
   auto tokenized_source =
       TokenizedBuffer::Lex(*source, ConsoleDiagnosticConsumer());
   auto parse_tree =
       ParseTree::Parse(tokenized_source, ConsoleDiagnosticConsumer());
-  parse_tree.Print(output_stream);
+  parse_tree.Print(output_stream_);
   return !tokenized_source.HasErrors() && !parse_tree.HasErrors();
 }
 
 auto Driver::ReportExtraArgs(llvm::StringRef subcommand_text,
                              llvm::ArrayRef<llvm::StringRef> args) -> void {
-  error_stream << "ERROR: Unexpected additional arguments to the '"
-               << subcommand_text << "' subcommand:";
+  error_stream_ << "ERROR: Unexpected additional arguments to the '"
+                << subcommand_text << "' subcommand:";
   for (auto arg : args) {
-    error_stream << " " << arg;
+    error_stream_ << " " << arg;
   }
 
-  error_stream << "\n";
+  error_stream_ << "\n";
 }
 
 }  // namespace Carbon
diff --git a/toolchain/driver/driver.h b/toolchain/driver/driver.h
@@ -23,12 +23,12 @@ class Driver {
  public:
   // Default constructed driver uses stderr for all error and informational
   // output.
-  Driver() : output_stream(llvm::outs()), error_stream(llvm::errs()) {}
+  Driver() : output_stream_(llvm::outs()), error_stream_(llvm::errs()) {}
 
   // Constructs a driver with any error or informational output directed to a
   // specified stream.
   Driver(llvm::raw_ostream& output_stream, llvm::raw_ostream& error_stream)
-      : output_stream(output_stream), error_stream(error_stream) {}
+      : output_stream_(output_stream), error_stream_(error_stream) {}
 
   // Parses the given arguments into both a subcommand to select the operation
   // to perform and any arguments to that subcommand.
@@ -71,8 +71,8 @@ class Driver {
   auto ReportExtraArgs(llvm::StringRef subcommand_text,
                        llvm::ArrayRef<llvm::StringRef> args) -> void;
 
-  llvm::raw_ostream& output_stream;
-  llvm::raw_ostream& error_stream;
+  llvm::raw_ostream& output_stream_;
+  llvm::raw_ostream& error_stream_;
 };
 
 }  // namespace Carbon

diff --git a/toolchain/driver/driver_test.cpp b/toolchain/driver/driver_test.cpp
@@ -22,32 +22,33 @@ namespace Yaml = Carbon::Testing::Yaml;
 
 /// A raw_ostream that makes it easy to repeatedly check streamed output.
 class RawTestOstream : public llvm::raw_ostream {
-  std::string buffer;
-
-  void write_impl(const char* ptr, size_t size) override {
-    buffer.append(ptr, ptr + size);
-  }
-
-  [[nodiscard]] auto current_pos() const -> uint64_t override {
-    return buffer.size();
-  }
-
  public:
   ~RawTestOstream() override {
     flush();
-    if (!buffer.empty()) {
-      ADD_FAILURE() << "Unchecked output:\n" << buffer;
+    if (!buffer_.empty()) {
+      ADD_FAILURE() << "Unchecked output:\n" << buffer_;
     }
   }
 
   /// Flushes the stream and returns the contents so far, clearing the stream
   /// back to empty.
   auto TakeStr() -> std::string {
     flush();
-    std::string result = std::move(buffer);
-    buffer.clear();
+    std::string result = std::move(buffer_);
+    buffer_.clear();
     return result;
   }
+
+ private:
+  void write_impl(const char* ptr, size_t size) override {
+    buffer_.append(ptr, ptr + size);
+  }
+
+  [[nodiscard]] auto current_pos() const -> uint64_t override {
+    return buffer_.size();
+  }
+
+  std::string buffer_;
 };
 
 TEST(DriverTest, FullCommandErrors) {