[PATCH] D74564: libclang: Add static build support for Windows

2020-02-16 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan added a comment.

Please, upload patches with context (-U).




Comment at: clang/include/clang-c/Platform.h:31
+#elif defined(CINDEX_EXPORTS)
+  #define CINDEX_LINKAGE __attribute__((visibility("default")))
+#endif

Is it different from just leaving CINDEX_LINKAGE empty?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74564/new/

https://reviews.llvm.org/D74564



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D74698: [CodeGen] -pg shouldn't add "frame-pointer"="all" fn attr w/ -mfentry

2020-02-16 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers updated this revision to Diff 244916.
nickdesaulniers added a comment.

- add test line


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74698/new/

https://reviews.llvm.org/D74698

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/CodeGen/fentry.c


Index: clang/test/CodeGen/fentry.c
===
--- clang/test/CodeGen/fentry.c
+++ clang/test/CodeGen/fentry.c
@@ -2,6 +2,7 @@
 // RUN: %clang_cc1 -pg -mfentry -triple x86_64-unknown-linux-gnu -emit-llvm -o 
- %s | FileCheck %s
 // RUN: %clang_cc1 -mfentry -triple i386-unknown-unknown -emit-llvm -o - %s | 
FileCheck -check-prefix=NOPG %s
 // RUN: %clang_cc1 -mfentry -triple x86_64-unknown-linux-gnu -emit-llvm -o - 
