Fix incorrect unknown flags error message.

Before, if both unknown and known flags were in XLA_FLAGS, the error would list the unknown flags and also incorrectly some known flags as being unknown. Now, only the unknown flags are shown.

The issue was tsl::Flags::Parse would set its argc and argv parameters to only have the unknown flags. The caller in parse_flags_from_env.cc would pass a pointer to the first element of a vector<char*> for the argv parameter. But then the caller would use the size of the vector<char*> when printing unknown flags, which was not mutated by tsl::Flags::Parse, instead of argc, which was mutated.

PiperOrigin-RevId: 723714502

Author: reedwm

third_party/xla/xla/parse_flags_from_env.cc

@@ -227,8 +227,8 @@ static void DieIfEnvHasUnknownFlagsLeft(absl::string_view envvar) {
   SetArgvFromEnv(envvar, env_argv);
 
   if (env_argv->argc != 1) {
+    auto unknown_flags = absl::MakeSpan(env_argv->argv).first(env_argv->argc);
     // Skip the first argv, which is the fake argv[0].
-    auto unknown_flags = absl::MakeSpan(env_argv->argv);
     unknown_flags.remove_prefix(1);
     LOG(QFATAL) << "Unknown flag" << (unknown_flags.size() > 1 ? "s" : "")
                 << " in " << envvar << ": "

third_party/xla/xla/parse_flags_from_env_test.cc

@@ -25,6 +25,7 @@ limitations under the License.
 #include <string>
 #include <vector>
 
+#include <gmock/gmock.h>
 #include <gtest/gtest.h>
 #include "absl/log/check.h"
 #include "absl/strings/str_format.h"
@@ -210,17 +211,50 @@ TEST(ParseFlagsFromEnv, ErrorOutOnUnknownFlag) {
   EXPECT_NE(child_status, 0);
 }
 
+TEST(ParseFlagsFromEnv, UknownFlagErrorMessage) {
+  const char* env =
+      "--unknown_flag_1=value --int_flag=3 --unknown_flag_2=value "
+      "--float_flag=3.0";
+
+  if (env == nullptr) {
+    // Might be set from previous tests.
+    tsl::unsetenv("TF_XLA_FLAGS");
+  } else {
+    tsl::setenv("TF_XLA_FLAGS", env, /*overwrite=*/true);
+  }
+  tsl::SubProcess child;
+  std::vector<std::string> argv;
+  argv.push_back(binary_name);
+  argv.push_back("--recursing");
+  child.SetProgram(binary_name, argv);
+  child.SetChannelAction(tsl::CHAN_STDOUT, tsl::ACTION_PIPE);
+  child.SetChannelAction(tsl::CHAN_STDERR, tsl::ACTION_PIPE);
+  EXPECT_TRUE(child.Start());
+  std::string stdout_str;
+  std::string stderr_str;
+
+  int child_status = child.Communicate(nullptr, &stdout_str, &stderr_str);
+  EXPECT_NE(child_status, 0);
+
+  EXPECT_THAT(
+      stderr_str,
+      ::testing::EndsWith("Unknown flags in TF_XLA_FLAGS: "
+                          "--unknown_flag_1=value --unknown_flag_2=value\n"));
+}
+
 }  // namespace xla
 
 int main(int argc, char* argv[]) {
   // Save name of binary so that it may invoke itself.
   xla::binary_name = argv[0];
   bool recursing = false;
   int32_t int_flag = 1;
+  float float_flag = 1.;
   const std::vector<tsl::Flag> flag_list = {
       tsl::Flag("recursing", &recursing,
                 "Whether the binary is being invoked recursively."),
       tsl::Flag("int_flag", &int_flag, "An integer flag to test with"),
+      tsl::Flag("float_flag", &float_flag, "A float flag to test with"),
   };
   std::string usage = tsl::Flags::Usage(argv[0], flag_list);
   xla::ParseFlagsFromEnvAndDieIfUnknown("TF_XLA_FLAGS", flag_list);