Small improvements and fixes

- Fixes bug in ValueRange, fixes doe300/VC4CL#88
- Some small performance improvements
- Fixes a few clang-tidy warnings

.clang-tidy

@@ -1,5 +1,5 @@
 ---
-Checks:          '*,-abseil*,-android*,-boost*,-fuchsia*,-google*,-llvm*,-*objc*,-openmp*,-hicpp-braces-around-statements,-readability-braces-around-statements,-misc-unused-parameters,-clang-diagnostic-shadow-field-in-constructor,-modernize-use-auto,-clang-diagnostic-c++98-*,-clang-diagnostic-padded,-cppcoreguidelines-pro-type-reinterpret-cast,-cert-err58-cpp,-hicpp-special-member-functions,-cppcoreguidelines-pro-bounds-array-to-pointer-decay,-modernize-use-using,-readability-else-after-return,-clang-analyzer-osx*,-clang-diagnostic-shadow,-clang-diagnostic-missing-braces,-clang-diagnostic-missing-prototypes,-misc-non-private-member-variables-in-classes,-cppcoreguidelines-non-private-member-variables-in-classes,-hicpp-uppercase-literal-suffix,-readability-uppercase-literal-suffix,-modernize-use-transparent-functors,-readability-implicit-bool-conversion,-cppcoreguidelines-pro-bounds-constant-array-index,-readability-magic-numbers,-cppcoreguidelines-avoid-magic-numbers,-readability-redundant-member-init,-cppcoreguidelines-pro-type-union-access,-hicpp-no-array-decay,-readability-named-parameter,-misc-unconventional-assign-operator,-modernize-use-default-member-init'
+Checks:          '*,-abseil*,-android*,-boost*,-fuchsia*,-google*,-llvm*,-*objc*,-openmp*,-hicpp-braces-around-statements,-readability-braces-around-statements,-misc-unused-parameters,-clang-diagnostic-shadow-field-in-constructor,-modernize-use-auto,-clang-diagnostic-c++98-*,-clang-diagnostic-padded,-cppcoreguidelines-pro-type-reinterpret-cast,-cert-err58-cpp,-hicpp-special-member-functions,-cppcoreguidelines-pro-bounds-array-to-pointer-decay,-modernize-use-using,-readability-else-after-return,-clang-analyzer-osx*,-clang-diagnostic-shadow,-clang-diagnostic-missing-braces,-clang-diagnostic-missing-prototypes,-misc-non-private-member-variables-in-classes,-cppcoreguidelines-non-private-member-variables-in-classes,-hicpp-uppercase-literal-suffix,-readability-uppercase-literal-suffix,-modernize-use-transparent-functors,-readability-implicit-bool-conversion,-cppcoreguidelines-pro-bounds-constant-array-index,-readability-magic-numbers,-cppcoreguidelines-avoid-magic-numbers,-readability-redundant-member-init,-cppcoreguidelines-pro-type-union-access,-hicpp-no-array-decay,-readability-named-parameter,-misc-unconventional-assign-operator,-cppcoreguidelines-c-copy-assignment-signature,-modernize-use-default-member-init,-cppcoreguidelines-pro-bounds-pointer-arithmetic,-modernize-return-braced-init-list,-hicpp-use-auto,-bugprone-forward-declaration-namespace,-cppcoreguidelines-owning-memory'
 WarningsAsErrors: ''
 HeaderFilterRegex: '.*'
 AnalyzeTemporaryDtors: true
@@ -137,6 +137,8 @@ CheckOptions:
     value:           ''
   - key:             modernize-use-noexcept.UseNoexceptFalse
     value:           '1'
+  - key:             modernize-use-override.AllowOverrideAndFinal
+    value:           '1'
   - key:             modernize-use-nullptr.NullMacros
     value:           'NULL'
   - key:             modernize-use-transparent-functors.SafeMode

include/Optional.h

@@ -225,7 +225,7 @@ namespace vc4c
         constexpr Optional(const T& value) : Base(value) {}
         constexpr Optional(T&& value) : Base(std::forward<T>(value)) {}
         constexpr Optional(const Optional<T>& other) : Base(other) {}
