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

BRAYNS-649 Add clang tidy #1284

Merged
merged 12 commits into from
Sep 9, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
47 changes: 40 additions & 7 deletions .clang-format
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@ AlignEscapedNewlines: DontAlign
AlignOperands: DontAlign
AlignTrailingComments: false
AllowAllArgumentsOnNextLine: false
AllowAllConstructorInitializersOnNextLine: false # Deprecated
AllowAllParametersOfDeclarationOnNextLine: false
AllowShortBlocksOnASingleLine: Never
AllowShortCaseLabelsOnASingleLine: false
AllowShortCompoundRequirementOnASingleLine: true
AllowShortEnumsOnASingleLine: false
AllowShortFunctionsOnASingleLine: None
AllowShortIfStatementsOnASingleLine: Never
Expand All @@ -26,13 +26,35 @@ AlwaysBreakTemplateDeclarations: Yes
BinPackArguments: false
BinPackParameters: false
BitFieldColonSpacing: Both
BraceWrapping:
AfterCaseLabel: true
AfterClass: true
AfterControlStatement: Always
AfterEnum: true
AfterFunction: true
AfterNamespace: true
AfterStruct: true
AfterUnion: true
AfterExternBlock: true
BeforeCatch: true
BeforeElse: true
BeforeLambdaBody: true
BeforeWhile: true
IndentBraces: false
SplitEmptyRecord: true
SplitEmptyNamespace: true
BreakAdjacentStringLiterals: true
BreakAfterAttributes: Never
BreakAfterReturnType: Automatic
BreakBeforeBinaryOperators: NonAssignment
BreakBeforeBraces: Allman
BreakBeforeConceptDeclarations: true
BreakBeforeBraces: Custom
BreakBeforeConceptDeclarations: Always
BreakBeforeTernaryOperators: true
BreakConstructorInitializers: AfterColon
BreakFunctionDefinitionParameters: false
BreakInheritanceList: AfterColon
BreakStringLiterals: true
BreakStringLiterals: false
BreakTemplateDeclarations: Yes
ColumnLimit: 120
CompactNamespaces: false
ConstructorInitializerIndentWidth: 4
Expand All @@ -50,12 +72,21 @@ IndentCaseLabels: false
IndentExternBlock: true
IndentGotoLabels: false
IndentPPDirectives: BeforeHash
IndentRequires: true
IndentRequiresClause: true
IndentWidth: 4
IndentWrappedFunctionNames: true
KeepEmptyLinesAtTheStartOfBlocks: false
InsertBraces: true
IntegerLiteralSeparator:
Binary: -1
Decimal: 3
Hex: -1
KeepEmptyLines:
AtEndOfFile: false
AtStartOfBlock: false
AtStartOfFile: false
LambdaBodyIndentation: Signature
Language: Cpp
MainIncludeChar: Quote
MaxEmptyLinesToKeep: 1
NamespaceIndentation: None
PackConstructorInitializers: Never
Expand All @@ -73,7 +104,9 @@ PointerAlignment: Right
QualifierAlignment: Custom
QualifierOrder: ["static", "inline", "const", "volatile", "type"]
ReferenceAlignment: Right
ReflowComments: true
ReflowComments: false
RequiresClausePosition: WithPreceding
RequiresExpressionIndentation: OuterScope
SeparateDefinitionBlocks: Always
ShortNamespaceLines: 1
SortIncludes: true
Expand Down
57 changes: 57 additions & 0 deletions .clang-tidy
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
Checks: -*
bugprone-*
cert-*
concurrency-*
cppcoreguidelines-*
misc-*
modernize-*
performance-*
portability-*
readability-*
-cppcoreguidelines-avoid-magic-numbers
-cppcoreguidelines-missing-std-forward
-cppcoreguidelines-pro-*
-misc-include-cleaner
-modernize-use-nodiscard
-modernize-use-trailing-return-type
-readability-identifier-length
-readability-magic-numbers

WarningsAsErrors: "*"
HeaderFilterRegex: "src/.*"

