[clang-tidy] Add `bugprone-reserved-identifier`

[logan-5]

Summary

This patch adds bugprone-reserved-identifier, which flags uses of __names _Like ::_this, which are reserved for the implementation. The check can optionally be inverted, i.e. configured to flag any names that are _not_ reserved, which may be useful for e.g. standard library implementors.

This diff is relative to, and dependent on, https://reviews.llvm.org/D72284. Not sure if there's a way to chain it or flag it as such within Phabricator, if so let me know.

[Eugene.Zelenko]

clang-tools-extra/docs/ReleaseNotes.rst

100

One sentence is enough, details belong to documentation.

[logan-5]

Addressed some nits.

[JonasToth]

clang-tools-extra/docs/clang-tidy/checks/bugprone-reserved-identifier.rst

35

i think you need double-tick here. The highlighting seems weird, usually its orangy, isn't it?

clang-tools-extra/test/clang-tidy/checkers/bugprone-reserved-identifier.cpp

14

missing tests with templates, in this file.

[Eugene.Zelenko]

clang-tools-extra/docs/clang-tidy/checks/bugprone-reserved-identifier.rst

35

No, single back-ticks for option values, double back-ticks for language constructs.

[logan-5]

Added tests for template parameters.

[aaron.ballman]

This check is missing a whole lot of reserved identifiers. For instance, in C++ it is missing everything from http://eel.is/c++draft/zombie.names and for C it is missing everything from future library directions. Are you intending to cover those cases as well?

clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp

124

I think the CERT module should also be given an alias for this check, as it seems to cover: https://wiki.sei.cmu.edu/confluence/display/c/DCL37-C.+Do+not+declare+or+define+a+reserved+identifier and https://wiki.sei.cmu.edu/confluence/display/cplusplus/DCL51-CPP.+Do+not+declare+or+define+a+reserved+identifier

clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp

36

Please drop top-level const qualifiers for parameters and locals -- that's not a style we typically use.

49–52

Can't this still result in a reserved identifier? e.g.,

int ___Foobar; // three underscores

93

if (llvm::any_of(Whitelist, Name)) return None;

99

AppendFailure per our usual naming conventions.

107

InProgressFixup

120–121

You can just return Info here without the if statement -- the effect is the same.

clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.h

22

Why just C++? C has reserved identifiers as well, and there's a lot of overlap between the two languages in terms of what's reserved.

[logan-5]

In D72378#1810763, @aaron.ballman wrote:

This check is missing a whole lot of reserved identifiers. For instance, in C++ it is missing everything from http://eel.is/c++draft/zombie.names and for C it is missing everything from future library directions. Are you intending to cover those cases as well?

I admit that those are outside the scope of what I had originally planned for this check -- I was primarily concerned about 'uglified' names, and writing a check that was 'invertible' in that regard. Now that you mention these, though, I do feel like this check doesn't live up to its name without including them.

I'd be interested in incorporating them. It doesn't sound difficult, but it does sound like it'd be a sizable addition to this diff. Still familiarizing with the workflow around here... would it be alright to leave a TODO comment in this patch and add them in an upcoming patch, to keep this one more self-contained?

clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp

49–52

___Foobar with 3 underscores will be fixed to _Foobar by this fixup, which is then passed through the underscore capital fixup, and that will be caught there. So it still works.

Thinking about it more, though, I do think the consume_front is unnecessary. Without it, __foo would get changed (by this fixup) to _foo, which will be corrected by a later fixup if that's still invalid. If not, that's a smaller and less opinionated change to the name than the current __foo -> foo.

I think I'll take out the consume_front("__") and update the tests to match.

clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.h

22

That's a good point. I'll do some tweaking to make sure this works well for C, including any places where C and C++ differ.

[logan-5]

Addressed nits. Added CERT aliases. Adjusted the check to work for both C and C++, including where they differ.

[Eugene.Zelenko]

Please rebase from trunk. I sorted Clang-tidy entries in Release Notes and aliases have own subsubsection. Please keep alphabetical order there.

clang-tools-extra/docs/clang-tidy/checks/cert-dcl51-cpp.rst

6 ↗

