Add clang-tidy check for overflow in constructing energy quantity fro…

…m int

tools/clang-tidy-plugin/CMakeLists.txt

@@ -34,6 +34,7 @@ set(CataAnalyzerSrc
         TranslationsInDebugMessagesCheck.cpp
         TranslatorCommentsCheck.cpp
         U8PathCheck.cpp
+        UnitOverflowCheck.cpp
         UnsequencedCallsCheck.cpp
         UnusedStaticsCheck.cpp
         UseLocalizedSortingCheck.cpp

tools/clang-tidy-plugin/CataTidyModule.cpp

@@ -30,6 +30,7 @@
 #include "TranslationsInDebugMessagesCheck.h"
 #include "TranslatorCommentsCheck.h"
 #include "U8PathCheck.h"
+#include "UnitOverflowCheck.h"
 #include "UnsequencedCallsCheck.h"
 #include "UnusedStaticsCheck.h"
 #include "UseLocalizedSortingCheck.h"
@@ -100,6 +101,7 @@ class CataModule : public ClangTidyModule
                 "cata-translations-in-debug-messages" );
             CheckFactories.registerCheck<TranslatorCommentsCheck>( "cata-translator-comments" );
             CheckFactories.registerCheck<U8PathCheck>( "cata-u8-path" );
+            CheckFactories.registerCheck<UnitOverflowCheck>( "cata-unit-overflow" );
             CheckFactories.registerCheck<UnsequencedCallsCheck>( "cata-unsequenced-calls" );
             CheckFactories.registerCheck<UnusedStaticsCheck>( "cata-unused-statics" );
             CheckFactories.registerCheck<UseLocalizedSortingCheck>( "cata-use-localized-sorting" );

tools/clang-tidy-plugin/UnitOverflowCheck.cpp