-        constexpr Optional(Optional<T>&& other) : Base(std::forward<Base>(other)) {}
+        constexpr Optional(Optional<T>&& other) noexcept : Base(std::forward<Base>(other)) {}
         ~Optional() noexcept = default;
 
         Optional& operator=(const Optional&) = default;

include/VC4C.h

@@ -21,13 +21,12 @@ namespace vc4c
     /*
      * Disassembles the given machine-code module and writes the converted code into output
      */
-    std::size_t disassembleModule(
-        std::istream& binary, std::ostream& output, const OutputMode outputMode = OutputMode::HEX);
+    std::size_t disassembleModule(std::istream& binary, std::ostream& output, OutputMode outputMode = OutputMode::HEX);
     /*
      * Disassembles the given machine code (containing only instructions) and writes the converted code into output
      */
     std::size_t disassembleCodeOnly(std::istream& binary, std::ostream& output, std::size_t numInstructions,
-        const OutputMode outputMode = OutputMode::HEX);
+        OutputMode outputMode = OutputMode::HEX);
 } /* namespace vc4c */
 
 #endif /* VC4C_H */

src/CMakeLists.txt

@@ -90,8 +90,8 @@ endif(MULTI_THREADED)
 if(VC4C_ENABLE_SPIRV_FRONTEND)
 	add_dependencies(${VC4C_LIBRARY_NAME} SPIRV-Dependencies)
 	target_link_libraries(${VC4C_LIBRARY_NAME} ${SPIRV_Tools_LIBS})
-	target_include_directories(${VC4C_LIBRARY_NAME} PRIVATE ${SPIRV_Headers_HEADERS})
-	target_include_directories(${VC4C_LIBRARY_NAME} PRIVATE ${SPIRV_Tools_HEADERS})
+	target_include_directories(${VC4C_LIBRARY_NAME} SYSTEM PRIVATE ${SPIRV_Headers_HEADERS})
+	target_include_directories(${VC4C_LIBRARY_NAME} SYSTEM PRIVATE ${SPIRV_Tools_HEADERS})
 
 	target_compile_definitions(${VC4C_LIBRARY_NAME} PRIVATE SPIRV_LLVM_SPIRV_PATH="${SPIRV_LLVM_SPIR_FOUND}" SPIRV_FRONTEND=1)
 	target_compile_definitions(${VC4C_PROGRAM_NAME} PRIVATE SPIRV_LLVM_SPIRV_PATH="${SPIRV_LLVM_SPIR_FOUND}" SPIRV_FRONTEND=1)
@@ -130,8 +130,8 @@ endif()
 
 # Download mpark/variant library as dependency
 add_dependencies(${VC4C_LIBRARY_NAME} variant-dependencies)
-target_include_directories(${VC4C_LIBRARY_NAME} PRIVATE ${variant_HEADERS})
-target_include_directories(${VC4C_PROGRAM_NAME} PRIVATE ${variant_HEADERS})
+target_include_directories(${VC4C_LIBRARY_NAME} SYSTEM PRIVATE ${variant_HEADERS})
+target_include_directories(${VC4C_PROGRAM_NAME} SYSTEM PRIVATE ${variant_HEADERS})
 
 if(VC4CL_STDLIB_DIR)
 	# Pre-compile VC4CL standard library files if development headers available and output files do not yet exist

src/CompilationError.cpp