(On Diff #237244)

Header length.

[aaron.ballman]

In D72378#1812620, @logan-5 wrote:

In D72378#1810763, @aaron.ballman wrote:

This check is missing a whole lot of reserved identifiers. For instance, in C++ it is missing everything from http://eel.is/c++draft/zombie.names and for C it is missing everything from future library directions. Are you intending to cover those cases as well?

I admit that those are outside the scope of what I had originally planned for this check -- I was primarily concerned about 'uglified' names, and writing a check that was 'invertible' in that regard. Now that you mention these, though, I do feel like this check doesn't live up to its name without including them.

I'd be interested in incorporating them. It doesn't sound difficult, but it does sound like it'd be a sizable addition to this diff. Still familiarizing with the workflow around here... would it be alright to leave a TODO comment in this patch and add them in an upcoming patch, to keep this one more self-contained?

I would be fine putting a TODO in the code and addressing it in a follow-up patch. It's not critical for the initial offering.

clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp

122

Sorry, I missed this before -- you can drop the else as well -- we don't use else after an unconditional return as part of our usual style guidelines.

153–162

Rather than use a complex conditional, I would prefer to see some string constants and a variable. It's a bit hard to reason about the diagnostic text as-is.

clang-tools-extra/docs/clang-tidy/checks/bugprone-reserved-identifier.rst

32

You should probably add some links here back to the CERT checks with some words like "this check also covers the following CERT secure coding guidelines" or somesuch.

[logan-5]

Added a TODO comment for catching more reserved names. Added links in documentation to CERT guidelines covered by the check. Pulled strings into named constants and made that logic easier to read.

[Mordante]

Adding Depends on D72284 would add the patches to the stack.
I like the patch!

[aaron.ballman]

I think this patch is getting pretty close to finished -- have you tried running it over a large corpus of code to see if it spots reserved identifiers (or unreserved ones)? If not, I would recommend running it over LLVM + clang + clang-tools-extra to see how it operates when looking for reserved identifiers and libc++ for unreserved ones.

clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp

28–35

I think you can reasonably combine these into a single diagnostic using %select. e.g.,
"declaration uses identifier %0, which is %select{a reserved identifier|not a reserved identifier|reserved in the global namespace}1"

I took out the bits about causing undefined behavior because that doesn't really add much to the diagnostic (the "is a reserved identifier" bit should be sufficient). Also, I removed the manual quoting around the %0 because it shouldn't be needed if you pass in a NamedDecl* as opposed to a string (which you should prefer doing because it automatically formats the identifier properly).

66

Elide braces.

clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.h

36

Personal preference to name this AllowedIdentifiers or something along those lines (and same for the user-facing option).

43

This class is marked final, so why are these protected rather than private?

clang-tools-extra/docs/clang-tidy/checks/bugprone-reserved-identifier.rst

9

You should mention that it currently does not check for all reserved names such as future library or zombie names.

[logan-5]

Renamed check option from "Whitelist" to "AllowedIdentifiers". Added note about missing checks in documentation. Changed to use a %select for diagnostic text. Some nits.

The check does due diligence when run over LLVM (after whitelisting a couple things), correctly flagging a few suspicious names scattered about.

The inverted mode is a bit cumbersome at the moment to use on libc++ (since libc++ legitimately defines lots of non-reserved names (that's its job)). One could whitelist every name the standard defines, but I'd like to add some logic in a future patch that is smarter about which names need to be reserved in inverted mode -- something like, flagging private class members but not public ones, for example.

clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp

28–35

I don't have access to a NamedDecl* for a couple reasons, one being that the same diagnostic is used for both declarations and macro definitions. The %select change sounds like a good one--I'll do that (or I already did it, depending when this comment shows up).

[aaron.ballman]

LGTM!

clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp

28–35

Ah, okay, that's a good point about macros.

[logan-5]

Great! In that case, I'll need help committing this, as well as the thing it depends on, https://reviews.llvm.org/D72284 (which has also been LGTM'd).

[aaron.ballman]

In D72378#1824586, @logan-5 wrote:

Great! In that case, I'll need help committing this, as well as the thing it depends on, https://reviews.llvm.org/D72284 (which has also been LGTM'd).

Great, can you rebase this patch on master? It does not apply cleanly currently.

c:\llvm-project>git apply C:\Users\aballman\Desktop\D72378.diff
C:/Users/aballman/Desktop/D72378.diff:364: trailing whitespace.
Checks for usages of identifiers reserved for use by the implementation.
C:/Users/aballman/Desktop/D72378.diff:371: trailing whitespace.
while the C++ standard strengthens this to reserve names with a double
C:/Users/aballman/Desktop/D72378.diff:378: trailing whitespace.
  namespace NS {
C:/Users/aballman/Desktop/D72378.diff:385: trailing whitespace.
The check can also be inverted, i.e. it can be configured to flag any
C:/Users/aballman/Desktop/D72378.diff:386: trailing whitespace.
identifier that is _not_ a reserved identifier. This mode is for use by e.g.
error: patch failed: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp:271
error: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp: patch does not apply
error: patch failed: clang-tools-extra/docs/ReleaseNotes.rst:94
error: clang-tools-extra/docs/ReleaseNotes.rst: patch does not apply

[logan-5]

Should be good now.

[aaron.ballman]

Thank you for the new checker -- I've committed on your behalf in 42a0355816d3bc125d59cbd07052c8886e78ca86

[thakis]

Looks like this isn't building on Windows: http://45.33.8.238/win/6039/step_4.txt

[thakis]

cl.exe prints a different error: http://lab.llvm.org:8011/builders/clang-x64-windows-msvc/builds/13716/steps/stage%201%20build/logs/stdio

[lebedev.ri]

So, i'm seeing an issue here:
https://godbolt.org/z/KM2qLa

I can't NOLINT it, because it is defined not in the source file, but in compile line.
And i can't whitelist it since there is no whitelist..

PTAL.

[aaron.ballman]

In D72378#1840353, @lebedev.ri wrote:

So, i'm seeing an issue here:
https://godbolt.org/z/KM2qLa

I can't NOLINT it, because it is defined not in the source file, but in compile line.
And i can't whitelist it since there is no whitelist..

PTAL.

Doesn't the AllowedIdentifiers option work for you?

[lebedev.ri]

In D72378#1840455, @aaron.ballman wrote:

In D72378#1840353, @lebedev.ri wrote:

So, i'm seeing an issue here:
https://godbolt.org/z/KM2qLa

I can't NOLINT it, because it is defined not in the source file, but in compile line.
And i can't whitelist it since there is no whitelist..

PTAL.

Doesn't the AllowedIdentifiers option work for you?

Uuuh. To be honest somehow i did not observe it when looking through
the docs (as indicated in it since there is no whitelist..),
It does work in this case, thanks. Sorry for false alarm.

[aaron.ballman]

In D72378#1840461, @lebedev.ri wrote:

In D72378#1840455, @aaron.ballman wrote:

In D72378#1840353, @lebedev.ri wrote:

So, i'm seeing an issue here:
https://godbolt.org/z/KM2qLa

I can't NOLINT it, because it is defined not in the source file, but in compile line.
And i can't whitelist it since there is no whitelist..

PTAL.

Doesn't the AllowedIdentifiers option work for you?

Uuuh. To be honest somehow i did not observe it when looking through
the docs (as indicated in it since there is no whitelist..),
It does work in this case, thanks. Sorry for false alarm.

No worries, I'm glad it's working for you!

clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp

 #include "MacroRepeatedSideEffectsCheck.h"
 #include "MisplacedOperatorInStrlenInAllocCheck.h"
 #include "MisplacedWideningCastCheck.h"
 #include "MoveForwardingReferenceCheck.h"
 #include "MultipleStatementMacroCheck.h"
 #include "NotNullTerminatedResultCheck.h"
 #include "ParentVirtualCallCheck.h"
 #include "PosixReturnCheck.h"
+#include "ReservedIdentifierCheck.h"
 #include "SizeofContainerCheck.h"
 #include "SizeofExpressionCheck.h"
 #include "StringConstructorCheck.h"
 #include "StringIntegerAssignmentCheck.h"
 #include "StringLiteralWithEmbeddedNulCheck.h"
 #include "SuspiciousEnumUsageCheck.h"
 #include "SuspiciousMemsetUsageCheck.h"
 #include "SuspiciousMissingCommaCheck.h"
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<cppcoreguidelines::NarrowingConversionsCheck>(
         "bugprone-narrowing-conversions");
     CheckFactories.registerCheck<NotNullTerminatedResultCheck>(
         "bugprone-not-null-terminated-result");
     CheckFactories.registerCheck<ParentVirtualCallCheck>(
         "bugprone-parent-virtual-call");
     CheckFactories.registerCheck<PosixReturnCheck>(
         "bugprone-posix-return");
+    CheckFactories.registerCheck<ReservedIdentifierCheck>(
+        "bugprone-reserved-identifier");
     CheckFactories.registerCheck<SizeofContainerCheck>(
         "bugprone-sizeof-container");
     CheckFactories.registerCheck<SizeofExpressionCheck>(
         "bugprone-sizeof-expression");
     CheckFactories.registerCheck<StringConstructorCheck>(
         "bugprone-string-constructor");
     CheckFactories.registerCheck<StringIntegerAssignmentCheck>(
         "bugprone-string-integer-assignment");

clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt

add_clang_library(clangTidyBugproneModule
   MacroRepeatedSideEffectsCheck.cpp
   MisplacedOperatorInStrlenInAllocCheck.cpp
   MisplacedWideningCastCheck.cpp
   MoveForwardingReferenceCheck.cpp
   MultipleStatementMacroCheck.cpp
   NotNullTerminatedResultCheck.cpp
   ParentVirtualCallCheck.cpp
   PosixReturnCheck.cpp
+  ReservedIdentifierCheck.cpp
   SizeofContainerCheck.cpp
   SizeofExpressionCheck.cpp
   StringConstructorCheck.cpp
   StringIntegerAssignmentCheck.cpp
   StringLiteralWithEmbeddedNulCheck.cpp
   SuspiciousEnumUsageCheck.cpp
   SuspiciousMemsetUsageCheck.cpp
   SuspiciousMissingCommaCheck.cpp

clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.h

+//===--- ReservedIdentifierCheck.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_BUGPRONE_RESERVEDIDENTIFIERCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_RESERVEDIDENTIFIERCHECK_H
+
+#include "../utils/RenamerClangTidyCheck.h"
+#include "llvm/ADT/Optional.h"
+#include <string>
+#include <vector>
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+/// Checks for usages of identifiers reserved for use by the C++ implementation.
+/// The C++ standard reserves the following names for such use:
+/// * identifiers with a double underscore anywhere;
+/// * identifiers that begin with an underscore followed by an uppercase letter;
+/// * identifiers in the global namespace that begin with an underscore.
+///
+/// Violating the naming rules above results in undefined behavior.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone-reserved-identifier.html
+class ReservedIdentifierCheck final : public RenamerClangTidyCheck {
+  const bool Invert;
+  const std::vector<std::string> Whitelist;
+
+public:
+  ReservedIdentifierCheck(StringRef Name, ClangTidyContext *Context);
+
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+
+protected:
+  llvm::Optional<FailureInfo>
+  GetDeclFailureInfo(const NamedDecl *Decl,
+                     const SourceManager &SM) const override;
+  llvm::Optional<FailureInfo>
+  GetMacroFailureInfo(const Token &MacroNameTok,
+                      const SourceManager &SM) const override;
+  DiagInfo GetDiagInfo(const NamingCheckId &ID,
+                       const NamingCheckFailure &Failure) const override;
+};
+
+} // namespace bugprone
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_RESERVEDIDENTIFIERCHECK_H

clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp

+//===--- ReservedIdentifierCheck.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 "ReservedIdentifierCheck.h"
+#include "../utils/Matchers.h"
+#include "../utils/OptionsUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include <algorithm>
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+ReservedIdentifierCheck::ReservedIdentifierCheck(StringRef Name,
+                                                 ClangTidyContext *Context)
+    : RenamerClangTidyCheck(Name, Context),
+      Invert(Options.get("Invert", false)),
+      Whitelist(utils::options::parseStringList(Options.get("Whitelist", ""))) {
+}
+
+void ReservedIdentifierCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "Invert", Invert);
+  Options.store(Opts, "Whitelist",
+                utils::options::serializeStringList(Whitelist));
+}
+
+static std::string collapseConsecutive(const StringRef str, const char c) {
+  std::string result;
+  std::unique_copy(
+      str.begin(), str.end(), std::back_inserter(result),
+      [c](const char a, const char b) { return a == c && b == c; });
+  return result;
+}
+
+static bool hasDoubleUnderscore(const StringRef Name) {
+  return Name.find("__") != StringRef::npos;
+}
+
+static Optional<std::string> getDoubleUnderscoreFixup(StringRef Name) {
+  if (hasDoubleUnderscore(Name)) {
+    Name.consume_front("__");
+    return collapseConsecutive(Name, '_');
+  }
+  return None;
+}
+
+static bool startsWithUnderscoreCapital(const StringRef Name) {
+  return Name.size() >= 2 && Name[0] == '_' && std::isupper(Name[1]);
+}
+
+static Optional<std::string> getUnderscoreCapitalFixup(const StringRef Name) {
+  if (startsWithUnderscoreCapital(Name)) {
+    return std::string(Name.drop_front(1));
+  }
+  return None;
+}
+
+static bool
+startsWithUnderscoreInGlobalNamespace(const StringRef Name,
+                                      const bool IsInGlobalNamespace) {
+  return IsInGlobalNamespace && Name.size() >= 1 && Name[0] == '_';
+}
+
+static Optional<std::string>
+getUnderscoreGlobalNamespaceFixup(const StringRef Name,
+                                  const bool IsInGlobalNamespace) {
+  if (startsWithUnderscoreInGlobalNamespace(Name, IsInGlobalNamespace)) {
+    return std::string(Name.drop_front(1));
+  }
+  return None;
+}
+
+static std::string getNonReservedFixup(std::string Name) {
+  assert(!Name.empty());
+  if (Name[0] == '_' || std::isupper(Name[0])) {
+    Name.insert(Name.begin(), '_');
+  } else {
+    Name.insert(Name.begin(), 2, '_');
+  }
+  return Name;
+}
+
+static Optional<RenamerClangTidyCheck::FailureInfo>
+getFailureInfoImpl(const StringRef Name, const bool IsInGlobalNamespace,
+                   const bool Invert, const ArrayRef<std::string> Whitelist) {
+  assert(!Name.empty());
+  if (std::find(Whitelist.begin(), Whitelist.end(), Name) != Whitelist.end())
+    return None;
+
+  using FailureInfo = RenamerClangTidyCheck::FailureInfo;
+  if (!Invert) {
+    Optional<FailureInfo> Info;
+    auto appendFailure = [&](StringRef Kind, std::string &&Fixup) {
+      if (!Info) {
+        Info = FailureInfo{Kind, std::move(Fixup)};
+      } else {
+        Info->KindName += Kind;
+        Info->Fixup = std::move(Fixup);
+      }
+    };
+    auto inProgressFixup = [&] {
+      return Info
+          .map([](const FailureInfo &Info) { return StringRef(Info.Fixup); })
+          .getValueOr(Name);
+    };
+    if (auto Fixup = getDoubleUnderscoreFixup(inProgressFixup())) {
+      appendFailure("du", *std::move(Fixup));
+    }
+    if (auto Fixup = getUnderscoreCapitalFixup(inProgressFixup())) {
+      appendFailure("uc", *std::move(Fixup));
+    }
+    if (auto Fixup = getUnderscoreGlobalNamespaceFixup(inProgressFixup(),
+                                                       IsInGlobalNamespace)) {
+      appendFailure("global-under", *std::move(Fixup));
+    }
+    if (Info) {
+      return Info;
+    }
+  } else {
+    if (!(hasDoubleUnderscore(Name) || startsWithUnderscoreCapital(Name) ||
+          startsWithUnderscoreInGlobalNamespace(Name, IsInGlobalNamespace))) {
+      return FailureInfo{"non-reserved", getNonReservedFixup(Name)};
+    }
+  }
+  return None;
+}
+
+Optional<RenamerClangTidyCheck::FailureInfo>
+ReservedIdentifierCheck::GetDeclFailureInfo(const NamedDecl *Decl,
+                                            const SourceManager &) const {
+  assert(Decl && Decl->getIdentifier() && !Decl->getName().empty() &&
+         !Decl->isImplicit() &&
+         "Decl must be an explicit identifier with a name.");
+  return getFailureInfoImpl(Decl->getName(),
+                            isa<TranslationUnitDecl>(Decl->getDeclContext()),
+                            Invert, Whitelist);
+}
+
+Optional<RenamerClangTidyCheck::FailureInfo>
+ReservedIdentifierCheck::GetMacroFailureInfo(const Token &MacroNameTok,
+                                             const SourceManager &) const {
+  return getFailureInfoImpl(MacroNameTok.getIdentifierInfo()->getName(), true,
+                            Invert, Whitelist);
+}
+
+RenamerClangTidyCheck::DiagInfo
+ReservedIdentifierCheck::GetDiagInfo(const NamingCheckId &ID,
+                                     const NamingCheckFailure &Failure) const {
+  return DiagInfo{
+      Failure.Info.KindName == "non-reserved"
+          ? "declaration uses identifier '%0', which is not a reserved "
+            "identifier"
+          : Failure.Info.KindName == "global-under"
+                ? "declaration uses identifier '%0', which is reserved in "
+                  "the global "
+                  "namespace; this causes undefined behavior"
+                : "declaration uses reserved identifier '%0', which causes "
+                  "undefined "
+                  "behavior",
+      [&](DiagnosticBuilder &diag) { diag << ID.second; }};
+};
+
+} // namespace bugprone
+} // namespace tidy
+} // namespace clang

