commit eaf066b3b8c2f6533cce1caed559540b1ca10252
author michajlo <michajlo@google.com> Wed May 15 14:12:57 2019 -0700
committer Copybara-Service <copybara-worker@google.com> Wed May 15 14:14:14 2019 -0700
tree 67d37c8494a440654aabc0bc861770acc5d37e52
parent 0659e6e3ef38d02baf1dfa3ef72167beab86ee9c

Update OptionProcessor's startup options naming/docs to better reflect reality

AFAICT, contrary to what the documentation, naming, and usage suggest,
OptionProcessor.parsed_startup_options_ can actually be unparsed and is never
null. I'm guessing at some point this was all true, but not now.

PiperOrigin-RevId: 248402326

src/main/cpp/option_processor.cc

diff --git a/src/main/cpp/option_processor.cc b/src/main/cpp/option_processor.cc
index b3d96b8..33808d5 100644
--- a/src/main/cpp/option_processor.cc
+++ b/src/main/cpp/option_processor.cc
@@ -54,7 +54,8 @@
     const WorkspaceLayout* workspace_layout,
     std::unique_ptr<StartupOptions> default_startup_options)
     : workspace_layout_(workspace_layout),
-      parsed_startup_options_(std::move(default_startup_options)),
+      startup_options_(std::move(default_startup_options)),
+      parse_options_called_(false),
       system_bazelrc_path_(BAZEL_SYSTEM_BAZELRC_PATH) {}
 
 OptionProcessor::OptionProcessor(
@@ -62,13 +63,14 @@
     std::unique_ptr<StartupOptions> default_startup_options,
     const std::string& system_bazelrc_path)
     : workspace_layout_(workspace_layout),
-      parsed_startup_options_(std::move(default_startup_options)),
+      startup_options_(std::move(default_startup_options)),
+      parse_options_called_(false),
       system_bazelrc_path_(system_bazelrc_path) {}
 
 std::unique_ptr<CommandLine> OptionProcessor::SplitCommandLine(
     vector<string> args, string* error) const {
   const string lowercase_product_name =
-      parsed_startup_options_->GetLowercaseProductName();
+      startup_options_->GetLowercaseProductName();
 
   if (args.empty()) {
     blaze_util::StringPrintf(error,
@@ -86,10 +88,10 @@
     // If the current argument is a valid nullary startup option such as
     // --master_bazelrc or --nomaster_bazelrc proceed to examine the next
     // argument.
-    if (parsed_startup_options_->IsNullary(current_arg)) {
+    if (startup_options_->IsNullary(current_arg)) {
       startup_args.push_back(current_arg);
       i++;
-    } else if (parsed_startup_options_->IsUnary(current_arg)) {
+    } else if (startup_options_->IsUnary(current_arg)) {
       // If the current argument is a valid unary startup option such as
       // --bazelrc there are two cases to consider.
 
@@ -453,6 +455,7 @@
 blaze_exit_code::ExitCode OptionProcessor::ParseOptions(
     const vector<string>& args, const string& workspace, const string& cwd,
     string* error) {
+  parse_options_called_ = true;
   cmd_line_ = SplitCommandLine(args, error);
   if (cmd_line_ == nullptr) {
     return blaze_exit_code::BAD_ARGV;
@@ -547,7 +550,7 @@
     rcstartup_flags.push_back(RcStartupFlag("", arg));
   }
 
-  return parsed_startup_options_->ProcessArgs(rcstartup_flags, error);
+  return startup_options_->ProcessArgs(rcstartup_flags, error);
 }
 
 static bool IsValidEnvName(const char* p) {
@@ -701,8 +704,8 @@
 }
 
 StartupOptions* OptionProcessor::GetParsedStartupOptions() const {
-  assert(parsed_startup_options_ != NULL);
-  return parsed_startup_options_.get();
+  assert(parse_options_called_);
+  return startup_options_.get();
 }
 
 }  // namespace blaze

src/main/cpp/option_processor.h

diff --git a/src/main/cpp/option_processor.h b/src/main/cpp/option_processor.h
index 5cb4ed3..bfe2070 100644
--- a/src/main/cpp/option_processor.h
+++ b/src/main/cpp/option_processor.h
@@ -105,6 +105,8 @@
   // Gets the arguments explicitly provided by the user's command line.
   std::vector<std::string> GetExplicitCommandArguments() const;
 
+  // Returns the underlying StartupOptions object with parsed values. Must
+  // only be called after ParseOptions.
   virtual StartupOptions* GetParsedStartupOptions() const;
 
   // Prints a message about the origin of startup options. This should be called
@@ -145,9 +147,12 @@
 
   const WorkspaceLayout* workspace_layout_;
 
-  // The startup options parsed from args, this field is initialized by
-  // ParseOptions.
-  std::unique_ptr<StartupOptions> parsed_startup_options_;
+  // The StartupOptions object defining the startup options which are accepted,
+  // and, after ParseOptions has been called, their values.
+  const std::unique_ptr<StartupOptions> startup_options_;
+
+  // Whether or not ParseOptions has been called.
+  bool parse_options_called_;
 
   // Path to the system-wide bazelrc configuration file.
   // This is configurable for testing purposes only.