Skip to content

Commit

Permalink
chore: fixed linter errors for ecc, numeric and common modules (#…
Browse files Browse the repository at this point in the history
…1714)

The majority of the barretenberg codebase does not conform to our C++
style guide rules.

This PR updates the `common`, `numeric` and `ecc` modules to conform to
the guide. These 3 modules should now produce no linter errors.

# Checklist:
Remove the checklist to signal you've completed it. Enable auto-merge if
the PR is ready to merge.
- [x] If the pull request requires a cryptography review (e.g.
cryptographic algorithm implementations) I have added the 'crypto' tag.
- [x] I have reviewed my diff in github, line by line and removed
unexpected formatting changes, testing logs, or commented-out code.
- [x] Every change is related to the PR description.
- [x] I have
[linked](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue)
this pull request to relevant issues (if any exist).

---------

Co-authored-by: kevaundray <kevtheappdev@gmail.com>
  • Loading branch information
zac-williamson and kevaundray authored Aug 25, 2023
1 parent d4ede19 commit 026273b
Show file tree
Hide file tree
Showing 99 changed files with 2,362 additions and 2,431 deletions.
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> int64_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>(
(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,82 @@ 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>;
};

/**
* @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 +173,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 +202,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 +231,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 +292,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 +331,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 +475,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 +522,8 @@ 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;
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-const-cast)
auto* pData = const_cast<uint8_t*>(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? not comfortable refactoring this myself. Issue #1807)
// 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

0 comments on commit 026273b

Please sign in to comment.