clang-tools-extra/docs/ReleaseNotes.rst

- New :doc:`bugprone-not-null-terminated-result
   <clang-tidy/checks/bugprone-not-null-terminated-result>` check
 
   Finds function calls where it is possible to cause a not null-terminated
   result. Usually the proper length of a string is ``strlen(str) + 1`` or equal
   length of this expression, because the null terminator needs an extra space.
   Without the null terminator it can result in undefined behaviour when the
   string is read.
 
+- New :doc:`bugprone-reserved-identifier
+  <clang-tidy/checks/bugprone-reserved-identifier>` check.
+
+  Checks for usages of identifiers reserved for use by the C++ implementation. 
+  The C++ standard reserves the following names for such use:
+  * identifiers with a double underscore anywhere;
+  * identifiers that begin with an underscore followed by an uppercase letter;
+  * identifiers in the global namespace that begin with an underscore.
+  
+  Violating the naming rules above results in undefined behavior.
+
 - New :doc:`cert-mem57-cpp
   <clang-tidy/checks/cert-mem57-cpp>` check.
 
   Checks if an object of type with extended alignment is allocated by using
   the default ``operator new``.
 
 - New alias :doc:`cert-pos44-c
   <clang-tidy/checks/cert-pos44-c>` to

clang-tools-extra/docs/clang-tidy/checks/bugprone-reserved-identifier.rst