%s | FileCheck -check-prefix=NOPG %s
+// RUN: %clang_cc1 -pg -mfentry -emit-llvm -o - %s | FileCheck 
-check-prefix=NOFP  %s
 
 int foo(void) {
   return 0;
@@ -16,3 +17,5 @@
 //CHECK-NOT: attributes #1 = { {{.*}}"fentry-call"="true"{{.*}} }
 //NOPG-NOT: attributes #0 = { {{.*}}"fentry-call"{{.*}} }
 //NOPG-NOT: attributes #1 = { {{.*}}"fentry-call"{{.*}} }
+//NOFP-NOT: "frame-pointer"="all"
+//NOFP: "frame-pointer"="none"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -526,7 +526,7 @@
 
 static bool useFramePointerForTargetByDefault(const ArgList ,
   const llvm::Triple ) {
-  if (Args.hasArg(options::OPT_pg))
+  if (Args.hasArg(options::OPT_pg) && !Args.hasArg(options::OPT_mfentry))
 return true;
 
   switch (Triple.getArch()) {
@@ -6150,7 +6150,8 @@
   }
 
   if (Arg *A = Args.getLastArg(options::OPT_pg))
-if (FPKeepKind == CodeGenOptions::FramePointerKind::None)
+if (FPKeepKind == CodeGenOptions::FramePointerKind::None &&
+!Args.hasArg(options::OPT_mfentry))
   D.Diag(diag::err_drv_argument_not_allowed_with) << "-fomit-frame-pointer"
   << A->getAsString(Args);
 


Index: clang/test/CodeGen/fentry.c
===
--- clang/test/CodeGen/fentry.c
+++ clang/test/CodeGen/fentry.c
@@ -2,6 +2,7 @@
 // RUN: %clang_cc1 -pg -mfentry -triple x86_64-unknown-linux-gnu -emit-llvm -o - %s | FileCheck %s
 // RUN: %clang_cc1 -mfentry -triple i386-unknown-unknown -emit-llvm -o - %s | FileCheck -check-prefix=NOPG %s
 // RUN: %clang_cc1 -mfentry -triple x86_64-unknown-linux-gnu -emit-llvm -o - %s | FileCheck -check-prefix=NOPG %s
+// RUN: %clang_cc1 -pg -mfentry -emit-llvm -o - %s | FileCheck -check-prefix=NOFP  %s
 
 int foo(void) {
   return 0;
@@ -16,3 +17,5 @@
 //CHECK-NOT: attributes #1 = { {{.*}}"fentry-call"="true"{{.*}} }
 //NOPG-NOT: attributes #0 = { {{.*}}"fentry-call"{{.*}} }
 //NOPG-NOT: attributes #1 = { {{.*}}"fentry-call"{{.*}} }
+//NOFP-NOT: "frame-pointer"="all"
+//NOFP: "frame-pointer"="none"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -526,7 +526,7 @@
 
 static bool useFramePointerForTargetByDefault(const ArgList ,
   const llvm::Triple ) {
-  if (Args.hasArg(options::OPT_pg))
+  if (Args.hasArg(options::OPT_pg) && !Args.hasArg(options::OPT_mfentry))
 return true;
 
   switch (Triple.getArch()) {
@@ -6150,7 +6150,8 @@
   }
 
   if (Arg *A = Args.getLastArg(options::OPT_pg))
-if (FPKeepKind == CodeGenOptions::FramePointerKind::None)
+if (FPKeepKind == CodeGenOptions::FramePointerKind::None &&
+!Args.hasArg(options::OPT_mfentry))
   D.Diag(diag::err_drv_argument_not_allowed_with) << "-fomit-frame-pointer"
   << A->getAsString(Args);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D74698: [CodeGen] -pg shouldn't add "frame-pointer"="all" fn attr w/ -mfentry

2020-02-16 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers created this revision.
nickdesaulniers added reviewers: void, manojgupta, dberris.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

$ clang -O2 -pg -mfentry foo.c

was adding frame pointers to all functions. This was exposed via
compiling the Linux kernel for x86_64 with CONFIG_FUNCTION_TRACER
enabled.

Fixes: pr/44934


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D74698

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/CodeGen/fentry.c


Index: clang/test/CodeGen/fentry.c
===
--- clang/test/CodeGen/fentry.c
+++ clang/test/CodeGen/fentry.c
@@ -2,6 +2,7 @@
 // RUN: %clang_cc1 -pg -mfentry -triple x86_64-unknown-linux-gnu -emit-llvm -o 
- %s | FileCheck %s
 // RUN: %clang_cc1 -mfentry -triple i386-unknown-unknown -emit-llvm -o - %s | 
FileCheck -check-prefix=NOPG %s
 // RUN: %clang_cc1 -mfentry -triple x86_64-unknown-linux-gnu -emit-llvm -o - 
%s | FileCheck -check-prefix=NOPG %s
+// RUN: %clang_cc1 -pg -mfentry -emit-llvm -o - %s | FileCheck 
-check-prefix=NOFP  %s
 
 int foo(void) {
   return 0;
@@ -16,3 +17,4 @@
 //CHECK-NOT: attributes #1 = { {{.*}}"fentry-call"="true"{{.*}} }
 //NOPG-NOT: attributes #0 = { {{.*}}"fentry-call"{{.*}} }
 //NOPG-NOT: attributes #1 = { {{.*}}"fentry-call"{{.*}} }
+//NOFP-NOT: "frame-pointer"="all"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -526,7 +526,7 @@
 
 static bool useFramePointerForTargetByDefault(const ArgList ,
   const llvm::Triple ) {
-  if (Args.hasArg(options::OPT_pg))
+  if (Args.hasArg(options::OPT_pg) && !Args.hasArg(options::OPT_mfentry))
 return true;
 
   switch (Triple.getArch()) {
@@ -6150,7 +6150,8 @@
   }
 
   if (Arg *A = Args.getLastArg(options::OPT_pg))
-if (FPKeepKind == CodeGenOptions::FramePointerKind::None)
+if (FPKeepKind == CodeGenOptions::FramePointerKind::None &&
+!Args.hasArg(options::OPT_mfentry))
   D.Diag(diag::err_drv_argument_not_allowed_with) << "-fomit-frame-pointer"
   << A->getAsString(Args);
 


Index: clang/test/CodeGen/fentry.c
===
--- clang/test/CodeGen/fentry.c
+++ clang/test/CodeGen/fentry.c
@@ -2,6 +2,7 @@
 // RUN: %clang_cc1 -pg -mfentry -triple x86_64-unknown-linux-gnu -emit-llvm -o - %s | FileCheck %s
 // RUN: %clang_cc1 -mfentry -triple i386-unknown-unknown -emit-llvm -o - %s | FileCheck -check-prefix=NOPG %s
 // RUN: %clang_cc1 -mfentry -triple x86_64-unknown-linux-gnu -emit-llvm -o - %s | FileCheck -check-prefix=NOPG %s
+// RUN: %clang_cc1 -pg -mfentry -emit-llvm -o - %s | FileCheck -check-prefix=NOFP  %s
 
 int foo(void) {
   return 0;
@@ -16,3 +17,4 @@
 //CHECK-NOT: attributes #1 = { {{.*}}"fentry-call"="true"{{.*}} }
 //NOPG-NOT: attributes #0 = { {{.*}}"fentry-call"{{.*}} }
 //NOPG-NOT: attributes #1 = { {{.*}}"fentry-call"{{.*}} }
+//NOFP-NOT: "frame-pointer"="all"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -526,7 +526,7 @@
 
 static bool useFramePointerForTargetByDefault(const ArgList ,
   const llvm::Triple ) {
-  if (Args.hasArg(options::OPT_pg))
+  if (Args.hasArg(options::OPT_pg) && !Args.hasArg(options::OPT_mfentry))
 return true;
 
   switch (Triple.getArch()) {
@@ -6150,7 +6150,8 @@
   }
 
   if (Arg *A = Args.getLastArg(options::OPT_pg))
-if (FPKeepKind == CodeGenOptions::FramePointerKind::None)
+if (FPKeepKind == CodeGenOptions::FramePointerKind::None &&
+!Args.hasArg(options::OPT_mfentry))
   D.Diag(diag::err_drv_argument_not_allowed_with) << "-fomit-frame-pointer"
   << A->getAsString(Args);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D72231: [Sema] Adds the pointer-to-int-cast diagnostic

2020-02-16 Thread Nathan Chancellor via Phabricator via cfe-commits
nathanchance added a comment.

There appear to a be semantic difference between GCC and clang with the current 
version of this patch which results in a lot of additional warnings in the 
Linux kernel: https://godbolt.org/z/eHFJd8


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72231/new/

https://reviews.llvm.org/D72231



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


RE: [PATCH] D70172: [CUDA][HIP][OpenMP] Emit deferred diagnostics by a post-parsing AST travese

2020-02-16 Thread Liu, Yaxun (Sam) via cfe-commits
[AMD Official Use Only - Internal Distribution Only]

Thanks a lot for fixing it.

Sam

-Original Message-
From: Fangrui Song via Phabricator  
Sent: Sunday, February 16, 2020 8:41 PM
To: Liu, Yaxun (Sam) ; t...@google.com; rjmcc...@gmail.com; 
rich...@metafoo.co.uk; jdoerf...@anl.gov; a.bat...@hotmail.com
Cc: mask...@google.com; michael.hl...@gmail.com; 
mariya.podchishcha...@intel.com; alexey.ba...@intel.com; 
zhang.guans...@gmail.com; her...@google.com; r...@google.com; 
cfe-commits@lists.llvm.org; mlek...@skidmore.edu; blitzrak...@gmail.com; 
shen...@google.com
Subject: [PATCH] D70172: [CUDA][HIP][OpenMP] Emit deferred diagnostics by a 
post-parsing AST travese

[CAUTION: External Email]

MaskRay added inline comments.



Comment at: clang/lib/Sema/Sema.cpp:14

+#include "UsedDeclVisitor.h"
 #include "clang/AST/ASTContext.h"

hliao wrote:
> this file is missing and breaks the build
Fixed by c7fa409bcadaf4ddba1862b2e52349e0ab03d1b4


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Freviews.llvm.org%2FD70172%2Fnew%2Fdata=02%7C01%7Cyaxun.liu%40amd.com%7Cd887ec3ab69146d8ee4e08d7b36392d6%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637175112565124973sdata=16dU6RUcV1M9%2BMoNbgytch179KaG6vr9fJO%2BzeYJ4z4%3Dreserved=0

https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Freviews.llvm.org%2FD70172data=02%7C01%7Cyaxun.liu%40amd.com%7Cd887ec3ab69146d8ee4e08d7b36392d6%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637175112565124973sdata=gDh1FqkmR7VzE73YXtGyX8Z3pr8FCJwjnZH9RaVv%2FCo%3Dreserved=0


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D70172: [CUDA][HIP][OpenMP] Emit deferred diagnostics by a post-parsing AST travese

2020-02-16 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments.



Comment at: clang/lib/Sema/Sema.cpp:14
 
+#include "UsedDeclVisitor.h"
 #include "clang/AST/ASTContext.h"

hliao wrote:
> this file is missing and breaks the build
Fixed by c7fa409bcadaf4ddba1862b2e52349e0ab03d1b4


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70172/new/

https://reviews.llvm.org/D70172



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] c7fa409 - [CUDA][HIP][OpenMP] Add lib/Sema/UsedDeclVisitor.h after D70172

2020-02-16 Thread Fangrui Song via cfe-commits

Author: Fangrui Song
Date: 2020-02-16T20:33:07-08:00
New Revision: c7fa409bcadaf4ddba1862b2e52349e0ab03d1b4

URL: 
https://github.com/llvm/llvm-project/commit/c7fa409bcadaf4ddba1862b2e52349e0ab03d1b4
DIFF: 
https://github.com/llvm/llvm-project/commit/c7fa409bcadaf4ddba1862b2e52349e0ab03d1b4.diff

LOG: [CUDA][HIP][OpenMP] Add lib/Sema/UsedDeclVisitor.h after D70172

Added: 
clang/lib/Sema/UsedDeclVisitor.h

Modified: 


Removed: 




diff  --git a/clang/lib/Sema/UsedDeclVisitor.h 
b/clang/lib/Sema/UsedDeclVisitor.h
new file mode 100644
index ..440029a1d567
--- /dev/null
+++ b/clang/lib/Sema/UsedDeclVisitor.h
@@ -0,0 +1,70 @@
+//===- UsedDeclVisitor.h - ODR-used declarations visitor *- 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
+//===--===//
+//
+//  This file defines UsedDeclVisitor, a CRTP class which visits all the
+//  declarations that are ODR-used by an expression or statement.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_LIB_SEMA_USEDDECLVISITOR_H
+#define LLVM_CLANG_LIB_SEMA_USEDDECLVISITOR_H
+
+#include "clang/AST/EvaluatedExprVisitor.h"
+#include "clang/Sema/SemaInternal.h"
+
+namespace clang {
+template 
+class UsedDeclVisitor : public EvaluatedExprVisitor {
+protected:
+  Sema 
+
+public:
+  typedef EvaluatedExprVisitor Inherited;
+
+  UsedDeclVisitor(Sema ) : Inherited(S.Context), S(S) {}
+
+  Derived () { return *static_cast(this); }
+
+  void VisitCXXBindTemporaryExpr(CXXBindTemporaryExpr *E) {
+asImpl().visitUsedDecl(
+E->getBeginLoc(),
+const_cast(E->getTemporary()->getDestructor()));
+asImpl().Visit(E->getSubExpr());
+  }
+
+  void VisitCXXNewExpr(CXXNewExpr *E) {
+if (E->getOperatorNew())
+  asImpl().visitUsedDecl(E->getBeginLoc(), E->getOperatorNew());
+if (E->getOperatorDelete())
+  asImpl().visitUsedDecl(E->getBeginLoc(), E->getOperatorDelete());
+Inherited::VisitCXXNewExpr(E);
+  }
+
+  void VisitCXXDeleteExpr(CXXDeleteExpr *E) {
+if (E->getOperatorDelete())
+  asImpl().visitUsedDecl(E->getBeginLoc(), E->getOperatorDelete());
+QualType Destroyed = S.Context.getBaseElementType(E->getDestroyedType());
+if (const RecordType *DestroyedRec = Destroyed->getAs()) {
+  CXXRecordDecl *Record = cast(DestroyedRec->getDecl());
+  asImpl().visitUsedDecl(E->getBeginLoc(), S.LookupDestructor(Record));
+}
+
+Inherited::VisitCXXDeleteExpr(E);
+  }
+
+  void VisitCXXConstructExpr(CXXConstructExpr *E) {
+asImpl().visitUsedDecl(E->getBeginLoc(), E->getConstructor());
+Inherited::VisitCXXConstructExpr(E);
+  }
+
+  void VisitCXXDefaultArgExpr(CXXDefaultArgExpr *E) {
+asImpl().Visit(E->getExpr());
+  }
+};
+} // end namespace clang
+
+#endif // LLVM_CLANG_LIB_SEMA_USEDDECLVISITOR_H



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D70172: [CUDA][HIP][OpenMP] Emit deferred diagnostics by a post-parsing AST travese

2020-02-16 Thread Michael Liao via Phabricator via cfe-commits
hliao added a comment.

one header is missing and breaks the build




Comment at: clang/lib/Sema/Sema.cpp:14
 
+#include "UsedDeclVisitor.h"
 #include "clang/AST/ASTContext.h"

this file is missing and breaks the build


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70172/new/

https://reviews.llvm.org/D70172



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D70172: [CUDA][HIP][OpenMP] Emit deferred diagnostics by a post-parsing AST travese

2020-02-16 Thread Yaxun Liu via Phabricator via cfe-commits
This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
yaxunl marked an inline comment as done.
Closed by commit rG1b978ddba05c: [CUDA][HIP][OpenMP] Emit deferred diagnostics 
by a post-parsing AST travese (authored by yaxunl).
Herald added a project: clang.

Changed prior to commit:
  https://reviews.llvm.org/D70172?vs=242296=244912#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70172/new/

https://reviews.llvm.org/D70172

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaCUDA.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/declare_target_messages.cpp
  clang/test/OpenMP/nvptx_target_exceptions_messages.cpp
  clang/test/SemaCUDA/bad-calls-on-same-line.cu
  clang/test/SemaCUDA/call-device-fn-from-host.cu
  clang/test/SemaCUDA/call-host-fn-from-device.cu
  clang/test/SemaCUDA/openmp-target.cu
  clang/test/SemaCUDA/trace-through-global.cu

Index: clang/test/SemaCUDA/trace-through-global.cu
===
--- clang/test/SemaCUDA/trace-through-global.cu
+++ clang/test/SemaCUDA/trace-through-global.cu
@@ -38,7 +38,7 @@
   // Notice that these two diagnostics are different: Because the call to hd1
   // is not dependent on T, the call to hd1 comes from 'launch_kernel', while
   // the call to hd3, being dependent, comes from 'launch_kernel'.
-  hd1(); // expected-note {{called by 'launch_kernel'}}
+  hd1(); // expected-note {{called by 'launch_kernel'}}
   hd3(T()); // expected-note {{called by 'launch_kernel'}}
 }
 
Index: clang/test/SemaCUDA/openmp-target.cu
===
--- clang/test/SemaCUDA/openmp-target.cu
+++ clang/test/SemaCUDA/openmp-target.cu
@@ -16,9 +16,9 @@
 void bazzz() {bazz();}
 #pragma omp declare target to(bazzz) device_type(nohost)
 void any() {bazz();} // expected-error {{function with 'device_type(nohost)' is not available on host}}
-void host1() {bazz();}
+void host1() {bazz();} // expected-error {{function with 'device_type(nohost)' is not available on host}}
 #pragma omp declare target to(host1) device_type(host)
-void host2() {bazz();}
+void host2() {bazz();} // expected-error {{function with 'device_type(nohost)' is not available on host}}
 #pragma omp declare target to(host2)
 void device() {host1();}
 #pragma omp declare target to(device) device_type(nohost)
Index: clang/test/SemaCUDA/call-host-fn-from-device.cu
===
--- clang/test/SemaCUDA/call-host-fn-from-device.cu
+++ clang/test/SemaCUDA/call-host-fn-from-device.cu
@@ -56,14 +56,14 @@
 }
 
 template  __host__ __device__ void hd2() { host_fn(); }
-// expected-error@-1 2 {{reference to __host__ function 'host_fn' in __host__ __device__ function}}
+// expected-error@-1 {{reference to __host__ function 'host_fn' in __host__ __device__ function}}
 __global__ void kernel() { hd2(); }
 
 __host__ __device__ void hd() { host_fn(); }
 // expected-error@-1 {{reference to __host__ function 'host_fn' in __host__ __device__ function}}
 
 template  __host__ __device__ void hd3() { host_fn(); }
-// expected-error@-1 2 {{reference to __host__ function 'host_fn' in __host__ __device__ function}}
+// expected-error@-1 {{reference to __host__ function 'host_fn' in __host__ __device__ function}}
 __device__ void device_fn() { hd3(); }
 
 // No error because this is never instantiated.
Index: clang/test/SemaCUDA/call-device-fn-from-host.cu
===
--- clang/test/SemaCUDA/call-device-fn-from-host.cu
+++ clang/test/SemaCUDA/call-device-fn-from-host.cu
@@ -1,7 +1,7 @@
 // RUN: %clang_cc1 %s --std=c++11 -triple x86_64-unknown-linux -emit-llvm -o - \
 // RUN:   -verify -verify-ignore-unexpected=note
 // RUN: %clang_cc1 %s --std=c++11 -triple x86_64-unknown-linux -emit-llvm -o - \
-// RUN:   -verify -verify-ignore-unexpected=note -fopenmp
+// RUN:   -verify=expected,omp -verify-ignore-unexpected=note -fopenmp
 
 // Note: This test won't work with -fsyntax-only, because some of these errors
 // are emitted during codegen.
@@ -39,7 +39,7 @@
 }
 
 template  __host__ __device__ void hd2() { device_fn(); }
-// expected-error@-1 2 {{reference to __device__ function 'device_fn' in __host__ __device__ function}}
+// expected-error@-1 {{reference to __device__ function 'device_fn' in __host__ __device__ function}}
 void host_fn() { hd2(); }
 
 __host__ __device__ void hd() { device_fn(); }
Index: clang/test/SemaCUDA/bad-calls-on-same-line.cu
===
--- clang/test/SemaCUDA/bad-calls-on-same-line.cu
+++ clang/test/SemaCUDA/bad-calls-on-same-line.cu
@@ -33,8 +33,8 @@
 
 void host_fn() {
   hd();
-  hd();  // expected-note 

[clang] 1b978dd - [CUDA][HIP][OpenMP] Emit deferred diagnostics by a post-parsing AST travese

2020-02-16 Thread Yaxun Liu via cfe-commits

Author: Yaxun (Sam) Liu
Date: 2020-02-16T22:44:33-05:00
New Revision: 1b978ddba05cb15e22b4e75adb5e7362ad861987

URL: 
https://github.com/llvm/llvm-project/commit/1b978ddba05cb15e22b4e75adb5e7362ad861987
DIFF: 
https://github.com/llvm/llvm-project/commit/1b978ddba05cb15e22b4e75adb5e7362ad861987.diff

LOG: [CUDA][HIP][OpenMP] Emit deferred diagnostics by a post-parsing AST travese

This patch removes the explicit call graph for CUDA/HIP/OpenMP deferred
diagnostics generated during parsing since it is error prone due to
incomplete information about function declarations during parsing. In stead,
this patch does a post-parsing AST traverse and emits deferred diagnostics
based on the use graph implicitly generated during the traverse.

Differential Revision: https://reviews.llvm.org/D70172

Added: 


Modified: 
clang/include/clang/Sema/Sema.h
clang/lib/Sema/Sema.cpp
clang/lib/Sema/SemaCUDA.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaExpr.cpp
clang/lib/Sema/SemaOpenMP.cpp
clang/test/OpenMP/declare_target_messages.cpp
clang/test/OpenMP/nvptx_target_exceptions_messages.cpp
clang/test/SemaCUDA/bad-calls-on-same-line.cu
clang/test/SemaCUDA/call-device-fn-from-host.cu
clang/test/SemaCUDA/call-host-fn-from-device.cu
clang/test/SemaCUDA/openmp-target.cu
clang/test/SemaCUDA/trace-through-global.cu

Removed: 




diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index a6430353b241..6a3add109b09 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -1464,6 +1464,12 @@ class Sema final {
 
   void emitAndClearUnusedLocalTypedefWarnings();
 
+  // Emit all deferred diagnostics.
+  void emitDeferredDiags();
+  // Emit any deferred diagnostics for FD and erase them from the map in which
+  // they're stored.
+  void emitDeferredDiags(FunctionDecl *FD, bool ShowCallStack);
+
   enum TUFragmentKind {
 /// The global module fragment, between 'module;' and a module-declaration.
 Global,
@@ -3683,7 +3689,8 @@ class Sema final {
 TemplateDiscarded, // Discarded due to uninstantiated templates
 Unknown,
   };
-  FunctionEmissionStatus getEmissionStatus(FunctionDecl *Decl);
+  FunctionEmissionStatus getEmissionStatus(FunctionDecl *Decl,
+   bool Final = false);
 
   // Whether the callee should be ignored in CUDA/HIP/OpenMP host/device check.
   bool shouldIgnoreInHostDeviceCheck(FunctionDecl *Callee);
@@ -9677,22 +9684,10 @@ class Sema final {
   /// Pop OpenMP function region for non-capturing function.
   void popOpenMPFunctionRegion(const sema::FunctionScopeInfo *OldFSI);
 
-  /// Check whether we're allowed to call Callee from the current function.
-  void checkOpenMPDeviceFunction(SourceLocation Loc, FunctionDecl *Callee,
- bool CheckForDelayedContext = true);
-
-  /// Check whether we're allowed to call Callee from the current function.
-  void checkOpenMPHostFunction(SourceLocation Loc, FunctionDecl *Callee,
-   bool CheckCaller = true);
-
   /// Check if the expression is allowed to be used in expressions for the
   /// OpenMP devices.
   void checkOpenMPDeviceExpr(const Expr *E);
 
-  /// Finishes analysis of the deferred functions calls that may be declared as
-  /// host/nohost during device/host compilation.
-  void finalizeOpenMPDelayedAnalysis();
-
   /// Checks if a type or a declaration is disabled due to the owning extension
   /// being disabled, and emits diagnostic messages if it is disabled.
   /// \param D type or declaration to be checked.
@@ -9875,6 +9870,11 @@ class Sema final {
   void
   checkDeclIsAllowedInOpenMPTarget(Expr *E, Decl *D,
SourceLocation IdLoc = SourceLocation());
+  /// Finishes analysis of the deferred functions calls that may be declared as
+  /// host/nohost during device/host compilation.
+  void finalizeOpenMPDelayedAnalysis(const FunctionDecl *Caller,
+ const FunctionDecl *Callee,
+ SourceLocation Loc);
   /// Return true inside OpenMP declare target region.
   bool isInOpenMPDeclareTargetContext() const {
 return DeclareTargetNestingLevel > 0;
@@ -11223,18 +11223,6 @@ class Sema final {
  /* Caller = */ FunctionDeclAndLoc>
   DeviceKnownEmittedFns;
 
-  /// A partial call graph maintained during CUDA/OpenMP device code 
compilation
-  /// to support deferred diagnostics.
-  ///
-  /// Functions are only added here if, at the time they're considered, they 
are
-  /// not known-emitted.  As soon as we discover that a function is
-  /// known-emitted, we remove it and everything it transitively calls from 
this
-  /// set and add those functions to DeviceKnownEmittedFns.
-  llvm::DenseMap,
- /* Callees = */ 

[PATCH] D73651: [OpenCL][CUDA][HIP][SYCL] Add norecurse

2020-02-16 Thread Yaxun Liu via Phabricator via cfe-commits
This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rGfb44b9db95a3: [OpenCL][CUDA][HIP][SYCL] Add norecurse 
(authored by yaxunl).
Herald added subscribers: kerbowa, nhaehnle, jvesely.
Herald added a project: clang.

Changed prior to commit:
  https://reviews.llvm.org/D73651?vs=242300=244906#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73651/new/

https://reviews.llvm.org/D73651

Files:
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/test/CodeGenCUDA/norecurse.cu
  clang/test/CodeGenCUDA/propagate-metadata.cu
  clang/test/CodeGenOpenCL/amdgpu-attrs.cl
  clang/test/CodeGenOpenCL/norecurse.cl
  clang/test/SemaCUDA/call-kernel-from-kernel.cu

Index: clang/test/SemaCUDA/call-kernel-from-kernel.cu
===
--- /dev/null
+++ clang/test/SemaCUDA/call-kernel-from-kernel.cu
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 %s --std=c++11 -triple x86_64-unknown-linux -emit-llvm -o - \
+// RUN:   -verify -fsyntax-only -verify-ignore-unexpected=note
+
+#include "Inputs/cuda.h"
+
+__global__ void kernel1();
+__global__ void kernel2() {
+  kernel1<<<1,1>>>(); // expected-error {{reference to __global__ function 'kernel1' in __global__ function}}
+}
Index: clang/test/CodeGenOpenCL/norecurse.cl
===
--- /dev/null
+++ clang/test/CodeGenOpenCL/norecurse.cl
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -O0 -emit-llvm -o - %s | FileCheck %s
+
+kernel void kernel1(int a) {}
+// CHECK: define{{.*}}@kernel1{{.*}}#[[ATTR:[0-9]*]]
+
+// CHECK: attributes #[[ATTR]] = {{.*}}norecurse
Index: clang/test/CodeGenOpenCL/amdgpu-attrs.cl
===
--- clang/test/CodeGenOpenCL/amdgpu-attrs.cl
+++ clang/test/CodeGenOpenCL/amdgpu-attrs.cl
@@ -162,33 +162,33 @@
 // CHECK-NOT: "amdgpu-num-sgpr"="0"
 // CHECK-NOT: "amdgpu-num-vgpr"="0"
 
-// CHECK-DAG: attributes [[FLAT_WORK_GROUP_SIZE_32_64]] = { convergent noinline nounwind optnone "amdgpu-flat-work-group-size"="32,64" "amdgpu-implicitarg-num-bytes"="56"
-// CHECK-DAG: attributes [[FLAT_WORK_GROUP_SIZE_64_64]] = { convergent noinline nounwind optnone "amdgpu-flat-work-group-size"="64,64" "amdgpu-implicitarg-num-bytes"="56"
-// CHECK-DAG: attributes [[FLAT_WORK_GROUP_SIZE_16_128]] = { convergent noinline nounwind optnone "amdgpu-flat-work-group-size"="16,128" "amdgpu-implicitarg-num-bytes"="56"
-
-// CHECK-DAG: attributes [[WAVES_PER_EU_2]] = { convergent noinline nounwind optnone "amdgpu-flat-work-group-size"="1,256"  "amdgpu-implicitarg-num-bytes"="56" "amdgpu-waves-per-eu"="2"
-
-// CHECK-DAG: attributes [[WAVES_PER_EU_2_4]] = { convergent noinline nounwind optnone "amdgpu-flat-work-group-size"="1,256" "amdgpu-implicitarg-num-bytes"="56" "amdgpu-waves-per-eu"="2,4"
-// CHECK-DAG: attributes [[NUM_SGPR_32]] = { convergent noinline nounwind optnone "amdgpu-flat-work-group-size"="1,256" "amdgpu-implicitarg-num-bytes"="56" "amdgpu-num-sgpr"="32"
-// CHECK-DAG: attributes [[NUM_VGPR_64]] = { convergent noinline nounwind optnone "amdgpu-flat-work-group-size"="1,256" "amdgpu-implicitarg-num-bytes"="56" "amdgpu-num-vgpr"="64"
-
-// CHECK-DAG: attributes [[FLAT_WORK_GROUP_SIZE_32_64_WAVES_PER_EU_2]] = { convergent noinline nounwind optnone "amdgpu-flat-work-group-size"="32,64" "amdgpu-implicitarg-num-bytes"="56" "amdgpu-waves-per-eu"="2"
-// CHECK-DAG: attributes [[FLAT_WORK_GROUP_SIZE_32_64_WAVES_PER_EU_2_4]] = { convergent noinline nounwind optnone "amdgpu-flat-work-group-size"="32,64" "amdgpu-implicitarg-num-bytes"="56" "amdgpu-waves-per-eu"="2,4"
-// CHECK-DAG: attributes [[FLAT_WORK_GROUP_SIZE_32_64_NUM_SGPR_32]] = { convergent noinline nounwind optnone "amdgpu-flat-work-group-size"="32,64" "amdgpu-implicitarg-num-bytes"="56" "amdgpu-num-sgpr"="32"
-// CHECK-DAG: attributes [[FLAT_WORK_GROUP_SIZE_32_64_NUM_VGPR_64]] = { convergent noinline nounwind optnone "amdgpu-flat-work-group-size"="32,64" "amdgpu-implicitarg-num-bytes"="56" "amdgpu-num-vgpr"="64"
-// CHECK-DAG: attributes [[WAVES_PER_EU_2_NUM_SGPR_32]] = { convergent noinline nounwind optnone "amdgpu-flat-work-group-size"="1,256" "amdgpu-implicitarg-num-bytes"="56" "amdgpu-num-sgpr"="32" "amdgpu-waves-per-eu"="2"
-// CHECK-DAG: attributes [[WAVES_PER_EU_2_NUM_VGPR_64]] = { convergent noinline nounwind optnone "amdgpu-flat-work-group-size"="1,256" "amdgpu-implicitarg-num-bytes"="56" "amdgpu-num-vgpr"="64" "amdgpu-waves-per-eu"="2"
-// CHECK-DAG: attributes [[WAVES_PER_EU_2_4_NUM_SGPR_32]] = { convergent noinline nounwind optnone "amdgpu-flat-work-group-size"="1,256" "amdgpu-implicitarg-num-bytes"="56" "amdgpu-num-sgpr"="32" "amdgpu-waves-per-eu"="2,4"
-// CHECK-DAG: attributes [[WAVES_PER_EU_2_4_NUM_VGPR_64]] = { convergent noinline nounwind optnone "amdgpu-flat-work-group-size"="1,256" 

[clang] fb44b9d - [OpenCL][CUDA][HIP][SYCL] Add norecurse

2020-02-16 Thread Yaxun Liu via cfe-commits

Author: Yaxun (Sam) Liu
Date: 2020-02-16T20:41:00-05:00
New Revision: fb44b9db95a333efdfa9a33ddc1778f97428f5f5

URL: 
https://github.com/llvm/llvm-project/commit/fb44b9db95a333efdfa9a33ddc1778f97428f5f5
DIFF: 
https://github.com/llvm/llvm-project/commit/fb44b9db95a333efdfa9a33ddc1778f97428f5f5.diff

LOG: [OpenCL][CUDA][HIP][SYCL] Add norecurse

norecurse function attr indicates the function is not called recursively
directly or indirectly.

Add norecurse to OpenCL functions, SYCL functions in device compilation
and CUDA/HIP kernels.

Although there is LLVM pass adding norecurse to functions, it only works
for whole-program compilation. Also FE adding norecurse can make that
pass run faster since functions with norecurse do not need to be checked
again.

Differential Revision: https://reviews.llvm.org/D73651

Added: 
clang/test/CodeGenCUDA/norecurse.cu
clang/test/CodeGenOpenCL/norecurse.cl
clang/test/SemaCUDA/call-kernel-from-kernel.cu

Modified: 
clang/lib/CodeGen/CodeGenFunction.cpp
clang/test/CodeGenCUDA/propagate-metadata.cu
clang/test/CodeGenOpenCL/amdgpu-attrs.cl

Removed: 




diff  --git a/clang/lib/CodeGen/CodeGenFunction.cpp 
b/clang/lib/CodeGen/CodeGenFunction.cpp
index bcd936638d61..d6c2afc51b04 100644
--- a/clang/lib/CodeGen/CodeGenFunction.cpp
+++ b/clang/lib/CodeGen/CodeGenFunction.cpp
@@ -918,10 +918,20 @@ void CodeGenFunction::StartFunction(GlobalDecl GD, 
QualType RetTy,
   // If we're in C++ mode and the function name is "main", it is guaranteed
   // to be norecurse by the standard (3.6.1.3 "The function main shall not be
   // used within a program").
-  if (getLangOpts().CPlusPlus)
-if (const FunctionDecl *FD = dyn_cast_or_null(D))
-  if (FD->isMain())
-Fn->addFnAttr(llvm::Attribute::NoRecurse);
+  //
+  // OpenCL C 2.0 v2.2-11 s6.9.i:
+  // Recursion is not supported.
+  //
+  // SYCL v1.2.1 s3.10:
+  // kernels cannot include RTTI information, exception classes,
+  // recursive code, virtual functions or make use of C++ libraries that
+  // are not compiled for the device.
+  if (const FunctionDecl *FD = dyn_cast_or_null(D)) {
+if ((getLangOpts().CPlusPlus && FD->isMain()) || getLangOpts().OpenCL ||
+getLangOpts().SYCLIsDevice ||
+(getLangOpts().CUDA && FD->hasAttr()))
+  Fn->addFnAttr(llvm::Attribute::NoRecurse);
+  }
 
   if (const FunctionDecl *FD = dyn_cast_or_null(D))
 if (FD->usesFPIntrin())

diff  --git a/clang/test/CodeGenCUDA/norecurse.cu 
b/clang/test/CodeGenCUDA/norecurse.cu
new file mode 100644
index ..07f0f83179fe
--- /dev/null
+++ b/clang/test/CodeGenCUDA/norecurse.cu
@@ -0,0 +1,15 @@
+// REQUIRES: amdgpu-registered-target
+// REQUIRES: nvptx-registered-target
+
+// RUN: %clang_cc1 -triple nvptx-nvidia-cuda -fcuda-is-device \
+// RUN: -emit-llvm -disable-llvm-passes -o - %s | FileCheck %s
+
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -fcuda-is-device \
+// RUN: -emit-llvm -disable-llvm-passes -o - -x hip %s | FileCheck %s
+
+#include "Inputs/cuda.h"
+
+__global__ void kernel1(int a) {}
+// CHECK: define{{.*}}@_Z7kernel1i{{.*}}#[[ATTR:[0-9]*]]
+
+// CHECK: attributes #[[ATTR]] = {{.*}}norecurse

diff  --git a/clang/test/CodeGenCUDA/propagate-metadata.cu 
b/clang/test/CodeGenCUDA/propagate-metadata.cu
index 45f9319f013f..4b1976939993 100644
--- a/clang/test/CodeGenCUDA/propagate-metadata.cu
+++ b/clang/test/CodeGenCUDA/propagate-metadata.cu
@@ -48,16 +48,33 @@ __global__ void kernel() { lib_fn(); }
 }
 
 // The kernel and lib function should have the same attributes.
-// CHECK: define void @kernel() [[attr:#[0-9]+]]
-// CHECK: define internal void @lib_fn() [[attr]]
+// CHECK: define void @kernel() [[kattr:#[0-9]+]]
+// CHECK: define internal void @lib_fn() [[fattr:#[0-9]+]]
 
 // FIXME: These -NOT checks do not work as intended and do not check on the 
same
 // line.
 
-// Check the attribute list.
-// CHECK: attributes [[attr]] = {
+// Check the attribute list for kernel.
+// CHECK: attributes [[kattr]] = {
 
 // CHECK-SAME: convergent
+// CHECK-SAME: norecurse
+
+// FTZ-NOT: "denormal-fp-math"
+
+// FTZ-SAME: "denormal-fp-math-f32"="preserve-sign,preserve-sign"
+// NOFTZ-SAME: "denormal-fp-math-f32"="ieee,ieee"
+
+// CHECK-SAME: "no-trapping-math"="true"
+
+// FAST-SAME: "unsafe-fp-math"="true"
+// NOFAST-NOT: "unsafe-fp-math"="true"
+
+// Check the attribute list for lib_fn.
+// CHECK: attributes [[fattr]] = {
+
+// CHECK-SAME: convergent
+// CHECK-NOT: norecurse
 
 // FTZ-NOT: "denormal-fp-math"
 

diff  --git a/clang/test/CodeGenOpenCL/amdgpu-attrs.cl 
b/clang/test/CodeGenOpenCL/amdgpu-attrs.cl
index b66714849342..13f8b1191c2b 100644
--- a/clang/test/CodeGenOpenCL/amdgpu-attrs.cl
+++ b/clang/test/CodeGenOpenCL/amdgpu-attrs.cl
@@ -162,33 +162,33 @@ kernel void default_kernel() {
 // CHECK-NOT: "amdgpu-num-sgpr"="0"
 // CHECK-NOT: "amdgpu-num-vgpr"="0"
 
-// CHECK-DAG: attributes 

[PATCH] D73755: [objc_direct] Small updates to help with adoption.

2020-02-16 Thread Pierre Habouzit via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG3adcc78a8071: [objc_direct] Small updates to help with 
adoption. (authored by MadCoder).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73755/new/

https://reviews.llvm.org/D73755

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDeclObjC.cpp
  clang/lib/Sema/SemaExprObjC.cpp
  clang/lib/Sema/SemaObjCProperty.cpp
  clang/test/FixIt/fixit-objc-direct.m
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/SemaObjC/category-direct-properties.m
  clang/test/SemaObjC/dynamic-direct-properties.m
  clang/test/SemaObjC/method-direct.m

Index: clang/test/SemaObjC/method-direct.m
===
--- clang/test/SemaObjC/method-direct.m
+++ clang/test/SemaObjC/method-direct.m
@@ -18,6 +18,7 @@
 + (void)classRootDirect __attribute__((objc_direct)); // expected-note {{previous declaration is here}};
 - (void)otherRootDirect __attribute__((objc_direct)); // expected-note {{direct method 'otherRootDirect' declared here}}
 + (void)otherClassRootDirect __attribute__((objc_direct)); // expected-note {{direct method 'otherClassRootDirect' declared here}}
++ (void)otherOtherClassRootDirect __attribute__((objc_direct)); // expected-note {{direct method 'otherOtherClassRootDirect' declared here}}
 - (void)notDirectInIface; // expected-note {{previous declaration is here}}
 + (void)classNotDirectInIface;// expected-note {{previous declaration is here}}
 @end
@@ -48,8 +49,9 @@
 + (void)classRootCategoryDirect2 __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
 @end
 
-__attribute__((objc_root_class, objc_direct_members)) // expected-error {{'objc_direct_members' attribute only applies to Objective-C implementation declarations and Objective-C containers}}
-@interface SubDirectFail : Root
+__attribute__((objc_direct_members))
+@interface SubDirectMembers : Root
+@property int foo; // expected-note {{previous declaration is here}}
 - (instancetype)init;
 @end
 
@@ -94,6 +96,8 @@
 + (void)otherClassRootDirect {
   [self someRootDirectMethod]; // expected-error {{messaging a Class with a method that is possibly direct}}
 }
++ (void)otherOtherClassRootDirect {
+}
 - (void)rootExtensionDirect {
 }
 + (void)classRootExtensionDirect {
@@ -135,6 +139,16 @@
 - (void)someValidSubMethod {
   [super otherRootDirect]; // expected-error {{messaging super with a direct method}}
 }
++ (void)someValidSubMethod {
+  [super otherOtherClassRootDirect]; // expected-error {{messaging super with a direct method}}
+}
+@end
+
+@implementation SubDirectMembers
+@dynamic foo; // expected-error {{direct property cannot be @dynamic}}
+- (instancetype)init {
+  return self;
+}
 @end
 
 extern void callMethod(id obj, Class cls);
Index: clang/test/SemaObjC/dynamic-direct-properties.m
===
--- /dev/null
+++ clang/test/SemaObjC/dynamic-direct-properties.m
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wselector-type-mismatch %s
+
+__attribute__((objc_root_class))
+@interface Foo
+@property() int dynamic_property;
+@property(direct) int direct_property; // expected-note {{previous declaration is here}}
+@end
+
+@implementation Foo
+@dynamic dynamic_property;
+@dynamic direct_property; // expected-error {{direct property cannot be @dynamic}}
+@end
+
+@interface Foo (Bar)
+@property() int dynamic_category_property;
+@property(direct) int direct_category_property; // expected-note {{previous declaration is here}}
+@end
+
+@implementation Foo (Bar)
+@dynamic dynamic_category_property;
+@dynamic direct_category_property; // expected-error {{direct property cannot be @dynamic}}
+@end
Index: clang/test/SemaObjC/category-direct-properties.m
===
--- /dev/null
+++ clang/test/SemaObjC/category-direct-properties.m
@@ -0,0 +1,273 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wselector-type-mismatch %s
+
+__attribute__((objc_root_class))
+@interface Inteface_Implementation
+@property(nonatomic, readonly) int normal_normal;
+@property(nonatomic, readonly, direct) int direct_normal;
+@property(nonatomic, readonly) int normal_direct; // expected-note {{previous declaration is here}}
+@property(nonatomic, readonly, direct) int direct_direct;
+@end
+
+@implementation Inteface_Implementation
+- (int)normal_normal {
+  return 42;
+}
+- (int)direct_normal {
+  return 42;
+}
+- (int)normal_direct __attribute__((objc_direct)) { // expected-error {{direct method implementation was previously declared not direct}}
+  return 42;
+}
+- (int)direct_direct __attribute__((objc_direct)) {
+  return 42;
+}
+@end
+
+__attribute__((objc_root_class))
+@interface 

[clang] 3adcc78 - [objc_direct] Small updates to help with adoption.

2020-02-16 Thread Pierre Habouzit via cfe-commits

Author: Pierre Habouzit
Date: 2020-02-16T16:32:41-08:00
New Revision: 3adcc78a8071e7d6dd9aa34979f9842a72a2d91b

URL: 
https://github.com/llvm/llvm-project/commit/3adcc78a8071e7d6dd9aa34979f9842a72a2d91b
DIFF: 
https://github.com/llvm/llvm-project/commit/3adcc78a8071e7d6dd9aa34979f9842a72a2d91b.diff

LOG: [objc_direct] Small updates to help with adoption.

Add fixits for messaging self in MRR or using super, as the intent is
clear, and it turns out people do that a lot more than expected.

Allow for objc_direct_members on main interfaces, it's extremely useful
for internal only classes, and proves to be quite annoying for adoption.

Add some better warnings around properties direct/non-direct clashes (it
was done for methods but properties were a miss).

Add some errors when direct properties are marked @dynamic.

Radar-Id: rdar://problem/58355212
Signed-off-by: Pierre Habouzit 
Differential Revision: https://reviews.llvm.org/D73755

Added: 
clang/test/FixIt/fixit-objc-direct.m
clang/test/SemaObjC/category-direct-properties.m
clang/test/SemaObjC/dynamic-direct-properties.m

Modified: 
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/AttrDocs.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaDeclObjC.cpp
clang/lib/Sema/SemaExprObjC.cpp
clang/lib/Sema/SemaObjCProperty.cpp
clang/test/Misc/pragma-attribute-supported-attributes-list.test
clang/test/SemaObjC/method-direct.m

Removed: 




diff  --git a/clang/include/clang/Basic/Attr.td 
b/clang/include/clang/Basic/Attr.td
index e1a61d0c49c1..a5b053209866 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -1926,7 +1926,7 @@ def ObjCDirect : Attr {
 
 def ObjCDirectMembers : Attr {
   let Spellings = [Clang<"objc_direct_members">];
-  let Subjects = SubjectList<[ObjCImpl, ObjCCategory], ErrorDiag>;
+  let Subjects = SubjectList<[ObjCImpl, ObjCInterface, ObjCCategory], 
ErrorDiag>;
   let LangOpts = [ObjC];
   let Documentation = [ObjCDirectMembersDocs];
 }

diff  --git a/clang/include/clang/Basic/AttrDocs.td 
b/clang/include/clang/Basic/AttrDocs.td
index 114dea0ba359..cc9d3c80c0da 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -4118,7 +4118,7 @@ documentation for more information.
 def ObjCDirectMembersDocs : Documentation {
   let Category = DocCatDecl;
   let Content = [{
-The ``objc_direct_members`` attribute can be placed on an  Objective-C
+The ``objc_direct_members`` attribute can be placed on an Objective-C
 ``@interface`` or ``@implementation`` to mark that methods declared
 therein should be considered direct by default.  See the documentation
 for ``objc_direct`` for more information about direct methods.
@@ -4127,9 +4127,7 @@ When ``objc_direct_members`` is placed on an 
``@interface`` block, every
 method in the block is considered to be declared as direct.  This includes any
 implicit method declarations introduced by property declarations.  If the 
method
 redeclares a non-direct method, the declaration is ill-formed, exactly as if 
the
-method was annotated with the ``objc_direct`` attribute.  
``objc_direct_members``
-cannot be placed on the primary interface of a class, only on category or class
-extension interfaces.
+method was annotated with the ``objc_direct`` attribute.
 
 When ``objc_direct_members`` is placed on an ``@implementation`` block,
 methods defined in the block are considered to be declared as direct unless

diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 111c8d8b6ed5..2d8e87f9dfb7 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -1019,8 +1019,8 @@ def err_objc_direct_on_protocol : Error<
   "'objc_direct' attribute cannot be applied to %select{methods|properties}0 "
   "declared in an Objective-C protocol">;
 def err_objc_direct_duplicate_decl : Error<
-  "%select{|direct }0method declaration conflicts "
-  "with previous %select{|direct }1declaration of method %2">;
+  "%select{|direct }0%select{method|property}1 declaration conflicts "
+  "with previous %select{|direct }2declaration of %select{method|property}1 
%3">;
 def err_objc_direct_impl_decl_mismatch : Error<
   "direct method was declared in %select{the primary interface|an extension|a 
category}0 "
   "but is implemented in %select{the primary interface|a category|a 
diff erent category}1">;
@@ -1036,6 +1036,8 @@ def warn_objc_direct_ignored : Warning<
 def warn_objc_direct_property_ignored : Warning<
   "direct attribute on property %0 ignored (not implemented by this 
Objective-C runtime)">,
   InGroup;
+def err_objc_direct_dynamic_property : Error<
+  "direct property cannot be @dynamic">;
 
 def warn_conflicting_overriding_ret_types : Warning<
   "conflicting return type in "


[PATCH] D72841: [RFC] Add support for pragma float_control, to control precision and exception behavior at the source level

2020-02-16 Thread Melanie Blower via Phabricator via cfe-commits
mibintc updated this revision to Diff 244894.
mibintc added a comment.

I found the problem in the #pragma float_control (push/pop) stack, it was just 
a dumb bug.

I also added -include-pch test cases, and added code to ASTWriter ASTReader to 
preserve the floatcontrol pragma stack

For 2 of the new floatcontrol test cases, I temporarily added XFAIL because the 
patch to default -ffp-precise=fast and ffp-contract=on has been withdrawn (will 
re-commit soon).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72841/new/

https://reviews.llvm.org/D72841

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/AST/Stmt.h
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Basic/PragmaKinds.h
  clang/include/clang/Basic/TokenKinds.def
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/include/clang/Serialization/ASTReader.h
  clang/include/clang/Serialization/ASTWriter.h
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParsePragma.cpp
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Parse/Parser.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaAttr.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/CodeGen/constrained-math-builtins.c
  clang/test/CodeGen/fast-math.c
  clang/test/CodeGen/fp-contract-on-pragma.cpp
  clang/test/CodeGen/fp-contract-pragma.cpp
  clang/test/CodeGen/fp-floatcontrol-class.cpp
  clang/test/CodeGen/fp-floatcontrol-pragma.cpp
  clang/test/CodeGen/fp-floatcontrol-stack.cpp
  clang/test/CodeGen/fpconstrained.c
  clang/test/CodeGen/fpconstrained.cpp
  clang/test/CodeGenOpenCL/builtins-amdgcn-dl-insts.cl
  clang/test/CodeGenOpenCL/builtins-amdgcn-gfx9.cl
  clang/test/CodeGenOpenCL/builtins-amdgcn-interp.cl
  clang/test/CodeGenOpenCL/builtins-amdgcn-mfma.cl
  clang/test/CodeGenOpenCL/builtins-amdgcn-vi.cl
  clang/test/CodeGenOpenCL/builtins-amdgcn.cl
  clang/test/CodeGenOpenCL/builtins-f16.cl
  clang/test/CodeGenOpenCL/builtins-r600.cl
  clang/test/CodeGenOpenCL/relaxed-fpmath.cl
  clang/test/CodeGenOpenCL/single-precision-constant.cl
  clang/test/PCH/pragma-floatcontrol.c
  clang/test/Parser/fp-floatcontrol-syntax.cpp
  llvm/include/llvm/IR/IRBuilder.h

Index: llvm/include/llvm/IR/IRBuilder.h
===
--- llvm/include/llvm/IR/IRBuilder.h
+++ llvm/include/llvm/IR/IRBuilder.h
@@ -215,6 +215,8 @@
   /// Get the flags to be applied to created floating point ops
   FastMathFlags getFastMathFlags() const { return FMF; }
 
+  FastMathFlags& getFastMathFlags() { return FMF; }
+
   /// Clear the fast-math flags.
   void clearFastMathFlags() { FMF.clear(); }
 
@@ -299,10 +301,16 @@
 IRBuilderBase 
 FastMathFlags FMF;
 MDNode *FPMathTag;
+bool IsFPConstrained;
+fp::ExceptionBehavior DefaultConstrainedExcept;
+fp::RoundingMode DefaultConstrainedRounding;
 
   public:
 FastMathFlagGuard(IRBuilderBase )
-: Builder(B), FMF(B.FMF), FPMathTag(B.DefaultFPMathTag) {}
+: Builder(B), FMF(B.FMF), FPMathTag(B.DefaultFPMathTag),
+  IsFPConstrained(B.IsFPConstrained),
+  DefaultConstrainedExcept(B.DefaultConstrainedExcept),
+  DefaultConstrainedRounding(B.DefaultConstrainedRounding) {}
 
 FastMathFlagGuard(const FastMathFlagGuard &) = delete;
 FastMathFlagGuard =(const FastMathFlagGuard &) = delete;
@@ -310,6 +318,9 @@
 ~FastMathFlagGuard() {
   Builder.FMF = FMF;
   Builder.DefaultFPMathTag = FPMathTag;
+  Builder.IsFPConstrained = IsFPConstrained;
+  Builder.DefaultConstrainedExcept = DefaultConstrainedExcept;
+  Builder.DefaultConstrainedRounding = DefaultConstrainedRounding;
 }
   };
 
Index: clang/test/Parser/fp-floatcontrol-syntax.cpp
===
--- /dev/null
+++ clang/test/Parser/fp-floatcontrol-syntax.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -DCHECK_ERROR %s
+
+float function_scope(float a) {
+#  pragma float_control(precise, on) junk // expected-warning {{extra tokens at end of '#pragma float_control' - ignored}}
+  return a;
+}
+
+#ifdef CHECK_ERROR
+#  pragma float_control(push)
+#  pragma float_control(pop)
+#  pragma float_control(precise,on,push)
+void check_stack() {
+#pragma float_control(push) // expected-error {{can only appear at file scope}}
+#pragma float_control(pop) // expected-error {{can only appear at file scope}}
+#pragma float_control(precise,on,push) // expected-error {{can only appear at file scope}}

[PATCH] D74463: [clang-tidy] Add new check avoid-adjacent-unrelated-parameters-of-the-same-type

2020-02-16 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment.

In D74463#1878290 , @vingeldal wrote:

> I am pretty sure that an option to allow short names would cause  a 
> relatively big hit on performance (relative to how it runs without the 
> option) for this check while also potentially causing some false negatives 
> (which I would very much like to avoid).


Highly unlikely, the additional name length check would only be ran on 
functions where the consecutive param types occur, then the check itself is 
very fast, you don't even need to compute the length as its stored in the 
identifier info, finally on cases where the name check prevents a diagnostic 
that is a huge performance win. The false negatives could kind of be an issue, 
but we could leave that up to the developer who is running the check.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74463/new/

https://reviews.llvm.org/D74463



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D69520: [libc++] Disallow dynamic -> static span conversions

2020-02-16 Thread Jan Wilken Dörrie via Phabricator via cfe-commits
jdoerrie added a comment.

Oh interesting! Yes, it looks like you are right.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69520/new/

https://reviews.llvm.org/D69520



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D69520: [libc++] Disallow dynamic -> static span conversions

2020-02-16 Thread Michael Schellenberger Costa via Phabricator via cfe-commits
miscco added a comment.

I believe this is superseded by the implementation of P1976R2 in D74577 



CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69520/new/

https://reviews.llvm.org/D69520



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] af20211 - [Sema] Fix pointer-to-int-cast for MSVC build bot

2020-02-16 Thread Mark de Wever via cfe-commits

Author: Mark de Wever
Date: 2020-02-16T19:09:59+01:00
New Revision: af202119443bf8759fb6ca9fd8d0282f27001cec

URL: 
https://github.com/llvm/llvm-project/commit/af202119443bf8759fb6ca9fd8d0282f27001cec
DIFF: 
https://github.com/llvm/llvm-project/commit/af202119443bf8759fb6ca9fd8d0282f27001cec.diff

LOG: [Sema] Fix pointer-to-int-cast for MSVC build bot

Revision 9658d895c81a breaks the clang-x64-windows-msvc build bot [1].
This should fix the unit test using the same method as used in 9658d895c81a.

Note I don't have access to a Windows system so the patch is based on the
errors generated by the bot.

[1] http://lab.llvm.org:8011/builders/clang-x64-windows-msvc/builds/14358

Differential Revision: https://reviews.llvm.org/D74694

Added: 


Modified: 
clang/test/Analysis/taint-generic.c
clang/test/Sema/conditional-expr.c
clang/test/Sema/darwin-align-cast.c

Removed: 




diff  --git a/clang/test/Analysis/taint-generic.c 
b/clang/test/Analysis/taint-generic.c
index 60a27c5ce464..a299501b1068 100644
--- a/clang/test/Analysis/taint-generic.c
+++ b/clang/test/Analysis/taint-generic.c
@@ -1,11 +1,11 @@
-// RUN: %clang_analyze_cc1 -Wno-format-security -verify %s \
+// RUN: %clang_analyze_cc1 -Wno-format-security -Wno-pointer-to-int-cast 
-verify %s \
 // RUN:   -analyzer-checker=alpha.security.taint \
 // RUN:   -analyzer-checker=core \
 // RUN:   -analyzer-checker=alpha.security.ArrayBoundV2 \
 // RUN:   -analyzer-config \
 // RUN: 
alpha.security.taint.TaintPropagation:Config=%S/Inputs/taint-generic-config.yaml
 
-// RUN: %clang_analyze_cc1 -Wno-format-security -verify %s \
+// RUN: %clang_analyze_cc1 -Wno-format-security -Wno-pointer-to-int-cast 
-verify %s \
 // RUN:   -DFILE_IS_STRUCT \
 // RUN:   -analyzer-checker=alpha.security.taint \
 // RUN:   -analyzer-checker=core \
@@ -13,7 +13,7 @@
 // RUN:   -analyzer-config \
 // RUN: 
alpha.security.taint.TaintPropagation:Config=%S/Inputs/taint-generic-config.yaml
 
-// RUN: not %clang_analyze_cc1 -verify %s \
+// RUN: not %clang_analyze_cc1 -Wno-pointer-to-int-cast -verify %s \
 // RUN:   -analyzer-checker=alpha.security.taint \
 // RUN:   -analyzer-config \
 // RUN: alpha.security.taint.TaintPropagation:Config=justguessit \

diff  --git a/clang/test/Sema/conditional-expr.c 
b/clang/test/Sema/conditional-expr.c
index 6b24ef49b3e6..7ff141d62e56 100644
--- a/clang/test/Sema/conditional-expr.c
+++ b/clang/test/Sema/conditional-expr.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -pedantic -Wsign-conversion %s
+// RUN: %clang_cc1 -fsyntax-only -Wno-pointer-to-int-cast -verify -pedantic 
-Wsign-conversion %s
 void foo() {
   *(0 ? (double *)0 : (void *)0) = 0;
   // FIXME: GCC doesn't consider the following two statements to be errors.

diff  --git a/clang/test/Sema/darwin-align-cast.c 
b/clang/test/Sema/darwin-align-cast.c
index bd357edceddf..3a1824901277 100644
--- a/clang/test/Sema/darwin-align-cast.c
+++ b/clang/test/Sema/darwin-align-cast.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wno-pointer-to-int-cast -verify %s
 // expected-no-diagnostics
 typedef long unsigned int __darwin_size_t;
 typedef long __darwin_ssize_t;



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D74694: [Sema] Fix pointer-to-int-cast for MSVC build bot

2020-02-16 Thread Mark de Wever via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGaf202119443b: [Sema] Fix pointer-to-int-cast for MSVC build 
bot (authored by Mordante).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74694/new/

https://reviews.llvm.org/D74694

Files:
  clang/test/Analysis/taint-generic.c
  clang/test/Sema/conditional-expr.c
  clang/test/Sema/darwin-align-cast.c


Index: clang/test/Sema/darwin-align-cast.c
===
--- clang/test/Sema/darwin-align-cast.c
+++ clang/test/Sema/darwin-align-cast.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wno-pointer-to-int-cast -verify %s
 // expected-no-diagnostics
 typedef long unsigned int __darwin_size_t;
 typedef long __darwin_ssize_t;
Index: clang/test/Sema/conditional-expr.c
===
--- clang/test/Sema/conditional-expr.c
+++ clang/test/Sema/conditional-expr.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -pedantic -Wsign-conversion %s
+// RUN: %clang_cc1 -fsyntax-only -Wno-pointer-to-int-cast -verify -pedantic 
-Wsign-conversion %s
 void foo() {
   *(0 ? (double *)0 : (void *)0) = 0;
   // FIXME: GCC doesn't consider the following two statements to be errors.
Index: clang/test/Analysis/taint-generic.c
===
--- clang/test/Analysis/taint-generic.c
+++ clang/test/Analysis/taint-generic.c
@@ -1,11 +1,11 @@
-// RUN: %clang_analyze_cc1 -Wno-format-security -verify %s \
+// RUN: %clang_analyze_cc1 -Wno-format-security -Wno-pointer-to-int-cast 
-verify %s \
 // RUN:   -analyzer-checker=alpha.security.taint \
 // RUN:   -analyzer-checker=core \
 // RUN:   -analyzer-checker=alpha.security.ArrayBoundV2 \
 // RUN:   -analyzer-config \
 // RUN: 
alpha.security.taint.TaintPropagation:Config=%S/Inputs/taint-generic-config.yaml
 
-// RUN: %clang_analyze_cc1 -Wno-format-security -verify %s \
+// RUN: %clang_analyze_cc1 -Wno-format-security -Wno-pointer-to-int-cast 
-verify %s \
 // RUN:   -DFILE_IS_STRUCT \
 // RUN:   -analyzer-checker=alpha.security.taint \
 // RUN:   -analyzer-checker=core \
@@ -13,7 +13,7 @@
 // RUN:   -analyzer-config \
 // RUN: 
alpha.security.taint.TaintPropagation:Config=%S/Inputs/taint-generic-config.yaml
 
-// RUN: not %clang_analyze_cc1 -verify %s \
+// RUN: not %clang_analyze_cc1 -Wno-pointer-to-int-cast -verify %s \
 // RUN:   -analyzer-checker=alpha.security.taint \
 // RUN:   -analyzer-config \
 // RUN: alpha.security.taint.TaintPropagation:Config=justguessit \


Index: clang/test/Sema/darwin-align-cast.c
===
--- clang/test/Sema/darwin-align-cast.c
+++ clang/test/Sema/darwin-align-cast.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wno-pointer-to-int-cast -verify %s
 // expected-no-diagnostics
 typedef long unsigned int __darwin_size_t;
 typedef long __darwin_ssize_t;
Index: clang/test/Sema/conditional-expr.c
===
--- clang/test/Sema/conditional-expr.c
+++ clang/test/Sema/conditional-expr.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -pedantic -Wsign-conversion %s
+// RUN: %clang_cc1 -fsyntax-only -Wno-pointer-to-int-cast -verify -pedantic -Wsign-conversion %s
 void foo() {
   *(0 ? (double *)0 : (void *)0) = 0;
   // FIXME: GCC doesn't consider the following two statements to be errors.
Index: clang/test/Analysis/taint-generic.c
===
--- clang/test/Analysis/taint-generic.c
+++ clang/test/Analysis/taint-generic.c
@@ -1,11 +1,11 @@
-// RUN: %clang_analyze_cc1 -Wno-format-security -verify %s \
+// RUN: %clang_analyze_cc1 -Wno-format-security -Wno-pointer-to-int-cast -verify %s \
 // RUN:   -analyzer-checker=alpha.security.taint \
 // RUN:   -analyzer-checker=core \
 // RUN:   -analyzer-checker=alpha.security.ArrayBoundV2 \
 // RUN:   -analyzer-config \
 // RUN: alpha.security.taint.TaintPropagation:Config=%S/Inputs/taint-generic-config.yaml
 
-// RUN: %clang_analyze_cc1 -Wno-format-security -verify %s \
+// RUN: %clang_analyze_cc1 -Wno-format-security -Wno-pointer-to-int-cast -verify %s \
 // RUN:   -DFILE_IS_STRUCT \
 // RUN:   -analyzer-checker=alpha.security.taint \
 // RUN:   -analyzer-checker=core \
@@ -13,7 +13,7 @@
 // RUN:   -analyzer-config \
 // RUN: alpha.security.taint.TaintPropagation:Config=%S/Inputs/taint-generic-config.yaml
 
-// RUN: not %clang_analyze_cc1 -verify %s \
+// RUN: not %clang_analyze_cc1 -Wno-pointer-to-int-cast -verify %s \
 // RUN:   -analyzer-checker=alpha.security.taint \
 // RUN:   -analyzer-config \
 // RUN: alpha.security.taint.TaintPropagation:Config=justguessit \
___
cfe-commits 

[PATCH] D74694: [Sema] Fix pointer-to-int-cast for MSVC build bot

2020-02-16 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 accepted this revision.
xbolva00 added a comment.
This revision is now accepted and ready to land.

Ok, please commit to fix the bot.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74694/new/

https://reviews.llvm.org/D74694



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D74694: [Sema] Fix pointer-to-int-cast for MSVC build bot

2020-02-16 Thread Mark de Wever via Phabricator via cfe-commits
Mordante added a comment.

Thanks for the quick review!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74694/new/

https://reviews.llvm.org/D74694



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D69520: [libc++] Disallow dynamic -> static span conversions

2020-02-16 Thread Jan Wilken Dörrie via Phabricator via cfe-commits
jdoerrie added a comment.

Friendly Ping


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69520/new/

https://reviews.llvm.org/D69520



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D73835: [IRBuilder] Virtualize IRBuilder

2020-02-16 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment.

@nikic: Can you please do compile-time benchmarks w/o this patch with e.g. php ?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73835/new/

https://reviews.llvm.org/D73835



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D72231: [Sema] Adds the pointer-to-int-cast diagnostic

2020-02-16 Thread Mark de Wever via Phabricator via cfe-commits
Mordante added a comment.

This revision caused issue for the MSVC build bot. I created a followup patch 
D74694 .


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72231/new/

https://reviews.llvm.org/D72231



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D74694: [Sema] Fix pointer-to-int-cast for MSVC build bot

2020-02-16 Thread Mark de Wever via Phabricator via cfe-commits
Mordante created this revision.
Mordante added reviewers: rsmith, dblaikie, rjmccall.
Mordante added a project: clang.
Herald added a subscriber: martong.

Revision 9658d895c81a 
 breaks 
the clang-x64-windows-msvc build bot [1].
This should fix the unit test using the same method as used in 9658d895c81a 
.

Note I don't have access to a Windows system so the patch is based on the 
errors generated by the bot.

[1] http://lab.llvm.org:8011/builders/clang-x64-windows-msvc/builds/14358


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D74694

Files:
  clang/test/Analysis/taint-generic.c
  clang/test/Sema/conditional-expr.c
  clang/test/Sema/darwin-align-cast.c


Index: clang/test/Sema/darwin-align-cast.c
===
--- clang/test/Sema/darwin-align-cast.c
+++ clang/test/Sema/darwin-align-cast.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wno-pointer-to-int-cast -verify %s
 // expected-no-diagnostics
 typedef long unsigned int __darwin_size_t;
 typedef long __darwin_ssize_t;
Index: clang/test/Sema/conditional-expr.c
===
--- clang/test/Sema/conditional-expr.c
+++ clang/test/Sema/conditional-expr.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -pedantic -Wsign-conversion %s
+// RUN: %clang_cc1 -fsyntax-only -Wno-pointer-to-int-cast -verify -pedantic 
-Wsign-conversion %s
 void foo() {
   *(0 ? (double *)0 : (void *)0) = 0;
   // FIXME: GCC doesn't consider the following two statements to be errors.
Index: clang/test/Analysis/taint-generic.c
===
--- clang/test/Analysis/taint-generic.c
+++ clang/test/Analysis/taint-generic.c
@@ -1,11 +1,11 @@
-// RUN: %clang_analyze_cc1 -Wno-format-security -verify %s \
+// RUN: %clang_analyze_cc1 -Wno-format-security -Wno-pointer-to-int-cast 
-verify %s \
 // RUN:   -analyzer-checker=alpha.security.taint \
 // RUN:   -analyzer-checker=core \
 // RUN:   -analyzer-checker=alpha.security.ArrayBoundV2 \
 // RUN:   -analyzer-config \
 // RUN: 
alpha.security.taint.TaintPropagation:Config=%S/Inputs/taint-generic-config.yaml
 
-// RUN: %clang_analyze_cc1 -Wno-format-security -verify %s \
+// RUN: %clang_analyze_cc1 -Wno-format-security -Wno-pointer-to-int-cast 
-verify %s \
 // RUN:   -DFILE_IS_STRUCT \
 // RUN:   -analyzer-checker=alpha.security.taint \
 // RUN:   -analyzer-checker=core \
@@ -13,7 +13,7 @@
 // RUN:   -analyzer-config \
 // RUN: 
alpha.security.taint.TaintPropagation:Config=%S/Inputs/taint-generic-config.yaml
 
-// RUN: not %clang_analyze_cc1 -verify %s \
+// RUN: not %clang_analyze_cc1 -Wno-pointer-to-int-cast -verify %s \
 // RUN:   -analyzer-checker=alpha.security.taint \
 // RUN:   -analyzer-config \
 // RUN: alpha.security.taint.TaintPropagation:Config=justguessit \


Index: clang/test/Sema/darwin-align-cast.c
===
--- clang/test/Sema/darwin-align-cast.c
+++ clang/test/Sema/darwin-align-cast.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wno-pointer-to-int-cast -verify %s
 // expected-no-diagnostics
 typedef long unsigned int __darwin_size_t;
 typedef long __darwin_ssize_t;
Index: clang/test/Sema/conditional-expr.c
===
--- clang/test/Sema/conditional-expr.c
+++ clang/test/Sema/conditional-expr.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -pedantic -Wsign-conversion %s
+// RUN: %clang_cc1 -fsyntax-only -Wno-pointer-to-int-cast -verify -pedantic -Wsign-conversion %s
 void foo() {
   *(0 ? (double *)0 : (void *)0) = 0;
   // FIXME: GCC doesn't consider the following two statements to be errors.
Index: clang/test/Analysis/taint-generic.c
===
--- clang/test/Analysis/taint-generic.c
+++ clang/test/Analysis/taint-generic.c
@@ -1,11 +1,11 @@
-// RUN: %clang_analyze_cc1 -Wno-format-security -verify %s \
+// RUN: %clang_analyze_cc1 -Wno-format-security -Wno-pointer-to-int-cast -verify %s \
 // RUN:   -analyzer-checker=alpha.security.taint \
 // RUN:   -analyzer-checker=core \
 // RUN:   -analyzer-checker=alpha.security.ArrayBoundV2 \
 // RUN:   -analyzer-config \
 // RUN: alpha.security.taint.TaintPropagation:Config=%S/Inputs/taint-generic-config.yaml
 
-// RUN: %clang_analyze_cc1 -Wno-format-security -verify %s \
+// RUN: %clang_analyze_cc1 -Wno-format-security -Wno-pointer-to-int-cast -verify %s \
 // RUN:   -DFILE_IS_STRUCT \
 // RUN:   -analyzer-checker=alpha.security.taint \
 // RUN:   -analyzer-checker=core \
@@ -13,7 +13,7 @@
 // RUN:   -analyzer-config \
 // RUN: 

[PATCH] D74463: [clang-tidy] Add new check avoid-adjacent-unrelated-parameters-of-the-same-type

2020-02-16 Thread Kim Viggedal via Phabricator via cfe-commits
vingeldal marked 2 inline comments as done.
vingeldal added a comment.

In D74463#1878144 , @njames93 wrote:

> I have a feeling this check is going to throw so many false positives that 
> it'll be too noisy to run on any real codebase. 
>  There should be a way to silence this when it is undesired like in the 
> example for `int max(int a, int b);`. 
>  A possible solution could be based on parameter name length, usually 
> interchangeable parameters have short names. 
>  Maybe an option could be added `IgnoreShortParamNamesSize` which takes an 
> int and disregards results if both params are shorter, set as `0` to disable 
> the option


I can't dispute that there will be false positives with this check, but I'm not 
sure there will be a problematic amount in many code bases.

I am pretty sure that an option to allow short names would cause  a relatively 
big hit on performance (relative to how it runs without the option) for this 
check while also potentially causing some false negatives (which I would very 
much like to avoid).
Since actually running the check is still optional I think it might be better 
to just not run this check on a code base where it gives a lot of false 
positives or suppress it in select header files where the false positives are 
plenty.
I'm feeling a bit reluctant to adding the suggested option, are you sure such 
an option would be worth the cost?
Also consider that even in the cases where the order of the parameters doesn't 
matter, like an averaging function, there is also the possibility to re-design 
to avoid this warning, by passing the parameters as some kind of collection of 
parameters.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74463/new/

https://reviews.llvm.org/D74463



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D71966: [Wdocumentation][RFC] Improve identifier's of \param

2020-02-16 Thread Mark de Wever via Phabricator via cfe-commits
Mordante planned changes to this revision.
Mordante marked an inline comment as done.
Mordante added inline comments.



Comment at: clang/lib/AST/CommentSema.cpp:1121
+  if (isFunctionOrMethodVariadic() &&
+  Typo.find_first_not_of('.') == StringRef::npos)
+return ParamCommandComment::VarArgParamIndex;

Mordante wrote:
> gribozavr2 wrote:
> > I'm not sure I like the heuristic.
> > 
> > ```
> > // \param foo.bar Something.
> > void foo(int foo_bar, ...);
> > ```
> > 
> > Here we would suggest to correct `foo.bar` to `...`.
> > 
> > Instead, I'd suggest to throw a "..." identifier into the typo corrector if 
> > the function is variadic, and let the edit distance decide.
> This heuristic works at the moment. The `lexIdentifier` will not accept 
> `foo.bar`. However since you requested to remove the `lexIdentifier` your 
> approach makes more sense.
Any suggestion how to do this? The `SimpleTypoCorrector` expects a `NamedDecl`, 
but AFAIK `...` isn't a `NamedDecl`.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71966/new/

https://reviews.llvm.org/D71966



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] 7c362b2 - [IRBuilder] Fix unnecessary IRBuilder copies; NFC

2020-02-16 Thread Nikita Popov via cfe-commits

Author: Nikita Popov
Date: 2020-02-16T17:57:18+01:00
New Revision: 7c362b25d7a9093d7d38171f2684876b63bb5a57

URL: 
https://github.com/llvm/llvm-project/commit/7c362b25d7a9093d7d38171f2684876b63bb5a57
DIFF: 
https://github.com/llvm/llvm-project/commit/7c362b25d7a9093d7d38171f2684876b63bb5a57.diff

LOG: [IRBuilder] Fix unnecessary IRBuilder copies; NFC

Fix a few cases where an IRBuilder is passed to a helper function
by value, while a by reference pass was intended.

Added: 


Modified: 
clang/lib/CodeGen/CGExprScalar.cpp
llvm/lib/Target/X86/X86InterleavedAccess.cpp
llvm/lib/Transforms/Instrumentation/BoundsChecking.cpp
llvm/lib/Transforms/Scalar/LowerMatrixIntrinsics.cpp
llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGExprScalar.cpp 
b/clang/lib/CodeGen/CGExprScalar.cpp
index 706aa43a5071..8d9bf17969a3 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -4683,7 +4683,7 @@ struct GEPOffsetAndOverflow {
 static GEPOffsetAndOverflow EmitGEPOffsetInBytes(Value *BasePtr, Value *GEPVal,
  llvm::LLVMContext ,
  CodeGenModule ,
- CGBuilderTy Builder) {
+ CGBuilderTy ) {
   const auto  = CGM.getDataLayout();
 
   // The total (signed) byte offset for the GEP.

diff  --git a/llvm/lib/Target/X86/X86InterleavedAccess.cpp 
b/llvm/lib/Target/X86/X86InterleavedAccess.cpp
index 36ee9d4ad382..f0288adf52ce 100644
--- a/llvm/lib/Target/X86/X86InterleavedAccess.cpp
+++ b/llvm/lib/Target/X86/X86InterleavedAccess.cpp
@@ -284,7 +284,7 @@ static void genShuffleBland(MVT VT, ArrayRef Mask,
 static void reorderSubVector(MVT VT, SmallVectorImpl 
,
   ArrayRef Vec, ArrayRef VPShuf,
   unsigned VecElems, unsigned Stride,
-  IRBuilder<> Builder) {
+  IRBuilder<> ) {
 
   if (VecElems == 16) {
 for (unsigned i = 0; i < Stride; i++)
@@ -519,7 +519,7 @@ static void DecodePALIGNRMask(MVT VT, unsigned Imm,
 // Invec[2] - |8|9|10|11|  Vec[2] - |2|5|8|11|
 
 static void concatSubVector(Value **Vec, ArrayRef InVec,
-unsigned VecElems, IRBuilder<> Builder) {
+unsigned VecElems, IRBuilder<> ) {
   if (VecElems == 16) {
 for (int i = 0; i < 3; i++)
   Vec[i] = InVec[i];

diff  --git a/llvm/lib/Transforms/Instrumentation/BoundsChecking.cpp 
b/llvm/lib/Transforms/Instrumentation/BoundsChecking.cpp
index 9abb62ac788c..3e7ebe54a00c 100644
--- a/llvm/lib/Transforms/Instrumentation/BoundsChecking.cpp
+++ b/llvm/lib/Transforms/Instrumentation/BoundsChecking.cpp
@@ -112,7 +112,7 @@ static Value *getBoundsCheckCond(Value *Ptr, Value *InstVal,
 ///
 /// \p GetTrapBB is a callable that returns the trap BB to use on failure.
 template 
-static void insertBoundsCheck(Value *Or, BuilderTy IRB, GetTrapBBT GetTrapBB) {
+static void insertBoundsCheck(Value *Or, BuilderTy , GetTrapBBT GetTrapBB) 
{
   // check if the comparison is always false
   ConstantInt *C = dyn_cast_or_null(Or);
   if (C) {

diff  --git a/llvm/lib/Transforms/Scalar/LowerMatrixIntrinsics.cpp 
b/llvm/lib/Transforms/Scalar/LowerMatrixIntrinsics.cpp
index 4047de13d0e2..5cfcb45e4bca 100644
--- a/llvm/lib/Transforms/Scalar/LowerMatrixIntrinsics.cpp
+++ b/llvm/lib/Transforms/Scalar/LowerMatrixIntrinsics.cpp
@@ -290,7 +290,7 @@ class LowerMatrixIntrinsics {
   /// Otherwie split the flat vector \p MatrixVal containing a matrix with
   /// shape \p SI into column vectors.
   ColumnMatrixTy getMatrix(Value *MatrixVal, const ShapeInfo ,
-   IRBuilder<> Builder) {
+   IRBuilder<> ) {
 VectorType *VType = dyn_cast(MatrixVal->getType());
 assert(VType && "MatrixVal must be a vector type");
 assert(VType->getNumElements() == SI.NumRows * SI.NumColumns &&
@@ -584,13 +584,13 @@ class LowerMatrixIntrinsics {
   }
 
   LoadInst *createColumnLoad(Value *ColumnPtr, Type *EltType,
- IRBuilder<> Builder) {
+ IRBuilder<> ) {
 return Builder.CreateAlignedLoad(
 ColumnPtr, Align(DL.getABITypeAlignment(EltType)), "col.load");
   }
 
   StoreInst *createColumnStore(Value *ColumnValue, Value *ColumnPtr,
-   Type *EltType, IRBuilder<> Builder) {
+   Type *EltType, IRBuilder<> ) {
 return Builder.CreateAlignedStore(ColumnValue, ColumnPtr,
   DL.getABITypeAlign(EltType));
   }
@@ -690,7 +690,7 @@ class LowerMatrixIntrinsics {
   /// Extract a column vector of \p NumElts starting at index (\p I, \p J) from
   /// the matrix \p LM represented as a vector of column vectors.
   Value *extractVector(const ColumnMatrixTy , unsigned I, unsigned J,
- 

[PATCH] D73835: [IRBuilder] Virtualize IRBuilder

2020-02-16 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a comment.

Tried what happens if the copy constructor is deleted: 
https://gist.github.com/nikic/1e6ab5bbf42cfe48e7b848e60a2df180 It looks like 
the referenced occurrence in DIBuilder was the only place that was using the 
copy constructor "for good reason", most of the other uses either copy the 
whole IRBuilder where they just want to pass a reference or copy the whole 
IRBuilder where they really just want an InsertPointGuard.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73835/new/

https://reviews.llvm.org/D73835



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D74692: [clang-tidy] Make bugprone-use-after-move ignore std::move for const values

2020-02-16 Thread Arthur O'Dwyer via cfe-commits
I see your point about how users who care should always be passing this
check alongside "performance-move-const-arg"; but IMVHO it still makes
sense for clang-tidy to warn unconditionally about code of the form

x = std::move(y);
use(y);

regardless of the incidental type of `y`. Sure, it's *technically* not
wrong if `y` is const... or if `y` is a primitive type such as `Widget*`...
or if `y` is a well-defined library type such as
`std::shared_ptr`... or if `y` is a library type that works in
practice, such as `std::list`
...
but regardless of its *technical* merits, the code is still *logically
semantically* wrong, and I think that's what the clang-tidy check should be
trying to diagnose.

my $.02,
Arthur

On Sun, Feb 16, 2020 at 10:44 AM Zinovy Nis via Phabricator via cfe-commits
 wrote:

> zinovy.nis updated this revision to Diff 244878.
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D74692/new/
>
> https://reviews.llvm.org/D74692
>
> Files:
>   clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
>   clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
>
>
> Index:
> clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
> ===
> --- clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
> +++ clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
> @@ -129,6 +129,14 @@
>// CHECK-NOTES: [[@LINE-3]]:15: note: move occurred here
>  }
>
> +// Simple case for const value: no actual move and no warning.
> +void simpleConst(const A a) {
> +  A other_a = std::move(a);
> +  a.foo();
> +  // CHECK-NOTES-NOT: [[@LINE-1]]:3: warning: 'a' used after it was moved
> +  // CHECK-NOTES-NOT: [[@LINE-3]]:15: note: move occurred here
> +}
> +
>  // A warning should only be emitted for one use-after-move.
>  void onlyFlagOneUseAfterMove() {
>A a;
> @@ -308,14 +316,15 @@
>  }
>
>  void lambdas() {
> -  // Use-after-moves inside a lambda should be detected.
> +  // Use-after-moves inside a lambda will not be detected as [a]
> +  // is passed as a const by default.
>{
>  A a;
>  auto lambda = [a] {
>std::move(a);
>a.foo();
> -  // CHECK-NOTES: [[@LINE-1]]:7: warning: 'a' used after it was moved
> -  // CHECK-NOTES: [[@LINE-3]]:7: note: move occurred here
> +  // CHECK-NOTES-NOT: [[@LINE-1]]:7: warning: 'a' used after it was
> moved
> +  // CHECK-NOTES-NOT: [[@LINE-3]]:7: note: move occurred here
>  };
>}
>// This is just as true if the variable was declared inside the lambda.
> @@ -720,15 +729,15 @@
>  // const pointer argument.
>  const A a;
>  std::move(a);
> -passByConstPointer();
> -// CHECK-NOTES: [[@LINE-1]]:25: warning: 'a' used after it was moved
> -// CHECK-NOTES: [[@LINE-3]]:5: note: move occurred here
> +passByConstPointer(); // expect no warning: the const value was not
> moved.
> +// CHECK-NOTES-NOT: [[@LINE-1]]:25: warning: 'a' used after it was
> moved
> +// CHECK-NOTES-NOT: [[@LINE-3]]:5: note: move occurred here
>}
>const A a;
>std::move(a);
> -  passByConstReference(a);
> -  // CHECK-NOTES: [[@LINE-1]]:24: warning: 'a' used after it was moved
> -  // CHECK-NOTES: [[@LINE-3]]:3: note: move occurred here
> +  passByConstReference(a); // expect no warning: the const value was not
> moved.
> +  // CHECK-NOTES-NOT: [[@LINE-1]]:24: warning: 'a' used after it was moved
> +  // CHECK-NOTES-NOT: [[@LINE-3]]:3: note: move occurred here
>  }
>
>  // Clearing a standard container using clear() is treated as a
> Index: clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
> ===
> --- clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
> +++ clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
> @@ -404,7 +404,8 @@
> hasArgument(0, declRefExpr().bind("arg")),
> anyOf(hasAncestor(lambdaExpr().bind("containing-lambda")),
>   hasAncestor(functionDecl().bind("containing-func"))),
> -   unless(inDecltypeOrTemplateArg()))
> +   unless(anyOf(inDecltypeOrTemplateArg(),
> +hasType(qualType(isConstQualified())
>.bind("call-move");
>
>Finder->addMatcher(
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D74669: [clang-tidy] New check: misc-no-include-cpp

2020-02-16 Thread Jonathan Roelofs via Phabricator via cfe-commits
jroelofs marked an inline comment as done.
jroelofs added inline comments.



Comment at: clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp:59
+Optional File = PP->LookupFile(
+HashLoc /* FIXME: lies */, (FileName + RE).str(), IsAngled, 
nullptr,
+nullptr, CurDir, nullptr, nullptr, nullptr, nullptr, nullptr);

FIXME: is this a reasonable `SourceLocation` to give to `LookupFile`?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74669/new/

https://reviews.llvm.org/D74669



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D74669: [clang-tidy] New check: misc-no-include-cpp

2020-02-16 Thread Jonathan Roelofs via Phabricator via cfe-commits
jroelofs updated this revision to Diff 244884.
jroelofs added a comment.

git format-patch


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74669/new/

https://reviews.llvm.org/D74669

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.h
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/a
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/a.cpp
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/a.hpp
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/c.c
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/c.cc
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/c.cxx
  clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-include.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-include.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-include.cpp
@@ -0,0 +1,18 @@
+// RUN: %check_clang_tidy %s bugprone-suspicious-include %t -- -- -isystem %S/Inputs/Headers
+
+// CHECK-MESSAGES: [[@LINE+4]]:1: warning: suspicious #include of file with .cpp extension
+// CHECK-MESSAGES: [[@LINE+3]]:1: note: did you mean to include 'a'?
+// CHECK-MESSAGES: [[@LINE+2]]:1: note: did you mean to include 'a.h'?
+// CHECK-MESSAGES: [[@LINE+1]]:1: note: did you mean to include 'a.hpp'?
+#include "a.cpp"
+
+#include "b.h"
+
+// CHECK-MESSAGES: [[@LINE+1]]:1: warning: suspicious #include of file with .c extension
+#include 
+
+// CHECK-MESSAGES: [[@LINE+1]]:1: warning: suspicious #include of file with .cc extension
+#include 
+
+// CHECK-MESSAGES: [[@LINE+1]]:1: warning: suspicious #include of file with .cxx extension
+#include 
Index: clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.h
@@ -0,0 +1,39 @@
+//===--- SuspiciousIncludeCheck.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_SUSPICIOUSINCLUDECHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SUSPICIOUSINCLUDECHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+/// Warns on inclusion of files whose names suggest that they're implementation
+/// files, instead of headers. E.g:
+///
+/// #include "foo.cpp"  // warning
+/// #include "bar.c"// warning
+/// #include "baz.h"// no diagnostic
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone-suspicious-include.html
+class SuspiciousIncludeCheck : public ClangTidyCheck {
+public:
+  SuspiciousIncludeCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+  void registerPPCallbacks(const SourceManager , Preprocessor *PP,
+   Preprocessor *ModuleExpanderPP) override;
+};
+
+} // namespace bugprone
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SUSPICIOUSINCLUDECHECK_H
Index: clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp
@@ -0,0 +1,73 @@
+//===--- SuspiciousIncludeCheck.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 "SuspiciousIncludeCheck.h"
+#include "clang/AST/ASTContext.h"
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+namespace {
+class SuspiciousIncludePPCallbacks : public PPCallbacks {
+public:
+  explicit SuspiciousIncludePPCallbacks(ClangTidyCheck ,
+const SourceManager ,
+Preprocessor *PP)
+  : Check(Check), PP(PP) {}
+
+  void InclusionDirective(SourceLocation HashLoc, const Token ,
+  StringRef FileName, bool IsAngled,
+  CharSourceRange FilenameRange, const FileEntry *File,
+  StringRef SearchPath, StringRef RelativePath,
+

[PATCH] D74669: [clang-tidy] New check: misc-no-include-cpp

2020-02-16 Thread Jonathan Roelofs via Phabricator via cfe-commits
jroelofs marked an inline comment as done.
jroelofs added inline comments.



Comment at: clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.h:26
+/// For the user-facing documentation see:
+/// 
http://clang.llvm.org/extra/clang-tidy/checks/bugprone-suspicious-include.html
+class SuspiciousIncludeCheck : public ClangTidyCheck {

FIXME: still need to write docs for this.



Comment at: clang-tools-extra/clang-tidy/misc/NoIncludeCPPCheck.cpp:50
+
+  (void)FilenameRange;
+  (void)File;

njames93 wrote:
> I have a feeling this is to suppress some unused parameter linting check. If 
> it is then it should be removed as unused parameters in an overridden 
> function shouldn't emit a warning.
> Same for below.
> 
> Side note: File a bug with whichever linter tool gave you that warning (if 
> there was one).
> 
Sorry, did this out of habit. Indeed it isn't actually needed.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74669/new/

https://reviews.llvm.org/D74669



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D74669: [clang-tidy] New check: misc-no-include-cpp

2020-02-16 Thread Jonathan Roelofs via Phabricator via cfe-commits
jroelofs marked 4 inline comments as done and an inline comment as not done.
jroelofs added inline comments.



Comment at: clang-tools-extra/clang-tidy/misc/NoIncludeCPPCheck.cpp:62
+  Check.diag(HashLoc, "suspicious #include")
+  << FixItHint::CreateReplacement(FilenameRange,
+  ((IsAngled ? "<" : "\"") + FileName +

njames93 wrote:
> jroelofs wrote:
> > njames93 wrote:
> > > This replacement is dangerous, I have a feeling no fix-it should be 
> > > provided or at least do a search of the include directories to see if 
> > > file you are trying to include actually does exist. The correct file 
> > > could be `*.hpp` like what boost uses for all its header files
> > Yeah, perhaps the FixIt should only be added if there is a single candidate 
> > replacement that exists on the `-I` path.
> > 
> > Another option is to not add FixIts at all, and instead emit a list of 
> > `note:`s suggesting each of the candidates.
> How about the case if someone wants to (for whatever reason) include 
> something like this:
> 
> ```
> #include "example.h"
> #include "example.cpp"
> ```
> That looks intentional and a fix it shouldn't be emitted.
I'd imagine that people doing this intentionally (unity builds come to mind) 
would turn off this check. That said, is there something better this check 
could be doing to accommodate those people?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74669/new/

https://reviews.llvm.org/D74669



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D74669: [clang-tidy] New check: misc-no-include-cpp

2020-02-16 Thread Jonathan Roelofs via Phabricator via cfe-commits
jroelofs updated this revision to Diff 244883.
jroelofs marked an inline comment as done.
jroelofs added a comment.

- Renamed to bugprone-suspicious-include
- Removed FixIts in favor of `note:` suggestions, iff the suggested file exists 
on the header search path.
- Removed unnecessary unused parameter casts-to-void.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74669/new/

https://reviews.llvm.org/D74669

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.h
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/a
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/a.cpp
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/a.hpp
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/c.c
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/c.cc
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/c.cxx
  clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-include.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-include.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-include.cpp
@@ -0,0 +1,19 @@
+// RUN: %check_clang_tidy %s bugprone-suspicious-include %t -- -- -isystem %S/Inputs/Headers
+
+// CHECK-MESSAGES: [[@LINE+4]]:1: warning: suspicious #include of file with .cpp extension
+// CHECK-MESSAGES: [[@LINE+3]]:1: note: did you mean to include 'a'?
+// CHECK-MESSAGES: [[@LINE+2]]:1: note: did you mean to include 'a.h'?
+// CHECK-MESSAGES: [[@LINE+1]]:1: note: did you mean to include 'a.hpp'?
+#include "a.cpp"
+
+#include "b.h"
+
+// CHECK-MESSAGES: [[@LINE+1]]:1: warning: suspicious #include of file with .c extension
+#include 
+
+// CHECK-MESSAGES: [[@LINE+1]]:1: warning: suspicious #include of file with .cc extension
+#include 
+
+// CHECK-MESSAGES: [[@LINE+1]]:1: warning: suspicious #include of file with .cxx extension
+#include 
+
Index: clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.h
@@ -0,0 +1,39 @@
+//===--- SuspiciousIncludeCheck.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_SUSPICIOUSINCLUDECHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SUSPICIOUSINCLUDECHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+/// Warns on inclusion of files whose names suggest that they're implementation
+/// files, instead of headers. E.g:
+///
+/// #include "foo.cpp"  // warning
+/// #include "bar.c"// warning
+/// #include "baz.h"// no diagnostic
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone-suspicious-include.html
+class SuspiciousIncludeCheck : public ClangTidyCheck {
+public:
+  SuspiciousIncludeCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+  void registerPPCallbacks(const SourceManager , Preprocessor *PP,
+   Preprocessor *ModuleExpanderPP) override;
+};
+
+} // namespace bugprone
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SUSPICIOUSINCLUDECHECK_H
Index: clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp
@@ -0,0 +1,74 @@
+//===--- SuspiciousIncludeCheck.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 "SuspiciousIncludeCheck.h"
+#include "clang/AST/ASTContext.h"
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+namespace {
+class SuspiciousIncludePPCallbacks : public PPCallbacks {
+public:
+  explicit SuspiciousIncludePPCallbacks(ClangTidyCheck ,
+const SourceManager ,
+Preprocessor *PP)
+  : Check(Check), PP(PP) {}
+
+  void 

[PATCH] D73835: [IRBuilder] Virtualize IRBuilder

2020-02-16 Thread Nikita Popov via Phabricator via cfe-commits
nikic reopened this revision.
nikic added a comment.
This revision is now accepted and ready to land.

Reverted because of failures on 
http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/22164.
 Not sure why this is the only builder that's failing, but I believe the 
problematic code is 
https://github.com/llvm/llvm-project/blob/af480e8c63b27ad247b8430cf124da8bcdf752f8/llvm/lib/IR/DIBuilder.cpp#L898-L910.
 Without NRVO this may copy `IRBuilder<>`, which is now self-referential due to 
the Folder/Inserter references in IRBuilderBase.

I'm not sure what the right way to fix this is. Should I add an explicit copy 
constructor? I guess that would also involve explicit move ctor and copy/move 
assignment, which seems pretty ugly. Maybe it makes more sense to explicitly 
delete it and just avoid the copy constructor in that particular code?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73835/new/

https://reviews.llvm.org/D73835



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D74692: [clang-tidy] Make bugprone-use-after-move ignore std::move for const values

2020-02-16 Thread Zinovy Nis via Phabricator via cfe-commits
zinovy.nis updated this revision to Diff 244878.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74692/new/

https://reviews.llvm.org/D74692

Files:
  clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
@@ -129,6 +129,14 @@
   // CHECK-NOTES: [[@LINE-3]]:15: note: move occurred here
 }
 
+// Simple case for const value: no actual move and no warning.
+void simpleConst(const A a) {
+  A other_a = std::move(a);
+  a.foo();
+  // CHECK-NOTES-NOT: [[@LINE-1]]:3: warning: 'a' used after it was moved
+  // CHECK-NOTES-NOT: [[@LINE-3]]:15: note: move occurred here
+}
+
 // A warning should only be emitted for one use-after-move.
 void onlyFlagOneUseAfterMove() {
   A a;
@@ -308,14 +316,15 @@
 }
 
 void lambdas() {
-  // Use-after-moves inside a lambda should be detected.
+  // Use-after-moves inside a lambda will not be detected as [a]
+  // is passed as a const by default.
   {
 A a;
 auto lambda = [a] {
   std::move(a);
   a.foo();
-  // CHECK-NOTES: [[@LINE-1]]:7: warning: 'a' used after it was moved
-  // CHECK-NOTES: [[@LINE-3]]:7: note: move occurred here
+  // CHECK-NOTES-NOT: [[@LINE-1]]:7: warning: 'a' used after it was moved
+  // CHECK-NOTES-NOT: [[@LINE-3]]:7: note: move occurred here
 };
   }
   // This is just as true if the variable was declared inside the lambda.
@@ -720,15 +729,15 @@
 // const pointer argument.
 const A a;
 std::move(a);
-passByConstPointer();
-// CHECK-NOTES: [[@LINE-1]]:25: warning: 'a' used after it was moved
-// CHECK-NOTES: [[@LINE-3]]:5: note: move occurred here
+passByConstPointer(); // expect no warning: the const value was not 
moved.
+// CHECK-NOTES-NOT: [[@LINE-1]]:25: warning: 'a' used after it was moved
+// CHECK-NOTES-NOT: [[@LINE-3]]:5: note: move occurred here
   }
   const A a;
   std::move(a);
-  passByConstReference(a);
-  // CHECK-NOTES: [[@LINE-1]]:24: warning: 'a' used after it was moved
-  // CHECK-NOTES: [[@LINE-3]]:3: note: move occurred here
+  passByConstReference(a); // expect no warning: the const value was not moved.
+  // CHECK-NOTES-NOT: [[@LINE-1]]:24: warning: 'a' used after it was moved
+  // CHECK-NOTES-NOT: [[@LINE-3]]:3: note: move occurred here
 }
 
 // Clearing a standard container using clear() is treated as a
Index: clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
@@ -404,7 +404,8 @@
hasArgument(0, declRefExpr().bind("arg")),
anyOf(hasAncestor(lambdaExpr().bind("containing-lambda")),
  hasAncestor(functionDecl().bind("containing-func"))),
-   unless(inDecltypeOrTemplateArg()))
+   unless(anyOf(inDecltypeOrTemplateArg(),
+hasType(qualType(isConstQualified())
   .bind("call-move");
 
   Finder->addMatcher(


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
@@ -129,6 +129,14 @@
   // CHECK-NOTES: [[@LINE-3]]:15: note: move occurred here
 }
 
+// Simple case for const value: no actual move and no warning.
+void simpleConst(const A a) {
+  A other_a = std::move(a);
+  a.foo();
+  // CHECK-NOTES-NOT: [[@LINE-1]]:3: warning: 'a' used after it was moved
+  // CHECK-NOTES-NOT: [[@LINE-3]]:15: note: move occurred here
+}
+
 // A warning should only be emitted for one use-after-move.
 void onlyFlagOneUseAfterMove() {
   A a;
@@ -308,14 +316,15 @@
 }
 
 void lambdas() {
-  // Use-after-moves inside a lambda should be detected.
+  // Use-after-moves inside a lambda will not be detected as [a]
+  // is passed as a const by default.
   {
 A a;
 auto lambda = [a] {
   std::move(a);
   a.foo();
-  // CHECK-NOTES: [[@LINE-1]]:7: warning: 'a' used after it was moved
-  // CHECK-NOTES: [[@LINE-3]]:7: note: move occurred here
+  // CHECK-NOTES-NOT: [[@LINE-1]]:7: warning: 'a' used after it was moved
+  // CHECK-NOTES-NOT: [[@LINE-3]]:7: note: move occurred here
 };
   }
   // This is just as true if the variable was declared inside the lambda.
@@ -720,15 +729,15 @@
 // const pointer argument.
 const A a;
 std::move(a);
-passByConstPointer();
-// CHECK-NOTES: [[@LINE-1]]:25: warning: 'a' used 

[PATCH] D74669: [clang-tidy] New check: misc-no-include-cpp

2020-02-16 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments.



Comment at: clang-tools-extra/clang-tidy/misc/NoIncludeCPPCheck.cpp:11-13
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;

You don't need to include or use the AST matchers in a preprocessor only check.



Comment at: clang-tools-extra/clang-tidy/misc/NoIncludeCPPCheck.cpp:50
+
+  (void)FilenameRange;
+  (void)File;

I have a feeling this is to suppress some unused parameter linting check. If it 
is then it should be removed as unused parameters in an overridden function 
shouldn't emit a warning.
Same for below.

Side note: File a bug with whichever linter tool gave you that warning (if 
there was one).




Comment at: clang-tools-extra/clang-tidy/misc/NoIncludeCPPCheck.cpp:62
+  Check.diag(HashLoc, "suspicious #include")
+  << FixItHint::CreateReplacement(FilenameRange,
+  ((IsAngled ? "<" : "\"") + FileName +

jroelofs wrote:
> njames93 wrote:
> > This replacement is dangerous, I have a feeling no fix-it should be 
> > provided or at least do a search of the include directories to see if file 
> > you are trying to include actually does exist. The correct file could be 
> > `*.hpp` like what boost uses for all its header files
> Yeah, perhaps the FixIt should only be added if there is a single candidate 
> replacement that exists on the `-I` path.
> 
> Another option is to not add FixIts at all, and instead emit a list of 
> `note:`s suggesting each of the candidates.
How about the case if someone wants to (for whatever reason) include something 
like this:

```
#include "example.h"
#include "example.cpp"
```
That looks intentional and a fix it shouldn't be emitted.



Comment at: clang-tools-extra/clang-tidy/misc/NoIncludeCPPCheck.h:31
+  : ClangTidyCheck(Name, Context) {}
+  void registerPPCallbacks(const SourceManager , Preprocessor *PP,
+   Preprocessor *ModuleExpanderPP);

This should be marked `override`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74669/new/

https://reviews.llvm.org/D74669



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D74692: [clang-tidy] Make bugprone-use-after-move ignore std::move for const values

2020-02-16 Thread Zinovy Nis via Phabricator via cfe-commits
zinovy.nis updated this revision to Diff 244876.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74692/new/

https://reviews.llvm.org/D74692

Files:
  clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
@@ -129,6 +129,14 @@
   // CHECK-NOTES: [[@LINE-3]]:15: note: move occurred here
 }
 
+// Simple case for const value.
+void simpleConst(const A a) {
+  A other_a = std::move(a);
+  a.foo();
+  // CHECK-NOTES-NOT: [[@LINE-1]]:3: warning: 'a' used after it was moved
+  // CHECK-NOTES-NOT: [[@LINE-3]]:15: note: move occurred here
+}
+
 // A warning should only be emitted for one use-after-move.
 void onlyFlagOneUseAfterMove() {
   A a;
@@ -314,8 +322,8 @@
 auto lambda = [a] {
   std::move(a);
   a.foo();
-  // CHECK-NOTES: [[@LINE-1]]:7: warning: 'a' used after it was moved
-  // CHECK-NOTES: [[@LINE-3]]:7: note: move occurred here
+  // CHECK-NOTES-NOT: [[@LINE-1]]:7: warning: 'a' used after it was moved
+  // CHECK-NOTES-NOT: [[@LINE-3]]:7: note: move occurred here
 };
   }
   // This is just as true if the variable was declared inside the lambda.
@@ -721,14 +729,14 @@
 const A a;
 std::move(a);
 passByConstPointer();
-// CHECK-NOTES: [[@LINE-1]]:25: warning: 'a' used after it was moved
-// CHECK-NOTES: [[@LINE-3]]:5: note: move occurred here
+// CHECK-NOTES-NOT: [[@LINE-1]]:25: warning: 'a' used after it was moved
+// CHECK-NOTES-NOT: [[@LINE-3]]:5: note: move occurred here
   }
   const A a;
   std::move(a);
   passByConstReference(a);
-  // CHECK-NOTES: [[@LINE-1]]:24: warning: 'a' used after it was moved
-  // CHECK-NOTES: [[@LINE-3]]:3: note: move occurred here
+  // CHECK-NOTES-NOT: [[@LINE-1]]:24: warning: 'a' used after it was moved
+  // CHECK-NOTES-NOT: [[@LINE-3]]:3: note: move occurred here
 }
 
 // Clearing a standard container using clear() is treated as a
Index: clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
@@ -404,7 +404,8 @@
hasArgument(0, declRefExpr().bind("arg")),
anyOf(hasAncestor(lambdaExpr().bind("containing-lambda")),
  hasAncestor(functionDecl().bind("containing-func"))),
-   unless(inDecltypeOrTemplateArg()))
+   unless(anyOf(inDecltypeOrTemplateArg(),
+hasType(qualType(isConstQualified())
   .bind("call-move");
 
   Finder->addMatcher(


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
@@ -129,6 +129,14 @@
   // CHECK-NOTES: [[@LINE-3]]:15: note: move occurred here
 }
 
+// Simple case for const value.
+void simpleConst(const A a) {
+  A other_a = std::move(a);
+  a.foo();
+  // CHECK-NOTES-NOT: [[@LINE-1]]:3: warning: 'a' used after it was moved
+  // CHECK-NOTES-NOT: [[@LINE-3]]:15: note: move occurred here
+}
+
 // A warning should only be emitted for one use-after-move.
 void onlyFlagOneUseAfterMove() {
   A a;
@@ -314,8 +322,8 @@
 auto lambda = [a] {
   std::move(a);
   a.foo();
-  // CHECK-NOTES: [[@LINE-1]]:7: warning: 'a' used after it was moved
-  // CHECK-NOTES: [[@LINE-3]]:7: note: move occurred here
+  // CHECK-NOTES-NOT: [[@LINE-1]]:7: warning: 'a' used after it was moved
+  // CHECK-NOTES-NOT: [[@LINE-3]]:7: note: move occurred here
 };
   }
   // This is just as true if the variable was declared inside the lambda.
@@ -721,14 +729,14 @@
 const A a;
 std::move(a);
 passByConstPointer();
-// CHECK-NOTES: [[@LINE-1]]:25: warning: 'a' used after it was moved
-// CHECK-NOTES: [[@LINE-3]]:5: note: move occurred here
+// CHECK-NOTES-NOT: [[@LINE-1]]:25: warning: 'a' used after it was moved
+// CHECK-NOTES-NOT: [[@LINE-3]]:5: note: move occurred here
   }
   const A a;
   std::move(a);
   passByConstReference(a);
-  // CHECK-NOTES: [[@LINE-1]]:24: warning: 'a' used after it was moved
-  // CHECK-NOTES: [[@LINE-3]]:3: note: move occurred here
+  // CHECK-NOTES-NOT: [[@LINE-1]]:24: warning: 'a' used after it was moved
+  // CHECK-NOTES-NOT: [[@LINE-3]]:3: note: move occurred here
 }
 
 // Clearing a standard container using clear() is treated as a
Index: clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp

[PATCH] D74692: [clang-tidy] Make bugprone-use-after-move ignore std::move for const values

2020-02-16 Thread Zinovy Nis via Phabricator via cfe-commits
zinovy.nis created this revision.
zinovy.nis added reviewers: mboehme, alexfh.
zinovy.nis added a project: clang-tools-extra.
Herald added subscribers: cfe-commits, mgehre, xazax.hun.
Herald added a project: clang.

std::move for const values is NO-OP, so it has no sense to report it in 
bugprone-use-after-move

  void simpleConst(const A a) {
A other_a = std::move(a); // no actual move takes place here,
a.foo();  // so this call is safe and must not be reported.
  }

Anyway, redundant std::move for consts can be found with 
```performance-move-const-arg``` check.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D74692

Files:
  clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
@@ -129,6 +129,14 @@
   // CHECK-NOTES: [[@LINE-3]]:15: note: move occurred here
 }
 
+// Simple case for const value.
+void simpleConst(const A a) {
+  A other_a = std::move(a);
+  a.foo();
+  // CHECK-NOTES-NOT: [[@LINE-1]]:3: warning: 'a' used after it was moved
+  // CHECK-NOTES-NOT: [[@LINE-3]]:15: note: move occurred here
+}
+
 // A warning should only be emitted for one use-after-move.
 void onlyFlagOneUseAfterMove() {
   A a;
@@ -314,8 +322,8 @@
 auto lambda = [a] {
   std::move(a);
   a.foo();
-  // CHECK-NOTES: [[@LINE-1]]:7: warning: 'a' used after it was moved
-  // CHECK-NOTES: [[@LINE-3]]:7: note: move occurred here
+  // CHECK-NOTES-NOT: [[@LINE-1]]:7: warning: 'a' used after it was moved
+  // CHECK-NOTES-NOT: [[@LINE-3]]:7: note: move occurred here
 };
   }
   // This is just as true if the variable was declared inside the lambda.
@@ -721,14 +729,14 @@
 const A a;
 std::move(a);
 passByConstPointer();
-// CHECK-NOTES: [[@LINE-1]]:25: warning: 'a' used after it was moved
-// CHECK-NOTES: [[@LINE-3]]:5: note: move occurred here
+// CHECK-NOTES-NOT: [[@LINE-1]]:25: warning: 'a' used after it was moved
+// CHECK-NOTES-NOT: [[@LINE-3]]:5: note: move occurred here
   }
   const A a;
   std::move(a);
   passByConstReference(a);
-  // CHECK-NOTES: [[@LINE-1]]:24: warning: 'a' used after it was moved
-  // CHECK-NOTES: [[@LINE-3]]:3: note: move occurred here
+  // CHECK-NOTES-NOT: [[@LINE-1]]:24: warning: 'a' used after it was moved
+  // CHECK-NOTES-NOT: [[@LINE-3]]:3: note: move occurred here
 }
 
 // Clearing a standard container using clear() is treated as a
Index: clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
@@ -425,13 +425,17 @@
 }
 
 void UseAfterMoveCheck::check(const MatchFinder::MatchResult ) {
+  const auto *Arg = Result.Nodes.getNodeAs("arg");
+
+  if (Arg->getType().isConstQualified())
+return;
+
   const auto *ContainingLambda =
   Result.Nodes.getNodeAs("containing-lambda");
   const auto *ContainingFunc =
   Result.Nodes.getNodeAs("containing-func");
   const auto *CallMove = Result.Nodes.getNodeAs("call-move");
   const auto *MovingCall = Result.Nodes.getNodeAs("moving-call");
-  const auto *Arg = Result.Nodes.getNodeAs("arg");
 
   if (!MovingCall || !MovingCall->getExprLoc().isValid())
 MovingCall = CallMove;


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
@@ -129,6 +129,14 @@
   // CHECK-NOTES: [[@LINE-3]]:15: note: move occurred here
 }
 
+// Simple case for const value.
+void simpleConst(const A a) {
+  A other_a = std::move(a);
+  a.foo();
+  // CHECK-NOTES-NOT: [[@LINE-1]]:3: warning: 'a' used after it was moved
+  // CHECK-NOTES-NOT: [[@LINE-3]]:15: note: move occurred here
+}
+
 // A warning should only be emitted for one use-after-move.
 void onlyFlagOneUseAfterMove() {
   A a;
@@ -314,8 +322,8 @@
 auto lambda = [a] {
   std::move(a);
   a.foo();
-  // CHECK-NOTES: [[@LINE-1]]:7: warning: 'a' used after it was moved
-  // CHECK-NOTES: [[@LINE-3]]:7: note: move occurred here
+  // CHECK-NOTES-NOT: [[@LINE-1]]:7: warning: 'a' used after it was moved
+  // CHECK-NOTES-NOT: [[@LINE-3]]:7: note: move occurred here
 };
   }
   // This is just as true if the variable was declared inside the lambda.
@@ -721,14 +729,14 @@
 const A a;
 std::move(a);
 passByConstPointer();
-// CHECK-NOTES: [[@LINE-1]]:25: warning: 'a' 

[PATCH] D74669: [clang-tidy] New check: misc-no-include-cpp

2020-02-16 Thread Jonathan Roelofs via Phabricator via cfe-commits
jroelofs marked an inline comment as done.
jroelofs added a comment.

In D74669#1878168 , @njames93 wrote:

> I have a feeling this check should be called something along the lines of 
> bugprone-suspicous-include.


That's a much better name, I like it.




Comment at: clang-tools-extra/clang-tidy/misc/NoIncludeCPPCheck.cpp:62
+  Check.diag(HashLoc, "suspicious #include")
+  << FixItHint::CreateReplacement(FilenameRange,
+  ((IsAngled ? "<" : "\"") + FileName +

njames93 wrote:
> This replacement is dangerous, I have a feeling no fix-it should be provided 
> or at least do a search of the include directories to see if file you are 
> trying to include actually does exist. The correct file could be `*.hpp` like 
> what boost uses for all its header files
Yeah, perhaps the FixIt should only be added if there is a single candidate 
replacement that exists on the `-I` path.

Another option is to not add FixIts at all, and instead emit a list of `note:`s 
suggesting each of the candidates.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74669/new/

https://reviews.llvm.org/D74669



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D72231: [Sema] Adds the pointer-to-int-cast diagnostic

2020-02-16 Thread Mark de Wever via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Mordante marked an inline comment as done.
Closed by commit rG9658d895c81a: [Sema] Adds the pointer-to-int-cast diagnostic 
(authored by Mordante).

Changed prior to commit:
  https://reviews.llvm.org/D72231?vs=242393=244872#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72231/new/

https://reviews.llvm.org/D72231

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaCast.cpp
  clang/test/Analysis/bstring.c
  clang/test/Analysis/casts.c
  clang/test/Analysis/misc-ps.c
  clang/test/Analysis/misc-ps.m
  clang/test/Analysis/ptr-arith.c
  clang/test/Analysis/svalbuilder-logic.c
  clang/test/Analysis/symbol-reaper.c
  clang/test/CodeGen/const-init.c
  clang/test/Sema/MicrosoftExtensions.c
  clang/test/Sema/block-return.c
  clang/test/Sema/cast.c
  clang/test/Sema/const-eval.c
  clang/test/Sema/init.c
  clang/test/Sema/offsetof.c
  clang/test/Sema/static-init.c
  clang/test/Sema/struct-decl.c
  clang/test/SemaObjC/arc.m
  clang/test/SemaObjC/gcc-cast-ext.m
  clang/test/SemaObjC/protocol-archane.m

Index: clang/test/SemaObjC/protocol-archane.m
===
--- clang/test/SemaObjC/protocol-archane.m
+++ clang/test/SemaObjC/protocol-archane.m
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -Wno-objc-root-class %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wno-pointer-to-int-cast -Wno-objc-root-class %s
 // rdar://5986251
 
 @protocol SomeProtocol
Index: clang/test/SemaObjC/gcc-cast-ext.m
===
--- clang/test/SemaObjC/gcc-cast-ext.m
+++ clang/test/SemaObjC/gcc-cast-ext.m
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -verify -fms-extensions -Wno-objc-root-class %s
+// RUN: %clang_cc1 -verify -Wno-pointer-to-int-cast -Wno-objc-root-class %s
 @class NSInvocation, NSMethodSignature, NSCoder, NSString, NSEnumerator;
 typedef struct _NSRange { } NSRange;
 
Index: clang/test/SemaObjC/arc.m
===
--- clang/test/SemaObjC/arc.m
+++ clang/test/SemaObjC/arc.m
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -triple x86_64-apple-darwin11 -fobjc-runtime-has-weak -fsyntax-only -fobjc-arc -fblocks -verify -Wno-objc-root-class %s
-// RUN: not %clang_cc1 -triple x86_64-apple-darwin11 -fobjc-runtime-has-weak -fsyntax-only -fobjc-arc -fblocks -Wno-objc-root-class -fdiagnostics-parseable-fixits %s 2>&1
+// RUN: %clang_cc1 -triple x86_64-apple-darwin11 -fobjc-runtime-has-weak -fsyntax-only -fobjc-arc -fblocks -verify -Wno-pointer-to-int-cast -Wno-objc-root-class %s
+// RUN: not %clang_cc1 -triple x86_64-apple-darwin11 -fobjc-runtime-has-weak -fsyntax-only -fobjc-arc -fblocks -Wno-pointer-to-int-cast -Wno-objc-root-class -fdiagnostics-parseable-fixits %s 2>&1
 
 typedef unsigned long NSUInteger;
 typedef const void * CFTypeRef;
Index: clang/test/Sema/struct-decl.c
===
--- clang/test/Sema/struct-decl.c
+++ clang/test/Sema/struct-decl.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -Wno-pointer-to-int-cast -fsyntax-only -verify %s
 // PR3459
 struct bar {
   char n[1];
Index: clang/test/Sema/static-init.c
===
--- clang/test/Sema/static-init.c
+++ clang/test/Sema/static-init.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -Wno-bool-conversion %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wno-pointer-to-int-cast -Wno-bool-conversion %s
 
 typedef __typeof((int*) 0 - (int*) 0) intptr_t;
 
Index: clang/test/Sema/offsetof.c
===
--- clang/test/Sema/offsetof.c
+++ clang/test/Sema/offsetof.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -Wno-pointer-to-int-cast -fsyntax-only -verify %s
 
 #define offsetof(TYPE, MEMBER) __builtin_offsetof (TYPE, MEMBER)
 
Index: clang/test/Sema/init.c
===
--- clang/test/Sema/init.c
+++ clang/test/Sema/init.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -verify -fsyntax-only -ffreestanding
+// RUN: %clang_cc1 %s -Wno-pointer-to-int-cast -verify -fsyntax-only -ffreestanding
 
 #include 
 #include 
Index: clang/test/Sema/const-eval.c
===
--- clang/test/Sema/const-eval.c
+++ clang/test/Sema/const-eval.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -triple x86_64-linux %s -Wno-tautological-pointer-compare
+// RUN: %clang_cc1 -fsyntax-only -verify -triple x86_64-linux %s -Wno-tautological-pointer-compare -Wno-pointer-to-int-cast
 
 #define EVAL_EXPR(testno, expr) int test##testno = sizeof(struct{char qq[expr];});
 int x;
Index: 

[PATCH] D72231: [Sema] Adds the pointer-to-int-cast diagnostic

2020-02-16 Thread Mark de Wever via Phabricator via cfe-commits
Mordante marked 2 inline comments as done.
Mordante added a comment.
Herald added a subscriber: martong.

Thanks for the review.

I'll have a look at your suggestion for a follow-up patch.




Comment at: clang/test/Sema/cast.c:1
-// RUN: %clang_cc1 -fsyntax-only -triple x86_64-unknown-unknown %s -verify
+// RUN: %clang_cc1 -fsyntax-only -triple x86_64-unknown-unknown 
-Wpointer-to-int-cast %s -verify
 

rsmith wrote:
> Don't specify the warning flag here, so that we have some test coverage that 
> this is enabled by default.
Good catch! This was needed before the diagnostic was enabled by default.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72231/new/

https://reviews.llvm.org/D72231



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] 9658d89 - [Sema] Adds the pointer-to-int-cast diagnostic

2020-02-16 Thread Mark de Wever via cfe-commits

Author: Mark de Wever
Date: 2020-02-16T15:38:25+01:00
New Revision: 9658d895c81aeb849b412b9332c88769588c7660

URL: 
https://github.com/llvm/llvm-project/commit/9658d895c81aeb849b412b9332c88769588c7660
DIFF: 
https://github.com/llvm/llvm-project/commit/9658d895c81aeb849b412b9332c88769588c7660.diff

LOG: [Sema] Adds the pointer-to-int-cast diagnostic

Converting a pointer to an integer whose result cannot represented in the
integer type is undefined behavior is C and prohibited in C++. C++ already
has a diagnostic when casting. This adds a diagnostic for C.

Since this diagnostic uses the range of the conversion it also modifies
int-to-pointer-cast diagnostic to use a range.

Fixes PR8718: No warning on casting between pointer and non-pointer-sized int

Differential Revision: https://reviews.llvm.org/D72231

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaCast.cpp
clang/test/Analysis/bstring.c
clang/test/Analysis/casts.c
clang/test/Analysis/misc-ps.c
clang/test/Analysis/misc-ps.m
clang/test/Analysis/ptr-arith.c
clang/test/Analysis/svalbuilder-logic.c
clang/test/Analysis/symbol-reaper.c
clang/test/CodeGen/const-init.c
clang/test/Sema/MicrosoftExtensions.c
clang/test/Sema/block-return.c
clang/test/Sema/cast.c
clang/test/Sema/const-eval.c
clang/test/Sema/init.c
clang/test/Sema/offsetof.c
clang/test/Sema/static-init.c
clang/test/Sema/struct-decl.c
clang/test/SemaObjC/arc.m
clang/test/SemaObjC/gcc-cast-ext.m
clang/test/SemaObjC/protocol-archane.m

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index b98abc82d247..ba3937dd83ed 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -51,7 +51,8 @@ Major New Features
 Improvements to Clang's diagnostics
 ^^^
 
-- ...
+- -Wpointer-to-int-cast is a new warning group. This group warns about C-style
+  casts of pointers to a integer type too small to hold all possible values.
 
 Non-comprehensive list of changes in this release
 -

diff  --git a/clang/include/clang/Basic/DiagnosticGroups.td 
b/clang/include/clang/Basic/DiagnosticGroups.td
index 8c54723cdbab..c10b6dc20323 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -836,6 +836,9 @@ def IncompatibleExceptionSpec : 
DiagGroup<"incompatible-exception-spec">;
 def IntToVoidPointerCast : DiagGroup<"int-to-void-pointer-cast">;
 def IntToPointerCast : DiagGroup<"int-to-pointer-cast",
  [IntToVoidPointerCast]>;
+def VoidPointerToIntCast : DiagGroup<"void-pointer-to-int-cast">;
+def PointerToIntCast : DiagGroup<"pointer-to-int-cast",
+ [VoidPointerToIntCast]>;
 
 def Move : DiagGroup<"move", [
 PessimizingMove,

diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 70897cb2816b..111c8d8b6ed5 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3658,6 +3658,15 @@ def warn_int_to_pointer_cast : Warning<
 def warn_int_to_void_pointer_cast : Warning<
   "cast to %1 from smaller integer type %0">,
   InGroup;
+def warn_pointer_to_int_cast : Warning<
+  "cast to smaller integer type %1 from %0">,
+  InGroup;
+def warn_void_pointer_to_int_cast : Warning<
+  "cast to smaller integer type %1 from %0">,
+  InGroup;
+def ext_ms_pointer_to_int_cast : ExtWarn<
+  "cast to smaller integer type %1 from %0 is a Microsoft extension">,
+InGroup;
 
 def warn_attribute_ignored_for_field_of_type : Warning<
   "%0 attribute ignored for field of type %1">,

diff  --git a/clang/lib/Sema/SemaCast.cpp b/clang/lib/Sema/SemaCast.cpp
index 7a8cbca1e3f1..a89cc4be53aa 100644
--- a/clang/lib/Sema/SemaCast.cpp
+++ b/clang/lib/Sema/SemaCast.cpp
@@ -1961,7 +1961,7 @@ static void DiagnoseCallingConvCast(Sema , const 
ExprResult ,
   << FD << DstCCName << FixItHint::CreateInsertion(NameLoc, CCAttrText);
 }
 
-static void checkIntToPointerCast(bool CStyle, SourceLocation Loc,
+static void checkIntToPointerCast(bool CStyle, const SourceRange ,
   const Expr *SrcExpr, QualType DestType,
   Sema ) {
   QualType SrcType = SrcExpr->getType();
@@ -1983,7 +1983,7 @@ static void checkIntToPointerCast(bool CStyle, 
SourceLocation Loc,
 unsigned Diag = DestType->isVoidPointerType() ?
   diag::warn_int_to_void_pointer_cast
 : diag::warn_int_to_pointer_cast;
-Self.Diag(Loc, Diag) << SrcType << DestType;
+Self.Diag(OpRange.getBegin(), Diag) << SrcType << DestType 

[PATCH] D73835: [IRBuilder] Virtualize IRBuilder

2020-02-16 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment.

This causes this build warning:

  ../../llvm/include/llvm/IR/IRBuilder.h:77:16: warning: 'anchor' overrides a 
member function but is not marked 'override' [-Winconsistent-missing-override]
virtual void anchor();
 ^
  ../../llvm/include/llvm/IR/IRBuilder.h:62:16: note: overridden virtual 
function is here
virtual void anchor();
 ^


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73835/new/

https://reviews.llvm.org/D73835



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D73834: Update for Clang 10 release notes

2020-02-16 Thread Mark de Wever via Phabricator via cfe-commits
Mordante closed this revision.
Mordante added a comment.

Phab registered the commit, but didn't close this review.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73834/new/

https://reviews.llvm.org/D73834



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D74669: [clang-tidy] New check: misc-no-include-cpp

2020-02-16 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment.

I have a feeling this check should be called something along the lines of 
bugprone-suspicous-include.




Comment at: clang-tools-extra/clang-tidy/misc/NoIncludeCPPCheck.cpp:62
+  Check.diag(HashLoc, "suspicious #include")
+  << FixItHint::CreateReplacement(FilenameRange,
+  ((IsAngled ? "<" : "\"") + FileName +

This replacement is dangerous, I have a feeling no fix-it should be provided or 
at least do a search of the include directories to see if file you are trying 
to include actually does exist. The correct file could be `*.hpp` like what 
boost uses for all its header files


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74669/new/

https://reviews.llvm.org/D74669



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D73835: [IRBuilder] Virtualize IRBuilder

2020-02-16 Thread Nikita Popov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG0765d3824d06: [IRBuilder] Virtualize IRBuilder (authored by 
nikic).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73835/new/

https://reviews.llvm.org/D73835

Files:
  clang/lib/CodeGen/CGBuilder.h
  llvm/include/llvm/Analysis/TargetFolder.h
  llvm/include/llvm/IR/ConstantFolder.h
  llvm/include/llvm/IR/IRBuilder.h
  llvm/include/llvm/IR/IRBuilderFolder.h
  llvm/include/llvm/IR/NoFolder.h
  llvm/lib/Analysis/ConstantFolding.cpp
  llvm/lib/IR/IRBuilder.cpp
  llvm/lib/Transforms/Scalar/SROA.cpp
  polly/include/polly/CodeGen/IRBuilder.h

Index: polly/include/polly/CodeGen/IRBuilder.h
===
--- polly/include/polly/CodeGen/IRBuilder.h
+++ polly/include/polly/CodeGen/IRBuilder.h
@@ -131,15 +131,14 @@
 ///
 /// This is used to add additional items such as e.g. the llvm.loop.parallel
 /// metadata.
-class IRInserter : protected llvm::IRBuilderDefaultInserter {
+class IRInserter : public llvm::IRBuilderDefaultInserter {
 public:
   IRInserter() = default;
   IRInserter(class ScopAnnotator ) : Annotator() {}
 
-protected:
   void InsertHelper(llvm::Instruction *I, const llvm::Twine ,
 llvm::BasicBlock *BB,
-llvm::BasicBlock::iterator InsertPt) const {
+llvm::BasicBlock::iterator InsertPt) const override {
 llvm::IRBuilderDefaultInserter::InsertHelper(I, Name, BB, InsertPt);
 if (Annotator)
   Annotator->annotate(I);
Index: llvm/lib/Transforms/Scalar/SROA.cpp
===
--- llvm/lib/Transforms/Scalar/SROA.cpp
+++ llvm/lib/Transforms/Scalar/SROA.cpp
@@ -139,9 +139,8 @@
 public:
   void SetNamePrefix(const Twine ) { Prefix = P.str(); }
 
-protected:
   void InsertHelper(Instruction *I, const Twine , BasicBlock *BB,
-BasicBlock::iterator InsertPt) const {
+BasicBlock::iterator InsertPt) const override {
 IRBuilderDefaultInserter::InsertHelper(I, getNameWithPrefix(Name), BB,
InsertPt);
   }
@@ -2368,7 +2367,8 @@
 Instruction *OldUserI = cast(OldUse->getUser());
 IRB.SetInsertPoint(OldUserI);
 IRB.SetCurrentDebugLocation(OldUserI->getDebugLoc());
-IRB.SetNamePrefix(Twine(NewAI.getName()) + "." + Twine(BeginOffset) + ".");
+IRB.getInserter().SetNamePrefix(
+Twine(NewAI.getName()) + "." + Twine(BeginOffset) + ".");
 
 CanSROA &= visit(cast(OldUse->getUser()));
 if (VecTy || IntTy)
Index: llvm/lib/IR/IRBuilder.cpp
===
--- llvm/lib/IR/IRBuilder.cpp
+++ llvm/lib/IR/IRBuilder.cpp
@@ -24,6 +24,7 @@
 #include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/Operator.h"
+#include "llvm/IR/NoFolder.h"
 #include "llvm/IR/Statepoint.h"
 #include "llvm/IR/Type.h"
 #include "llvm/IR/Value.h"
@@ -784,3 +785,8 @@
   Function *Fn = Intrinsic::getDeclaration(M, ID, Types);
   return createCallHelper(Fn, Args, this, Name, FMFSource);
 }
+
+void IRBuilderDefaultInserter::anchor() {}
+void IRBuilderCallbackInserter::anchor() {}
+void ConstantFolder::anchor() {}
+void NoFolder::anchor() {}
Index: llvm/lib/Analysis/ConstantFolding.cpp
===
--- llvm/lib/Analysis/ConstantFolding.cpp
+++ llvm/lib/Analysis/ConstantFolding.cpp
@@ -23,6 +23,7 @@
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Analysis/TargetFolder.h"
 #include "llvm/Analysis/TargetLibraryInfo.h"
 #include "llvm/Analysis/ValueTracking.h"
 #include "llvm/Analysis/VectorUtils.h"
@@ -2660,3 +2661,5 @@
 
   return false;
 }
+
+void TargetFolder::anchor() {}
Index: llvm/include/llvm/IR/NoFolder.h
===
--- llvm/include/llvm/IR/NoFolder.h
+++ llvm/include/llvm/IR/NoFolder.h
@@ -26,11 +26,14 @@
 #include "llvm/IR/InstrTypes.h"
 #include "llvm/IR/Instruction.h"
 #include "llvm/IR/Instructions.h"
+#include "llvm/IR/IRBuilderFolder.h"
 
 namespace llvm {
 
 /// NoFolder - Create "constants" (actually, instructions) with no folding.
-class NoFolder {
+class NoFolder final : public IRBuilderFolder {
+  virtual void anchor();
+
 public:
   explicit NoFolder() = default;
 
@@ -39,73 +42,76 @@
   //======//
 
   Instruction *CreateAdd(Constant *LHS, Constant *RHS,
- bool HasNUW = false, bool HasNSW = false) const {
+ bool HasNUW = false,
+ bool HasNSW = false) const override {
 BinaryOperator *BO = BinaryOperator::CreateAdd(LHS, RHS);
 if (HasNUW) 

[clang] 0765d38 - [IRBuilder] Virtualize IRBuilder

2020-02-16 Thread Nikita Popov via cfe-commits

Author: Nikita Popov
Date: 2020-02-16T13:48:55+01:00
New Revision: 0765d3824d069f37596bc5a890399099b776c2a0

URL: 
https://github.com/llvm/llvm-project/commit/0765d3824d069f37596bc5a890399099b776c2a0
DIFF: 
https://github.com/llvm/llvm-project/commit/0765d3824d069f37596bc5a890399099b776c2a0.diff

LOG: [IRBuilder] Virtualize IRBuilder

Related llvm-dev thread:
http://lists.llvm.org/pipermail/llvm-dev/2020-February/138951.html

This patch moves the IRBuilder from templating over the constant
folder and inserter towards making both of these virtual.
There are a couple of motivations for this:

1. It's not possible to share code between use-sites that use
different IRBuilder folders/inserters (short of templating the code
and moving it into headers).
2. Methods currently defined on IRBuilderBase (which is not templated)
do not use the custom inserter, resulting in subtle bugs (e.g.
incorrect InstCombine worklist management). It would be possible to
move those into the templated IRBuilder, but...
3. The vast majority of the IRBuilder implementation has to live
in the header, because it depends on the template arguments.
4. We have many unnecessary dependencies on IRBuilder.h,
because it is not easy to forward-declare. (Significant parts of
the backend depend on it via TargetLowering.h, for example.)

This patch addresses the issue by making the following changes:

* IRBuilderDefaultInserter::InsertHelper becomes virtual.
  IRBuilderBase accepts a reference to it.
* IRBuilderFolder is introduced as a virtual base class. It is
 implemented by ConstantFolder (default), NoFolder and TargetFolder.
  IRBuilderBase has a reference to this as well.
* All the logic is moved from IRBuilder to IRBuilderBase. This means
  that methods can in the future replace their IRBuilder<> & uses
  (or other specific IRBuilder types) with IRBuilderBase & and thus
  be usable with different IRBuilders.
* The IRBuilder class is now a thin wrapper around IRBuilderBase.
  Essentially it only stores the folder and inserter and takes care
  of constructing the base builder.

What this patch doesn't do, but should be simple followups after this change:

* Fixing use of the inserter for creation methods originally defined
  on IRBuilderBase.
* Replacing IRBuilder<> uses in arguments with IRBuilderBase, where useful.
* Moving code from the IRBuilder header to the source file.

>From the user perspective, these changes should be mostly transparent:
The only thing that consumers using a custom inserted may need to do is
inherit from IRBuilderDefaultInserter publicly and mark their InsertHelper
as public.

Differential Revision: https://reviews.llvm.org/D73835

Added: 
llvm/include/llvm/IR/IRBuilderFolder.h

Modified: 
clang/lib/CodeGen/CGBuilder.h
llvm/include/llvm/Analysis/TargetFolder.h
llvm/include/llvm/IR/ConstantFolder.h
llvm/include/llvm/IR/IRBuilder.h
llvm/include/llvm/IR/NoFolder.h
llvm/lib/Analysis/ConstantFolding.cpp
llvm/lib/IR/IRBuilder.cpp
llvm/lib/Transforms/Scalar/SROA.cpp
polly/include/polly/CodeGen/IRBuilder.h

Removed: 




diff  --git a/clang/lib/CodeGen/CGBuilder.h b/clang/lib/CodeGen/CGBuilder.h
index 7687f7990d99..19233f395b84 100644
--- a/clang/lib/CodeGen/CGBuilder.h
+++ b/clang/lib/CodeGen/CGBuilder.h
@@ -22,16 +22,15 @@ class CodeGenFunction;
 /// This is an IRBuilder insertion helper that forwards to
 /// CodeGenFunction::InsertHelper, which adds necessary metadata to
 /// instructions.
-class CGBuilderInserter : protected llvm::IRBuilderDefaultInserter {
+class CGBuilderInserter : public llvm::IRBuilderDefaultInserter {
 public:
   CGBuilderInserter() = default;
   explicit CGBuilderInserter(CodeGenFunction *CGF) : CGF(CGF) {}
 
-protected:
   /// This forwards to CodeGenFunction::InsertHelper.
   void InsertHelper(llvm::Instruction *I, const llvm::Twine ,
 llvm::BasicBlock *BB,
-llvm::BasicBlock::iterator InsertPt) const;
+llvm::BasicBlock::iterator InsertPt) const override;
 private:
   CodeGenFunction *CGF = nullptr;
 };

diff  --git a/llvm/include/llvm/Analysis/TargetFolder.h 
b/llvm/include/llvm/Analysis/TargetFolder.h
index 7ab6562be440..712c1a2e8118 100644
--- a/llvm/include/llvm/Analysis/TargetFolder.h
+++ b/llvm/include/llvm/Analysis/TargetFolder.h
@@ -22,13 +22,14 @@
 #include "llvm/Analysis/ConstantFolding.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/InstrTypes.h"
+#include "llvm/IR/IRBuilderFolder.h"
 
 namespace llvm {
 
 class DataLayout;
 
 /// TargetFolder - Create constants with target dependent folding.
-class TargetFolder {
+class TargetFolder final : public IRBuilderFolder {
   const DataLayout 
 
   /// Fold - Fold the constant using target specific information.
@@ -38,6 +39,8 @@ class TargetFolder {
 return C;
   }
 
+  virtual void anchor();
+
 public:
   explicit TargetFolder(const DataLayout ) : 

[PATCH] D74463: [clang-tidy] Add new check avoid-adjacent-unrelated-parameters-of-the-same-type

2020-02-16 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidAdjacentParametersOfTheSameTypeCheck.cpp:18
+namespace cppcoreguidelines {
+// FIXME: Make sure this matcher only accepts functions with minimum 2
+// parameters

njames93 wrote:
> You'd need to create a matcher for that
> 
> ```
> AST_MATCHER(FunctionDecl, hasMoreThan2Args) {
>   return Node.getNumParams() > 2;
> }
> ```
> 
Sorry you'd need
```
AST_MATCHER(FunctionDecl, has2orMoreArgs) {
  return Node.getNumParams() >= 2;
}
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74463/new/

https://reviews.llvm.org/D74463



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D74463: [clang-tidy] Add new check avoid-adjacent-unrelated-parameters-of-the-same-type

2020-02-16 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment.

I have a feeling this check is going to throw so many false positives that 
it'll be too noisy to run on any real codebase. 
There should be a way to silence this when it is undesired like in the example 
for `int max(int a, int b);`. 
A possible solution could be based on parameter name length, usually 
interchangeable parameters have short names. 
Maybe an option could be added `IgnoreShortParamNamesSize` which takes an int 
and disregards results if both params are shorter, set as `0` to disable the 
option




Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidAdjacentParametersOfTheSameTypeCheck.cpp:18
+namespace cppcoreguidelines {
+// FIXME: Make sure this matcher only accepts functions with minimum 2
+// parameters

You'd need to create a matcher for that

```
AST_MATCHER(FunctionDecl, hasMoreThan2Args) {
  return Node.getNumParams() > 2;
}
```




Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidAdjacentParametersOfTheSameTypeCheck.cpp:29
+  Result.Nodes.getNodeAs("multi-parameter-function");
+  bool adjacentParametersOfTheSameTypeFound = false;
+  auto parameters = MatchedDecl->parameters();

Please follow the naming convention of CamelCase for all Variables, same goes 
for all other occurances below



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidAdjacentParametersOfTheSameTypeCheck.cpp:30
+  bool adjacentParametersOfTheSameTypeFound = false;
+  auto parameters = MatchedDecl->parameters();
+  auto parameter = parameters.begin();

Don't use auto when the type isn't spelled out in the initialisation, the below 
cases are ok as they are iterators



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:72
 ^^
+- New :doc:`cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type
+  
`
 check.

A new line is needed above here



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type.rst:29
+int max(int a, int b);
\ No newline at end of file


New line



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type.cpp:58
+// CHECK-MESSAGES: :[[@LINE-3]]:17: warning: function 'func_member3' has 
adjacent parameters of the same type. If the order of these parameters matter 
consider rewriting this to avoid a mixup of parameters. 
[cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type]
\ No newline at end of file


New line


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74463/new/

https://reviews.llvm.org/D74463



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D74689: [clang-tidy] Better custom class support for performance-inefficient-vector-operation

2020-02-16 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 244867.
njames93 added a comment.

- New line and undo clang format artefact


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74689/new/

https://reviews.llvm.org/D74689

Files:
  clang-tools-extra/clang-tidy/performance/InefficientVectorOperationCheck.cpp
  clang-tools-extra/clang-tidy/performance/InefficientVectorOperationCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/performance-inefficient-vector-operation.rst
  
clang-tools-extra/test/clang-tidy/checkers/performance-inefficient-vector-operation.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/performance-inefficient-vector-operation.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/performance-inefficient-vector-operation.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/performance-inefficient-vector-operation.cpp
@@ -1,7 +1,12 @@
 // RUN: %check_clang_tidy %s performance-inefficient-vector-operation %t -- \
 // RUN: -format-style=llvm \
 // RUN: -config='{CheckOptions: \
-// RUN:  [{key: performance-inefficient-vector-operation.EnableProto, value: 1}]}'
+// RUN:  [{key: performance-inefficient-vector-operation.EnableProto, value: 1}, \
+// RUN:   {key: performance-inefficient-vector-operation.VectorLikeClasses, value : MyContainer}, \
+// RUN:   {key: performance-inefficient-vector-operation.SupportedRanges, value : MyContainer}, \
+// RUN:   {key: performance-inefficient-vector-operation.ReserveNames, value : Reserve}, \
+// RUN:   {key: performance-inefficient-vector-operation.AppendNames, value : PushBack}, \
+// RUN:   {key: performance-inefficient-vector-operation.SizeNames, value : Size}, ]}'
 
 namespace std {
 
@@ -359,3 +364,160 @@
 }
   }
 }
+
+namespace OptionsValidMatchDefault {
+template 
+class MyContainer {
+public:
+  unsigned size();
+  T *begin() const;
+  T *end() const;
+  void push_back(const T &);
+  void reserve(unsigned);
+};
+
+void foo(const MyContainer ) {
+  MyContainer CC1;
+  // CHECK-FIXES: {{^}}  CC1.reserve(C.size());
+  for (auto I : C) {
+CC1.push_back(I);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'push_back' is called
+  }
+}
+} // namespace OptionsValidMatchDefault
+
+namespace OptionsValidMatchDifferentMethods {
+template 
+class MyContainer {
+public:
+  unsigned Size();
+  T *begin() const;
+  T *end() const;
+  void PushBack(const T &);
+  void Reserve(unsigned);
+};
+
+void foo(const MyContainer ) {
+  MyContainer CC2;
+  // CHECK-FIXES: {{^}}  CC2.Reserve(C.Size());
+  for (auto I : C) {
+CC2.PushBack(I);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'PushBack' is called
+  }
+}
+} // namespace OptionsValidMatchDifferentMethods
+
+namespace UnknownContainer {
+template 
+class MyUContainer {
+public:
+  unsigned size();
+  T *begin() const;
+  T *end() const;
+  void push_back(const T &);
+  void reserve(unsigned);
+};
+
+void foo(const MyUContainer ) {
+  // MyUContainer isn't specified as a VectorLikeClass in the Config Options
+  MyUContainer CC3;
+  // CHECK-FIXES-NOT: {{^}}  CC3.reserve(C.size());
+  for (auto I : C) {
+CC3.push_back(I);
+// CHECK-MESSAGES-NOT: :[[@LINE-1]]:5: warning: 'push_back' is called
+  }
+}
+} // namespace UnknownContainer
+
+namespace PrivateMethods {
+namespace Size {
+template 
+class MyContainer {
+  unsigned size();
+
+public:
+  T *begin() const;
+  T *end() const;
+  void push_back(const T &);
+  void reserve(unsigned);
+};
+
+void foo(const MyContainer ) {
+  // MyContainer::size is private, so calling it will be invalid
+  MyContainer CC4;
+  // CHECK-FIXES-NOT: {{^}}  CC4.reserve(C.size());
+  for (auto I : C) {
+CC4.push_back(I);
+// CHECK-MESSAGES-NOT: :[[@LINE-1]]:5: warning: 'push_back' is called
+  }
+}
+} // namespace Size
+namespace Reserve {
+template 
+class MyContainer {
+public:
+  unsigned size();
+  T *begin() const;
+  T *end() const;
+  void push_back(const T &);
+
+private:
+  void reserve(unsigned);
+};
+
+void foo(const MyContainer ) {
+  // MyContainer::reserve is private, so calling it will be invalid
+  MyContainer CC5;
+  // CHECK-FIXES-NOT: {{^}}  CC5.reserve(C.size());
+  for (auto I : C) {
+CC5.push_back(I);
+// CHECK-MESSAGES-NOT: :[[@LINE-1]]:5: warning: 'push_back' is called
+  }
+}
+} // namespace Reserve
+} // namespace PrivateMethods
+
+namespace BadSignatures {
+namespace Size {
+template 
+class MyContainer {
+public:
+  char *size();
+  T *begin() const;
+  T *end() const;
+  void push_back(const T &);
+  void reserve(unsigned);
+};
+
+void foo(const MyContainer ) {
+  // MyContainer::size doesn't return an integral type(char *), so ignore this class
+  MyContainer CC6;
+  // CHECK-FIXES-NOT: {{^}}  CC6.reserve(C.size());
+  for (auto I : C) {
+CC6.push_back(I);
+// CHECK-MESSAGES-NOT: :[[@LINE-1]]:5: warning: 'push_back' is called
+  }
+}
+} // namespace Size
+namespace 

[PATCH] D74689: [clang-tidy] Better custom class support for performance-inefficient-vector-operation

2020-02-16 Thread Nathan James via Phabricator via cfe-commits
njames93 created this revision.
Herald added subscribers: cfe-commits, xazax.hun.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D74689

Files:
  clang-tools-extra/clang-tidy/performance/InefficientVectorOperationCheck.cpp
  clang-tools-extra/clang-tidy/performance/InefficientVectorOperationCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/performance-inefficient-vector-operation.rst
  
clang-tools-extra/test/clang-tidy/checkers/performance-inefficient-vector-operation.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/performance-inefficient-vector-operation.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/performance-inefficient-vector-operation.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/performance-inefficient-vector-operation.cpp
@@ -1,7 +1,12 @@
 // RUN: %check_clang_tidy %s performance-inefficient-vector-operation %t -- \
 // RUN: -format-style=llvm \
 // RUN: -config='{CheckOptions: \
-// RUN:  [{key: performance-inefficient-vector-operation.EnableProto, value: 1}]}'
+// RUN:  [{key: performance-inefficient-vector-operation.EnableProto, value: 1}, \
+// RUN:   {key: performance-inefficient-vector-operation.VectorLikeClasses, value : MyContainer}, \
+// RUN:   {key: performance-inefficient-vector-operation.SupportedRanges, value : MyContainer}, \
+// RUN:   {key: performance-inefficient-vector-operation.ReserveNames, value : Reserve}, \
+// RUN:   {key: performance-inefficient-vector-operation.AppendNames, value : PushBack}, \
+// RUN:   {key: performance-inefficient-vector-operation.SizeNames, value : Size}, ]}'
 
 namespace std {
 
@@ -359,3 +364,160 @@
 }
   }
 }
+
+namespace OptionsValidMatchDefault {
+template 
+class MyContainer {
+public:
+  unsigned size();
+  T *begin() const;
+  T *end() const;
+  void push_back(const T &);
+  void reserve(unsigned);
+};
+
+void foo(const MyContainer ) {
+  MyContainer CC1;
+  // CHECK-FIXES: {{^}}  CC1.reserve(C.size());
+  for (auto I : C) {
+CC1.push_back(I);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'push_back' is called
+  }
+}
+} // namespace OptionsValidMatchDefault
+
+namespace OptionsValidMatchDifferentMethods {
+template 
+class MyContainer {
+public:
+  unsigned Size();
+  T *begin() const;
+  T *end() const;
+  void PushBack(const T &);
+  void Reserve(unsigned);
+};
+
+void foo(const MyContainer ) {
+  MyContainer CC2;
+  // CHECK-FIXES: {{^}}  CC2.Reserve(C.Size());
+  for (auto I : C) {
+CC2.PushBack(I);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'PushBack' is called
+  }
+}
+} // namespace OptionsValidMatchDifferentMethods
+
+namespace UnknownContainer {
+template 
+class MyUContainer {
+public:
+  unsigned size();
+  T *begin() const;
+  T *end() const;
+  void push_back(const T &);
+  void reserve(unsigned);
+};
+
+void foo(const MyUContainer ) {
+  // MyUContainer isn't specified as a VectorLikeClass in the Config Options
+  MyUContainer CC3;
+  // CHECK-FIXES-NOT: {{^}}  CC3.reserve(C.size());
+  for (auto I : C) {
+CC3.push_back(I);
+// CHECK-MESSAGES-NOT: :[[@LINE-1]]:5: warning: 'push_back' is called
+  }
+}
+} // namespace UnknownContainer
+
+namespace PrivateMethods {
+namespace Size {
+template 
+class MyContainer {
+  unsigned size();
+
+public:
+  T *begin() const;
+  T *end() const;
+  void push_back(const T &);
+  void reserve(unsigned);
+};
+
+void foo(const MyContainer ) {
+  // MyContainer::size is private, so calling it will be invalid
+  MyContainer CC4;
+  // CHECK-FIXES-NOT: {{^}}  CC4.reserve(C.size());
+  for (auto I : C) {
+CC4.push_back(I);
+// CHECK-MESSAGES-NOT: :[[@LINE-1]]:5: warning: 'push_back' is called
+  }
+}
+} // namespace Size
+namespace Reserve {
+template 
+class MyContainer {
+public:
+  unsigned size();
+  T *begin() const;
+  T *end() const;
+  void push_back(const T &);
+
+private:
+  void reserve(unsigned);
+};
+
+void foo(const MyContainer ) {
+  // MyContainer::reserve is private, so calling it will be invalid
+  MyContainer CC5;
+  // CHECK-FIXES-NOT: {{^}}  CC5.reserve(C.size());
+  for (auto I : C) {
+CC5.push_back(I);
+// CHECK-MESSAGES-NOT: :[[@LINE-1]]:5: warning: 'push_back' is called
+  }
+}
+} // namespace Reserve
+} // namespace PrivateMethods
+
+namespace BadSignatures {
+namespace Size {
+template 
+class MyContainer {
+public:
+  char *size();
+  T *begin() const;
+  T *end() const;
+  void push_back(const T &);
+  void reserve(unsigned);
+};
+
+void foo(const MyContainer ) {
+  // MyContainer::size doesn't return an integral type(char *), so ignore this class
+  MyContainer CC6;
+  // CHECK-FIXES-NOT: {{^}}  CC6.reserve(C.size());
+  for (auto I : C) {
+CC6.push_back(I);
+// CHECK-MESSAGES-NOT: :[[@LINE-1]]:5: warning: 'push_back' is called
+  }
+}
+} // namespace Size
+namespace Reserve {
+template 
+class MyContainer {
+public:
+  unsigned size();
+  T 

[PATCH] D74684: [Sema][C++] Adopt DR2171 relaxed triviality requirements

2020-02-16 Thread Nicholas Allegra via Phabricator via cfe-commits
comex created this revision.
comex added a reviewer: rsmith.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

When defining a copy constructor or assignment operator as either explicitly 
defaulted (`= default`) or deleted (`= delete`), it's possible to give it a 
different parameter list from the normal `(const Foo &)`.

For `= default`, the only allowed change [1] is to use a non-const reference 
instead of a const one:

struct X {

  X(X &) = default;

};

For `= delete`, it's also possible to add additional parameters with default 
values, as well as variadic parameters (`...`).

(Besides the parameter list, it's also possible to change the exception 
specification and ref-qualifiers, but those never affected triviality.)

Originally, C++11 (which introduced explicit defaulting) had a rule that any 
modification of the parameter list would prevent the function from being 
considered trivial, so e.g. is_trivially_copyable_v would be false.  
However, Defect Report 2171 [2] (2016) removed that rule entirely.  Up until 
now, that change hasn't been implemented in Clang; this patch implements it.

In addition, Clang currently applies a similar rule to deleted *default* 
constructors, which AFAICT is not in compliance with any published version of 
the spec.  So this fails when it should pass:

struct X {

  X(...) = delete;

};
static_assert(__has_trivial_constructor(X));

This patch also fixes that.

This is an ABI-breaking change, since the existence of trivial constructors 
affects whether a type is "trivial for the purposes of calls" [3].  I checked 
other compilers to see whether they implement the DR:

- GCC has never treated such functions as nontrivial, even before the DR. [4]
- MSVC currently doesn't treat them as nontrivial [5]; I haven't checked old 
versions since they're not on Godbolt.

Thus the change would improve Clang's compatibility with those compilers.

Implementation-wise, this patch is simple enough: it just removes the code in 
Sema::SpecialMemberIsTrivial that checked the parameter list.  In terms of 
tests, there were already a number of tests verifying the old behavior, so the 
patch merely updates them for the new behavior.

[1] https://eel.is/c++draft/dcl.fct.def.default#2
[2] http://www.open-std.org/Jtc1/sc22/wg21/docs/cwg_defects.html#2171
[3] https://quuxplusone.github.io/blog/2018/05/02/trivial-abi-101/
[4] https://gcc.godbolt.org/z/GU_wyz (note GCC version)
[5] https://gcc.godbolt.org/z/VYiMn3


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D74684

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CXX/class/class.union/p1.cpp
  clang/test/CXX/special/class.copy/p12-0x.cpp
  clang/test/CXX/special/class.copy/p25-0x.cpp
  clang/test/CXX/special/class.ctor/p5-0x.cpp

Index: clang/test/CXX/special/class.ctor/p5-0x.cpp
===
--- clang/test/CXX/special/class.ctor/p5-0x.cpp
+++ clang/test/CXX/special/class.ctor/p5-0x.cpp
@@ -175,30 +175,27 @@
 // has a trivial default constructor.
 ASSERT_NONTRIVIAL(NonTrivialDefCtor6, , NonTrivialDefCtor1 t;)
 
-// FIXME: No core issue number yet.
-// - its parameter-declaration-clause is equivalent to that of an implicit declaration.
-struct NonTrivialDefCtor7 {
-  NonTrivialDefCtor7(...) = delete;
+// Otherwise, the default constructor is non-trivial.
+struct Trivial2 {
+  Trivial2(...) = delete;
 };
-static_assert(!__has_trivial_constructor(NonTrivialDefCtor7), "");
-struct NonTrivialDefCtor8 {
-  NonTrivialDefCtor8(int = 0) = delete;
+static_assert(__has_trivial_constructor(Trivial2), "Trivial2 is trivial");
+struct Trivial3 {
+  Trivial3(int = 0) = delete;
 };
-static_assert(!__has_trivial_constructor(NonTrivialDefCtor8), "");
-
-// Otherwise, the default constructor is non-trivial.
+static_assert(__has_trivial_constructor(Trivial3), "Trivial3 is trivial");
 
-class Trivial2 { Trivial2() = delete; };
-static_assert(__has_trivial_constructor(Trivial2), "Trivial2 is trivial");
+class Trivial4 { Trivial4() = delete; };
+static_assert(__has_trivial_constructor(Trivial4), "Trivial4 is trivial");
 
-class Trivial3 { Trivial3() = default; };
-static_assert(__has_trivial_constructor(Trivial3), "Trivial3 is trivial");
+class Trivial5 { Trivial5() = default; };
+static_assert(__has_trivial_constructor(Trivial5), "Trivial5 is trivial");
 
-template class Trivial4 { Trivial4() = default; };
-static_assert(__has_trivial_constructor(Trivial4), "Trivial4 is trivial");
+template class Trivial6 { Trivial6() = default; };
+static_assert(__has_trivial_constructor(Trivial6), "Trivial6 is trivial");
 
-template class Trivial5 { Trivial5() = delete; };
-static_assert(__has_trivial_constructor(Trivial5), "Trivial5 is trivial");
+template class Trivial7 { Trivial7() = delete; };
+static_assert(__has_trivial_constructor(Trivial7), "Trivial7 is trivial");
 
 namespace PR14558 {
   // Ensure we determine whether 

[PATCH] D73865: [CodeGenModule] Assume dso_local for -fpic -fno-semantic-interposition

2020-02-16 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay planned changes to this revision.
MaskRay added a comment.

I am thinking whether we may need fine-grained 
`-fno-semantic-interposition-{data,function}`. I've noticed one problem with an 
internal target `_multiarray_umath.so` `npy_ma_str_array_finalize`. I need to 
make more investigation, but there is a chance that we can default to 
`-fno-semantic-interposition-function` but not 
`-fno-semantic-interposition-data`.

There is an issue that should be fixed in LTO. dso_local needs to be removed 
for a split LTO unit. Inferring `dso_local` when `-fPIC 
-fno-semantic-interposition` can cause problem to `-fsanitize=cfi 
-fsanitize-cfi-cross-dso -fsplit-lto-unit`. For my own note, 
`absl/base:sysinfo_test` can cause `error: relocation R_X86_64_PC32 cannot be 
used against symbol vtable for BaseArena; recompile with -fPIC`.

At least, before the LTO issue is fixed, we cannot assume dso_local for 
-fno-semantic-position (even if specified explicitly).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73865/new/

https://reviews.llvm.org/D73865



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits