[clang-tidy] Avoid overflow when dumping unsigned integer values

[ealcdan]

Summary

Some options take the maximum unsigned integer value as default, but
they are being dumped to a string as integers. This makes -dump-config
write invalid '-1' values for these options. This change fixes this
issue by using utostr if the option is unsigned.

Change-Id: I22d481047330a89984d6eb27bb02f85c71d42bdb

Fixes: #60217

[PiotrZSL]

This wont fix issue, simply because problem happen during reading -1, not during dumping, and per documentation -1 was a valid value. Same issue happen in other checks that use -1. Most probably best way would be to change those variables into std::int64_t, in C++ creating any structure of size above max int will fail with error anyway, so using signed i64 should be sufficient, and would also accept -1.

[PiotrZSL]

I agree that this change is an improvement that is welcome, but that's not a fix for a #60217.

[ealcdan]

Thanks for the review. How should I procceed? Should I remove the "Fixes:" entry from the commit message?

clang-tools-extra/clang-tidy/ClangTidyCheck.h

public:
     /// Stores an option with the check-local name \p LocalName with
     /// string value \p Value to \p Options.
     void store(ClangTidyOptions::OptionMap &Options, StringRef LocalName,
                StringRef Value) const;
 
     /// Stores an option with the check-local name \p LocalName with
     /// integer value \p Value to \p Options.
     template <typename T>
-    std::enable_if_t<std::is_integral<T>::value>
+    std::enable_if_t<std::is_integral<T>::value && std::is_signed<T>::value>
     store(ClangTidyOptions::OptionMap &Options, StringRef LocalName,
           T Value) const {
       storeInt(Options, LocalName, Value);
     }
 
+    /// Stores an option with the check-local name \p LocalName with
+    /// unsigned integer value \p Value to \p Options.
+    template <typename T>
+    std::enable_if_t<std::is_unsigned<T>::value>
+    store(ClangTidyOptions::OptionMap &Options, StringRef LocalName,
+          T Value) const {
+      storeUnsigned(Options, LocalName, Value);
+    }
+
     /// Stores an option with the check-local name \p LocalName as the string
     /// representation of the Enum \p Value to \p Options.
     ///
     /// \ref clang::tidy::OptionEnumMapping must be specialized for ``T`` to
     /// supply the mapping required to convert between ``T`` and a string.
     template <typename T>
     std::enable_if_t<std::is_enum<T>::value>
     store(ClangTidyOptions::OptionMap &Options, StringRef LocalName,
typeEraseMapping() const {
         Result.emplace_back(static_cast<int64_t>(MappedItem.first),
                             MappedItem.second);
       }
       return Result;
     }
 
     void storeInt(ClangTidyOptions::OptionMap &Options, StringRef LocalName,
                   int64_t Value) const;
-
+    void storeUnsigned(ClangTidyOptions::OptionMap &Options,
+                       StringRef LocalName, uint64_t Value) const;
 
     std::string NamePrefix;
     const ClangTidyOptions::OptionMap &CheckOptions;
     ClangTidyContext *Context;
   };
 
 private:
   void run(const ast_matchers::MatchFinder::MatchResult &Result) override;

clang-tools-extra/clang-tidy/ClangTidyCheck.cpp

 }
 
 void ClangTidyCheck::OptionsView::storeInt(ClangTidyOptions::OptionMap &Options,
                                            StringRef LocalName,
                                            int64_t Value) const {
   store(Options, LocalName, llvm::itostr(Value));
 }
 
+void ClangTidyCheck::OptionsView::storeUnsigned(
+    ClangTidyOptions::OptionMap &Options, StringRef LocalName,
+    uint64_t Value) const {
+  store(Options, LocalName, llvm::utostr(Value));
+}
+
 template <>
 void ClangTidyCheck::OptionsView::store<bool>(
     ClangTidyOptions::OptionMap &Options, StringRef LocalName,
     bool Value) const {
   store(Options, LocalName, Value ? StringRef("true") : StringRef("false"));
 }
 
 std::optional<int64_t> ClangTidyCheck::OptionsView::getEnumInt(