+.. title:: clang-tidy - bugprone-reserved-identifier
+
+bugprone-reserved-identifier
+============================
+
+Checks for usages of identifiers reserved for use by the C++ implementation. 
+The C++ standard reserves the following names for such use:
+* identifiers with a double underscore anywhere;
+* identifiers that begin with an underscore followed by an uppercase letter;
+* identifiers in the global namespace that begin with an underscore.
+
+Violating the naming rules above results in undefined behavior.
+
+.. code-block:: c++
+
+  namespace NS { 
+    void __f(); // name is not allowed in user code
+    using _Int = int; // same with this
+    #define cool__macro // also this
+  }
+  int _g(); // disallowed in global namespace only
+
+The check can also be inverted, i.e. it can be configured to flag any 
+identifier that is _not_ a reserved identifier. This mode is for use by e.g. 
+standard library implementors, to ensure they don't infringe on the user 
+namespace.
+
+Options
+-------
+
+.. option:: Invert
+
+   If non-zero, inverts the check, i.e. flags names that are not reserved. 
+   Default is `0`.
+
+.. option:: Whitelist
+
+   Semicolon-separated list of names that the check ignores. Default is an 
+   empty list.

clang-tools-extra/test/clang-tidy/checkers/Inputs/bugprone-reserved-identifier/system/system-header.h

