Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: fixed linter errors for ecc, numeric and common modules #1714

Merged
merged 15 commits into from
Aug 25, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#pragma once
#include "assert.h"
#include <cassert>
#include <iostream>
#include <stdexcept>
#include <string>
Expand Down
15 changes: 9 additions & 6 deletions circuits/cpp/barretenberg/cpp/src/barretenberg/common/c_bind.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,13 @@ void thread_test_entry_point(test_threads_data* v)
// Do some meaningless work.
std::for_each(data.begin(), data.end(), [](auto& i) { i = i & 0x80; });
std::for_each(data.begin(), data.end(), [](auto& i) { i = i | 0x01; });
std::for_each(data.begin(), data.end(), [](auto& i) { i = (uint8_t)(i << 3); });
std::for_each(data.begin(), data.end(), [](auto& i) { i = static_cast<uint8_t>(i << 3); });
(v->counter)++;
}
// info("thread end with counter at: ", static_cast<size_t>(v->counter), " ", t.seconds(), "s");
}

void thread_test_abort_entry_point(void*)
void thread_test_abort_entry_point(void* /*unused*/)
{
info("thread_test_abort aborting");
std::abort();
Expand Down Expand Up @@ -58,7 +58,7 @@ WASM_EXPORT void test_threads(uint32_t const* thread_num, uint32_t const* iterat

WASM_EXPORT void test_thread_abort()
{
std::thread thread(thread_test_abort_entry_point, (void*)0);
std::thread thread(thread_test_abort_entry_point, (void*)nullptr);
thread.join();
}

Expand All @@ -72,9 +72,12 @@ WASM_EXPORT void test_abort()

WASM_EXPORT void test_stdout_stderr()
{
fprintf(stdout, "c: hello stdout!");
fflush(stdout);
fprintf(stderr, "c: hello stderr!");
// refactoring our file access methods to fix this warning is not worth it!
// NOLINTBEGIN(cppcoreguidelines-pro-type-vararg)
static_cast<void>(fprintf(stdout, "c: hello stdout!"));
static_cast<void>(fflush(stdout));
static_cast<void>(fprintf(stderr, "c: hello stderr!"));
// NOLINTEND(cppcoreguidelines-pro-type-vararg)
std::cout << "c++: hello stdout!" << std::flush;
std::cerr << "c++: hello stderr!";
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#pragma once

#include <stddef.h>
#include <cstddef>
#include <tuple>
#include <utility>

Expand Down Expand Up @@ -165,7 +165,7 @@ constexpr auto concatenate_arrays(const std::array<Type, sizes>&... arrays)
* cannot be constexpr)
*/
template <typename T, std::size_t... Is>
constexpr std::array<T, sizeof...(Is)> create_array(T value, std::index_sequence<Is...>)
constexpr std::array<T, sizeof...(Is)> create_array(T value, std::index_sequence<Is...> /*unused*/)
{
// cast Is to void to remove the warning: unused value
std::array<T, sizeof...(Is)> result = { { (static_cast<void>(Is), value)... } };
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#pragma once
#include <algorithm>
#include <stddef.h>
#include <cstddef>
#include <string>
#include <vector>

Expand Down Expand Up @@ -53,7 +53,7 @@ InnerCont flatten(Cont<InnerCont, Args...> const& in)

// Return the first index at which a given item can be found in the vector.
// Only safe for vectors with length less than the size_t overflow size.
template <typename T> long index_of(std::vector<T> const& vec, T const& item)
template <typename T> size_t index_of(std::vector<T> const& vec, T const& item)
{
auto const& begin = vec.begin();
auto const& end = vec.end();
Expand Down
162 changes: 86 additions & 76 deletions circuits/cpp/barretenberg/cpp/src/barretenberg/common/fuzzer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include "barretenberg/proof_system/circuit_builder/turbo_circuit_builder.hpp"
#include <concepts>

// NOLINTBEGIN(cppcoreguidelines-macro-usage, google-runtime-int)
#define PARENS ()

// Rescan macro tokens 256 times
Expand Down Expand Up @@ -68,7 +69,9 @@ class FastRandom {
FastRandom(uint32_t seed) { reseed(seed); }
uint32_t next()
{
state = static_cast<uint32_t>((uint64_t(state) * uint64_t(363364578) + uint64_t(537)) % uint64_t(3758096939));
state = static_cast<uint32_t>(
zac-williamson marked this conversation as resolved.
Show resolved Hide resolved
(static_cast<uint64_t>(state) * static_cast<uint64_t>(363364578) + static_cast<uint64_t>(537)) %
static_cast<uint64_t>(3758096939));
return state;
}
void reseed(uint32_t seed)
Expand All @@ -85,80 +88,83 @@ class FastRandom {
*
* @tparam T
*/
template <typename T>
concept SimpleRng = requires(T a) {
{
a.next()
} -> std::convertible_to<uint32_t>;
};
template <typename T> concept SimpleRng = requires(T a)
{
{
a.next()
}
->std::convertible_to<uint32_t>;
};
/**
* @brief Concept for forcing ArgumentSizes to be size_t
*
* @tparam T
*/
template <typename T>
concept InstructionArgumentSizes = requires {
{
std::make_tuple(T::CONSTANT,
T::WITNESS,
T::CONSTANT_WITNESS,
T::ADD,
T::SUBTRACT,
T::MULTIPLY,
T::DIVIDE,
T::ADD_TWO,
T::MADD,
T::MULT_MADD,
T::MSUB_DIV,
T::SQR,
T::SQR_ADD,
T::SUBTRACT_WITH_CONSTRAINT,
T::DIVIDE_WITH_CONSTRAINTS,
T::SLICE,
T::ASSERT_ZERO,
T::ASSERT_NOT_ZERO)
} -> std::same_as<std::tuple<size_t>>;
};
template <typename T> concept InstructionArgumentSizes = requires
{
{
std::make_tuple(T::CONSTANT,
T::WITNESS,
T::CONSTANT_WITNESS,
T::ADD,
T::SUBTRACT,
T::MULTIPLY,
T::DIVIDE,
T::ADD_TWO,
T::MADD,
T::MULT_MADD,
T::MSUB_DIV,
T::SQR,
T::SQR_ADD,
T::SUBTRACT_WITH_CONSTRAINT,
T::DIVIDE_WITH_CONSTRAINTS,
T::SLICE,
T::ASSERT_ZERO,
T::ASSERT_NOT_ZERO)
}
->std::same_as<std::tuple<size_t>>;
};

/**
* @brief Concept for Havoc Configurations
*
* @tparam T
*/
template <typename T>
concept HavocConfigConstraint =
requires {
{
std::make_tuple(T::GEN_MUTATION_COUNT_LOG, T::GEN_STRUCTURAL_MUTATION_PROBABILITY)
} -> std::same_as<std::tuple<size_t>>;
T::GEN_MUTATION_COUNT_LOG <= 7;
};
template <typename T> concept HavocConfigConstraint = requires
{
{
std::make_tuple(T::GEN_MUTATION_COUNT_LOG, T::GEN_STRUCTURAL_MUTATION_PROBABILITY)
}
->std::same_as<std::tuple<size_t>>;
T::GEN_MUTATION_COUNT_LOG <= 7;
};
/**
* @brief Concept specifying the class used by the fuzzer
*
* @tparam T
*/
template <typename T>
concept ArithmeticFuzzHelperConstraint = requires {
typename T::ArgSizes;
typename T::Instruction;
typename T::ExecutionState;
typename T::ExecutionHandler;
InstructionArgumentSizes<typename T::ArgSizes>;
// HavocConfigConstraint<typename T::HavocConfig>;
};
template <typename T> concept ArithmeticFuzzHelperConstraint = requires
{
typename T::ArgSizes;
typename T::Instruction;
typename T::ExecutionState;
typename T::ExecutionHandler;
InstructionArgumentSizes<typename T::ArgSizes>;
// HavocConfigConstraint<typename T::HavocConfig>;
zac-williamson marked this conversation as resolved.
Show resolved Hide resolved
};

/**
* @brief Fuzzer uses only composers with check_circuit function
*
* @tparam T
*/
template <typename T>
concept CheckableComposer = requires(T a) {
{
a.check_circuit()
} -> std::same_as<bool>;
};
template <typename T> concept CheckableComposer = requires(T a)
{
{
a.check_circuit()
}
->std::same_as<bool>;
};

/**
* @brief The fuzzer can use a postprocessing function that is specific to the type being fuzzed
Expand All @@ -168,23 +174,25 @@ concept CheckableComposer = requires(T a) {
* @tparam Context The class containing the full context
*/
template <typename T, typename Composer, typename Context>
concept PostProcessingEnabled = requires(Composer composer, Context context) {
{
T::postProcess(&composer, context)
} -> std::same_as<bool>;
};
concept PostProcessingEnabled = requires(Composer composer, Context context)
{
{
T::postProcess(&composer, context)
}
->std::same_as<bool>;
};

/**
* @brief This concept is used when we want to limit the number of executions of certain instructions (for example,
* divisions and multiplications in bigfield start to bog down the fuzzer)
*
* @tparam T
*/
template <typename T>
concept InstructionWeightsEnabled = requires {
typename T::InstructionWeights;
T::InstructionWeights::_LIMIT;
};
template <typename T> concept InstructionWeightsEnabled = requires
{
typename T::InstructionWeights;
T::InstructionWeights::_LIMIT;
};

/**
* @brief Mutate the value of a field element
Expand All @@ -195,9 +203,7 @@ concept InstructionWeightsEnabled = requires {
* @param havoc_config Mutation configuration
* @return Mutated element
*/
template <typename T, typename FF>
inline static FF mutateFieldElement(FF e, T& rng)
requires SimpleRng<T>
template <typename T, typename FF> inline static FF mutateFieldElement(FF e, T& rng) requires SimpleRng<T>
{
// With a certain probability, we apply changes to the Montgomery form, rather than the plain form. This
// has merit, since the computation is performed in montgomery form and comparisons are often performed
Expand Down Expand Up @@ -226,7 +232,8 @@ inline static FF mutateFieldElement(FF e, T& rng)
if (choice < 2) {
// Delegate mutation to libfuzzer (bit/byte mutations, autodictionary, etc)
MONT_CONVERSION_LOCAL
LLVMFuzzerMutate((uint8_t*)&value_data, sizeof(uint256_t), sizeof(uint256_t));
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
LLVMFuzzerMutate(reinterpret_cast<uint8_t*>(&value_data), sizeof(uint256_t), sizeof(uint256_t));
INV_MONT_CONVERSION_LOCAL
} else if (choice < 3) { // 25% to use small additions

Expand Down Expand Up @@ -286,9 +293,7 @@ inline static FF mutateFieldElement(FF e, T& rng)
*
* @tparam T
*/
template <typename T>
requires ArithmeticFuzzHelperConstraint<T>
class ArithmeticFuzzHelper {
template <typename T> requires ArithmeticFuzzHelperConstraint<T> class ArithmeticFuzzHelper {
private:
/**
* @brief Mutator swapping two instructions together
Expand Down Expand Up @@ -327,7 +332,7 @@ class ArithmeticFuzzHelper {
if (instructions_count == 0) {
return;
}
if (rng.next() & 1) {
if ((rng.next() & 1) != 0U) {
instructions.erase(instructions.begin() + (rng.next() % instructions_count));
} else {
// We get the logarithm of number of instructions and subtract 1 to delete at most half
Expand Down Expand Up @@ -471,7 +476,8 @@ class ArithmeticFuzzHelper {
std::vector<typename T::Instruction> result;
// Choose the size of th resulting vector
const size_t final_result_size = rng.next() % (vecA_size + vecB_size) + 1;
size_t indexA = 0, indexB = 0;
size_t indexA = 0;
size_t indexB = 0;
size_t* inIndex = &indexA;
size_t inSize = vecA_size;
auto inIterator = vecA.begin();
Expand Down Expand Up @@ -517,7 +523,7 @@ class ArithmeticFuzzHelper {
static std::vector<typename T::Instruction> parseDataIntoInstructions(const uint8_t* Data, size_t Size)
{
std::vector<typename T::Instruction> fuzzingInstructions;
uint8_t* pData = (uint8_t*)Data;
const auto* pData = Data;
size_t size_left = Size;
while (size_left != 0) {
uint8_t chosen_operation = *pData;
Expand Down Expand Up @@ -591,12 +597,14 @@ class ArithmeticFuzzHelper {
* @param instructions
*/
template <typename Composer>
inline static void executeInstructions(std::vector<typename T::Instruction>& instructions)
requires CheckableComposer<Composer>
// TODO(@Rumata888)(from Zac: maybe try to fix? I'm not comfortable refactoring this myself!)
zac-williamson marked this conversation as resolved.
Show resolved Hide resolved
// NOLINTNEXTLINE(readability-function-size, google-readability-function-size)
inline static void executeInstructions(
std::vector<typename T::Instruction>& instructions) requires CheckableComposer<Composer>
{
typename T::ExecutionState state;
Composer composer = Composer();
circuit_should_fail = false;
bool circuit_should_fail = false;
size_t total_instruction_weight = 0;
(void)total_instruction_weight;
for (auto& instruction : instructions) {
Expand Down Expand Up @@ -690,3 +698,5 @@ constexpr void RunWithComposers(const uint8_t* Data, const size_t Size, FastRand
RunWithComposer<Fuzzer, proof_system::TurboCircuitBuilder>(Data, Size, VarianceRNG);
}
}

// NOLINTEND(cppcoreguidelines-macro-usage, google-runtime-int)
Loading