[lldb][NFC] Move OptionDefinition from lldb-private-types.h to its own Utility header

Also moves the curious isprint8 function (which was used to check whether we have a
valid short option) into the struct and documents it.

lldb/include/lldb/Interpreter/Options.h

@@ -14,6 +14,7 @@
 
 #include "lldb/Utility/Args.h"
 #include "lldb/Utility/CompletionRequest.h"
+#include "lldb/Utility/OptionDefinition.h"
 #include "lldb/Utility/Status.h"
 #include "lldb/lldb-defines.h"
 #include "lldb/lldb-private.h"
@@ -40,12 +41,6 @@ struct OptionArgElement {
 
 typedef std::vector<OptionArgElement> OptionElementVector;
 
-static inline bool isprint8(int ch) {
-  if (ch & 0xffffff00u)
-    return false;
-  return llvm::isPrint(ch);
-}
-
 /// \class Options Options.h "lldb/Interpreter/Options.h"
 /// A command line option parsing protocol class.
 ///

lldb/include/lldb/Utility/OptionDefinition.h

@@ -0,0 +1,55 @@
+//===-- OptionDefinition.h --------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDB_UTILITY_OPTIONDEFINITION_H
+#define LLDB_UTILITY_OPTIONDEFINITION_H
+
+#include "lldb/lldb-enumerations.h"
+#include "lldb/lldb-private-types.h"
+#include "llvm/ADT/StringExtras.h"
+#include <cstdint>
+
+namespace lldb_private {
+struct OptionDefinition {
+  /// Used to mark options that can be used together.  If
+  /// `(1 << n & usage_mask) != 0` then this option belongs to option set n.
+  uint32_t usage_mask;
+  /// This option is required (in the current usage level).
+  bool required;
+  /// Full name for this option.
+  const char *long_option;
+  /// Single character for this option. If the option doesn't use a short
+  /// option character, this has to be a integer value that is not a printable
+  /// ASCII code point and also unique in the used set of options.
+  /// @see OptionDefinition::HasShortOption
+  int short_option;
+  /// no_argument, required_argument or optional_argument
+  int option_has_arg;
+  /// If non-NULL, option is valid iff |validator->IsValid()|, otherwise
+  /// always valid.
+  OptionValidator *validator;
+  /// If not empty, an array of enum values.
+  OptionEnumValues enum_values;
+  /// The kind of completion for this option.
+  /// Contains values of the CommandCompletions::CommonCompletionTypes enum.
+  uint32_t completion_type;
+  /// Type of argument this option takes.
+  lldb::CommandArgumentType argument_type;
+  /// Full text explaining what this options does and what (if any) argument to
+  /// pass it.
+  const char *usage_text;
+
+  /// Whether this has a short option character.
+  bool HasShortOption() const {
+    // See the short_option documentation for more.
+    return llvm::isPrint(short_option);
+  }
+};
+} // namespace lldb_private
+
+#endif // LLDB_UTILITY_OPTIONDEFINITION_H

lldb/include/lldb/lldb-private-types.h

@@ -107,33 +107,6 @@ struct OptionValidator {
   virtual const char *LongConditionString() const = 0;
 };
 
-struct OptionDefinition {
-  /// Used to mark options that can be used together.  If
-  /// `(1 << n & usage_mask) != 0` then this option belongs to option set n.
-  uint32_t usage_mask;
-  /// This option is required (in the current usage level).
-  bool required;
-  /// Full name for this option.
-  const char *long_option;
-  /// Single character for this option.
-  int short_option;
-  /// no_argument, required_argument or optional_argument
-  int option_has_arg;
-  /// If non-NULL, option is valid iff |validator->IsValid()|, otherwise
-  /// always valid.
-  OptionValidator *validator;
-  /// If not empty, an array of enum values.
-  OptionEnumValues enum_values;
-  /// The kind of completion for this option.
-  /// Contains values of the CommandCompletions::CommonCompletionTypes enum.
-  uint32_t completion_type;
-  /// Type of argument this option takes.
-  lldb::CommandArgumentType argument_type;
-  /// Full text explaining what this options does and what (if any) argument to
-  /// pass it.
-  const char *usage_text;
-};
-
 typedef struct type128 { uint64_t x[2]; } type128;
 typedef struct type256 { uint64_t x[4]; } type256;
 

lldb/source/Host/common/OptionParser.cpp

@@ -8,6 +8,7 @@
 
 #include "lldb/Host/OptionParser.h"
 #include "lldb/Host/HostGetOpt.h"
+#include "lldb/Utility/OptionDefinition.h"
 #include "lldb/lldb-private-types.h"
 
 #include <vector>

lldb/source/Interpreter/Options.cpp

@@ -223,7 +223,7 @@ Option *Options::GetLongOptions() {
         std::map<int, uint32_t>::const_iterator pos =
             option_seen.find(short_opt);
         StreamString strm;
-        if (isprint8(short_opt))
+        if (defs[i].HasShortOption())
           Host::SystemLog(Host::eSystemLogError,
                           "option[%u] --%s has a short option -%c that "
                           "conflicts with option[%u] --%s, short option won't "
@@ -355,9 +355,7 @@ enum OptionDisplayType {
 static bool PrintOption(const OptionDefinition &opt_def,
                         OptionDisplayType display_type, const char *header,
                         const char *footer, bool show_optional, Stream &strm) {
-  const bool has_short_option = isprint8(opt_def.short_option) != 0;
-
-  if (display_type == eDisplayShortOption && !has_short_option)
+  if (display_type == eDisplayShortOption && !opt_def.HasShortOption())
     return false;
 
   if (header && header[0])
@@ -366,7 +364,7 @@ static bool PrintOption(const OptionDefinition &opt_def,
   if (show_optional && !opt_def.required)
     strm.PutChar('[');
   const bool show_short_option =
-      has_short_option && display_type != eDisplayLongOption;
+      opt_def.HasShortOption() && display_type != eDisplayLongOption;
   if (show_short_option)
     strm.Printf("-%c", opt_def.short_option);
   else
@@ -445,7 +443,7 @@ void Options::GenerateOptionUsage(Stream &strm, CommandObject *cmd,
       std::set<int> options;
       std::set<int>::const_iterator options_pos, options_end;
       for (auto &def : opt_defs) {
-        if (def.usage_mask & opt_set_mask && isprint8(def.short_option)) {
+        if (def.usage_mask & opt_set_mask && def.HasShortOption()) {
           // Add current option to the end of out_stream.
 
           if (def.required && def.option_has_arg == OptionParser::eNoArgument) {
@@ -470,7 +468,7 @@ void Options::GenerateOptionUsage(Stream &strm, CommandObject *cmd,
 
       options.clear();
       for (auto &def : opt_defs) {
-        if (def.usage_mask & opt_set_mask && isprint8(def.short_option)) {
+        if (def.usage_mask & opt_set_mask && def.HasShortOption()) {
           // Add current option to the end of out_stream.
 
           if (!def.required &&
@@ -498,7 +496,7 @@ void Options::GenerateOptionUsage(Stream &strm, CommandObject *cmd,
       // First go through and print the required options (list them up front).
 
       for (auto &def : opt_defs) {
-        if (def.usage_mask & opt_set_mask && isprint8(def.short_option)) {
+        if (def.usage_mask & opt_set_mask && def.HasShortOption()) {
           if (def.required && def.option_has_arg != OptionParser::eNoArgument)
             PrintOption(def, eDisplayBestOption, " ", nullptr, true, strm);
         }
@@ -579,7 +577,7 @@ void Options::GenerateOptionUsage(Stream &strm, CommandObject *cmd,
       arg_name_str.Printf("<%s>", CommandObject::GetArgumentName(arg_type));
 
       strm.Indent();
-      if (opt_defs[i].short_option && isprint8(opt_defs[i].short_option)) {
+      if (opt_defs[i].short_option && opt_defs[i].HasShortOption()) {
         PrintOption(opt_defs[i], eDisplayShortOption, nullptr, nullptr, false,
                     strm);
         PrintOption(opt_defs[i], eDisplayLongOption, " ( ", " )", false, strm);