+namespace std {
+
+void __f() {}
+
+template <class _Tp>
+class reference_wrapper {
+public:
+  typedef _Tp type;
+
+private:
+  type *__f_;
+
+public:
+  reference_wrapper(type &__f)
+      : __f_(&__f) {}
+  // access
+  operator type &() const { return *__f_; }
+  type &get() const { return *__f_; }
+};
+
+template <class _Tp>
+inline reference_wrapper<_Tp>
+ref(_Tp &__t) noexcept {
+  return reference_wrapper<_Tp>(__t);
+}
+
+template <class _Tp>
+inline reference_wrapper<_Tp>
+ref(reference_wrapper<_Tp> __t) noexcept {
+  return ref(__t.get());
+}
+
+} // namespace std

clang-tools-extra/test/clang-tidy/checkers/Inputs/bugprone-reserved-identifier/user-header.h

+#define _HEADER_MACRO(m) int m = 0
+
+namespace _Header_Ns {
+class _Header_Object {
+  int _Header_Member;
+};
+
+float _Header_Global;
+
+void _Header_Function() {}
+
+using _Header_Alias = int;
+} // namespace _Header_Ns
+
+//
+
+#define __header_macro(m) int m = 0
+
+namespace __header_ns {
+class __header_object {
+  int __header_member;
+};
+
+float __header_global;
+
+void __header_function() {}
+
+using __header_alias = int;
+} // namespace __header_ns
+
+//
+
+#define header_macro__m(m) int m = 0
+
+namespace header_ns__n {
+class header_object__o {
+  int header_member__m;
+};
+
+float header_global__g;
+
+void header_function__f() {}
+
+using header_alias__a = int;
+} // namespace header_ns__n
+
+//
+
+#define _header_macro(m) int m = 0
+
+namespace _header_ns {}
+class _header_object {};
+
+float _header_global;
+
+void _header_function() {}
+
+using _header_alias = int;

