Differential D118743 Diff 416347 clang-tools-extra/clang-tidy/modernize/UseInlineConstVariablesInHeadersCheck.cpp

Changeset View

Standalone View

clang-tools-extra/clang-tidy/modernize/UseInlineConstVariablesInHeadersCheck.cpp

This file was added.

				//===--- UseInlineConstVariablesInHeadersCheck.cpp - clang-tidy -----------===//
				//
				// 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 "UseInlineConstVariablesInHeadersCheck.h"
				#include "clang/AST/ASTContext.h"
				#include "clang/ASTMatchers/ASTMatchFinder.h"

				using namespace clang::ast_matchers;

				namespace clang {
				namespace tidy {
				namespace modernize {

				namespace {

				AST_MATCHER(NamedDecl, isInAnonymousNamespace) {
				return Node.isInAnonymousNamespace();
				}

				carlosgalvezpUnsubmitted Done Reply Inline Actions I recently added this matcher into ASTMatchers, so you can remove this custom matcher :) carlosgalvezp: I recently added this matcher into ASTMatchers, so you can remove this custom matcher :)
				AST_MATCHER(VarDecl, isTemplateVariable) {
				return Node.isTemplated() \|\| isa<VarTemplateSpecializationDecl>(Node);
				}

				AST_MATCHER(VarDecl, isExternallyVisible) { return Node.isExternallyVisible(); }

				} // namespace

				UseInlineConstVariablesInHeadersCheck::UseInlineConstVariablesInHeadersCheck(
				StringRef Name, ClangTidyContext *Context)
				: ClangTidyCheck(Name, Context),
				RawStringHeaderFileExtensions(Options.getLocalOrGlobal(
				"HeaderFileExtensions", utils::defaultHeaderFileExtensions())),
				CheckNonInline(Options.getLocalOrGlobal("CheckNonInline", true)),
				CheckExtern(Options.getLocalOrGlobal("CheckExtern", true)) {
				if (!utils::parseFileExtensions(RawStringHeaderFileExtensions,
				HeaderFileExtensions,
				utils::defaultFileExtensionDelimiters())) {
				this->configurationDiag("Invalid header file extension: '%0'")
				<< RawStringHeaderFileExtensions;
				}
				}

				void UseInlineConstVariablesInHeadersCheck::storeOptions(
				ClangTidyOptions::OptionMap &Opts) {
				Options.store(Opts, "HeaderFileExtensions", RawStringHeaderFileExtensions);
				Options.store(Opts, "CheckNonInline", CheckNonInline);
				Options.store(Opts, "CheckExtern", CheckExtern);
				}

				void UseInlineConstVariablesInHeadersCheck::registerMatchers(
				MatchFinder *Finder) {
				auto NonInlineConstVarDecl =
				varDecl(hasGlobalStorage(),
				hasDeclContext(anyOf(translationUnitDecl(),
				namespaceDecl())), // is at file scope
				hasType(isConstQualified()), // const-qualified
				unless(anyOf(
				isInAnonymousNamespace(), // not within an anonymous namespace
				isTemplateVariable(), // non-template
				isInline(), // non-inline
				hasType(isVolatileQualified()), // non-volatile
				isExternC() // not "extern C" variable
				)));

				LegalizeAdulthoodUnsubmitted Done Reply Inline Actions Comment: Why don't we have a variadic `unless(...)` matcher? Is there any utility to reordering these `unless()` clauses in order of "most likely encountered first" to get an early out when matching? LegalizeAdulthood: Comment: Why don't we have a variadic `unless(...)` matcher? Is there any utility to…
				njames93Unsubmitted Not Done Reply Inline Actions does `unless(isVarInline(), isExternC())` mean `unless(isVarInline() && isExternC())` or `unless(isVarInline() \|\| isExternC())` This kind of ambiguity could be addressed with `unlessAllOf` and `unlessAnyOf` but I don't think its worth the effort. njames93: does `unless(isVarInline(), isExternC())` mean `unless(isVarInline() && isExternC())` or…
				IzaronAuthorUnsubmitted Done Reply Inline Actions Thanks, it is more convenient! reordering these unless() clauses in order of "most likely encountered first" I reordered them based on my own approximal estimation. We could make a benchmark to observe the impact of different orders, but it goes beyond this patch. Izaron: Thanks, it is more convenient! > reordering these unless() clauses in order of "most likely…
				if (CheckNonInline)
				Finder->addMatcher(varDecl(NonInlineConstVarDecl, isDefinition(),
				unless(isExternallyVisible()))
				.bind("non-inline-var-definition"),
				this);
				if (CheckExtern)
				Finder->addMatcher(varDecl(NonInlineConstVarDecl, isExternallyVisible())
				.bind("extern-var-declaration"),
				this);
				}

				void UseInlineConstVariablesInHeadersCheck::check(
				const MatchFinder::MatchResult &Result) {
				const VarDecl *D = nullptr;
				StringRef Msg;
				bool InsertInlineKeyword = false;

				if ((D = Result.Nodes.getNodeAs<VarDecl>("non-inline-var-definition"))) {
				Msg = "global constant %0 should be marked as 'inline'";
				InsertInlineKeyword = true;
				} else {
				D = Result.Nodes.getNodeAs<VarDecl>("extern-var-declaration");
				Msg = "global constant %0 should be converted to C++17 'inline variable'";
				}

				// Ignore if it comes not from a header
				if (!utils::isSpellingLocInHeaderFile(D->getBeginLoc(), *Result.SourceManager,
				HeaderFileExtensions))
				return;

				DiagnosticBuilder Diag = diag(D->getLocation(), Msg) << D;
				if (InsertInlineKeyword)
				Diag << FixItHint::CreateInsertion(D->getBeginLoc(), "inline ");
				}

				} // namespace modernize
				} // namespace tidy
				} // namespace clang