CheckOptions:
- key: readability-identifier-naming.ClassCase
value: CamelCase
- key: readability-identifier-naming.ClassMemberCase
value: CamelCase
- key: readability-identifier-naming.ConstexprVariableCase
value: CamelCase
- key: readability-identifier-naming.EnumCase
value: CamelCase
- key: readability-identifier-naming.EnumConstantCase
value: CamelCase
- key: readability-identifier-naming.FunctionCase
value: camelBack
- key: readability-identifier-naming.FunctionIgnoredRegexp
value: main|wmain|wWinMain|begin|end
- key: readability-identifier-naming.GlobalConstantCase
value: camelBack
- key: readability-identifier-naming.StaticConstantCase
value: camelBack
- key: readability-identifier-naming.StaticVariableCase
value: camelBack
- key: readability-identifier-naming.MacroDefinitionCase
value: UPPER_CASE
- key: readability-identifier-naming.MemberCase
value: camelBack
- key: readability-identifier-naming.PrivateMemberPrefix
value: _
- key: readability-identifier-naming.NamespaceCase
value: camelBack
- key: readability-identifier-naming.ParameterCase
value: camelBack
- key: readability-identifier-naming.TypeAliasCase
value: CamelCase
- key: readability-identifier-naming.VariableCase
value: camelBack
11 changes: 9 additions & 2 deletions .github/workflows/linter.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,15 @@ jobs:
steps:
- name: Checkout repository
uses: actions/checkout@v4
- name: Update APT package list
run: sudo apt-get update
- name: Download and install LLVM
run: |
wget https://apt.llvm.org/llvm.sh
sudo bash llvm.sh 20
- name: Install clang-format
run: sudo apt-get -y install clang-format-20
- name: Run clang-format
run: |
apt update && apt install -y clang-format-15
SOURCES=$(find apps src tests \( -name "*.h" -or -name "*.cpp" \))
clang-format-15 --dry-run --Werror $SOURCES
clang-format-20 --dry-run --Werror $SOURCES
20 changes: 20 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,26 @@ The following cmake options (shown with their default value) can be used during
* **BRAYNS_ENABLE_CIRCUITS** (Default ON) - Activate circuit support.
* **BRAYNS_ENABLE_ATLAS** - (Default ON) Activate atlas support.

### Run linters

Run clang format as follows:

$ SOURCES=$(find apps src tests \( -name "*.h" -or -name "*.cpp" \))
$ clang-format-20 --Werror $SOURCES

Run clang tidy as follows:

Use cmake config to generate build commands:

"cacheVariables": {
"CMAKE_EXPORT_COMPILE_COMMANDS": true,
"CMAKE_CXX_STANDARD_INCLUDE_DIRECTORIES": "/usr/include/c++/11;/usr/include/x86_64-linux-gnu/c++/11"
}

Run with

$ run-clang-tidy-20 -p build/user-debug


## Running

Expand Down
12 changes: 12 additions & 0 deletions scripts/install_llvm.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# Copyright (c) 2015-2024, EPFL/Blue Brain Project
# All rights reserved. Do not distribute without permission.
# Responsible Author: Adrien Fleury <[email protected]>
#
# This file is part of Brayns <https://github.com/BlueBrain/Brayns>

wget https://apt.llvm.org/llvm.sh
chmod +x llvm.sh
./llvm.sh 20

apt-get install clang-format-20
apt-get install clang-tidy-20
2 changes: 1 addition & 1 deletion src/brayns/core/Launcher.h
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ struct ArgvSettingsReflector<ServiceSettings>
.defaultValue("localhost");
builder.option("port", [](auto &settings) { return &settings.port; })
.description("Websocket server port")
.defaultValue(5000);
.defaultValue(5'000);
builder.option("max-thread-count", [](auto &settings) { return &settings.maxThreadCount; })
.description("Maximum number of threads for the websocket server")
.defaultValue(10);
Expand Down
10 changes: 6 additions & 4 deletions src/brayns/core/cli/CommandLine.h
Original file line number Diff line number Diff line change
Expand Up @@ -131,9 +131,11 @@ template<typename T>
struct ArgvReflector;

template<typename T>
concept ReflectedArgvOption = std::same_as<std::string, decltype(ArgvReflector<T>::getType())>
&& std::same_as<std::string, decltype(ArgvReflector<T>::toString(T()))>
&& std::same_as<T, decltype(ArgvReflector<T>::parse(std::string_view()))>;
concept ReflectedArgvOption = requires(T value) {
{ ArgvReflector<T>::getType() } -> std::same_as<std::string>;
{ ArgvReflector<T>::toString(value) } -> std::same_as<std::string>;
{ ArgvReflector<T>::parse(std::string_view()) } -> std::same_as<T>;
};

template<>
struct ArgvReflector<bool>
Expand All @@ -160,7 +162,7 @@ struct ArgvReflector<bool>
};

template<typename T>
requires std::is_arithmetic_v<T>
requires std::is_arithmetic_v<T>
struct ArgvReflector<T>
{
static std::string getType()
Expand Down
2 changes: 1 addition & 1 deletion src/brayns/core/codecs/PngCodec.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ std::string encodePng(const ImageView &image)
.message = {},
};

auto rowStride = PNG_IMAGE_ROW_STRIDE(png);
auto rowStride = png_int_32(PNG_IMAGE_ROW_STRIDE(png));

if (rowOrder == RowOrder::BottomUp)
{
Expand Down
6 changes: 3 additions & 3 deletions src/brayns/core/engine/Data.h
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ Data<T> allocateData(Device &device, std::size_t itemCount)
}