clang-tools-extra/test/clang-tidy/checkers/bugprone-reserved-identifier-invert.cpp

+// RUN: %check_clang_tidy %s bugprone-reserved-identifier %t -- \
+// RUN:   -config='{CheckOptions: [ \
+// RUN:     {key: bugprone-reserved-identifier.Invert, value: 1}, \
+// RUN:     {key: bugprone-reserved-identifier.Whitelist, value: std;reference_wrapper;ref;cref;type;get}, \
+// RUN:   ]}' -- \
+// RUN:   -I%S/Inputs/bugprone-reserved-identifier \
+// RUN:   -isystem %S/Inputs/bugprone-reserved-identifier/system
+
+namespace std {
+
+void __f() {}
+
+void f();
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: declaration uses identifier 'f', which is not a reserved identifier [bugprone-reserved-identifier]
+// CHECK-FIXES: {{^}}void __f();{{$}}
+struct helper {};
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: declaration uses identifier 'helper', which is not a reserved identifier [bugprone-reserved-identifier]
+// CHECK-FIXES: {{^}}struct __helper {};{{$}}
+struct Helper {};
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: declaration uses identifier 'Helper', which is not a reserved identifier [bugprone-reserved-identifier]
+// CHECK-FIXES: {{^}}struct _Helper {};{{$}}
+struct _helper2 {};
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: declaration uses identifier '_helper2', which is not a reserved identifier [bugprone-reserved-identifier]
+// CHECK-FIXES: {{^}}struct __helper2 {};{{$}}
+
+template <class _Tp>
+class reference_wrapper {
+public:
+  typedef _Tp type;
+
+private:
+  type *__f_;
+
+public:
+  reference_wrapper(type &__f)
+      : __f_(&__f) {}
+  // access
+  operator type &() const { return *__f_; }
+  type &get() const { return *__f_; }
+};
+
+template <class _Tp>
+inline reference_wrapper<_Tp>
+ref(_Tp &__t) noexcept {
+  return reference_wrapper<_Tp>(__t);
+}
+
+template <class _Tp>
+inline reference_wrapper<_Tp>
+ref(reference_wrapper<_Tp> __t) noexcept {
+  return ref(__t.get());
+}
+
+template <class Up>
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: declaration uses identifier 'Up', which is not a reserved identifier [bugprone-reserved-identifier]
+// CHECK-FIXES: {{^}}template <class _Up>{{$}}
+inline reference_wrapper<const Up>
+cref(const Up &u) noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: declaration uses identifier 'u', which is not a reserved identifier [bugprone-reserved-identifier]
+  // CHECK-FIXES: {{^}}cref(const Up &__u) noexcept {{{$}}
+  return reference_wrapper<const Up>(u);
+}
+
+template <class _Tp>
+inline reference_wrapper<_Tp>
+cref(reference_wrapper<const _Tp> __t) noexcept {
+  return cref(__t.get());
+}
+
+} // namespace std

