diff --git a/.clang-tidy.in b/.clang-tidy.in index 7c772f3533b..838c09d8873 100644 --- a/.clang-tidy.in +++ b/.clang-tidy.in @@ -34,22 +34,23 @@ Checks: '-*, modernize-replace-random-shuffle, modernize-shrink-to-fit, modernize-unary-static-assert, + mongo-assert-check, mongo-cctype-check, mongo-config-header-check, mongo-collection-sharding-runtime-check, mongo-cxx20-banned-includes-check, mongo-cxx20-std-chrono-check, mongo-header-bracket-check, - mongo-std-atomic-check, mongo-macro-definition-leaks-check, mongo-mutex-check, - mongo-assert-check, + mongo-polyfill-check, mongo-rand-check, + mongo-std-atomic-check, mongo-std-optional-check, + mongo-trace-check, mongo-uninterruptible-lock-guard-check, mongo-unstructured-log-check, mongo-volatile-check, - mongo-trace-check, mongo-fcv-constant-check, performance-faster-string-find, performance-implicit-conversion-in-loop, diff --git a/buildscripts/linter/simplecpplint.py b/buildscripts/linter/simplecpplint.py index 54ca204a2c1..7e171eaa368 100644 --- a/buildscripts/linter/simplecpplint.py +++ b/buildscripts/linter/simplecpplint.py @@ -8,52 +8,8 @@ import logging import re import sys - -def _make_polyfill_regex(): - polyfill_required_names = [ - '_', - 'adopt_lock', - 'async', - 'chrono', - 'condition_variable', - 'condition_variable_any', - 'cv_status', - 'defer_lock', - 'future', - 'future_status', - 'get_terminate', - 'launch', - 'lock_guard', - 'mutex', - 'notify_all_at_thread_exit', - 'packaged_task', - 'promise', - 'recursive_mutex', - 'set_terminate', - 'shared_lock', - 'shared_mutex', - 'shared_timed_mutex', - 'this_thread(?!::at_thread_exit)', - 'thread', - 'timed_mutex', - 'try_to_lock', - 'unique_lock', - 'unordered_map', - 'unordered_multimap', - 'unordered_multiset', - 'unordered_set', - ] - - qualified_names = ['boost::' + name + "\\b" for name in polyfill_required_names] - qualified_names.extend('std::' + name + "\\b" for name in polyfill_required_names) - qualified_names_regex = '|'.join(qualified_names) - return re.compile('(' + qualified_names_regex + ')') - - _RE_LINT = re.compile("//.*NOLINT") _RE_COMMENT_STRIP = re.compile("//.*") -_RE_PATTERN_MONGO_POLYFILL = _make_polyfill_regex() - _RE_GENERIC_FCV_COMMENT = re.compile(r'\(Generic FCV reference\):') GENERIC_FCV = [ r'::kLatest', @@ -144,7 +100,6 @@ class Linter: if not self.clean_lines[linenum]: continue - self._check_for_mongo_polyfill(linenum) self._check_for_c_stdlib_headers(linenum) # Relax the rule of commenting generic FCV references for files directly related to FCV @@ -203,15 +158,6 @@ class Linter: self.clean_lines.append(clean_line) - def _check_for_mongo_polyfill(self, linenum): - line = self.clean_lines[linenum] - match = _RE_PATTERN_MONGO_POLYFILL.search(line) - if match: - self._error( - linenum, 'mongodb/polyfill', - 'Illegal use of banned name from std::/boost:: for "%s", use mongo::stdx:: variant instead' - % (match.group(0))) - def _license_error(self, linenum, msg, category='legal/license'): style_url = 'https://github.com/mongodb/mongo/wiki/Server-Code-Style' self._error(linenum, category, '{} See {}'.format(msg, style_url)) diff --git a/src/mongo/tools/mongo_tidy_checks/MongoPolyFillCheck.cpp b/src/mongo/tools/mongo_tidy_checks/MongoPolyFillCheck.cpp new file mode 100644 index 00000000000..02869735838 --- /dev/null +++ b/src/mongo/tools/mongo_tidy_checks/MongoPolyFillCheck.cpp @@ -0,0 +1,126 @@ +/** + * Copyright (C) 2023-present MongoDB, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the Server Side Public License, version 1, + * as published by MongoDB, Inc. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * Server Side Public License for more details. + * + * You should have received a copy of the Server Side Public License + * along with this program. If not, see + * . + * + * As a special exception, the copyright holders give permission to link the + * code of portions of this program with the OpenSSL library under certain + * conditions as described in each individual source file and distribute + * linked combinations including the program with the OpenSSL library. You + * must comply with the Server Side Public License in all respects for + * all of the code used other than as permitted herein. If you modify file(s) + * with this exception, you may extend this exception to your version of the + * file(s), but you are not obligated to do so. If you do not wish to do so, + * delete this exception statement from your version. If you delete this + * exception statement from all source files in the program, then also delete + * it in the license file. + */ + +#include "MongoPolyFillCheck.h" + +#include + +namespace mongo::tidy { + +using namespace clang; +using namespace clang::ast_matchers; + +// Generate a list of fully qualified polyfill names by prefixing each name +// in the input list with 'std::' and 'boost::' +std::vector generateQualifiedPolyfillNames( + const std::vector& bannedNames) { + std::vector fullyBannedNames; + for (const auto& name : bannedNames) { + fullyBannedNames.push_back("std::" + name); + fullyBannedNames.push_back("boost::" + name); + } + return fullyBannedNames; +} + +// List of base polyfill names from the std and boost namespaces to be checked +std::vector MongoPolyFillCheck::basePolyfillNames = {"adopt_lock", + "async", + "chrono", + "condition_variable", + "condition_variable_any", + "cv_status", + "defer_lock", + "future", + "future_status", + "get_terminate", + "launch", + "lock_guard", + "mutex", + "notify_all_at_thread_exit", + "packaged_task", + "promise", + "recursive_mutex", + "set_terminate", + "shared_lock", + "shared_mutex", + "shared_timed_mutex", + "this_thread", + "thread", + "timed_mutex", + "try_to_lock", + "unique_lock", + "unordered_map", + "unordered_multimap", + "unordered_multiset", + "unordered_set"}; + +MongoPolyFillCheck::MongoPolyFillCheck(StringRef Name, clang::tidy::ClangTidyContext* Context) + : ClangTidyCheck(Name, Context) { + // Generate a list of fully polyfill names + fullyQualifiedPolyfillNames = generateQualifiedPolyfillNames(basePolyfillNames); +} + + +void MongoPolyFillCheck::registerMatchers(ast_matchers::MatchFinder* Finder) { + // Create an ArrayRef from the vector of banned names. This provides a + // lightweight, non-owning reference to the array of names. + std::vector basePolyfillNamesRefVector(basePolyfillNames.begin(), + basePolyfillNames.end()); + llvm::ArrayRef basePolyfillNamesRefArray(basePolyfillNamesRefVector); + + // Register an AST Matcher to find type declarations that use any of the banned names + Finder->addMatcher(loc(hasUnqualifiedDesugaredType(recordType( + hasDeclaration(namedDecl(hasAnyName(basePolyfillNamesRefArray)))))) + .bind("bannedNames"), + this); +} + +void MongoPolyFillCheck::check(const ast_matchers::MatchFinder::MatchResult& Result) { + const auto* MatchedTypeLoc = Result.Nodes.getNodeAs("bannedNames"); + if (MatchedTypeLoc) { + auto typeStr = MatchedTypeLoc->getType().getAsString(); + // we catch this_thread but not this_thread::at_thread_exit + if (typeStr.find("this_thread::at_thread_exit") != std::string::npos) + return; + + // Check if the type string starts with 'std' or 'boost' and contains a banned name. + for (const auto& name : fullyQualifiedPolyfillNames) { + if ((typeStr.find("std") == 0 || typeStr.find("boost") == 0) && + typeStr.find(name) != std::string::npos) { + auto location = MatchedTypeLoc->getBeginLoc(); + if (location.isValid()) + diag(MatchedTypeLoc->getBeginLoc(), + "Illegal use of banned name from std::/boost:: for %0, use mongo::stdx:: " + "variant instead") + << name; + } + } + } +} +} // namespace mongo::tidy diff --git a/src/mongo/tools/mongo_tidy_checks/MongoPolyFillCheck.h b/src/mongo/tools/mongo_tidy_checks/MongoPolyFillCheck.h new file mode 100644 index 00000000000..9de13d51287 --- /dev/null +++ b/src/mongo/tools/mongo_tidy_checks/MongoPolyFillCheck.h @@ -0,0 +1,56 @@ +/** + * Copyright (C) 2023-present MongoDB, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the Server Side Public License, version 1, + * as published by MongoDB, Inc. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * Server Side Public License for more details. + * + * You should have received a copy of the Server Side Public License + * along with this program. If not, see + * . + * + * As a special exception, the copyright holders give permission to link the + * code of portions of this program with the OpenSSL library under certain + * conditions as described in each individual source file and distribute + * linked combinations including the program with the OpenSSL library. You + * must comply with the Server Side Public License in all respects for + * all of the code used other than as permitted herein. If you modify file(s) + * with this exception, you may extend this exception to your version of the + * file(s), but you are not obligated to do so. If you do not wish to do so, + * delete this exception statement from your version. If you delete this + * exception statement from all source files in the program, then also delete + * it in the license file. + */ +#pragma once + +#include +#include + +namespace mongo::tidy { + +/** + * MongoPolyFillCheck is a custom clang-tidy check for detecting + * the usage of listed names from the std or boost namespace in the source code. + * + * It extends ClangTidyCheck and overrides the registerMatchers + * and check functions. The registerMatchers function adds matchers + * to identify the usage of banned names, while the check function + * flags the matched occurrences. + */ +class MongoPolyFillCheck : public clang::tidy::ClangTidyCheck { + +public: + MongoPolyFillCheck(clang::StringRef Name, clang::tidy::ClangTidyContext* Context); + void registerMatchers(clang::ast_matchers::MatchFinder* Finder) override; + void check(const clang::ast_matchers::MatchFinder::MatchResult& Result) override; + static std::vector basePolyfillNames; + +private: + std::vector fullyQualifiedPolyfillNames; +}; +} // namespace mongo::tidy diff --git a/src/mongo/tools/mongo_tidy_checks/MongoTidyModule.cpp b/src/mongo/tools/mongo_tidy_checks/MongoTidyModule.cpp index 237fb6bc3e0..6e1e0378bd0 100644 --- a/src/mongo/tools/mongo_tidy_checks/MongoTidyModule.cpp +++ b/src/mongo/tools/mongo_tidy_checks/MongoTidyModule.cpp @@ -37,6 +37,7 @@ #include "MongoHeaderBracketCheck.h" #include "MongoMacroDefinitionLeaksCheck.h" #include "MongoMutexCheck.h" +#include "MongoPolyFillCheck.h" #include "MongoRandCheck.h" #include "MongoStdAtomicCheck.h" #include "MongoStdOptionalCheck.h" @@ -77,6 +78,7 @@ public: CheckFactories.registerCheck( "mongo-macro-definition-leaks-check"); CheckFactories.registerCheck("mongo-rand-check"); + CheckFactories.registerCheck("mongo-polyfill-check"); } }; diff --git a/src/mongo/tools/mongo_tidy_checks/SConscript b/src/mongo/tools/mongo_tidy_checks/SConscript index d2a4eaf390d..6cdefe31a71 100644 --- a/src/mongo/tools/mongo_tidy_checks/SConscript +++ b/src/mongo/tools/mongo_tidy_checks/SConscript @@ -129,6 +129,7 @@ mongo_custom_check = env.SharedLibrary( "MongoCxx20StdChronoCheck.cpp", "MongoStdOptionalCheck.cpp", "MongoVolatileCheck.cpp", + "MongoPolyFillCheck.cpp", "MongoStdAtomicCheck.cpp", "MongoTidyModule.cpp", "MongoTraceCheck.cpp", diff --git a/src/mongo/tools/mongo_tidy_checks/tests/MongoTidyCheck_unittest.py b/src/mongo/tools/mongo_tidy_checks/tests/MongoTidyCheck_unittest.py index 2cc62b56295..1b5fd8c6461 100644 --- a/src/mongo/tools/mongo_tidy_checks/tests/MongoTidyCheck_unittest.py +++ b/src/mongo/tools/mongo_tidy_checks/tests/MongoTidyCheck_unittest.py @@ -337,6 +337,22 @@ class MongoTidyTests(unittest.TestCase): self.run_clang_tidy() + def test_MongoPolyFillCheck(self): + self.write_config( + textwrap.dedent("""\ + Checks: '-*,mongo-polyfill-check' + WarningsAsErrors: '*' + """)) + + self.expected_output = [ + "error: Illegal use of banned name from std::/boost:: for std::mutex, use mongo::stdx:: variant instead", + "error: Illegal use of banned name from std::/boost:: for std::future, use mongo::stdx:: variant instead", + "error: Illegal use of banned name from std::/boost:: for std::condition_variable, use mongo::stdx:: variant instead", + "error: Illegal use of banned name from std::/boost:: for std::unordered_map, use mongo::stdx:: variant instead", + "error: Illegal use of banned name from std::/boost:: for boost::unordered_map, use mongo::stdx:: variant instead", + ] + + self.run_clang_tidy() def test_MongoRandCheck(self): self.write_config( diff --git a/src/mongo/tools/mongo_tidy_checks/tests/SConscript b/src/mongo/tools/mongo_tidy_checks/tests/SConscript index 863499e104a..15c242282cd 100644 --- a/src/mongo/tools/mongo_tidy_checks/tests/SConscript +++ b/src/mongo/tools/mongo_tidy_checks/tests/SConscript @@ -14,6 +14,7 @@ if env.GetOption('ninja') == 'disabled': mongo_tidy_test_env.Append(CPPPATH=[ '.', '#src', + '#src/third_party/boost', ], ) # These test files will purposefully be error prone, so we can disable warnings any warnings we expect @@ -42,6 +43,7 @@ if env.GetOption('ninja') == 'disabled': 'test_MongoCollectionShardingRuntimeCheck.cpp', 'test_MongoMacroDefinitionLeaksCheck.cpp', 'test_MongoRandCheck.cpp', + 'test_MongoPolyFillCheck.cpp', ] # So that we can do fast runs, we will generate a separate compilation database file for each diff --git a/src/mongo/tools/mongo_tidy_checks/tests/test_MongoPolyFillCheck.cpp b/src/mongo/tools/mongo_tidy_checks/tests/test_MongoPolyFillCheck.cpp new file mode 100644 index 00000000000..fca700f5b1d --- /dev/null +++ b/src/mongo/tools/mongo_tidy_checks/tests/test_MongoPolyFillCheck.cpp @@ -0,0 +1,16 @@ +#include +#include +#include +#include +#include + +namespace mongo { +void mongoPolyFillCheckTest() { + std::mutex myMutex; + std::future myFuture; + std::condition_variable cv; + std::unordered_map myMap; + boost::unordered_map boostMap; +} + +} // namespace mongo