@@ -124,7 +124,7 @@ static void demangleAndPrint(char* line, int i)
 void CompilationError::logBacktrace()
 {
 #if defined(DEBUG_MODE) and defined(__GNUC__)
-    std::array<void*, 256> funcPointers;
+    std::array<void*, 256> funcPointers{};
     int numFuncs = backtrace(funcPointers.data(), funcPointers.size());
 
     char** strings = backtrace_symbols(funcPointers.data(), numFuncs);

src/Compiler.cpp

@@ -178,7 +178,7 @@ std::unique_ptr<logging::Logger> logging::LOGGER(new logging::ColoredLogger(std:
 void vc4c::setLogger(std::wostream& outputStream, const bool coloredOutput, const LogLevel level)
 {
     if(coloredOutput)
-        logging::LOGGER.reset(new logging::ColoredLogger(outputStream, static_cast<logging::Level>(level)));
+        logging::LOGGER = std::make_unique<logging::ColoredLogger>(outputStream, static_cast<logging::Level>(level));
     else
-        logging::LOGGER.reset(new logging::StreamLogger(outputStream, static_cast<logging::Level>(level)));
+        logging::LOGGER = std::make_unique<logging::StreamLogger>(outputStream, static_cast<logging::Level>(level));
 }

src/Disassembler.cpp

@@ -217,7 +217,7 @@ LCOV_EXCL_STOP
 
 static std::string readString(std::istream& binary, uint64_t stringLength)
 {
-    std::array<char, 1024> buffer;
+    std::array<char, 1024> buffer = {0};
 
     binary.read(buffer.data(), static_cast<std::streamsize>(stringLength));
     const std::string name(buffer.data(), stringLength);

src/Graph.h

@@ -64,6 +64,7 @@ namespace vc4c
 
         Node(const Node&) = delete;
         Node(Node&&) noexcept = delete;
+        ~Node() noexcept = default;
 
         Node& operator=(const Node&) = delete;
         Node& operator=(Node&&) noexcept = delete;
@@ -467,6 +468,7 @@ namespace vc4c
 
         Edge(const Edge&) = delete;
         Edge(Edge&&) noexcept = delete;
+        ~Edge() noexcept = default;
 
         Edge& operator=(const Edge&) = delete;
         Edge& operator=(Edge&&) noexcept = delete;
@@ -596,6 +598,7 @@ namespace vc4c
         }
         Graph(const Graph&) = delete;
         Graph(Graph&&) noexcept = delete;
+        ~Graph() noexcept = default;
 
         Graph& operator=(const Graph&) = delete;
         Graph& operator=(Graph&&) noexcept = delete;

src/HalfType.cpp

@@ -9,7 +9,7 @@ static uint16_t extractHalfExponent(float f)
 {
     if(f == 0.0f || f == -0.0f)
         return 0;
-    auto exp = (bit_cast<float, uint32_t>(f) & 0x7F800000) >> 23;
+    auto exp = (bit_cast<float, uint32_t>(f) & 0x7F800000u) >> 23u;
     if(exp == 0)
         // subnormal value -> flush to zero
         return 0;
@@ -18,21 +18,21 @@ static uint16_t extractHalfExponent(float f)
         return 0x1F;
 
     return std::min(static_cast<uint16_t>(exp - 127 /* floating-point exponent bias*/ + 15 /* half exponent bias */),
-        static_cast<uint16_t>(0x1F));
+        uint16_t{0x1F});
 }
 
 static uint16_t extractHalfMantissa(float f)
 {
-    auto exp = ((bit_cast<float, uint32_t>(f) & 0x7F800000) >> 23) - 127 + 15;
+    auto exp = ((bit_cast<float, uint32_t>(f) & 0x7F800000u) >> 23u) - 127 + 15;
     if(exp >= 0x1F)
         // will be converted to +-Inf -> mantissa of zero
         return 0;
     // TODO wrong for subnormals, will convert x * 2^-127 to x*2^-15
-    return static_cast<uint16_t>((bit_cast<float, uint32_t>(f) & 0x007FFFFF) >> 13);
+    return static_cast<uint16_t>((bit_cast<float, uint32_t>(f) & 0x007FFFFFu) >> 13u);
 }
 
 Binary16::Binary16(float val) :
-    sign(static_cast<uint16_t>(bit_cast<float, uint32_t>(val) >> 31)), exponent(extractHalfExponent(val)),
+    sign(static_cast<uint16_t>(bit_cast<float, uint32_t>(val) >> 31u)), exponent(extractHalfExponent(val)),
     fraction(extractHalfMantissa(val))
 {
 }
@@ -45,8 +45,8 @@ Binary16::operator float() const
         return sign ? -std::numeric_limits<float>::infinity() : std::numeric_limits<float>::infinity();
     if(isNaN())
         return std::numeric_limits<float>::quiet_NaN();
-    uint32_t tmp = (static_cast<uint32_t>(sign) << 31) | ((static_cast<uint32_t>(exponent) + (127 - 15)) << 23) |
-        (static_cast<uint32_t>(fraction) << 13);
+    uint32_t tmp = (static_cast<uint32_t>(sign) << 31u) | ((static_cast<uint32_t>(exponent) + (127 - 15)) << 23u) |
+        (static_cast<uint32_t>(fraction) << 13u);
     // TODO rewrite (value-mapping instead of bit-wise?) and make constexpr,
     // also constructor
     return bit_cast<uint32_t, float>(tmp);

src/HalfType.h

@@ -25,8 +25,8 @@ namespace vc4c
         uint16_t fraction : 10;
 
         constexpr explicit Binary16(uint16_t val = 0) :
-            sign(static_cast<uint16_t>(val >> 15)), exponent(static_cast<uint16_t>((val & 0x7C00) >> 10)),
-            fraction(static_cast<uint16_t>(val & 0x3FF))
+            sign(static_cast<uint16_t>(val >> 15u)), exponent(static_cast<uint16_t>((val & 0x7C00u) >> 10u)),
+            fraction(static_cast<uint16_t>(val & 0x3FFu))
         {
         }
 
@@ -41,7 +41,7 @@ namespace vc4c
 
         inline constexpr operator uint16_t() const
         {
-            return static_cast<uint16_t>((sign << 15) | (exponent << 10) | (fraction));
+            return static_cast<uint16_t>((sign << 15u) | (exponent << 10u) | (fraction));
         }
 
         inline constexpr bool isZero() const
@@ -70,12 +70,11 @@ namespace vc4c
     constexpr Binary16 HALF_INF(0, 0x1F, 0);
 
     static_assert(sizeof(Binary16) == 2, "Binary16 type has wrong size!");
-    static_assert(static_cast<uint16_t>(Binary16(static_cast<uint16_t>(0x1234))) == static_cast<uint16_t>(0x1234),
-        "Bit conversion failed!");
-    static_assert((Binary16(static_cast<uint16_t>(0))).isZero(), "Bitwise check failed!");
-    static_assert((Binary16(static_cast<uint16_t>(0x123))).isSubnormal(), "Bitwise check failed!");
-    static_assert((Binary16(static_cast<uint16_t>(0x7C00))).isInf(), "Bitwise check failed!");
-    static_assert((Binary16(static_cast<uint16_t>(0xFFFF))).isNaN(), "Bitwise check failed!");
+    static_assert(static_cast<uint16_t>(Binary16(uint16_t{0x1234})) == uint16_t{0x1234}, "Bit conversion failed!");
+    static_assert((Binary16(uint16_t{0})).isZero(), "Bitwise check failed!");
+    static_assert((Binary16(uint16_t{0x123})).isSubnormal(), "Bitwise check failed!");
+    static_assert((Binary16(uint16_t{0x7C00})).isInf(), "Bitwise check failed!");
+    static_assert((Binary16(uint16_t{0xFFFF})).isNaN(), "Bitwise check failed!");
 
     inline Binary16 operator""_h(long double val)
     {

src/Locals.h

@@ -189,8 +189,7 @@ namespace vc4c
         struct RAIILock
         {
             using UnlockFunc = std::function<void()>;
-            explicit RAIILock() : func(nullptr) {}
-            RAIILock(UnlockFunc&& f) : func(std::move(f)) {}
+            explicit RAIILock(UnlockFunc&& f = nullptr) : func(std::move(f)) {}
             RAIILock(const RAIILock&) = delete;
             RAIILock(RAIILock&& other) noexcept : func(std::move(other.func))
             {

src/ProcessUtil.cpp

@@ -139,7 +139,7 @@ static int runReadOnlySubprocess(const std::string& command, std::ostream* outpu
         std::size_t numRead = 0;
         while((numRead = fread(buffer.data(), sizeof(char), buffer.size(), fd)) > 0)
         {
-            output->write(buffer.data(), numRead);
+            output->write(buffer.data(), static_cast<std::streamsize>(numRead));
         }
         if(ferror(fd))
         {
@@ -161,7 +161,7 @@ static int runWriteOnlySubprocess(const std::string& command, std::istream* inpu
         std::array<char, 512> buffer;
         while(input->read(buffer.data(), buffer.size()))
         {
-            fwrite(buffer.data(), sizeof(char), input->gcount(), fd);
+            fwrite(buffer.data(), sizeof(char), static_cast<std::size_t>(input->gcount()), fd);
         }
         if(ferror(fd))
         {
@@ -247,7 +247,7 @@ int vc4c::runProcess(const std::string& command, std::istream* stdin, std::ostre
         {
             in.read(buffer.data(), buffer.size());
             numBytes = in.gcount();
-            write(pipes[STD_IN][WRITE], buffer.data(), in.gcount());
+            write(pipes[STD_IN][WRITE], buffer.data(), static_cast<std::size_t>(in.gcount()));
             if(numBytes != buffer.size())
                 break;
         }

src/Profiler.cpp

@@ -106,7 +106,8 @@ void profiler::dumpProfileResults(bool writeAsWarning)
             logFunc() << std::setw(40) << entry.name << std::setw(7)
                       << std::chrono::duration_cast<std::chrono::milliseconds>(entry.duration).count() << " ms"
                       << std::setw(12) << entry.duration.count() << " us" << std::setw(10) << entry.invocations
-                      << " calls" << std::setw(12) << entry.duration.count() / entry.invocations << " us/call"
+                      << " calls" << std::setw(12)
+                      << static_cast<std::size_t>(entry.duration.count()) / entry.invocations << " us/call"
                       << std::setw(64) << entry.fileName << "#" << entry.lineNumber << logging::endl;
         }
 
@@ -115,9 +116,9 @@ void profiler::dumpProfileResults(bool writeAsWarning)
         for(const Counter& counter : counts)
         {
             logFunc() << std::setw(40) << counter.name << std::setw(7) << counter.count << " counts" << std::setw(5)
-                      << counter.invocations << " calls" << std::setw(6) << counter.count / counter.invocations
-                      << " avg./call" << std::setw(8) << (counter.prevCounter == SIZE_MAX ? "" : "diff") << std::setw(7)
-                      << std::showpos
+                      << counter.invocations << " calls" << std::setw(6)
+                      << static_cast<std::size_t>(counter.count) / counter.invocations << " avg./call" << std::setw(8)
+                      << (counter.prevCounter == SIZE_MAX ? "" : "diff") << std::setw(7) << std::showpos
                       << (counter.prevCounter == SIZE_MAX ? 0 : counter.count - counters[counter.prevCounter].count)
                       << " (" << std::setw(5) << std::showpos
                       << (counter.prevCounter == SIZE_MAX ?

src/ThreadPool.h

@@ -19,7 +19,7 @@ namespace vc4c
     class ThreadPool
     {
     public:
-        ThreadPool(const std::string& poolName, unsigned numThreads = std::thread::hardware_concurrency());
+        explicit ThreadPool(const std::string& poolName, unsigned numThreads = std::thread::hardware_concurrency());
         ThreadPool(const ThreadPool&) = delete;
         ThreadPool(ThreadPool&&) noexcept = delete;
         ~ThreadPool();

src/Values.cpp

@@ -386,11 +386,11 @@ std::string SmallImmediate::to_string() const
         return (std::to_string(static_cast<int>(value) - 32) + " (") + std::to_string(static_cast<int>(value)) + ")";
     if(value <= 39)
         // 1.0, ..., 128.0
-        return (std::to_string(static_cast<float>(1 << (static_cast<int>(value) - 32))) + " (") +
+        return (std::to_string(static_cast<float>(1u << (static_cast<int>(value) - 32))) + " (") +
             std::to_string(static_cast<int>(value)) + ")";
     if(value <= 47)
         // 1/256, ..., 1/2
-        return (std::to_string(1.0f / static_cast<float>(1 << (48 - static_cast<int>(value)))) + " (") +
+        return (std::to_string(1.0f / static_cast<float>(1u << (48 - static_cast<int>(value)))) + " (") +
             std::to_string(static_cast<int>(value)) + ")";
     if(value == 48)
         return "<< r5";
@@ -416,10 +416,10 @@ Optional<float> SmallImmediate::getFloatingValue() const noexcept
 {
     if(value >= 32 && value <= 39)
         // 1.0, ..., 128.0
-        return static_cast<float>(1 << (static_cast<unsigned>(value) - 32));
+        return static_cast<float>(1u << (static_cast<unsigned>(value) - 32));
     if(value >= 40 && value <= 47)
         // 1/256, ..., 1/2
-        return 1.0f / static_cast<float>(1 << (48 - static_cast<unsigned>(value)));
+        return 1.0f / static_cast<float>(1u << (48 - static_cast<unsigned>(value)));
     return {};
 }
 
@@ -540,7 +540,7 @@ bool SIMDVector::isUndefined() const
 SIMDVector SIMDVector::transform(const std::function<Literal(Literal)>& transformOp) const &
 {
     SIMDVector copy;
-    for(unsigned i = 0; i < elements.size(); ++i)
+    for(std::size_t i = 0; i < elements.size(); ++i)
     {
         copy.elements[i] = transformOp(elements[i]);
     }

src/analysis/ValueRange.cpp

@@ -293,7 +293,7 @@ void ValueRange::update(const Optional<Value>& constant, const FastMap<const Loc
          * y is in range [0, constant] (unsigned)
          */
         if(auto litArg = op->findLiteralArgument())
-            extendBoundaries(0, static_cast<int64_t>(litArg->literal().unsignedInt()));
+            extendBoundaries(0, static_cast<int64_t>(litArg->getLiteralValue()->unsignedInt()));
         else
             throw CompilationError(CompilationStep::GENERAL,
                 "Failed to get literal argument for operation which reads a literal value", op->to_string());

src/intermediate/Helper.cpp

@@ -96,7 +96,7 @@ InstructionWalker intermediate::insertRestoreSign(
 {
     if(src.getLiteralValue() && sign.getLiteralValue())
     {
-        dest = sign.isZeroInitializer() ? src : Value(Literal(-src.literal().signedInt()), src.type);
+        dest = sign.isZeroInitializer() ? src : Value(Literal(-src.getLiteralValue()->signedInt()), src.type);
     }
     else
     {

src/intermediate/TypeConversions.cpp

@@ -298,12 +298,13 @@ InstructionWalker intermediate::insertSignExtension(InstructionWalker it, Method
     {
         // out = asr(shl(in, bit_diff) bit_diff)
         // where bit_diff is the difference to full 32-bit
-        Value widthDiff(Literal(static_cast<int32_t>(32 - src.type.getScalarBitCount())), TYPE_INT8);
+        Literal diffLit(static_cast<int32_t>(32 - src.type.getScalarBitCount()));
+        Value widthDiff(diffLit, TYPE_INT8);
 
         if(!allowLiteral)
         {
             Value tmp = method.addNewLocal(TYPE_INT8, "%sext");
-            it.emplace(new LoadImmediate(tmp, widthDiff.literal()));
+            it.emplace(new LoadImmediate(tmp, diffLit));
             it.nextInBlock();
             widthDiff = tmp;
         }

src/intermediate/VectorHelper.cpp

@@ -105,7 +105,7 @@ InstructionWalker intermediate::insertVectorRotation(InstructionWalker it, const
     }
 
     // 2. create rotation instruction
-    if(appliedOffset.hasLiteral(INT_ZERO.literal()))
+    if(appliedOffset.hasLiteral(0_lit))
         // a rotation by 0 is a simple move
         assign(it, dest) = src;
     else