clang-tools-extra/test/clang-tidy/checkers/bugprone-reserved-identifier.cpp

+// RUN: %check_clang_tidy %s bugprone-reserved-identifier %t -- -- \
+// RUN:   -I%S/Inputs/bugprone-reserved-identifier \
+// RUN:   -isystem %S/Inputs/bugprone-reserved-identifier/system
+
+// no warnings expected without -header-filter=
+#include "user-header.h"
+#include <system-header.h>
+
+#define _MACRO(m) int m = 0
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: declaration uses reserved identifier '_MACRO', which causes undefined behavior [bugprone-reserved-identifier]
+// CHECK-FIXES: {{^}}#define MACRO(m) int m = 0{{$}}
+
+namespace _Ns {
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: declaration uses reserved identifier '_Ns', which causes undefined behavior [bugprone-reserved-identifier]
+// CHECK-FIXES: {{^}}namespace Ns {{{$}}
+
+class _Object {
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration uses reserved identifier '_Object', which causes undefined behavior [bugprone-reserved-identifier]
+  // CHECK-FIXES: {{^}}class Object {{{$}}
+  int _Member;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration uses reserved identifier '_Member', which causes undefined behavior [bugprone-reserved-identifier]
+  // CHECK-FIXES: {{^}}  int Member;{{$}}
+};
+
+float _Global;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration uses reserved identifier '_Global', which causes undefined behavior [bugprone-reserved-identifier]
+// CHECK-FIXES: {{^}}float Global;{{$}}
+
+void _Function() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: declaration uses reserved identifier '_Function', which causes undefined behavior [bugprone-reserved-identifier]
+// CHECK-FIXES: {{^}}void Function() {}{{$}}
+
+using _Alias = int;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration uses reserved identifier '_Alias', which causes undefined behavior [bugprone-reserved-identifier]
+// CHECK-FIXES: {{^}}using Alias = int;{{$}}
+
+} // namespace _Ns
+
+//
+
+#define __macro(m) int m = 0
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: declaration uses reserved identifier '__macro', which causes undefined behavior [bugprone-reserved-identifier]
+// CHECK-FIXES: {{^}}#define macro(m) int m = 0{{$}}
+
+namespace __ns {
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: declaration uses reserved identifier '__ns', which causes undefined behavior [bugprone-reserved-identifier]
+// CHECK-FIXES: {{^}}namespace ns {{{$}}
+class __object {
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration uses reserved identifier '__object', which causes undefined behavior [bugprone-reserved-identifier]
+  // CHECK-FIXES: {{^}}class object {{{$}}
+  int __member;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration uses reserved identifier '__member', which causes undefined behavior [bugprone-reserved-identifier]
+  // CHECK-FIXES: {{^}}  int member;{{$}}
+};
+
+float __global;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration uses reserved identifier '__global', which causes undefined behavior [bugprone-reserved-identifier]
+// CHECK-FIXES: {{^}}float global;{{$}}
+
+void __function() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: declaration uses reserved identifier '__function', which causes undefined behavior [bugprone-reserved-identifier]
+// CHECK-FIXES: {{^}}void function() {}{{$}}
+
+using __alias = int;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration uses reserved identifier '__alias', which causes undefined behavior [bugprone-reserved-identifier]
+// CHECK-FIXES: {{^}}using alias = int;{{$}}
+
+} // namespace __ns
+
+//
+
+#define macro___m(m) int m = 0
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: declaration uses reserved identifier 'macro___m', which causes undefined behavior [bugprone-reserved-identifier]
+// CHECK-FIXES: {{^}}#define macro_m(m) int m = 0{{$}}
+
+namespace ns___n {
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: declaration uses reserved identifier 'ns___n', which causes undefined behavior [bugprone-reserved-identifier]
+// CHECK-FIXES: {{^}}namespace ns_n {{{$}}
+class object___o {
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration uses reserved identifier 'object___o', which causes undefined behavior [bugprone-reserved-identifier]
+  // CHECK-FIXES: {{^}}class object_o {{{$}}
+  int member___m;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration uses reserved identifier 'member___m', which causes undefined behavior [bugprone-reserved-identifier]
+  // CHECK-FIXES: {{^}}  int member_m;{{$}}
+};
+
+float global___g;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration uses reserved identifier 'global___g', which causes undefined behavior [bugprone-reserved-identifier]
+// CHECK-FIXES: {{^}}float global_g;{{$}}
+
+void function___f() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: declaration uses reserved identifier 'function___f', which causes undefined behavior [bugprone-reserved-identifier]
+// CHECK-FIXES: {{^}}void function_f() {}{{$}}
+
+using alias___a = int;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration uses reserved identifier 'alias___a', which causes undefined behavior [bugprone-reserved-identifier]
+// CHECK-FIXES: {{^}}using alias_a = int;{{$}}
+
+} // namespace ns___n
+
+//
+
+#define _macro(m) int m = 0
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: declaration uses identifier '_macro', which is reserved in the global namespace; this causes undefined behavior [bugprone-reserved-identifier]
+// CHECK-FIXES: {{^}}#define macro(m) int m = 0{{$}}
+
+namespace _ns {
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: declaration uses identifier '_ns', which is reserved in the global namespace; this causes undefined behavior [bugprone-reserved-identifier]
+// CHECK-FIXES: {{^}}namespace ns {{{$}}
+int _i;
+// no warning
+} // namespace _ns
+class _object {
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration uses identifier '_object', which is reserved in the global namespace; this causes undefined behavior [bugprone-reserved-identifier]
+  // CHECK-FIXES: {{^}}class object {{{$}}
+  int _member;
+  // no warning
+};
+float _global;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration uses identifier '_global', which is reserved in the global namespace; this causes undefined behavior [bugprone-reserved-identifier]
+// CHECK-FIXES: {{^}}float global;{{$}}
+void _function() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: declaration uses identifier '_function', which is reserved in the global namespace; this causes undefined behavior [bugprone-reserved-identifier]
+// CHECK-FIXES: {{^}}void function() {}{{$}}
+using _alias = int;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration uses identifier '_alias', which is reserved in the global namespace; this causes undefined behavior [bugprone-reserved-identifier]
+// CHECK-FIXES: {{^}}using alias = int;{{$}}
+
+void _float() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: declaration uses identifier '_float', which is reserved in the global namespace; this causes undefined behavior; cannot be fixed because 'float' would conflict with a keyword [bugprone-reserved-identifier]
+// CHECK-FIXES: {{^}}void _float() {}{{$}}
+
+#define SOME_MACRO
+int SOME__MACRO;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: declaration uses reserved identifier 'SOME__MACRO', which causes undefined behavior; cannot be fixed because 'SOME_MACRO' would conflict with a macro definition [bugprone-reserved-identifier]
+// CHECK-FIXES: {{^}}int SOME__MACRO;{{$}}
+
+void _TWO__PROBLEMS() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: declaration uses reserved identifier '_TWO__PROBLEMS', which causes undefined behavior [bugprone-reserved-identifier]
+// CHECK-FIXES: {{^}}void TWO_PROBLEMS() {}{{$}}
+void _two__problems() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: declaration uses reserved identifier '_two__problems', which causes undefined behavior [bugprone-reserved-identifier]
+// CHECK-FIXES: {{^}}void two_problems() {}{{$}}
+
+int __;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: declaration uses reserved identifier '__', which causes undefined behavior; cannot be fixed automatically [bugprone-reserved-identifier]
+// CHECK-FIXES: {{^}}int __;{{$}}
+
+int _________;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: declaration uses reserved identifier '_________', which causes undefined behavior; cannot be fixed automatically [bugprone-reserved-identifier]
+// CHECK-FIXES: {{^}}int _________;{{$}}
+
+int _;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: declaration uses identifier '_', which is reserved in the global namespace; this causes undefined behavior; cannot be fixed automatically [bugprone-reserved-identifier]
+// CHECK-FIXES: {{^}}int _;{{$}}
+
+// these should pass
+#define MACRO(m) int m = 0
+
+namespace Ns {
+class Object {
+  int Member;
+};
+float Global;
+
+void Function() {}
+using Alias = int;
+} // namespace Ns
+namespace ns_ {
+class object_ {
+  int member_;
+};
+float global_;
+void function_() {}
+using alias_ = int;
+} // namespace ns_
+
+class object_ {
+  int member_;
+};
+float global_;
+void function_() {}
+using alias_ = int;