template<OsprayDataType T, std::ranges::range U>
requires(std::convertible_to<std::ranges::range_value_t<U>, T>)
requires std::convertible_to<std::ranges::range_value_t<U>, T>
Data<T> createData(Device &device, U &&items)
{
auto data = allocateData<T>(device, items.size());
Expand Down Expand Up @@ -249,15 +249,15 @@ Data3D<OutputType> createDataView3D(const Data3D<T> &data, const DataRegion3D &r
template<OsprayDataType OutputType, OsprayDataType T>
Data2D<OutputType> createDataView2D(const Data2D<T> &data, const DataRegion2D &region)
{
auto region3D = DataRegion3D{Size3(region.itemCount, 1), Size3(region.stride, 0), region.offset};
auto region3D = DataRegion3D{Size3(region.itemCount, 1), Stride3(region.stride, 0), region.offset};
auto view = createDataView3D<OutputType>(data, region3D);
return Data2D<OutputType>(std::move(view));
}

template<OsprayDataType OutputType, OsprayDataType T>
Data<OutputType> createDataView(const Data<T> &data, const DataRegion &region)
{
auto region3D = DataRegion3D{Size3(region.itemCount, 1, 1), Size3(region.stride, 0, 0), region.offset};
auto region3D = DataRegion3D{Size3(region.itemCount, 1, 1), Stride3(region.stride, 0, 0), region.offset};
auto view = createDataView3D<OutputType>(data, region3D);
return Data<OutputType>(std::move(view));
}
Expand Down
2 changes: 1 addition & 1 deletion src/brayns/core/json/JsonSchema.h
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,6 @@ struct JsonSchema
std::optional<std::size_t> maxItems = {};
std::map<std::string, JsonSchema> properties = {};

auto operator<=>(const JsonSchema &) const = default;
bool operator==(const JsonSchema &) const = default;
};
}
16 changes: 8 additions & 8 deletions src/brayns/core/jsonrpc/Errors.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,42 +58,42 @@ const JsonValue &JsonRpcException::getData() const
}

ParseError::ParseError(const std::string &message):
JsonRpcException(-32700, message)
JsonRpcException(-32'700, message)
{
}

InvalidRequest::InvalidRequest(const std::string &message):
JsonRpcException(-32600, message)
JsonRpcException(-32'600, message)
{
}

InvalidRequest::InvalidRequest(const std::string &message, const std::vector<JsonSchemaError> &errors):
JsonRpcException(-32600, message, serializeToJson(errors))
JsonRpcException(-32'600, message, serializeToJson(errors))
{
}

MethodNotFound::MethodNotFound(const std::string &method):
JsonRpcException(-32601, fmt::format("Method not found: '{}'", method))
JsonRpcException(-32'601, fmt::format("Method not found: '{}'", method))
{
}

InvalidParams::InvalidParams(const std::string &message):
JsonRpcException(-32602, message)
JsonRpcException(-32'602, message)
{
}

InvalidParams::InvalidParams(const std::string &message, const std::vector<JsonSchemaError> &errors):
JsonRpcException(-32602, message, serializeToJson(errors))
JsonRpcException(-32'602, message, serializeToJson(errors))
{
}

InternalError::InternalError(const std::string &message):
JsonRpcException(-32603, message)
JsonRpcException(-32'603, message)
{
}

InternalError::InternalError(const std::exception &e):
JsonRpcException(-32603, e.what())
JsonRpcException(-32'603, e.what())
{
}
}
8 changes: 5 additions & 3 deletions src/brayns/core/jsonrpc/PayloadReflector.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,11 @@ template<typename T>
struct PayloadReflector;

template<typename T>
concept ReflectedPayload = std::same_as<JsonSchema, decltype(PayloadReflector<T>::getSchema())>
&& std::same_as<Payload, decltype(PayloadReflector<T>::serialize(std::declval<T>()))>
&& std::same_as<T, decltype(PayloadReflector<T>::deserialize(Payload()))>;
concept ReflectedPayload = requires(T value) {
{ PayloadReflector<T>::getSchema() } -> std::same_as<JsonSchema>;
{ PayloadReflector<T>::serialize(std::move(value)) } -> std::same_as<Payload>;
{ PayloadReflector<T>::deserialize(Payload()) } -> std::same_as<T>;
};

template<>
struct PayloadReflector<Payload>
Expand Down
13 changes: 7 additions & 6 deletions src/brayns/core/objects/common/Binary.h
Original file line number Diff line number Diff line change
Expand Up @@ -70,12 +70,13 @@ Data3D<T> createData3DFromBinaryOf(Device &device, const Size3 &itemCount, std::

if (reduceMultiply(itemCount) != totalItemCount)
{
throw InvalidParams(fmt::format(
"Item count in binary {} does not match given item count {}x{}x{}",
totalItemCount,
itemCount.x,
itemCount.y,
itemCount.z));
throw InvalidParams(
fmt::format(
"Item count in binary {} does not match given item count {}x{}x{}",
totalItemCount,
itemCount.x,
itemCount.y,
itemCount.z));
}

auto data = allocateData3D<T>(device, itemCount);
Expand Down
Loading