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

[CQT-190] [Infra] Add clang-tidy #270

Merged
merged 23 commits into from
Nov 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
7db16cf
Add an initial version of .clang-tidy.
rturrado Oct 22, 2024
26afd02
Add -misc-unused-alias-decls, as they seem to be false positives.
rturrado Nov 7, 2024
8d2f723
Add cpp-linters badge to README.md.
rturrado Nov 22, 2024
b4594e4
Enable clang-tidy checks in linters workflow.
rturrado Nov 22, 2024
9fbac22
Change the value of two variables from True to ON.
rturrado Nov 22, 2024
822704c
Update cpp-linters job checks.
rturrado Nov 22, 2024
2341fc7
Change cpp-linters job to build gcc/Linux/x64 before running the lint…
rturrado Nov 22, 2024
56e2e70
Change cpp-linters job to build gcc/Linux/x64 before running the lint…
rturrado Nov 22, 2024
6a4e4db
Merge remote-tracking branch 'origin/CQT-190-Infra-Add-clang-tidy' in…
rturrado Nov 22, 2024
d605959
Trying to correctly set cpp-linter-action database (folder where comp…
rturrado Nov 22, 2024
c0edcb5
Building emscripten binaries (so that cpp-linters can find emscripten…
rturrado Nov 22, 2024
7d8301a
Building emscripten binaries (so that cpp-linters can find emscripten…
rturrado Nov 22, 2024
e471a5f
Building gcc/Linux/x64 but ignoring emscripten_wrapper.hpp.
rturrado Nov 22, 2024
fdc9cd8
Run cpp-linters both for gcc and clang.
rturrado Nov 22, 2024
c3efeb4
Temporarily remove clang compiler from cpp-linters job.
rturrado Nov 22, 2024
ddca492
Test multi-line notice message.
rturrado Nov 22, 2024
7d68140
run_cpp_linters.py: change default build folder to build/Release.
rturrado Nov 22, 2024
4a37b4c
linters.yaml: change multi-line notice warning message.
rturrado Nov 22, 2024
13807f2
Remove a couple of linter warnings:
rturrado Nov 25, 2024
d486d60
Update docs.
rturrado Nov 25, 2024
05e7b4a
Apply suggestions from code review
rturrado Nov 29, 2024
9861384
Merge branch 'develop' into CQT-190-Infra-Add-clang-tidy
rturrado Nov 29, 2024
13ef7ab
Merge remote-tracking branch 'origin/CQT-190-Infra-Add-clang-tidy' in…
rturrado Nov 29, 2024
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
48 changes: 48 additions & 0 deletions .clang-tidy
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
Checks: |
-*,
clang-analyzer-*,
clang-diagnostic-*,
misc-*,
-misc-const-correctness,
-misc-include-cleaner
-misc-non-private-member-variables-in-classes,
-misc-no-recursion,
-misc-unused-alias-decls,
-misc-unused-parameters,
-misc-use-anonymous-namespace,
modernize-*,
-modernize-use-trailing-return-type,
readability-identifier-naming,

CheckOptions:
- key: readability-identifier-naming.ClassCase
value: CamelCase
- key: readability-identifier-naming.ClassIgnoredRegexp
value: "bf_cp|f_cp|uf_cp"
- key: readability-identifier-naming.EnumCase
value: CamelCase
- key: readability-identifier-naming.FunctionCase
value: lower_case
- key: readability-identifier-naming.FunctionIgnoredRegexp
value: ".*_|SetUp|MOCK_METHOD|TEST|TEST_F|TestBody\
|DescribeNegationTo|DescribeTo|MatchAndExplain|ValuesEq\
|addErrorListener|copyNodeAnnotation|expandNodeAnnotation|setNodeAnnotation|syntaxError\
|visit.*"
- key: readability-identifier-naming.MemberCase
value: lower_case
- key: readability-identifier-naming.MemberIgnoredRegexp
value: ".*_"
- key: readability-identifier-naming.ParameterCase
value: lower_case
- key: readability-identifier-naming.ParameterIgnoredRegexp
value: ".*_|AnalysisResult|ParseResult"
- key: readability-identifier-naming.UnionCase
value: CamelCase
- key: readability-identifier-naming.VariableCase
value: lower_case
- key: readability-identifier-naming.IgnoreMainLikeFunctions
value: 1
- key: readability-redundant-member-init.IgnoreBaseInCopyConstructors
value: 1
- key: modernize-use-default-member-init.UseAssignment
value: 1
16 changes: 13 additions & 3 deletions .github/workflows/linters.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,28 @@ jobs:
steps:
- name: Checkout
uses: actions/checkout@v4
- name: Build C++ tests (gcc-clang/Linux/x64)
uses: ./.github/actions/cpp-tests
with:
build_type: Release
conan_profile_host: conan/profiles/tests-release-gcc-linux-x64
shell: bash
- name: Run C++ linters
uses: cpp-linter/cpp-linter-action@v2
id: linter
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
with:
database: '${{ github.workspace }}/build/Release'
extensions: 'cpp,hpp'
ignore: 'emscripten/emscripten_wrapper.hpp'
style: 'file' # use .clang-format config file
tidy-checks: '-*' # disable clang-tidy checks
tidy-checks: '' # use .clang-tidy config file
version: 18
- name: Fail fast
if: steps.linter.outputs.clang-format-checks-failed > 0
if: steps.linter.outputs.checks-failed > 0
run: |
echo "::notice::Try executing 'python3 ./scripts/run_cpp_linters.py .' to fix linter issues."
echo "::notice::Try running the following commands to fix and/or understand linter issues:"
echo "::notice:: conan build . -pr:a=conan/profiles/tests-release-gcc-linux-x64 -b missing"
echo "::notice:: python3 ./scripts/run_cpp_linters.py ."
exit 1
4 changes: 3 additions & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,11 @@ project(cqasm LANGUAGES C CXX)
if(NOT TARGET cqasm)

set(CMAKE_CXX_STANDARD 20)
set(CMAKE_CXX_STANDARD_REQUIRED True)
set(CMAKE_CXX_STANDARD_REQUIRED ON)
set(CMAKE_CXX_EXTENSIONS OFF)

set(CMAKE_EXPORT_COMPILE_COMMANDS ON)

# Library type option. Default is a static library.
option(BUILD_SHARED_LIBS
"whether the cqasm library should be built as a shared object or as a static library"
Expand Down
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

[![CI](https://github.com/QuTech-Delft/libqasm/workflows/Test/badge.svg)](https://github.com/qutech-delft/libqasm/actions)
[![Conan Center](https://img.shields.io/conan/v/libqasm)](https://conan.io/center/recipes/libqasm)
[![cpp-linter](https://github.com/cpp-linter/cpp-linter-action/actions/workflows/cpp-linter.yml/badge.svg)](https://github.com/cpp-linter/cpp-linter-action/actions/workflows/cpp-linter.yml)
[![PyPI](https://badgen.net/pypi/v/libqasm)](https://pypi.org/project/libqasm/)
![OS](https://img.shields.io/badge/os-emscripten%20%7C%20linux%20%7C%20macos%20%7C%20windows-blue?style=flat-square)
[![License](https://img.shields.io/badge/License-Apache_2.0-blue.svg)](https://opensource.org/licenses/Apache-2.0)
Expand Down
10 changes: 8 additions & 2 deletions docs/dev-guide/dev-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ Tests are disabled by default. To enable them, use `-c tools.build:skip_test=Fal
Build and serve on `http://127.0.0.1:8000/`.

```shell
export PTYHONPATH=./scripts/python
export PYTHONPATH=./scripts/python
mkdocs serve
```

Expand All @@ -137,12 +137,18 @@ Continuous Integration will fail if the files do not adhere to a series of forma
It is recommended to run these linters before pushing any change:

```shell
conan build . -pr:a=conan/profiles/tests-release-gcc-linux-x64 -b missing
python3 ./scripts/run_cpp_linters.py .
```

!!! note

The linters require `clang-format-18` and `clang-tidy-18` to be installed on the system.
- The linters require`clang-format-18` and `clang-tidy-18`.
- It is mandatory to have a build before running the linters.
- `clang-tidy` expects to find a `compile_commands.json` in a build folder.
- It is recommended to build with _gcc_ in _Release_ mode.
- We have observed `clang-tidy` fails to find some standard headers when compiling with _clang_.
- `run_cpp_linters.py` can receive a build folder as second argument, but defaults to `build/Release`.

## Docker

Expand Down
1 change: 1 addition & 0 deletions emscripten/emscripten_wrapper.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ struct EmscriptenWrapper {
std::string analyze_string_to_json(const std::string& data, const std::string& file_name);
};

// NOLINTNEXTLINE
EMSCRIPTEN_BINDINGS(CqasmJS) {
emscripten::class_<EmscriptenWrapper>("EmscriptenWrapper")
.constructor()
Expand Down
4 changes: 2 additions & 2 deletions include/libqasm/error.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -75,15 +75,15 @@ class Error : public std::runtime_error {
/**
* Returns the message exception-style.
*/
const char* what() const noexcept override;
[[nodiscard]] const char* what() const noexcept override;

/**
* Returns a string with a JSON representation of an Error.
* The JSON representation follows the Language Server Protocol (LSP) specification.
* Every error is mapped to an LSP Diagnostic structure:
* Severity is hardcoded to 1 at the moment (value corresponding to an Error level)
*/
std::string to_json() const;
[[nodiscard]] std::string to_json() const;
};

/**
Expand Down
1 change: 0 additions & 1 deletion include/libqasm/overload.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
#include <utility> // pair
#include <vector>

#include "libqasm/overload.hpp"
#include "libqasm/tree.hpp"

/**
Expand Down
2 changes: 2 additions & 0 deletions include/libqasm/v3x/analysis_result.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,10 @@ using Root = ast::One<semantic::Program>;
*/
class AnalysisFailed : public std::runtime_error {
public:
// NOLINTBEGIN
AnalysisFailed()
: std::runtime_error{ "cQASM analysis failed" } {};
// NOLINTEND
};

/**
Expand Down
1 change: 0 additions & 1 deletion include/libqasm/v3x/analyzer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
#include <string>

#include "analysis_result.hpp"
#include "analyzer.hpp"
#include "libqasm/v3x/ast.hpp"
#include "libqasm/v3x/core_function.hpp"
#include "libqasm/v3x/parse_helper.hpp"
Expand Down
6 changes: 3 additions & 3 deletions include/libqasm/v3x/antlr_custom_error_listener.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,12 @@ class AntlrCustomErrorListener : public antlr4::BaseErrorListener {
*/
std::optional<std::string> file_name_;

void syntaxError(antlr4::Recognizer* recognizer, antlr4::Token* offendingSymbol, size_t line,
size_t charPositionInLine, const std::string& msg, std::exception_ptr e) override;
void syntaxError(antlr4::Recognizer* recognizer, antlr4::Token* offending_symbol, size_t line,
size_t char_position_in_line, const std::string& msg, std::exception_ptr e) override;

public:
explicit AntlrCustomErrorListener(const std::optional<std::string>& file_name);
void syntaxError(size_t line, size_t charPositionInLine, const std::string& msg);
void syntaxError(size_t line, size_t char_position_in_line, const std::string& msg);
};

} // namespace cqasm::v3x::parser
4 changes: 2 additions & 2 deletions include/libqasm/v3x/antlr_scanner.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ class FileAntlrScanner : public AntlrScanner {

public:
FileAntlrScanner(std::unique_ptr<BaseSyntacticAnalyzer> build_visitor_up,
std::unique_ptr<AntlrCustomErrorListener> error_listener_up, const std::string& file_path);
std::unique_ptr<AntlrCustomErrorListener> error_listener_up, std::string file_path);

~FileAntlrScanner() override;

Expand All @@ -54,7 +54,7 @@ class StringAntlrScanner : public AntlrScanner {

public:
StringAntlrScanner(std::unique_ptr<BaseSyntacticAnalyzer> build_visitor_up,
std::unique_ptr<AntlrCustomErrorListener> error_listener_up, const std::string& data);
std::unique_ptr<AntlrCustomErrorListener> error_listener_up, std::string data);

~StringAntlrScanner() override;

Expand Down
8 changes: 1 addition & 7 deletions include/libqasm/v3x/core_function.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,14 +49,8 @@ class CoreFunction : public tree::Base {
* param_types is a shorthand type specification string as parsed by cqasm::types::from_spec().
* return_type is a shorthand type specification char as parsed by cqasm::types::from_spec().
*/
CoreFunction(std::string name, const std::string& param_types, const char return_type);

CoreFunction() = default;
~CoreFunction() override = default;
CoreFunction(const CoreFunction& t) = default;
CoreFunction(CoreFunction&& t) noexcept = default;
CoreFunction& operator=(const CoreFunction& t) = default;
CoreFunction& operator=(CoreFunction&& t) noexcept = default;
CoreFunction(std::string name, const std::string& param_types, const char return_type);

bool operator==(const CoreFunction& rhs) const;
inline bool operator!=(const CoreFunction& rhs) const { return !(*this == rhs); }
Expand Down
9 changes: 5 additions & 4 deletions include/libqasm/v3x/cqasm_python.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -146,24 +146,25 @@ class V3xAnalyzer {
/**
* Parses and analyzes a file containing a cQASM v3.0 program.
*/
std::vector<std::string> analyze_file(const std::string& file_name) const;
[[nodiscard]] std::vector<std::string> analyze_file(const std::string& file_name) const;

/**
* Parses and analyzes a file containing a cQASM v3.0 program.
*/
std::string analyze_file_to_json(const std::string& file_name) const;
[[nodiscard]] std::string analyze_file_to_json(const std::string& file_name) const;

/**
* Parses and analyzes a string containing a cQASM v3.0 program.
*
* `file_name` is optional. It is only used when reporting errors.
*/
std::vector<std::string> analyze_string(const std::string& data, const std::string& file_name = "") const;
[[nodiscard]] std::vector<std::string> analyze_string(
const std::string& data, const std::string& file_name = "") const;

/**
* Parses and analyzes a string containing a cQASM v3.0 program.
*
* `file_name` is optional. It is only used when reporting errors.
*/
std::string analyze_string_to_json(const std::string& data, const std::string& file_name = "") const;
[[nodiscard]] std::string analyze_string_to_json(const std::string& data, const std::string& file_name = "") const;
};
2 changes: 2 additions & 0 deletions include/libqasm/v3x/resolver.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ using Values = values::Values;
// Analysis errors //
//-----------------//

// NOLINTBEGIN
/**
* Exception for failed name resolutions.
*/
Expand All @@ -50,6 +51,7 @@ CQASM_ANALYSIS_ERROR(OverloadResolutionFailure);
* Exception for failed resolutions.
*/
CQASM_ANALYSIS_ERROR(ResolutionFailure);
// NOLINTEND

//------------------------//
// OverloadedNameResolver //
Expand Down
4 changes: 2 additions & 2 deletions include/libqasm/v3x/syntactic_analyzer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ class SyntacticAnalyzer : public BaseSyntacticAnalyzer {
std::int64_t get_int_value(antlr4::tree::TerminalNode* node) const;
double get_float_value(antlr4::tree::TerminalNode* node) const;

tree::Maybe<ast::IntegerLiteral> getArraySize(CqasmParser::ArraySizeDeclarationContext* context);
tree::Maybe<ast::IntegerLiteral> get_array_size(CqasmParser::ArraySizeDeclarationContext* context);

template <typename Context>
tree::One<ast::Variable> visitVariable(Context* context) {
Expand Down Expand Up @@ -107,7 +107,7 @@ class SyntacticAnalyzer : public BaseSyntacticAnalyzer {
std::any visitFloatLiteral(CqasmParser::FloatLiteralContext* context) override;

explicit SyntacticAnalyzer(const std::optional<std::string>& file_name);
void addErrorListener(AntlrCustomErrorListener* errorListener) override;
void addErrorListener(AntlrCustomErrorListener* error_listener) override;
void syntaxError(size_t line, size_t char_position_in_line, const std::string& text) const override;
void setNodeAnnotation(const ast::One<ast::Node>& node, antlr4::Token* token) const override;
void expandNodeAnnotation(const ast::One<ast::Node>& node, antlr4::Token* token) const override;
Expand Down
2 changes: 1 addition & 1 deletion include/libqasm/v3x/syntactic_analyzer_base.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ namespace cqasm::v3x::parser {

class BaseSyntacticAnalyzer : public CqasmParserVisitor {
public:
virtual void addErrorListener(AntlrCustomErrorListener* errorListener) = 0;
virtual void addErrorListener(AntlrCustomErrorListener* error_listener) = 0;
virtual void syntaxError(size_t line, size_t char_position_in_line, const std::string& text) const = 0;
virtual void setNodeAnnotation(const ast::One<ast::Node>& node, antlr4::Token* token) const = 0;
virtual void expandNodeAnnotation(const ast::One<ast::Node>& node, antlr4::Token* token) const = 0;
Expand Down
40 changes: 26 additions & 14 deletions scripts/run_cpp_linters.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,19 +9,20 @@
def print_usage():
usage_str = '''
Usage:
python3 run_cpp_linters.py <ROOT_FOLDER>
python3 run_cpp_linters.py <ROOT_FOLDER> [<BUILD_FOLDER>]
Where:
ROOT_FOLDER: folder containing the .clang-format, .clang-tidy, and C++ files.
BUILD_FOLDER: build folder (optional, defaulted to 'build/Release').
Example:
python3 run_cpp_linters.py .
python3 run_cpp_linters.py . build/Debug
'''
logging.info(usage_str)


def get_list_of_cpp_files(root_folder: os.PathLike) -> list[str]:
ret: list[str] = []
file_types = ['cpp', 'hpp']
input_subfolders = ['emscripten', 'include', 'src', 'test']
input_subfolders = ['include', 'src', 'test']
for input_subfolder in input_subfolders:
input_folder = os.path.join(root_folder, input_subfolder)
for root, dirs, files in os.walk(input_folder):
Expand All @@ -32,35 +33,46 @@ def get_list_of_cpp_files(root_folder: os.PathLike) -> list[str]:
return ret


def run_clang_format(root_folder: os.PathLike):
def run_clang_format(root_folder: os.PathLike, file_list_str: str) -> None:
rturrado marked this conversation as resolved.
Show resolved Hide resolved
logging.info("Running clang-format")
file_list = get_list_of_cpp_files(root_folder)
try:
format_file_path = os.path.join(root_folder, ".clang-format")
file_list_str = " ".join(file_list)
command = f"clang-format-18 -i --style=file:{format_file_path} --verbose {file_list_str}"
options = f"-i --style=file:{format_file_path} --verbose"
command = f"clang-format-18 {options} {file_list_str}"
subprocess.run(command, shell=True)
except FileNotFoundError as err:
print("Error running clang-format: {}".format(err.strerror))
exit(3)


def run_clang_tidy(root_folder: os.PathLike):
pass
def run_clang_tidy(root_folder: os.PathLike, build_folder: os.PathLike, file_list_str: str) -> None :
logging.info("Running clang-tidy")
try:
config_file_path = os.path.join(root_folder, ".clang-tidy")
options = f"--config-file={config_file_path} --fix --format-style=file -p {build_folder} --use-color"
command = f"clang-tidy-18 {options} {file_list_str}"
subprocess.run(command, shell=True)
except FileNotFoundError as err:
print("Error running clang-tidy: {}".format(err.strerror))
exit(4)


def run_cpp_linters(root_folder: os.PathLike):
def run_cpp_linters(root_folder: os.PathLike, build_folder: os.PathLike):
logging.info("Running C++ linters")
run_clang_format(root_folder)
run_clang_tidy(root_folder)
file_list = get_list_of_cpp_files(root_folder)
file_list_str = " ".join(file_list)
run_clang_format(root_folder, file_list_str)
run_clang_tidy(root_folder, build_folder, file_list_str)


def main(argv):
logging.basicConfig(level=logging.INFO, format='%(message)s')
if len(argv) != 2:
if len(argv) < 2 or len(argv) > 3:
rturrado marked this conversation as resolved.
Show resolved Hide resolved
print_usage()
exit(2)
run_cpp_linters(os.path.abspath(argv[1]))
root_folder = os.path.abspath(argv[1])
build_folder = os.path.abspath(argv[2] if len(argv) == 3 else os.path.join("build", "Release"))
run_cpp_linters(root_folder, build_folder)

if __name__ == '__main__':
main(sys.argv)
3 changes: 1 addition & 2 deletions src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -248,8 +248,7 @@ if(CMAKE_COMPILER_IS_GNUCXX)
-Wall -Wextra -Werror -Wfatal-errors
-fPIC
-Wno-error=deprecated-declarations
-Wno-error=maybe-uninitialized
-Wno-error=restrict
-Wno-error=uninitialized
-Wno-error=sign-compare
-Wno-error=unused-parameter
)
Expand Down
Loading