zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.79k stars 6.58k forks source link

Establish C++ coding guidelines #45195

Open stephanosio opened 2 years ago

stephanosio commented 2 years ago

Introduction

Establish C++ coding guidelines to be used in the Zephyr repository, and implement means to enforce them.

Problem description

While we have multiple coding and style guideline checkers such as checkpatch.pl and guideline_check.py for the C language, we do not for the C++ language. It is necessary to establish and enforce repository-wide C++ coding and style guidelines in order to ensure code quality and consistency.

Proposed change

  1. Adopt one or more C++ coding and style guidelines for use in the Zephyr repository.
    • Refer to the "Guideline Candidates" below.
  2. Adopt tools to enforce such guidelines in the CI.
  3. Document the guidelines.

Guideline Candidates

(please feel free to edit this RFC and add any other guidelines that you think would be good for consideration)

Enforcement Tool Candidates

stephanosio commented 2 years ago

cc @alexanderwachter @yperess @nashif @carlescufi @MaureenHelm @mbolivar-nordic @tejlmand

yperess commented 2 years ago

Hey, just wanted to post here and let you know I haven't forgotten about this. I'm looking at the options and will post my thoughts about this as soon as I have formed a strong opinion.

chrta commented 2 years ago

I think it might be worth to consider the styles supported by clang-tidy and check if one of them can be used, see https://clang.llvm.org/docs/ClangFormatStyleOptions.html#configurable-format-style-options (below BasedOnStyle) There are also links to style guides from other projects.

yperess commented 2 years ago

OK, so a likely unpopular opinion. I've uploaded a .clang-format file here with a rough guideline. It's non standard in C/C++ but it's close to the style guide for Kotlin. The driving factor here is to minimize diffs and merge conflicts. clang-format.txt

stephanosio commented 2 years ago

@yperess Can you provide some examples/sample code in that style? It is hard to see what it looks like with just the .clang-format file.

yperess commented 2 years ago

@yperess Can you provide some examples/sample code in that style? It is hard to see what it looks like with just the .clang-format file.

Sure, here are some examples, it mostly revolves around having things on separate lines. It takes up more vertical space, but makes reviews easier and results in fewer merge conflicts.

int action(
  int color,
  char alpha,
  float
);

int fooNS::FooClass::action(
    int color,
    char alpha,
    float
) {
  return doSomething(
      color,
      alpha
  );
}

ptr
  ->getSelf()
  ->getSelf()
  ->getSelf()
  ->getSelf();
stephanosio commented 2 years ago

FYI @cfriedt

cfriedt commented 2 years ago

I added some comments in https://github.com/zephyrproject-rtos/zephyr/pull/41307, but IMHO, it makes sense to consolidate on clang-format because it supports both C and C++.

cfriedt commented 2 years ago

In terms of linter (clang-tidy), it's possible to run it only on changes too https://clang.llvm.org/extra/doxygen/clang-tidy-diff_8py_source.html @stephanosio

It might also be good to mention clang-format in the description, as it's more of a code formatter and clang-tidy is more of a linter. Of course, scan-build for static analysis, etc.

aborisovich commented 1 year ago

C++ Core Guidelines, maintained by the Standard C++ Foundation certainly has my vote. Those guys really though it through. In addition to those guidelines, please consider following convention of file namings (I'm not sure how it may be enforced except core review remarks):

This way it would be easy to find files that should not be included by C code. Just an idea.