[clang-tidy] add check cppcoreguidelines-pro-type-union-access

[mgehre]

Summary

This check flags all access to members of unions. Passing unions as a
whole is not flagged.

Reading from a union member assumes that member was the last one
written, and writing to a union member assumes another member with a
nontrivial destructor had its destructor called. This is fragile because
it cannot generally be enforced to be safe in the language and so relies
on programmer discipline to get it right.

This rule is part of the "Type safety" profile of the C++ Core
Guidelines, see
https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#-type7-avoid-accessing-members-of-raw-unions-prefer-variant-instead

[sbenza]

Seems straightforward.

[sbenza]

clang-tidy/cppcoreguidelines/ProTypeUnionAccessCheck.cpp

20 ↗

(On Diff #37515)

This should check that we are in C++ code.

if (!getLangOpts().CPlusPlus)
  return;

clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS support)
 
 add_clang_library(clangTidyCppCoreGuidelinesModule
   CppCoreGuidelinesTidyModule.cpp
   ProBoundsPointerArithmeticCheck.cpp
   ProTypeConstCastCheck.cpp
   ProTypeReinterpretCastCheck.cpp
   ProTypeStaticCastDowncastCheck.cpp
+  ProTypeUnionAccessCheck.cpp
 
   LINK_LIBS
   clangAST
   clangASTMatchers
   clangBasic
   clangLex
   clangTidy
   clangTidyCERTModule
   clangTidyMiscModule
   clangTidyUtils
   clangTooling
   )

clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp

 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
 #include "../cert/VariadicFunctionDefCheck.h"
 #include "../misc/AssignOperatorSignatureCheck.h"
 #include "ProBoundsPointerArithmeticCheck.h"
 #include "ProTypeConstCastCheck.h"
 #include "ProTypeReinterpretCastCheck.h"
 #include "ProTypeStaticCastDowncastCheck.h"
+#include "ProTypeUnionAccessCheck.h"
 
 namespace clang {
 namespace tidy {
 namespace cppcoreguidelines {
 
 /// A module containing checks of the C++ Core Guidelines
 class CppCoreGuidelinesModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<ProBoundsPointerArithmeticCheck>(
         "cppcoreguidelines-pro-bounds-pointer-arithmetic");
     CheckFactories.registerCheck<ProTypeConstCastCheck>(
         "cppcoreguidelines-pro-type-const-cast");
     CheckFactories.registerCheck<VariadicFunctionDefCheck>(
         "cppcoreguidelines-pro-type-vararg-def");
     CheckFactories.registerCheck<ProTypeReinterpretCastCheck>(
         "cppcoreguidelines-pro-type-reinterpret-cast");
     CheckFactories.registerCheck<ProTypeStaticCastDowncastCheck>(
         "cppcoreguidelines-pro-type-static-cast-downcast");
+    CheckFactories.registerCheck<ProTypeUnionAccessCheck>(
+        "cppcoreguidelines-pro-type-union-access");
     CheckFactories.registerCheck<misc::AssignOperatorSignatureCheck>(
         "cppcoreguidelines-c-copy-assignment-signature");
   }
 };
 
 // Register the LLVMTidyModule using this statically initialized variable.
 static ClangTidyModuleRegistry::Add<CppCoreGuidelinesModule>
     X("cppcoreguidelines-module", "Adds checks for the C++ Core Guidelines.");

clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeUnionAccessCheck.h

+//===--- ProTypeUnionAccessCheck.h - clang-tidy------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_PRO_TYPE_UNION_ACCESS_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_PRO_TYPE_UNION_ACCESS_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+
+/// This check flags all access to members of unions.
+/// Access to a union as a whole (e.g. passing to a function) is not flagged.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-pro-type-union-access.html
+class ProTypeUnionAccessCheck : public ClangTidyCheck {
+public:
+  ProTypeUnionAccessCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_PRO_TYPE_UNION_ACCESS_H
+

clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeUnionAccessCheck.cpp

+//===--- ProTypeUnionAccessCheck.cpp - clang-tidy--------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "ProTypeUnionAccessCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+
+void ProTypeUnionAccessCheck::registerMatchers(MatchFinder *Finder) {
+  if (!getLangOpts().CPlusPlus)
+    return;
+
+  Finder->addMatcher(memberExpr(hasObjectExpression(hasType(recordDecl(isUnion())))).bind("expr"), this);
+}
+
+void ProTypeUnionAccessCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *Matched = Result.Nodes.getNodeAs<MemberExpr>("expr");
+  diag(Matched->getMemberLoc(), "do not access members of unions; use (boost::)variant instead");
+}
+
+} // namespace tidy
+} // namespace clang
+

clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-pro-type-union-access.rst

+cppcoreguidelines-pro-type-union-access
+=======================================
+
+This check flags all access to members of unions. Passing unions as a whole is not flagged.
+
+Reading from a union member assumes that member was the last one written, and writing to a union member assumes another member with a nontrivial destructor had its destructor called. This is fragile because it cannot generally be enforced to be safe in the language and so relies on programmer discipline to get it right.
+
+This rule is part of the "Type safety" profile of the C++ Core Guidelines, see
+https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#-type7-avoid-accessing-members-of-raw-unions-prefer-variant-instead

clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst

 List of clang-tidy Checks
 =========================
 
 .. toctree::
    cert-setlongjmp
    cert-variadic-function-def
    cppcoreguidelines-pro-bounds-pointer-arithmetic
    cppcoreguidelines-pro-type-const-cast
    cppcoreguidelines-pro-type-reinterpret-cast
    cppcoreguidelines-pro-type-static-cast-downcast
+   cppcoreguidelines-pro-type-union-access
    google-build-explicit-make-pair
    google-build-namespaces
    google-build-using-namespace
    google-explicit-constructor
    google-global-names-in-headers
    google-readability-braces-around-statements
    google-readability-casting
    google-readability-function-size

clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-pro-type-union-access.cpp

+// RUN: %python %S/check_clang_tidy.py %s cppcoreguidelines-pro-type-union-access %t
+
+union U {
+  bool union_member1;
+  char union_member2;
+} u;
+
+struct S {
+  int non_union_member;
+  union {
+    bool union_member;
+  };
+  union {
+    char union_member2;
+  } u;
+} s;
+
+
+void f(char);
+void f2(U);
+void f3(U&);
+void f4(U*);
+
+void check()
+{
+  u.union_member1 = true;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not access members of unions; use (boost::)variant instead [cppcoreguidelines-pro-type-union-access]
+  auto b = u.union_member2;
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: do not access members of unions; use (boost::)variant instead
+  auto a = &s.union_member;
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: do not access members of unions; use (boost::)variant instead
+  f(s.u.union_member2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: do not access members of unions; use (boost::)variant instead
+
+  s.non_union_member = 2; // OK
+
+  U u2 = u; // OK
+  f2(u); // OK
+  f3(u); // OK
+  f4(&u); // OK
+}