merge main into amd-staging (llvm#862)

Author: ronlieb

clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp

@@ -7,7 +7,6 @@
 //===----------------------------------------------------------------------===//
 
 #include "UnnecessaryValueParamCheck.h"
-
 #include "../utils/DeclRefExprUtils.h"
 #include "../utils/FixItHintUtils.h"
 #include "../utils/Matchers.h"
@@ -30,14 +29,6 @@ std::string paramNameOrIndex(StringRef Name, size_t Index) {
       .str();
 }
 
-bool isReferencedOutsideOfCallExpr(const FunctionDecl &Function,
-                                   ASTContext &Context) {
-  auto Matches = match(declRefExpr(to(functionDecl(equalsNode(&Function))),
-                                   unless(hasAncestor(callExpr()))),
-                       Context);
-  return !Matches.empty();
-}
-
 bool hasLoopStmtAncestor(const DeclRefExpr &DeclRef, const Decl &Decl,
                          ASTContext &Context) {
   auto Matches = match(
@@ -155,12 +146,9 @@ void UnnecessaryValueParamCheck::handleConstRefFix(const FunctionDecl &Function,
   // Do not propose fixes when:
   // 1. the ParmVarDecl is in a macro, since we cannot place them correctly
   // 2. the function is virtual as it might break overrides
-  // 3. the function is referenced outside of a call expression within the
-  //    compilation unit as the signature change could introduce build errors.
-  // 4. the function is an explicit template/ specialization.
+  // 3. the function is an explicit template/ specialization.
   const auto *Method = llvm::dyn_cast<CXXMethodDecl>(&Function);
   if (Param.getBeginLoc().isMacroID() || (Method && Method->isVirtual()) ||
-      isReferencedOutsideOfCallExpr(Function, Context) ||
       Function.getTemplateSpecializationKind() == TSK_ExplicitSpecialization)
     return;
   for (const auto *FunctionDecl = &Function; FunctionDecl != nullptr;

clang-tools-extra/docs/ReleaseNotes.rst

@@ -115,6 +115,11 @@ Changes in existing checks
   <clang-tidy/checks/misc/redundant-expression>` check by providing additional
   examples and fixing some macro related false positives.
 
+- Improved :doc:`performance/unnecessary-value-param
+  <clang-tidy/checks/performance/unnecessary-value-param>` check performance by
+  tolerating fix-it breaking compilation when functions is used as pointers 
+  to avoid matching usage of functions within the current compilation unit.
+
 Removed checks
 ^^^^^^^^^^^^^^
 

clang-tools-extra/docs/clang-tidy/checks/performance/unnecessary-value-param.rst

@@ -54,6 +54,10 @@ Will become:
     Field = std::move(Value);
   }
 
+Because the fix-it needs to change the signature of the function, it may break
+builds if the function is used in multiple translation units or some codes
+depends on funcion signatures.
+
 Options
 -------
 

clang-tools-extra/test/clang-tidy/checkers/performance/unnecessary-value-param.cpp

@@ -332,11 +332,7 @@ void PositiveNonConstDeclaration(const ExpensiveToCopyType A) {
 
 void PositiveOnlyMessageAsReferencedInCompilationUnit(ExpensiveToCopyType A) {
   // CHECK-MESSAGES: [[@LINE-1]]:75: warning: the parameter 'A' is copied
-  // CHECK-FIXES: void PositiveOnlyMessageAsReferencedInCompilationUnit(ExpensiveToCopyType A) {
-}
-
-void ReferenceFunctionOutsideOfCallExpr() {
-  void (*ptr)(ExpensiveToCopyType) = &PositiveOnlyMessageAsReferencedInCompilationUnit;
+  // CHECK-FIXES: void PositiveOnlyMessageAsReferencedInCompilationUnit(const ExpensiveToCopyType& A) {
 }
 
 void PositiveMessageAndFixAsFunctionIsCalled(ExpensiveToCopyType A) {

clang/docs/ReleaseNotes.rst

@@ -199,6 +199,12 @@ Improvements to Clang's diagnostics
 - Diagnostics on chained comparisons (``a < b < c``) are now an error by default. This can be disabled with
   ``-Wno-error=parentheses``.
 
+- The :doc:`ThreadSafetyAnalysis` now supports ``-Wthread-safety-pointer``,
+  which enables warning on passing or returning pointers to guarded variables
+  as function arguments or return value respectively. Note that
+  :doc:`ThreadSafetyAnalysis` still does not perform alias analysis. The
+  feature will be default-enabled with ``-Wthread-safety`` in a future release.
+
 Improvements to Clang's time-trace
 ----------------------------------
 

clang/docs/ThreadSafetyAnalysis.rst

@@ -515,8 +515,12 @@ Warning flags
   + ``-Wthread-safety-analysis``: The core analysis.
   + ``-Wthread-safety-precise``: Requires that mutex expressions match precisely.
        This warning can be disabled for code which has a lot of aliases.
-  + ``-Wthread-safety-reference``: Checks when guarded members are passed by reference.
+  + ``-Wthread-safety-reference``: Checks when guarded members are passed or
+    returned by reference.
 
+* ``-Wthread-safety-pointer``: Checks when passing or returning pointers to
+  guarded variables, or pointers to guarded data, as function argument or
+  return value respectively.
 
 :ref:`negative` are an experimental feature, which are enabled with:
 

clang/include/clang/AST/ASTContext.h

@@ -410,14 +410,8 @@ class ASTContext : public RefCountedBase<ASTContext> {
   /// The identifier 'NSCopying'.
   IdentifierInfo *NSCopyingName = nullptr;
 
-  /// The identifier '__make_integer_seq'.
-  mutable IdentifierInfo *MakeIntegerSeqName = nullptr;
-
-  /// The identifier '__type_pack_element'.
-  mutable IdentifierInfo *TypePackElementName = nullptr;
-
-  /// The identifier '__builtin_common_type'.
-  mutable IdentifierInfo *BuiltinCommonTypeName = nullptr;
+#define BuiltinTemplate(BTName) mutable IdentifierInfo *Name##BTName = nullptr;
+#include "clang/Basic/BuiltinTemplates.inc"
 
   QualType ObjCConstantStringType;
   mutable RecordDecl *CFConstantStringTagDecl = nullptr;
@@ -629,9 +623,10 @@ class ASTContext : public RefCountedBase<ASTContext> {
 
   TranslationUnitDecl *TUDecl = nullptr;
   mutable ExternCContextDecl *ExternCContext = nullptr;
-  mutable BuiltinTemplateDecl *MakeIntegerSeqDecl = nullptr;
-  mutable BuiltinTemplateDecl *TypePackElementDecl = nullptr;
-  mutable BuiltinTemplateDecl *BuiltinCommonTypeDecl = nullptr;
+
+#define BuiltinTemplate(BTName)                                                \
+  mutable BuiltinTemplateDecl *Decl##BTName = nullptr;
+#include "clang/Basic/BuiltinTemplates.inc"
 
   /// The associated SourceManager object.
   SourceManager &SourceMgr;
@@ -1157,9 +1152,9 @@ class ASTContext : public RefCountedBase<ASTContext> {
   }
 
   ExternCContextDecl *getExternCContextDecl() const;
-  BuiltinTemplateDecl *getMakeIntegerSeqDecl() const;
-  BuiltinTemplateDecl *getTypePackElementDecl() const;
-  BuiltinTemplateDecl *getBuiltinCommonTypeDecl() const;
+
+#define BuiltinTemplate(BTName) BuiltinTemplateDecl *get##BTName##Decl() const;
+#include "clang/Basic/BuiltinTemplates.inc"
 
   // Builtin Types.
   CanQualType VoidTy;
@@ -2107,23 +2102,13 @@ class ASTContext : public RefCountedBase<ASTContext> {
     return BoolName;
   }
 
-  IdentifierInfo *getMakeIntegerSeqName() const {
-    if (!MakeIntegerSeqName)
-      MakeIntegerSeqName = &Idents.get("__make_integer_seq");
-    return MakeIntegerSeqName;
-  }
-
-  IdentifierInfo *getTypePackElementName() const {
-    if (!TypePackElementName)
-      TypePackElementName = &Idents.get("__type_pack_element");
-    return TypePackElementName;
-  }
-
-  IdentifierInfo *getBuiltinCommonTypeName() const {
-    if (!BuiltinCommonTypeName)
-      BuiltinCommonTypeName = &Idents.get("__builtin_common_type");
-    return BuiltinCommonTypeName;
+#define BuiltinTemplate(BTName)                                                \
+  IdentifierInfo *get##BTName##Name() const {                                  \
+    if (!Name##BTName)                                                         \
+      Name##BTName = &Idents.get(#BTName);                                     \
+    return Name##BTName;                                                       \
   }
+#include "clang/Basic/BuiltinTemplates.inc"
 
   /// Retrieve the Objective-C "instancetype" type, if already known;
   /// otherwise, returns a NULL type;

clang/include/clang/AST/DeclID.h

@@ -71,20 +71,14 @@ enum PredefinedDeclIDs {
   /// The extern "C" context.
   PREDEF_DECL_EXTERN_C_CONTEXT_ID,
 
-  /// The internal '__make_integer_seq' template.
-  PREDEF_DECL_MAKE_INTEGER_SEQ_ID,
-
   /// The internal '__NSConstantString' typedef.
   PREDEF_DECL_CF_CONSTANT_STRING_ID,
 
   /// The internal '__NSConstantString' tag type.
   PREDEF_DECL_CF_CONSTANT_STRING_TAG_ID,
 
-  /// The internal '__type_pack_element' template.
-  PREDEF_DECL_TYPE_PACK_ELEMENT_ID,
-
-  /// The internal '__builtin_common_type' template.
-  PREDEF_DECL_COMMON_TYPE_ID,
+#define BuiltinTemplate(BTName) PREDEF_DECL##BTName##_ID,
+#include "clang/Basic/BuiltinTemplates.inc"
 
   /// The number of declaration IDs that are predefined.
   NUM_PREDEF_DECL_IDS

clang/include/clang/Analysis/Analyses/ThreadSafety.h

@@ -54,6 +54,18 @@ enum ProtectedOperationKind {
 
   /// Returning a pt-guarded variable by reference.
   POK_PtReturnByRef,
+
+  /// Passing pointer to a guarded variable.
+  POK_PassPointer,
+
+  /// Passing a pt-guarded pointer.
+  POK_PtPassPointer,
+
+  /// Returning pointer to a guarded variable.
+  POK_ReturnPointer,
+
+  /// Returning a pt-guarded pointer.
+  POK_PtReturnPointer,
 };
 
 /// This enum distinguishes between different kinds of lock actions. For

clang/include/clang/Basic/BuiltinTemplates.td

@@ -0,0 +1,52 @@
+//===--- BuiltinTemplates.td - Clang builtin template aliases ---*- 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
+//
+//===----------------------------------------------------------------------===//
+
+class TemplateArg<string name> {
+  string Name = name;
+}
+
+class Template<list<TemplateArg> args, string name> : TemplateArg<name> {
+  list<TemplateArg> Args = args;
+}
+
+class Class<string name, bit is_variadic = 0> : TemplateArg<name> {
+  bit IsVariadic = is_variadic;
+}
+
+class NTTP<string type_name, string name, bit is_variadic = 0> : TemplateArg<name> {
+  string TypeName = type_name;
+  bit IsVariadic = is_variadic;
+}
+
+class BuiltinNTTP<string type_name> : TemplateArg<""> {
+  string TypeName = type_name;
+}
+
+def SizeT : BuiltinNTTP<"size_t"> {}
+
+class BuiltinTemplate<list<TemplateArg> template_head> {
+  list<TemplateArg> TemplateHead = template_head;
+}
+
+// template <template <class T, T... Ints> IntSeq, class T, T N>
+def __make_integer_seq : BuiltinTemplate<
+  [Template<[Class<"T">, NTTP<"T", "Ints", /*is_variadic=*/1>], "IntSeq">, Class<"T">, NTTP<"T", "N">]>;
+
+// template <size_t, class... T>
+def __type_pack_element : BuiltinTemplate<
+  [SizeT, Class<"T", /*is_variadic=*/1>]>;
+
+// template <template <class... Args> BaseTemplate,
+//           template <class TypeMember> HasTypeMember,
+//           class HasNoTypeMember
+//           class... Ts>
+def __builtin_common_type : BuiltinTemplate<
+  [Template<[Class<"Args", /*is_variadic=*/1>], "BaseTemplate">,
+   Template<[Class<"TypeMember">], "HasTypeMember">,
+   Class<"HasNoTypeMember">,
+   Class<"Ts", /*is_variadic=*/1>]>;