Skip to content

Uniform initialisation for cpp11 #244

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

Closed
Closed
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
4 changes: 2 additions & 2 deletions clang-tools-extra/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ add_subdirectory(tool-template)

# Add the common testsuite after all the tools.
if(CLANG_INCLUDE_TESTS)
add_subdirectory(test)
add_subdirectory(unittests)
add_subdirectory(test)
add_subdirectory(unittests)
endif()

option(CLANG_TOOLS_EXTRA_INCLUDE_DOCS "Generate build targets for the Clang Extra Tools docs."
Expand Down
2 changes: 2 additions & 0 deletions clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ set(LLVM_LINK_COMPONENTS support)

add_clang_library(clangTidyCppCoreGuidelinesModule
AvoidGotoCheck.cpp
ConstCorrectnessCheck.cpp
CppCoreGuidelinesTidyModule.cpp
InitVariablesCheck.cpp
InterfacesGlobalInitCheck.cpp
Expand All @@ -23,6 +24,7 @@ add_clang_library(clangTidyCppCoreGuidelinesModule
SpecialMemberFunctionsCheck.cpp

LINK_LIBS
clangAnalysis
clangAST
clangASTMatchers
clangBasic
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,178 @@
//===--- ConstCorrectnessCheck.cpp - clang-tidy -----------------*- C++ -*-===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//

#include "../utils/FixItHintUtils.h"
#include "ConstCorrectnessCheck.h"
#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/ASTMatchers/ASTMatchers.h"

using namespace clang::ast_matchers;

namespace clang {
namespace tidy {
namespace cppcoreguidelines {

namespace {
// FIXME: This matcher exists in some other code-review as well.
// It should probably move to ASTMatchers.
AST_MATCHER(VarDecl, isLocal) { return Node.isLocalVarDecl(); }
AST_MATCHER_P(DeclStmt, containsDeclaration2,
ast_matchers::internal::Matcher<Decl>, InnerMatcher) {
return ast_matchers::internal::matchesFirstInPointerRange(
InnerMatcher, Node.decl_begin(), Node.decl_end(), Finder, Builder);
}
AST_MATCHER(ReferenceType, isSpelledAsLValue) {
return Node.isSpelledAsLValue();
}
AST_MATCHER(Type, isDependentType) { return Node.isDependentType(); }
} // namespace

void ConstCorrectnessCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
Options.store(Opts, "AnalyzeValues", AnalyzeValues);
Options.store(Opts, "AnalyzeReferences", AnalyzeReferences);
Options.store(Opts, "WarnPointersAsValues", WarnPointersAsValues);

Options.store(Opts, "TransformValues", TransformValues);
Options.store(Opts, "TransformReferences", TransformReferences);
Options.store(Opts, "TransformPointersAsValues", TransformPointersAsValues);
}

void ConstCorrectnessCheck::registerMatchers(MatchFinder *Finder) {
const auto ConstType = hasType(isConstQualified());
const auto ConstReference = hasType(references(isConstQualified()));
const auto RValueReference = hasType(
referenceType(anyOf(rValueReferenceType(), unless(isSpelledAsLValue()))));

const auto TemplateType = anyOf(
hasType(hasCanonicalType(templateTypeParmType())),
hasType(substTemplateTypeParmType()), hasType(isDependentType()),
// References to template types, their substitutions or typedefs to
// template types need to be considered as well.
hasType(referenceType(pointee(hasCanonicalType(templateTypeParmType())))),
hasType(referenceType(pointee(substTemplateTypeParmType()))));

const auto AutoTemplateType = varDecl(
anyOf(hasType(autoType()), hasType(referenceType(pointee(autoType()))),
hasType(pointerType(pointee(autoType())))),
hasInitializer(isInstantiationDependent()));

const auto FunctionPointerRef =
hasType(hasCanonicalType(referenceType(pointee(functionType()))));

// Match local variables which could be 'const' if not modified later.
// Example: `int i = 10` would match `int i`.
const auto LocalValDecl = varDecl(
allOf(isLocal(), hasInitializer(anything()),
unless(anyOf(ConstType, ConstReference, TemplateType,
AutoTemplateType, RValueReference, FunctionPointerRef,
hasType(cxxRecordDecl(isLambda())), isImplicit()))));

// Match the function scope for which the analysis of all local variables
// shall be run.
const auto FunctionScope = functionDecl(hasBody(
compoundStmt(findAll(declStmt(allOf(containsDeclaration2(
LocalValDecl.bind("local-value")),
unless(has(decompositionDecl()))))
.bind("decl-stmt")))
.bind("scope")));

Finder->addMatcher(FunctionScope, this);
}

/// Classify for a variable in what the Const-Check is interested.
enum class VariableCategory { Value, Reference, Pointer };

void ConstCorrectnessCheck::check(const MatchFinder::MatchResult &Result) {
const auto *LocalScope = Result.Nodes.getNodeAs<CompoundStmt>("scope");
assert(LocalScope && "Did not match scope for local variable");
registerScope(LocalScope, Result.Context);

const auto *Variable = Result.Nodes.getNodeAs<VarDecl>("local-value");
assert(Variable && "Did not match local variable definition");

VariableCategory VC = VariableCategory::Value;
if (Variable->getType()->isReferenceType())
VC = VariableCategory::Reference;
if (Variable->getType()->isPointerType())
VC = VariableCategory::Pointer;

// Each variable can only in one category: Value, Pointer, Reference.
// Analysis can be controlled for every category.
if (VC == VariableCategory::Reference && !AnalyzeReferences)
return;

if (VC == VariableCategory::Reference &&
Variable->getType()->getPointeeType()->isPointerType() &&
!WarnPointersAsValues)
return;

if (VC == VariableCategory::Pointer && !WarnPointersAsValues)
return;

if (VC == VariableCategory::Value && !AnalyzeValues)
return;

// Offload const-analysis to utility function.
if (ScopesCache[LocalScope]->isMutated(Variable))
return;

auto Diag = diag(Variable->getBeginLoc(),
"variable %0 of type %1 can be declared 'const'")
<< Variable << Variable->getType();

const auto *VarDeclStmt = Result.Nodes.getNodeAs<DeclStmt>("decl-stmt");

// It can not be guaranteed that the variable is declared isolated, therefore
// a transformation might effect the other variables as well and be incorrect.
if (VarDeclStmt == nullptr || !VarDeclStmt->isSingleDecl())
return;

using namespace utils::fixit;
using llvm::Optional;
if (VC == VariableCategory::Value && TransformValues) {
if (Optional<FixItHint> Fix = addQualifierToVarDecl(
*Variable, *Result.Context, DeclSpec::TQ_const,
QualifierTarget::Value, QualifierPolicy::Left)) {
Diag << *Fix;
// FIXME: Add '{}' for default initialization if no user-defined default
// constructor exists and there is no initializer.
}
return;
}

if (VC == VariableCategory::Reference && TransformReferences) {
if (Optional<FixItHint> Fix = addQualifierToVarDecl(
*Variable, *Result.Context, DeclSpec::TQ_const,
QualifierTarget::Value, QualifierPolicy::Left))
Diag << *Fix;
return;
}

if (VC == VariableCategory::Pointer) {
if (WarnPointersAsValues && TransformPointersAsValues) {
if (Optional<FixItHint> Fix = addQualifierToVarDecl(
*Variable, *Result.Context, DeclSpec::TQ_const,
QualifierTarget::Value, QualifierPolicy::Left))
Diag << *Fix;
}
return;
}
}

void ConstCorrectnessCheck::registerScope(const CompoundStmt *LocalScope,
ASTContext *Context) {
if (ScopesCache.find(LocalScope) == ScopesCache.end())
ScopesCache.insert(std::make_pair(
LocalScope,
std::make_unique<ExprMutationAnalyzer>(*LocalScope, *Context)));
}

} // namespace cppcoreguidelines
} // namespace tidy
} // namespace clang
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
//===--- ConstCorrectnessCheck.h - clang-tidy -------------------*- C++ -*-===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//

#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_CONSTCORRECTNESSCHECK_H
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_CONSTCORRECTNESSCHECK_H

#include "../ClangTidy.h"
#include "clang/Analysis/Analyses/ExprMutationAnalyzer.h"

namespace clang {
namespace tidy {

namespace cppcoreguidelines {

/// This check warns on variables which could be declared const but are not.
///
/// For the user-facing documentation see:
/// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-const.html
class ConstCorrectnessCheck : public ClangTidyCheck {
public:
ConstCorrectnessCheck(StringRef Name, ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
AnalyzeValues(Options.get("AnalyzeValues", 1)),
AnalyzeReferences(Options.get("AnalyzeReferences", 1)),
WarnPointersAsValues(Options.get("WarnPointersAsValues", 0)),
TransformValues(Options.get("TransformValues", 1)),
TransformReferences(Options.get("TransformReferences", 1)),
TransformPointersAsValues(Options.get("TransformPointersAsValues", 0)) {
}

void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;

private:
void registerScope(const CompoundStmt *LocalScope, ASTContext *Context);

using MutationAnalyzer = std::unique_ptr<ExprMutationAnalyzer>;
llvm::DenseMap<const CompoundStmt *, MutationAnalyzer> ScopesCache;

const bool AnalyzeValues;
const bool AnalyzeReferences;
const bool WarnPointersAsValues;

const bool TransformValues;
const bool TransformReferences;
const bool TransformPointersAsValues;
};

} // namespace cppcoreguidelines
} // namespace tidy
} // namespace clang

#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_CONSTCORRECTNESSCHECK_H
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "../modernize/UseOverrideCheck.h"
#include "../readability/MagicNumbersCheck.h"
#include "AvoidGotoCheck.h"
#include "ConstCorrectnessCheck.h"
#include "InitVariablesCheck.h"
#include "InterfacesGlobalInitCheck.h"
#include "MacroUsageCheck.h"
Expand Down Expand Up @@ -48,6 +49,8 @@ class CppCoreGuidelinesModule : public ClangTidyModule {
"cppcoreguidelines-avoid-goto");
CheckFactories.registerCheck<readability::MagicNumbersCheck>(
"cppcoreguidelines-avoid-magic-numbers");
CheckFactories.registerCheck<ConstCorrectnessCheck>(
"cppcoreguidelines-const-correctness");
CheckFactories.registerCheck<modernize::UseOverrideCheck>(
"cppcoreguidelines-explicit-virtual-functions");
CheckFactories.registerCheck<InitVariablesCheck>(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
//
//===----------------------------------------------------------------------===//

#include <regex>

#include "InitVariablesCheck.h"

#include "clang/AST/ASTContext.h"
Expand All @@ -26,7 +28,9 @@ InitVariablesCheck::InitVariablesCheck(StringRef Name,
: ClangTidyCheck(Name, Context),
IncludeStyle(utils::IncludeSorter::parseIncludeStyle(
Options.getLocalOrGlobal("IncludeStyle", "llvm"))),
MathHeader(Options.get("MathHeader", "math.h")) {}
MathHeader(getLangOpts().CPlusPlus11
? Options.get("MathHeader", "cmath")
: Options.get("MathHeader", "math.h")) {}

void InitVariablesCheck::registerMatchers(MatchFinder *Finder) {
std::string BadDecl = "badDecl";
Expand Down Expand Up @@ -75,9 +79,11 @@ void InitVariablesCheck::check(const MatchFinder::MatchResult &Result) {
const char *InitializationString = nullptr;
bool AddMathInclude = false;

if (TypePtr->isIntegerType())
if (TypePtr->isBooleanType()) {
InitializationString = " = false";
} else if (TypePtr->isIntegerType()) {
InitializationString = " = 0";
else if (TypePtr->isFloatingType()) {
} else if (TypePtr->isFloatingType()) {
InitializationString = " = NAN";
AddMathInclude = true;
} else if (TypePtr->isAnyPointerType()) {
Expand All @@ -87,6 +93,12 @@ void InitVariablesCheck::check(const MatchFinder::MatchResult &Result) {
InitializationString = " = NULL";
}

if (InitializationString && getLangOpts().CPlusPlus11) {
std::regex re(" = (.*)");
InitializationString =
std::regex_replace(InitializationString, re, "{$1}").c_str();
}

if (InitializationString) {
auto Diagnostic =
diag(MatchedDecl->getLocation(), "variable %0 is not initialized")
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
#!/bin/bash

# http://redsymbol.net/articles/unofficial-bash-strict-mode/
set -euo pipefail

BASEDIR=$1

echo "Fixing instances of double const qualification for files in $BASEDIR..."
for i in $(find $BASEDIR -type f -iregex '.*\.\(h\|hxx\|hpp\|hh\|c\|cpp\|cxx\|cc\)$');do
echo "Checking $i"
sed -i 's/const const/const/g' $i
done
5 changes: 5 additions & 0 deletions clang-tools-extra/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,11 @@ New checks
Checks whether there are local variables that are declared without an initial
value.

- New :doc:`cppcoreguidelines-const-correctness
<clang-tidy/checks/cppcoreguidelines-const-correctness>` check.

Suggest adding ``const`` to unmodified local variables.

- New :doc:`darwin-dispatch-once-nonstatic
<clang-tidy/checks/darwin-dispatch-once-nonstatic>` check.

Expand Down
Loading