clang-tidy v. -Weffc++

Open
#54 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
5/5
Estimated time
Over a week
Newbie friendliness
30/100
Issue type
Feature
Clarity
Needs clarification
Activity status
Stale
Tech stack
cpp
Domain
tooling

Research direction

Start with clang-tidy's readability-redundant-member-init documentation and the linked mapbox/cpp#37, wagyu#69, and wagyu#70 discussions; no repository file or test is identified. Determine an agreed approach for -Weffc++, clang-tidy guidance, and detecting uninitialized members; done requires a documented recommendation and configuration.

Written by the indexing model from the issue text.

Description

Context

Recent versions of clang-tidy added readability-redundant-member-init: https://clang.llvm.org/extra/clang-tidy/checks/readability-redundant-member-init.html

This is handy! When a class member has a default initializer it is redundent to initialize it in the member list.

But, because some types don't have default initializers, forgetting to initialize them in the member list (or using c++11 initialization in the class definition) can lead to serious trouble like mapbox/wagyu#69 - refs mapbox/wagyu#70.

So, this is the reason, at https://github.com/mapbox/cpp/issues/37#issuecomment-336200744, that we recommend using g++ and the -Weffc++ flag because it can catch this (note, clang++ plus -Weffc++ cannot):

-Weffc++ - useful when building with g++ (does not do much with clang++). With g++ it can catch uninitialized class members and prevent crashes like mapbox/wagyu#69 - refs mapbox/wagyu#70

Problem

  • -Weffc++ will warn on all class members not explicitly initialized in the initializer list
  • clang-tidy will automatically remove variables from the initializer list that have default constructors

So, the two will fight: causing each other warnings. For this reason I think we should likely:

  • let clang-tidy win
  • recommend no longer using g++ with -Weffc++
  • figure out what alternative way we can catch when members, without default initializers, are uninitialized (without needing to use -Weffc++). Maybe another clang-tidy check?
Dominant language
No language data
Stars
110
Forks
17
No merged PRs in 30d

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from mapbox/cpp

All issues in mapbox/cpp

Similar issues

More DevTools issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.