@@ -0,0 +1,86 @@
+#include "UnitOverflowCheck.h"
+#include "Utils.h"
+
+#include <clang/ASTMatchers/ASTMatchers.h>
+#include <clang/Basic/Diagnostic.h>
+
+#include <map>
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::cata
+{
+
+struct QuantityUnit {
+    std::string_view Unit;
+    std::int64_t ConversionFactor;
+};
+
+static const std::map<std::string_view, QuantityUnit> FunctionAndQuantityTypes {
+    {"from_joule", {"energy", 1'000LL}},
+    {"from_kilojoule", {"energy", 1'000'000LL}},
+};
+
+void UnitOverflowCheck::registerMatchers( ast_matchers::MatchFinder *Finder )
+{
+    for( const auto &[functionName, unit] : FunctionAndQuantityTypes ) {
+        Finder->addMatcher(
+            callExpr( callee( functionDecl( hasName( functionName ) ).bind( "func" ) ),
+                      hasArgument( 0, expr( hasType( isInteger() ) ).bind( "arg" ) ) ),
+            this
+        );
+    }
+}
+
+void UnitOverflowCheck::check( const ast_matchers::MatchFinder::MatchResult &Result )
+{
+    const FunctionDecl *func = Result.Nodes.getNodeAs<FunctionDecl>( "func" );
+    const Expr *arg = Result.Nodes.getNodeAs<Expr>( "arg" );
+    if( !func || !arg ) {
+        return;
+    }
+    const QualType type = arg->getType();
+    const std::int64_t width = Result.Context->getTypeInfo( type ).Width;
+    if( width >= 64 ) {
+        return;
+    }
+    const SourceManager &sourceManager = *Result.SourceManager;
+    if( sourceManager.getFilename( arg->getBeginLoc() ).ends_with( "src/units.h" ) ) {
+        return;
+    }
+    const std::string functionName = func->getNameAsString();
+    if( const IntegerLiteral *literal = dyn_cast<IntegerLiteral>( arg ) ) {
+        const bool isSigned = literal->getType()->isSignedIntegerType();
+        std::int64_t minVal = 0;
+        std::int64_t maxVal = 0;
+        if( isSigned ) {
+            minVal = llvm::APInt::getSignedMinValue( width ).getSExtValue();
+            maxVal = llvm::APInt::getSignedMaxValue( width ).getSExtValue();
+        } else {
+            minVal = llvm::APInt::getMinValue( width ).getSExtValue();
+            maxVal = llvm::APInt::getMaxValue( width ).getSExtValue();
+        }
+        const std::int64_t multiplier = FunctionAndQuantityTypes.at( functionName ).ConversionFactor;
+        const std::int64_t val = literal->getValue().getSExtValue() * multiplier;
+        if( val < minVal || val > maxVal ) {
+            emitDiag( arg->getBeginLoc(), FunctionAndQuantityTypes.at( functionName ).Unit, type.getAsString(),
+                      functionName, FixItHint::CreateReplacement( arg->getSourceRange(),
+                              ( getText( Result, arg ) + Twine( "LL" ) ).str() ) );
+        }
+    } else {
+        emitDiag( arg->getBeginLoc(), FunctionAndQuantityTypes.at( functionName ).Unit, type.getAsString(),
+                  functionName, FixItHint::CreateReplacement( arg->getSourceRange(),
+                          ( Twine( "static_cast<std::int64_t>( " ) + getText( Result, arg ) + " )" ).str() ) );
+    }
+}
+
+void UnitOverflowCheck::emitDiag( const SourceLocation &loc, const std::string_view QuantityType,
+                                  const std::string_view ValueType,
+                                  const std::string_view FunctionName, const clang::FixItHint &fix )
+{
+    diag( loc,
+          "constructing %0 quantity from '%1' can overflow in '%2' in multiplying with the conversion factor" )
+            << QuantityType << ValueType << FunctionName << fix;
+}
+
+} // namespace clang::tidy::cata

tools/clang-tidy-plugin/UnitOverflowCheck.h

@@ -0,0 +1,34 @@
+#ifndef CATA_TOOLS_CLANG_TIDY_PLUGIN_UNITOVERFLOWCHECK_H
+#define CATA_TOOLS_CLANG_TIDY_PLUGIN_UNITOVERFLOWCHECK_H
+
+#include <clang-tidy/ClangTidy.h>
+#include <clang-tidy/ClangTidyCheck.h>
+#include <clang/ASTMatchers/ASTMatchFinder.h>
+
+namespace clang
+{
+
+namespace tidy
+{
+class ClangTidyContext;
+
+namespace cata
+{
+
+class UnitOverflowCheck : public ClangTidyCheck
+{
+    public:
+        UnitOverflowCheck( StringRef Name, ClangTidyContext *Context ) : ClangTidyCheck( Name, Context ) {}
+        void registerMatchers( ast_matchers::MatchFinder *Finder ) override;
+        void check( const ast_matchers::MatchFinder::MatchResult &Result ) override;
+
+    protected:
+        void emitDiag( const SourceLocation &loc, std::string_view QuantityType,
+                       std::string_view ValueType, std::string_view FunctionName, const FixItHint &fix );
+};
+
+} // namespace cata
+} // namespace tidy
+} // namespace clang
+
+#endif // CATA_TOOLS_CLANG_TIDY_PLUGIN_UNITOVERFLOWCHECK_H

tools/clang-tidy-plugin/test/unit-overflow.cpp

@@ -0,0 +1,48 @@
+// RUN: %check_clang_tidy -allow-stdinc %s cata-unit-overflow %t -- --load=%cata_plugin -- -isystem %cata_include -isystem %cata_include/third-party
+
+#include "units.h"
+
+units::energy f_energy_0()
+{
+    return units::from_millijoule( 3'000 );
+}
+
+units::energy f_energy_1()
+{
+    return units::from_joule( 3'000 );
+}
+
+units::energy f_energy_2()
+{
+    return units::from_joule( 3'000'000 );
+    // CHECK-MESSAGES: warning: constructing energy quantity from 'int' can overflow in 'from_joule' in multiplying with the conversion factor [cata-unit-overflow]
+    // CHECK-FIXES: return units::from_joule( 3'000'000LL );
+}
+
+units::energy f_energy_3()
+{
+    int joule = 3'000'000;
+    return units::from_joule( joule );
+    // CHECK-MESSAGES: warning: constructing energy quantity from 'int' can overflow in 'from_joule' in multiplying with the conversion factor [cata-unit-overflow]
+    // CHECK-FIXES: return units::from_joule( static_cast<std::int64_t>( joule ) );
+}
+
+units::energy f_energy_4()
+{
+    return units::from_kilojoule( 2'000 );
+}
+
+units::energy f_energy_5()
+{
+    return units::from_kilojoule( 3'000 );
+    // CHECK-MESSAGES: warning: constructing energy quantity from 'int' can overflow in 'from_kilojoule' in multiplying with the conversion factor [cata-unit-overflow]
+    // CHECK-FIXES: return units::from_kilojoule( 3'000LL );
+}
+
+units::energy f_energy_6()
+{
+    int kj = 3'000;
+    return units::from_kilojoule( kj );
+    // CHECK-MESSAGES: warning: constructing energy quantity from 'int' can overflow in 'from_kilojoule' in multiplying with the conversion factor [cata-unit-overflow]
+    // CHECK-FIXES: return units::from_kilojoule( static_cast<std::int64_t>( kj ) );
+}