[PATCH] D73425: [PPC] Fix platform definitions when compiling FreeBSD powerpc64 as LE

2020-01-25 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment.
Herald added a subscriber: wuzish.

At a minimum, `test/Driver/freebsd.c` and `test/Preprocessor/init-ppc64.c` 
should be updated for `powerpcle-unknown-freebsd` tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73425



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


[PATCH] D73425: [PPC] Fix platform definitions when compiling FreeBSD powerpc64 as LE

2020-01-25 Thread Brandon Bergren via Phabricator via cfe-commits
Bdragon28 created this revision.
Bdragon28 added a project: PowerPC.
Herald added subscribers: cfe-commits, steven.zhang, shchenz, jsji, 
krytarowski, arichardson, nemanjai, emaste.
Herald added a project: clang.

As a prerequisite to doing experimental buids of pieces of FreeBSD PowerPC64 as 
little-endian, allow actually targeting it.

This is needed so basic platform definitions are pulled in. Without it, the 
compiler will only run freestanding.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D73425

Files:
  clang/lib/Basic/Targets.cpp


Index: clang/lib/Basic/Targets.cpp
===
--- clang/lib/Basic/Targets.cpp
+++ clang/lib/Basic/Targets.cpp
@@ -352,6 +352,8 @@
 switch (os) {
 case llvm::Triple::Linux:
   return new LinuxTargetInfo(Triple, Opts);
+case llvm::Triple::FreeBSD:
+  return new FreeBSDTargetInfo(Triple, Opts);
 case llvm::Triple::NetBSD:
   return new NetBSDTargetInfo(Triple, Opts);
 default:


Index: clang/lib/Basic/Targets.cpp
===
--- clang/lib/Basic/Targets.cpp
+++ clang/lib/Basic/Targets.cpp
@@ -352,6 +352,8 @@
 switch (os) {
 case llvm::Triple::Linux:
   return new LinuxTargetInfo(Triple, Opts);
+case llvm::Triple::FreeBSD:
+  return new FreeBSDTargetInfo(Triple, Opts);
 case llvm::Triple::NetBSD:
   return new NetBSDTargetInfo(Triple, Opts);
 default:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D72553: [clang-tidy] Add performance-prefer-preincrement check

2020-01-25 Thread pre-merge checks [bot] via Phabricator via cfe-commits
merge_guards_bot added a comment.

{icon times-circle color=red} Unit tests: fail. 62197 tests passed, 1 failed 
and 815 were skipped.

  failed: 
libc++.std/thread/thread_mutex/thread_mutex_requirements/thread_timedmutex_requirements/thread_timedmutex_class/lock.pass.cpp

{icon times-circle color=red} clang-tidy: fail. clang-tidy found 0 errors and 1 
warnings 
.
 0 of them are added as review comments below (why? 
).

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 


//Pre-merge checks is in beta. Report issue 
.
 Please join beta  or enable 
it for your project 
.//


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72553



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


[PATCH] D72553: [clang-tidy] Add performance-prefer-preincrement check

2020-01-25 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 240402.
njames93 marked 5 inline comments as done.
njames93 added a comment.

- Address some nits, more test cases need adding though


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72553

Files:
  clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
  clang-tools-extra/clang-tidy/performance/CMakeLists.txt
  clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
  clang-tools-extra/clang-tidy/performance/PreferPreIncrementCheck.cpp
  clang-tools-extra/clang-tidy/performance/PreferPreIncrementCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/llvm-prefer-pre-increment.rst
  clang-tools-extra/docs/clang-tidy/checks/performance-prefer-pre-increment.rst
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/performance-prefer-pre-increment/iterator.h
  
clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment-disable-cpp-opcalls.cpp
  clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment.c
  
clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment.cpp
@@ -0,0 +1,45 @@
+// RUN: %check_clang_tidy %s performance-prefer-pre-increment %t -- -- \
+// RUN: -I%S/Inputs/performance-prefer-pre-increment
+
+#include "iterator.h"
+
+void foo() {
+  int Array[32];
+  Iterator It([0]);
+  It++;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use pre-increment instead of post-increment
+  // CHECK-FIXES: {{^}}  ++It;
+  It--;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use pre-decrement instead of post-decrement
+  // CHECK-FIXES: {{^}}  --It;
+  (*It)++;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use pre-increment instead of post-increment
+  // CHECK-FIXES: {{^}}  ++(*It);
+  (*It)--;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use pre-decrement instead of post-decrement
+  // CHECK-FIXES: {{^}}  --(*It);
+
+  *It++;
+  // CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use pre-increment instead of post-increment
+  *It--;
+  // CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use pre-decrement instead of post-decrement
+
+  PostfixIterator PfIt([0]);
+  PfIt++;
+  // CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use pre-increment instead of post-increment
+  // CHECK-FIXES-NOT: {{^}}  ++PfIt;
+  PfIt--;
+  // CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use pre-decrement instead of post-decrement
+  // CHECK-FIXES-NOT: {{^}}  --PfIt;
+  (*PfIt)++;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use pre-increment instead of post-increment
+  // CHECK-FIXES: {{^}}  ++(*PfIt);
+  (*PfIt)--;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: Use pre-decrement instead of post-decrement
+  // CHECK-FIXES: {{^}}  --(*PfIt);
+
+  *PfIt++;
+  // CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use pre-increment instead of post-increment
+  *PfIt--;
+  // CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use pre-decrement instead of post-decrement
+}
Index: clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment.c
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment.c
@@ -0,0 +1,128 @@
+// RUN: %check_clang_tidy %s performance-prefer-pre-increment %t
+
+#define INC(X) X++
+#define DEC(X) X--
+
+void foo(int A) {
+  for (int I = 0; I < 10; I++) {
+// CHECK-MESSAGES: [[@LINE-1]]:27: warning: Use pre-increment instead of post-increment
+// CHECK-FIXES: {{^}}  for (int I = 0; I < 10; ++I) {
+  }
+  for (int I = 0; I < 10; ++I) {
+// CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use pre-increment instead of post-increment
+  }
+  for (int I = 0; I < 10; A = I++) {
+// CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use pre-increment instead of post-increment
+  }
+
+  for (int I = 10; I < 0; I--) {
+// CHECK-MESSAGES: [[@LINE-1]]:27: warning: Use pre-decrement instead of post-decrement
+// CHECK-FIXES: {{^}}  for (int I = 10; I < 0; --I) {
+  }
+  for (int I = 10; I < 0; --I) {
+// CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use pre-decrement instead of post-decrement
+  }
+  for (int I = 10; I < 0; A = I--) {
+// CHECK-MESSAGES-NOT: [[@LINE-1]]:{{\d*}}: warning: Use pre-decrement instead of post-decrement
+  }
+
+  for (int I = 0; I < 10; INC(I)) {
+// CHECK-MESSAGES: [[@LINE-1]]:31: warning: Use pre-increment instead of post-increment
+  }
+  for (int I = 0; I < 10; DEC(I)) {
+// CHECK-MESSAGES: [[@LINE-1]]:31: warning: Use pre-decrement instead of post-decrement
+  }
+  for (int I = 0; I < 10; A = INC(I)) {
+   

[PATCH] D72553: [clang-tidy] Add performance-prefer-preincrement check

2020-01-25 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/performance/PreferPreIncrementCheck.cpp:63
+
+  if (!getLangOpts().CPlusPlus || !TransformCxxOpCalls)
+return;

JonasToth wrote:
> Why would you deactivate the check for c-code?
> I think the `for(int i = 0; i < 10; ++i)` could be a requirement in c-coding 
> styles, too. 
I only deactivate operator calls for c-code as they aren't a thing in c. The 
unary operation expr still runs in c-code



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment-disable-cpp-opcalls.cpp:44
+};
+
+void foo() {

JonasToth wrote:
> Test-cases that I would like to see:
> 
> - only the post-fix operator is overloaded for the class --> best-case this 
> is detected and a fix is not provided
> - iterator-inheritance: the base-class provides the operator-overloads --> 
> does matching work? There might be an implicit cast for example
> - the iterator-type is type-dependent --> maybe fixing should not be done or 
> even the warning should not be emitted, because there might be only a 
> post-fix available in some instantiations (see point 1). I do mean something 
> like this `template  void f() { T::iterator it; it++; }`
There are test cases for only post fix operator overloading. basically it 
doesn't warn or provided a fix it as that isn't valid. I feel like there could 
be a seperate check that detects classes that overload operator++(int) but not 
operator++() but thats not what this check is for.
I'll take a look at the other cases tomorrow


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72553



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


[PATCH] D73098: [clang-tidy] readability-identifier-naming disregards parameters restrictions on main like functions

2020-01-25 Thread pre-merge checks [bot] via Phabricator via cfe-commits
merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 62196 tests passed, 0 failed 
and 815 were skipped.

{icon check-circle color=green} clang-tidy: pass.

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 


//Pre-merge checks is in beta. Report issue 
.
 Please join beta  or enable 
it for your project 
.//


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73098



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


[PATCH] D73098: [clang-tidy] readability-identifier-naming disregards parameters restrictions on main like functions

2020-01-25 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 240400.
njames93 added a comment.

- Address nits


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73098

Files:
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
  clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-main-like.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-main-like.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-main-like.cpp
@@ -0,0 +1,88 @@
+// RUN: %check_clang_tidy %s readability-identifier-naming %t -- \
+// RUN:   -config='{CheckOptions: [ \
+// RUN: {key: readability-identifier-naming.ParameterCase, value: CamelCase}, \
+// RUN: {key: readability-identifier-naming.IgnoreMainLikeFunctions, value: 1} \
+// RUN:  ]}'
+
+int mainLike(int argc, char **argv);
+int mainLike(int argc, char **argv, const char **env);
+int mainLike(int argc, const char **argv);
+int mainLike(int argc, const char **argv, const char **env);
+int mainLike(int argc, char *argv[]);
+int mainLike(int argc, const char *argv[]);
+int mainLike(int argc, char *argv[], char *env[]);
+int mainLike(int argc, const char *argv[], const char *env[]);
+void notMain(int argc, char **argv);
+// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: invalid case style for parameter 'argc'
+// CHECK-MESSAGES: :[[@LINE-2]]:31: warning: invalid case style for parameter 'argv'
+void notMain(int argc, char **argv, char **env);
+// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: invalid case style for parameter 'argc'
+// CHECK-MESSAGES: :[[@LINE-2]]:31: warning: invalid case style for parameter 'argv'
+// CHECK-MESSAGES: :[[@LINE-3]]:44: warning: invalid case style for parameter 'env'
+int notMain(int argc, char **argv, char **env, int Extra);
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: invalid case style for parameter 'argc'
+// CHECK-MESSAGES: :[[@LINE-2]]:30: warning: invalid case style for parameter 'argv'
+// CHECK-MESSAGES: :[[@LINE-3]]:43: warning: invalid case style for parameter 'env'
+int notMain(int argc, char **argv, int Extra);
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: invalid case style for parameter 'argc'
+// CHECK-MESSAGES: :[[@LINE-2]]:30: warning: invalid case style for parameter 'argv'
+int notMain(int argc, char *argv);
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: invalid case style for parameter 'argc'
+// CHECK-MESSAGES: :[[@LINE-2]]:29: warning: invalid case style for parameter 'argv'
+int notMain(unsigned argc, char **argv);
+// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: invalid case style for parameter 'argc'
+// CHECK-MESSAGES: :[[@LINE-2]]:35: warning: invalid case style for parameter 'argv'
+int notMain(long argc, char *argv);
+// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: invalid case style for parameter 'argc'
+// CHECK-MESSAGES: :[[@LINE-2]]:30: warning: invalid case style for parameter 'argv'
+int notMain(int argc, char16_t **argv);
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: invalid case style for parameter 'argc'
+// CHECK-MESSAGES: :[[@LINE-2]]:34: warning: invalid case style for parameter 'argv'
+int notMain(int argc, char argv[]);
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: invalid case style for parameter 'argc'
+// CHECK-MESSAGES: :[[@LINE-2]]:28: warning: invalid case style for parameter 'argv'
+typedef char myFunChar;
+typedef int myFunInt;
+typedef char **myFunCharPtr;
+typedef long myFunLong;
+myFunInt mainLikeTypedef(myFunInt argc, myFunChar **argv);
+int mainLikeTypedef(int argc, myFunCharPtr argv);
+int notMainTypedef(myFunLong argc, char **argv);
+// CHECK-MESSAGES: :[[@LINE-1]]:30: warning: invalid case style for parameter 'argc'
+// CHECK-MESSAGES: :[[@LINE-2]]:43: warning: invalid case style for parameter 'argv'
+
+// Don't flag as name contains the word main
+int myMainFunction(int argc, char *argv[]);
+
+// This is fine, named with wmain and has wchar ptr.
+int wmainLike(int argc, wchar_t *argv[]);
+
+// Flag this as has signature of main, but named as wmain.
+int wmainLike(int argc, char *argv[]);
+// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: invalid case style for parameter 'argc'
+// CHECK-MESSAGES: :[[@LINE-2]]:31: warning: invalid case style for parameter 'argv'
+
+struct Foo {
+  Foo(int argc, char *argv[]) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: invalid case style for parameter 'argc'
+  // CHECK-MESSAGES: :[[@LINE-2]]:23: warning: invalid case style for parameter 'argv'
+
+  int mainPub(int argc, char *argv[]);
+  static int mainPubStatic(int argc, char *argv[]);
+
+protected:
+  int mainProt(int argc, char *argv[]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: invalid case style for parameter 'argc'
+  // 

[PATCH] D73418: [WPD] Emit vcall_visibility metadata for MicrosoftCXXABI

2020-01-25 Thread Teresa Johnson via Phabricator via cfe-commits
tejohnson updated this revision to Diff 240395.
tejohnson added a comment.

Improve test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73418

Files:
  clang/lib/CodeGen/MicrosoftCXXABI.cpp
  clang/test/CodeGenCXX/vcall-visibility-metadata.cpp


Index: clang/test/CodeGenCXX/vcall-visibility-metadata.cpp
===
--- clang/test/CodeGenCXX/vcall-visibility-metadata.cpp
+++ clang/test/CodeGenCXX/vcall-visibility-metadata.cpp
@@ -1,5 +1,6 @@
 // RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -emit-llvm 
-fvirtual-function-elimination -fwhole-program-vtables -o - %s | FileCheck %s 
--check-prefix=CHECK --check-prefix=CHECK-VFE
 // RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -emit-llvm 
-fwhole-program-vtables -o - %s | FileCheck %s --check-prefix=CHECK 
--check-prefix=CHECK-NOVFE
+// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-pc-windows-msvc -emit-llvm 
-fwhole-program-vtables -o - %s | FileCheck %s --check-prefix=CHECK-MS 
--check-prefix=CHECK-NOVFE
 
 // Check that in ThinLTO we also get vcall_visibility summary entries in the 
bitcode
 // RUN: %clang_cc1 -flto=thin -flto-unit -triple x86_64-unknown-linux 
-emit-llvm-bc -fwhole-program-vtables -o - %s | llvm-dis -o - | FileCheck %s 
--check-prefix=CHECK --check-prefix=CHECK-NOVFE --check-prefix=CHECK-SUMMARY
@@ -8,6 +9,7 @@
 // Anonymous namespace.
 namespace {
 // CHECK: @_ZTVN12_GLOBAL__N_11AE = {{.*}} !vcall_visibility [[VIS_TU:![0-9]+]]
+// CHECK-MS: @anon.{{.*}} = private unnamed_addr constant 
{{.*}}struct.(anonymous namespace)::A{{.*}} !vcall_visibility [[VIS_TU:![0-9]+]]
 struct A {
   A() {}
   virtual int f() { return 1; }
@@ -20,6 +22,7 @@
 
 // Hidden visibility.
 // CHECK: @_ZTV1B = {{.*}} !vcall_visibility [[VIS_DSO:![0-9]+]]
+// CHECK-MS: @anon.{{.*}} = private unnamed_addr constant {{.*}}struct.B{{.*}} 
!vcall_visibility [[VIS_DSO:![0-9]+]]
 struct __attribute__((visibility("hidden"))) B {
   B() {}
   virtual int f() { return 1; }
@@ -31,6 +34,8 @@
 
 // Default visibility.
 // CHECK-NOT: @_ZTV1C = {{.*}} !vcall_visibility
+// On MS default is hidden
+// CHECK-MS: @anon.{{.*}} = private unnamed_addr constant {{.*}}struct.C{{.*}} 
!vcall_visibility [[VIS_DSO]]
 struct __attribute__((visibility("default"))) C {
   C() {}
   virtual int f() { return 1; }
@@ -42,6 +47,7 @@
 
 // Hidden visibility, public LTO visibility.
 // CHECK-NOT: @_ZTV1D = {{.*}} !vcall_visibility
+// CHECK-MS-NOT: @anon.{{.*}} = private unnamed_addr constant 
{{.*}}struct.D{{.*}} !vcall_visibility
 struct __attribute__((visibility("hidden"))) [[clang::lto_visibility_public]] 
D {
   D() {}
   virtual int f() { return 1; }
@@ -53,6 +59,8 @@
 
 // Hidden visibility, but inherits from class with default visibility.
 // CHECK-NOT: @_ZTV1E = {{.*}} !vcall_visibility
+// On MS default is hidden
+// CHECK-MS: @anon.{{.*}} = private unnamed_addr constant {{.*}}struct.E{{.*}} 
!vcall_visibility [[VIS_DSO]]
 struct __attribute__((visibility("hidden"))) E : C {
   E() {}
   virtual int f() { return 1; }
@@ -64,6 +72,8 @@
 
 // Anonymous namespace, but inherits from class with default visibility.
 // CHECK-NOT: @_ZTVN12_GLOBAL__N_11FE = {{.*}} !vcall_visibility
+// On MS default is hidden
+// CHECK-MS: @anon.{{.*}} = private unnamed_addr constant 
{{.*}}struct.(anonymous namespace)::F{{.*}} !vcall_visibility [[VIS_DSO]]
 namespace {
 struct __attribute__((visibility("hidden"))) F : C {
   F() {}
@@ -77,6 +87,7 @@
 
 // Anonymous namespace, but inherits from class with hidden visibility.
 // CHECK: @_ZTVN12_GLOBAL__N_11GE = {{.*}} !vcall_visibility 
[[VIS_DSO:![0-9]+]]
+// CHECK-MS: @anon.{{.*}} = private unnamed_addr constant 
{{.*}}struct.(anonymous namespace)::G{{.*}} !vcall_visibility [[VIS_DSO]]
 namespace {
 struct __attribute__((visibility("hidden"))) G : B {
   G() {}
@@ -87,6 +98,8 @@
   return new G();
 }
 
+// CHECK-MS-DAG: [[VIS_DSO]] = !{i64 1}
+// CHECK-MS-DAG: [[VIS_TU]] = !{i64 2}
 // CHECK-DAG: [[VIS_DSO]] = !{i64 1}
 // CHECK-DAG: [[VIS_TU]] = !{i64 2}
 // CHECK-VFE-DAG: !{i32 1, !"Virtual Function Elim", i32 1}
Index: clang/lib/CodeGen/MicrosoftCXXABI.cpp
===
--- clang/lib/CodeGen/MicrosoftCXXABI.cpp
+++ clang/lib/CodeGen/MicrosoftCXXABI.cpp
@@ -1621,6 +1621,15 @@
   if (!CGM.getCodeGenOpts().LTOUnit)
 return;
 
+  // TODO: Should VirtualFunctionElimination also be supported here?
+  // See similar handling in CodeGenModule::EmitVTableTypeMetadata.
+  if (CGM.getCodeGenOpts().WholeProgramVTables) {
+llvm::GlobalObject::VCallVisibility TypeVis =
+CGM.GetVCallVisibilityLevel(RD);
+if (TypeVis != llvm::GlobalObject::VCallVisibilityPublic)
+  VTable->setVCallVisibilityMetadata(TypeVis);
+  }
+
   // The location of the first virtual function pointer in the virtual table,
   // aka the "address point" on 

[PATCH] D73418: [WPD] Emit vcall_visibility metadata for MicrosoftCXXABI

2020-01-25 Thread Teresa Johnson via Phabricator via cfe-commits
tejohnson added a comment.

I fixed the test matching as I found that the names generated for the anonymous 
namespace comdats depend on the module identifier in some way. I changed the 
matching to look at the struct type embedded in the initializer, which is also 
more intuitive. I also moved the new MS checks to be alongside the companion 
Itanium checks. And I added comments noting that MSVC gets DSO (hidden) 
visibility by default.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73418



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


[clang] 713562f - [Concepts] Transform constraints of non-template functions to ConstantEvaluated

2020-01-25 Thread Saar Raz via cfe-commits

Author: Saar Raz
Date: 2020-01-25T23:00:24+02:00
New Revision: 713562f54858f10bf8998ee21ff2c7e7bad0d177

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

LOG: [Concepts] Transform constraints of non-template functions to 
ConstantEvaluated

We would previously try to evaluate atomic constraints of non-template 
functions as-is,
and since they are now unevaluated at first, this would cause incorrect 
evaluation (bugs #44657, #44656).

Substitute into atomic constraints of non-template functions as we would atomic 
constraints
of template functions, in order to rebuild the expressions in a 
constant-evaluated context.

Added: 


Modified: 
clang/include/clang/AST/ASTConcept.h
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Sema/Sema.h
clang/lib/AST/ASTConcept.cpp
clang/lib/Sema/SemaConcept.cpp
clang/lib/Sema/SemaExpr.cpp
clang/lib/Sema/SemaExprCXX.cpp
clang/lib/Sema/SemaOverload.cpp
clang/lib/Sema/SemaTemplateInstantiate.cpp
clang/test/SemaTemplate/cxx2a-constraint-exprs.cpp

Removed: 




diff  --git a/clang/include/clang/AST/ASTConcept.h 
b/clang/include/clang/AST/ASTConcept.h
index 30c4706d2a15..3ebaad4eafdd 100644
--- a/clang/include/clang/AST/ASTConcept.h
+++ b/clang/include/clang/AST/ASTConcept.h
@@ -29,14 +29,14 @@ class ConceptSpecializationExpr;
 class ConstraintSatisfaction : public llvm::FoldingSetNode {
   // The template-like entity that 'owns' the constraint checked here (can be a
   // constrained entity or a concept).
-  NamedDecl *ConstraintOwner = nullptr;
+  const NamedDecl *ConstraintOwner = nullptr;
   llvm::SmallVector TemplateArgs;
 
 public:
 
   ConstraintSatisfaction() = default;
 
-  ConstraintSatisfaction(NamedDecl *ConstraintOwner,
+  ConstraintSatisfaction(const NamedDecl *ConstraintOwner,
  ArrayRef TemplateArgs) :
   ConstraintOwner(ConstraintOwner), TemplateArgs(TemplateArgs.begin(),
  TemplateArgs.end()) { }
@@ -57,7 +57,7 @@ class ConstraintSatisfaction : public llvm::FoldingSetNode {
   }
 
   static void Profile(llvm::FoldingSetNodeID , const ASTContext ,
-  NamedDecl *ConstraintOwner,
+  const NamedDecl *ConstraintOwner,
   ArrayRef TemplateArgs);
 };
 

diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index b49c222c8fb7..5e88c6ee86ab 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -4686,6 +4686,8 @@ def note_checking_constraints_for_var_spec_id_here : Note<
 def note_checking_constraints_for_class_spec_id_here : Note<
   "while checking constraint satisfaction for class template partial "
   "specialization '%0' required here">;
+def note_checking_constraints_for_function_here : Note<
+  "while checking constraint satisfaction for function '%0' required here">;
 def note_constraint_substitution_here : Note<
   "while substituting template arguments into constraint expression here">;
 def note_constraint_normalization_here : Note<

diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 5ab74e4cd662..64a6793aa411 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -6288,7 +6288,7 @@ class Sema final {
   /// \returns true if an error occurred and satisfaction could not be checked,
   /// false otherwise.
   bool CheckConstraintSatisfaction(
-  NamedDecl *Template, ArrayRef ConstraintExprs,
+  const NamedDecl *Template, ArrayRef ConstraintExprs,
   ArrayRef TemplateArgs,
   SourceRange TemplateIDRange, ConstraintSatisfaction );
 
@@ -6301,6 +6301,17 @@ class Sema final {
   bool CheckConstraintSatisfaction(const Expr *ConstraintExpr,
ConstraintSatisfaction );
 
+  /// Check whether the given function decl's trailing requires clause is
+  /// satisfied, if any. Returns false and updates Satisfaction with the
+  /// satisfaction verdict if successful, emits a diagnostic and returns true 
if
+  /// an error occured and satisfaction could not be determined.
+  ///
+  /// \returns true if an error occurred, false otherwise.
+  bool CheckFunctionConstraints(const FunctionDecl *FD,
+ConstraintSatisfaction ,
+SourceLocation UsageLoc = SourceLocation());
+
+
   /// \brief Ensure that the given template arguments satisfy the constraints
   /// associated with the given template, emitting a diagnostic if they do not.
   ///

diff  --git a/clang/lib/AST/ASTConcept.cpp b/clang/lib/AST/ASTConcept.cpp
index c28a06bdf0b2..549088ad4a8a 100644
--- 

[PATCH] D73203: [clang-tidy] Prevent a remove only fixit causing a conflict when enclosed by another fixit

2020-01-25 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment.

We need tests for that. There should be tests for the exporting-feature. Maybe 
they could provide a good starting point on adding tests. (not sure if there is 
unit-test code for isolated testing, but some tests do exist!)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73203



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


[PATCH] D72553: [clang-tidy] Add performance-prefer-preincrement check

2020-01-25 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment.

Yeah, the libc++ failures are not your fault :)
My 2 cents added.




Comment at: 
clang-tools-extra/clang-tidy/performance/PreferPreIncrementCheck.cpp:63
+
+  if (!getLangOpts().CPlusPlus || !TransformCxxOpCalls)
+return;

Why would you deactivate the check for c-code?
I think the `for(int i = 0; i < 10; ++i)` could be a requirement in c-coding 
styles, too. 



Comment at: 
clang-tools-extra/clang-tidy/performance/PreferPreIncrementCheck.cpp:86
+  DiagnosticBuilder Diag =
+  diag(Range.getBegin(), "Use Pre-%0 instead of Post-%0")
+  << (IsIncrement ? "increment" : "decrement");

i think `pre` and `post` instead of capitalized.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment-disable-cpp-opcalls.cpp:11
+  Iterator(T *Pointer) : Current(Pointer) {}
+  T *() const { return *Current; }
+  Iterator ++() { return ++Current, *this; }

Please extract the common iterator class into a header and include it in the 
tests. this is done in other tests as well, just grep for `include`.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/performance-prefer-pre-increment-disable-cpp-opcalls.cpp:44
+};
+
+void foo() {

Test-cases that I would like to see:

- only the post-fix operator is overloaded for the class --> best-case this is 
detected and a fix is not provided
- iterator-inheritance: the base-class provides the operator-overloads --> does 
matching work? There might be an implicit cast for example
- the iterator-type is type-dependent --> maybe fixing should not be done or 
even the warning should not be emitted, because there might be only a post-fix 
available in some instantiations (see point 1). I do mean something like this 
`template  void f() { T::iterator it; it++; }`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72553



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


[PATCH] D72810: [LifetimeAnalysis] Add support for lifetime annotations on functions

2020-01-25 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

Thank you for the work on this!




Comment at: clang/include/clang/AST/LifetimeAttr.h:24
+/// An abstract memory location that participates in defining a lifetime
+/// contract. A lifetime contract constrains lifetime of a
+/// LifetimeContractVariable to be at least as big as the lifetime of other

constrains lifetime -> constrains the lifetime



Comment at: clang/include/clang/AST/LifetimeAttr.h:25
+/// contract. A lifetime contract constrains lifetime of a
+/// LifetimeContractVariable to be at least as big as the lifetime of other
+/// LifetimeContractVariables.

big -> long



Comment at: clang/include/clang/AST/LifetimeAttr.h:29
+/// The memory locations that we can describe are: return values of a function,
+/// this pointer, any function parameter, a "drilldown" expression based on
+/// function parameters, null etc.

What is a drilldown expression? That's not a term I'm familiar with.



Comment at: clang/include/clang/AST/LifetimeAttr.h:70
+  bool operator<(const LifetimeContractVariable ) const {
+if (Tag != O.Tag)
+  return Tag < O.Tag;

I think it would be easier to read if the pattern was: `if (Foo < Bar) return 
true;` as opposed to checking inequality before returning the comparison.



Comment at: clang/include/clang/AST/LifetimeAttr.h:79
+  if (RD != O.RD)
+return std::less()(RD, O.RD);
+

This will have non deterministic behavior between program executions -- are you 
sure that's what we want? Similar below for fields.



Comment at: clang/include/clang/AST/LifetimeAttr.h:153-154
+
+  LifetimeContractVariable(TagType T) : Tag(T) {}
+  LifetimeContractVariable(const RecordDecl *RD) : RD(RD), Tag(This) {}
+  LifetimeContractVariable(const ParmVarDecl *PVD, int Deref)

These constructors should probably be `explicit`?



Comment at: clang/include/clang/AST/LifetimeAttr.h:163
+// Maps each annotated entity to a lifetime set.
+using LifetimeContracts = std::map;
+

xazax.hun wrote:
> gribozavr2 wrote:
> > Generally, DenseMap and DenseSet are faster. Any reason to not use them 
> > instead?
> No specific reason other than I am always insecure about the choice. Dense 
> data structures are great for small objects and I do not know where the break 
> even point is and never really did any benchmarks. Is there some guidelines 
> somewhere for what object size should we prefer one over the other?
I usually figure that if it's less than two pointers in size, it's sufficiently 
small, but maybe others have a different opinion. This class is probably a bit 
large for using dense containers.

I do worry a little bit about the hoops we're jumping through to make the class 
orderable -- is there a reason to avoid an unordered container instead?



Comment at: clang/include/clang/Basic/Attr.td:2902
 
+def LifetimeContract : InheritableAttr {
+  let Spellings = [CXX11<"gsl", "pre">, CXX11<"gsl", "post">];

I think the attribute should probably only be supported in C++ for now, and 
should have `let LangOpts = [CPlusPlus];` in the declaration. WDYT?



Comment at: clang/include/clang/Basic/Attr.td:2941
+  }];
+  let Documentation = [Undocumented]; // FIXME
+}

I agree with the FIXME comment. ;-)



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:3342
+  InGroup, DefaultIgnore;
+def warn_dump_lifetime_contracts : Warning<"%0">, 
InGroup, DefaultIgnore;
+

xazax.hun wrote:
> gribozavr2 wrote:
> > xazax.hun wrote:
> > > gribozavr2 wrote:
> > > > I think a warning that is a debugging aid is new territory.
> > > Do you think a cc1 flag would be sufficient or do you have other 
> > > ideas/preferences?
> > Yes, I think that would be quite standard (similar to dumping the AST, 
> > dumping the CFG etc.)
> I started to look into a cc1 flag, but it looks like it needs a bit more 
> plumbing I expected as some does not have access to the CompilerInvocation 
> object or the FrontendOptions. While it is not too hard to pass an additional 
> boolean to Sema I also started to think about the user/developer experience 
> (ok, regular users are not expected to do debugging). The advantage of 
> warnings are that we get a source location for free, so it is really easy to 
> see where the message is originated from. And while it is true that I cannot 
> think of any examples of using warnings for debugging the Clang Static 
> Analyzer is full of checks that are only dumping debug data as warnings.
I also prefer a cc1 flag -- using warnings to control debugging behavior is 
novel and doesn't seem like something we should encourage. I don't think you 
would need to pass any additional flags to Sema however; I would expect this to 
show up in a 

[PATCH] D73007: [Sema] Avoid Wrange-loop-analysis false positives

2020-01-25 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D73007#1839648 , @aaronpuchert 
wrote:

> Here is a proposal: we add two children to `-Wrange-loop-analysis`.
>
> - `-Wrange-loop-construct` warns about possibly unintended constructor calls. 
> This might be in `-Wall`. It contains
>   - `warn_for_range_copy`: loop variable A of type B creates a copy from type 
> C
>   - `warn_for_range_const_reference_copy`: loop variable A  is initialized 
> with a value of a different type resulting in a copy
> - `-Wrange-loop-bind-ref[erence]` warns about misleading use of reference 
> types. This might **not** be in `-Wall`. It contains
>   - `warn_for_range_variable_always_copy`: loop variable A is always a copy 
> because the range of type B does not return a reference


I think this seems like a reasonable approach.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73007



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


[PATCH] D73098: [clang-tidy] readability-identifier-naming disregards parameters restrictions on main like functions

2020-01-25 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM aside from some minor nits.




Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:340
+return false;
+  if (FDecl->getAccess() == AS_private || FDecl->getAccess() == AS_protected)
+return false;

I'd flip the logic to `!= AS_public` to be more clear that we only care about 
public members.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst:833
+
+When set to ``1`` functions that have a similar signature to ``main`` won't
+enforce checks on the names of their parameters.

We should document that this defaults to `0` and mention `wmain` as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73098



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


[PATCH] D72448: [clang-tidy] readability-redundant-string-init now flags redundant initialisation in Field Decls and Constructor Initialisers

2020-01-25 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/readability/RedundantStringInitCheck.cpp:58
+  }
+  if (B.isInvalid() || E.isInvalid())
+return llvm::None;

Should we be worried about macros here?

It looks a bit like we're ignoring macros entirely for this check, so maybe 
that can be done as a separate patch instead. The situation I am worried about 
is:
```
#if SOMETHING
#define INIT ""
#else
#define INIT "haha"
#endif

std::string S = INIT;
```



Comment at: 
clang-tools-extra/clang-tidy/readability/RedundantStringInitCheck.cpp:110
   namedDecl(
-  varDecl(
-  hasType(hasUnqualifiedDesugaredType(recordType(
-  hasDeclaration(cxxRecordDecl(hasStringTypeName),
-  hasInitializer(expr(ignoringImplicit(anyOf(
-  EmptyStringCtorExpr, EmptyStringCtorExprWithTemporaries)
-  .bind("vardecl"),
+  varDecl(StringType, hasInitializer(EmptyStringInit)).bind("vardecl"),
   unless(parmVarDecl())),

Should this also match on something like `std::string foo{};` as a redundant 
init? Similar question for the other cases. (Could be done in a follow-up patch 
if desired.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72448



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


[PATCH] D73090: [clang-tidy] Fix PR#44528 'modernize-use-using and enums'

2020-01-25 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp:28
  this);
-  // This matcher used to find structs defined in source code within typedefs.
+  // This matcher`s used to find structs/enums defined in source code within 
typedefs.
   // They appear in the AST just *prior* to the typedefs.

It looks like there's a backtick in the comment rather than a quotation mark, 
that should probably be `matcher's` instead. Also, instead of `struct/enums`, I 
think it should be `tag declarations` (unions count, for instance).



Comment at: clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp:30-31
   // They appear in the AST just *prior* to the typedefs.
-  Finder->addMatcher(cxxRecordDecl(unless(isImplicit())).bind("struct"), this);
+  Finder->addMatcher(cxxRecordDecl(unless(isImplicit())).bind("tagdecl"), 
this);
+  Finder->addMatcher(enumDecl(unless(isImplicit())).bind("tagdecl"), this);
 }

Rather than matching on these, I think it would make more sense to add a new 
matcher for `tagDecl()`. It can be local to the check for now, or you can add 
it to the AST matchers header as a separate patch and then base this patch off 
that work.


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

https://reviews.llvm.org/D73090



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


[PATCH] D65994: Extended FPOptions with new attributes

2020-01-25 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM, but I'll echo what @rjmccall said about this level of granularity being a 
bit too fine for review for future patches (we expect changes to be testable, 
which this is, but only if you also review other patches and notice the tests 
and how they relate to this patch, which can be easy for reviewers to miss).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65994



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


[PATCH] D73413: [clang-tidy] Add check to detect external definitions with no header declaration

2020-01-25 Thread pre-merge checks [bot] via Phabricator via cfe-commits
merge_guards_bot added a comment.

{icon times-circle color=red} Unit tests: fail. 62128 tests passed, 5 failed 
and 807 were skipped.

  failed: libc++.std/language_support/cmp/cmp_partialord/partialord.pass.cpp
  failed: libc++.std/language_support/cmp/cmp_strongeq/cmp.strongeq.pass.cpp
  failed: libc++.std/language_support/cmp/cmp_strongord/strongord.pass.cpp
  failed: libc++.std/language_support/cmp/cmp_weakeq/cmp.weakeq.pass.cpp
  failed: libc++.std/language_support/cmp/cmp_weakord/weakord.pass.cpp

{icon times-circle color=red} clang-tidy: fail. clang-tidy found 0 errors and 1 
warnings 
.
 0 of them are added as review comments below (why? 
).

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 


//Pre-merge checks is in beta. Report issue 
.
 Please join beta  or enable 
it for your project 
.//


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73413



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


[PATCH] D73413: [clang-tidy] Add check to detect external definitions with no header declaration

2020-01-25 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 240387.
njames93 added a comment.

- Warn extern declarations in source files


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73413

Files:
  clang-tools-extra/clang-tidy/misc/CMakeLists.txt
  clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
  clang-tools-extra/clang-tidy/misc/MissingHeaderFileDeclarationCheck.cpp
  clang-tools-extra/clang-tidy/misc/MissingHeaderFileDeclarationCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/docs/clang-tidy/checks/misc-missing-header-file-declaration.rst
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/misc-missing-header-file-declaration/misc-missing-header-file-declaration.cpp.tmp.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/misc-missing-header-file-declaration/wrong_header.h
  
clang-tools-extra/test/clang-tidy/checkers/misc-missing-header-file-declaration-any-header.cpp
  
clang-tools-extra/test/clang-tidy/checkers/misc-missing-header-file-declaration.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/misc-missing-header-file-declaration.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc-missing-header-file-declaration.cpp
@@ -0,0 +1,91 @@
+// RUN: %check_clang_tidy %s misc-missing-header-file-declaration %t -- \
+// RUN: -- -I%S/Inputs/misc-missing-header-file-declaration
+
+#include "misc-missing-header-file-declaration.cpp.tmp.h"
+// Include file has to be named *.cpp.tmp.h to account for the script renaming
+// this file to *.cpp.tmp.cpp.
+// Outside this test environment it will work normally
+//  -> 
+#include "wrong_header.h"
+
+// These declarations should be ignored by the check as they are in the same
+// file.
+extern bool DeclInSource;
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: Variable 'DeclInSource' is declared as extern in a source file
+extern void declInSource();
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: Function 'declInSource' is declared as extern in a source file
+
+// These declarations should be ignored by the check as they are in the same
+// file, however there is a corresponding decl in the header that will prevent
+// a failing check.
+extern bool DeclInBoth;
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: Variable 'DeclInBoth' is declared as extern in a source file
+extern void declInBoth();
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: Function 'declInBoth' is declared as extern in a source file
+
+// No external linkage so no warning
+static bool StaticOK = false;
+constexpr bool ConstexprOK = false;
+inline void inlineOK() {}
+static void staticOK() {}
+constexpr bool constexprOK() { return true; }
+
+// External linkage but decl in header so no warning
+bool DeclInHeader = false;
+bool DeclInBoth = false;
+void declInHeader() {}
+void declInBoth() {}
+
+//Decls don't appear in corresponding header so issue a warning
+bool DeclInSource = false;
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Variable 'DeclInSource' is defined with external linkage
+bool NoDecl = false;
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Variable 'NoDecl' is defined with external linkage
+void declInSource() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Function 'declInSource' is defined with external linkage
+void noDecl() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Function 'noDecl' is defined with external linkage
+
+bool DeclInWrongHeader = false;
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Variable 'DeclInWrongHeader' is defined with external linkage
+void declInWrongHeader() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Function 'declInWrongHeader' is defined with external linkage
+
+// Decls in an anonymous namespace don't have external linkage, so no warning
+// should be emitted
+namespace {
+bool AnonNS = false;
+void anonNS() {}
+} // namespace
+
+// Ensure in namespace definitions are correctly resolved
+namespace ns1 {
+bool NS = false;
+void nS() {}
+} // namespace ns1
+
+// Ensure out of namespace definitions are correctly resolved
+bool /*namespace*/ ns2::NS = false;
+void /*namespace*/ ns2::nS() {}
+
+// Static class members declared in the header shouldn't be warned on.
+int /*struct*/ Foo::Bar = 0;
+
+// main is special, don't warn for it.
+int main() {
+}
+
+template 
+void templateFuncNoHeader() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Function 'templateFuncNoHeader' is defined with external linkage
+
+// Warn on explicit instantiations
+template <>
+void templateFuncNoHeader() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Function 'templateFuncNoHeader' is defined with external linkage
+
+static void foo() {
+  // We don't want warnings for these implicit instantations
+  templateFuncNoHeader();
+  templateFuncNoHeader();
+}
Index: 

[clang] 0f34ea5 - [perf-training] Update ' (in-process)' prefix handling

2020-01-25 Thread Francis Visoiu Mistrih via cfe-commits

Author: Francis Visoiu Mistrih
Date: 2020-01-25T09:14:24-08:00
New Revision: 0f34ea5dc3cb3efea12dac1fa28b4d3db0cebc75

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

LOG: [perf-training] Update ' (in-process)' prefix handling

A recent change added a new line after the prefix, so it's now part of
the prefix list.

Added: 


Modified: 
clang/utils/perf-training/perf-helper.py

Removed: 




diff  --git a/clang/utils/perf-training/perf-helper.py 
b/clang/utils/perf-training/perf-helper.py
index 58eef65c6e73..88708a92712a 100644
--- a/clang/utils/perf-training/perf-helper.py
+++ b/clang/utils/perf-training/perf-helper.py
@@ -123,6 +123,7 @@ def get_cc1_command_for_args(cmd, env):
   ln.startswith('Thread model:') or
   ln.startswith('InstalledDir:') or
   ln.startswith('LLVM Profile Note') or
+  ln.startswith(' (in-process)') or
   ' version ' in ln):
   continue
   cc_commands.append(ln)
@@ -131,15 +132,7 @@ def get_cc1_command_for_args(cmd, env):
   print('Fatal error: unable to determine cc1 command: %r' % cc_output)
   exit(1)
 
-  cc_command = cc_commands[0]
-
-  # When cc1 runs in the same process as the driver, it prefixes the cc1
-  # invocation with ' (in-process)'. Skip it.
-  skip_pfx_line = ' (in-process)'
-  if cc_command.startswith(skip_pfx_line):
-  cc_command = cc_command[len(skip_pfx_line):]
-
-  cc1_cmd = shlex.split(cc_command)
+  cc1_cmd = shlex.split(cc_commands[0])
   if not cc1_cmd:
   print('Fatal error: unable to determine cc1 command: %r' % cc_output)
   exit(1)



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


[PATCH] D71566: New checks for fortified sprintf

2020-01-25 Thread serge via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG6d485ff455ea: Improve static checks for sprintf and 
__builtin___sprintf_chk (authored by serge-sans-paille).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71566

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/warn-fortify-source.c

Index: clang/test/Sema/warn-fortify-source.c
===
--- clang/test/Sema/warn-fortify-source.c
+++ clang/test/Sema/warn-fortify-source.c
@@ -11,6 +11,8 @@
 extern "C" {
 #endif
 
+extern int sprintf(char *str, const char *format, ...);
+
 #if defined(USE_PASS_OBJECT_SIZE)
 void *memcpy(void *dst, const void *src, size_t c);
 static void *memcpy(void *dst __attribute__((pass_object_size(1))), const void *src, size_t c) __attribute__((overloadable)) __asm__("merp");
@@ -96,6 +98,91 @@
   __builtin_vsnprintf(buf, 11, "merp", list); // expected-warning {{'vsnprintf' size argument is too large; destination buffer has size 10, but size argument is 11}}
 }
 
+void call_sprintf_chk(char *buf) {
+  __builtin___sprintf_chk(buf, 1, 6, "hell\n");
+  __builtin___sprintf_chk(buf, 1, 5, "hell\n"); // expected-warning {{'sprintf' will always overflow; destination buffer has size 5, but format string expands to at least 6}}
+  __builtin___sprintf_chk(buf, 1, 6, "hell\0 boy"); // expected-warning {{format string contains '\0' within the string body}}
+  __builtin___sprintf_chk(buf, 1, 2, "hell\0 boy"); // expected-warning {{format string contains '\0' within the string body}}
+  // expected-warning@-1 {{'sprintf' will always overflow; destination buffer has size 2, but format string expands to at least 5}}
+  __builtin___sprintf_chk(buf, 1, 6, "hello");
+  __builtin___sprintf_chk(buf, 1, 5, "hello"); // expected-warning {{'sprintf' will always overflow; destination buffer has size 5, but format string expands to at least 6}}
+  __builtin___sprintf_chk(buf, 1, 2, "%c", '9');
+  __builtin___sprintf_chk(buf, 1, 1, "%c", '9'); // expected-warning {{'sprintf' will always overflow; destination buffer has size 1, but format string expands to at least 2}}
+  __builtin___sprintf_chk(buf, 1, 2, "%d", 9);
+  __builtin___sprintf_chk(buf, 1, 1, "%d", 9); // expected-warning {{'sprintf' will always overflow; destination buffer has size 1, but format string expands to at least 2}}
+  __builtin___sprintf_chk(buf, 1, 2, "%i", 9);
+  __builtin___sprintf_chk(buf, 1, 1, "%i", 9); // expected-warning {{'sprintf' will always overflow; destination buffer has size 1, but format string expands to at least 2}}
+  __builtin___sprintf_chk(buf, 1, 2, "%o", 9);
+  __builtin___sprintf_chk(buf, 1, 1, "%o", 9); // expected-warning {{'sprintf' will always overflow; destination buffer has size 1, but format string expands to at least 2}}
+  __builtin___sprintf_chk(buf, 1, 2, "%u", 9);
+  __builtin___sprintf_chk(buf, 1, 1, "%u", 9); // expected-warning {{'sprintf' will always overflow; destination buffer has size 1, but format string expands to at least 2}}
+  __builtin___sprintf_chk(buf, 1, 2, "%x", 9);
+  __builtin___sprintf_chk(buf, 1, 1, "%x", 9); // expected-warning {{'sprintf' will always overflow; destination buffer has size 1, but format string expands to at least 2}}
+  __builtin___sprintf_chk(buf, 1, 2, "%X", 9);
+  __builtin___sprintf_chk(buf, 1, 1, "%X", 9); // expected-warning {{'sprintf' will always overflow; destination buffer has size 1, but format string expands to at least 2}}
+  __builtin___sprintf_chk(buf, 1, 2, "%hhd", (char)9);
+  __builtin___sprintf_chk(buf, 1, 1, "%hhd", (char)9); // expected-warning {{'sprintf' will always overflow; destination buffer has size 1, but format string expands to at least 2}}
+  __builtin___sprintf_chk(buf, 1, 2, "%hd", (short)9);
+  __builtin___sprintf_chk(buf, 1, 1, "%hd", (short)9); // expected-warning {{'sprintf' will always overflow; destination buffer has size 1, but format string expands to at least 2}}
+  __builtin___sprintf_chk(buf, 1, 2, "%ld", 9l);
+  __builtin___sprintf_chk(buf, 1, 1, "%ld", 9l); // expected-warning {{'sprintf' will always overflow; destination buffer has size 1, but format string expands to at least 2}}
+  __builtin___sprintf_chk(buf, 1, 2, "%lld", 9ll);
+  __builtin___sprintf_chk(buf, 1, 1, "%lld", 9ll); // expected-warning {{'sprintf' will always overflow; destination buffer has size 1, but format string expands to at least 2}}
+  __builtin___sprintf_chk(buf, 1, 2, "%%");
+  __builtin___sprintf_chk(buf, 1, 1, "%%"); // expected-warning {{'sprintf' will always overflow; destination buffer has size 1, but format string expands to at least 2}}
+  __builtin___sprintf_chk(buf, 1, 4, "%#x", 9);
+  __builtin___sprintf_chk(buf, 1, 3, "%#x", 9); // expected-warning {{'sprintf' will always overflow; destination buffer has size 3, but format string expands 

[PATCH] D72378: [clang-tidy] Add `bugprone-reserved-identifier`

2020-01-25 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D72378#1840461 , @lebedev.ri wrote:

> In D72378#1840455 , @aaron.ballman 
> wrote:
>
> > In D72378#1840353 , @lebedev.ri 
> > wrote:
> >
> > > So, i'm seeing an issue here:
> > >  https://godbolt.org/z/KM2qLa
> > >
> > > I can't `NOLINT` it, because it is defined not in the source file, but in 
> > > compile line.
> > >  And i can't whitelist it since there is no whitelist..
> > >
> > > PTAL.
> >
> >
> > Doesn't the `AllowedIdentifiers` option work for you?
>
>
> Uuuh. To be honest somehow i did not observe it when looking through
>  the docs (as indicated in `it since there is no whitelist..`),
>  It does work in this case, thanks. Sorry for false alarm.


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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72378



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


[PATCH] D72378: [clang-tidy] Add `bugprone-reserved-identifier`

2020-01-25 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment.

In D72378#1840455 , @aaron.ballman 
wrote:

> In D72378#1840353 , @lebedev.ri 
> wrote:
>
> > So, i'm seeing an issue here:
> >  https://godbolt.org/z/KM2qLa
> >
> > I can't `NOLINT` it, because it is defined not in the source file, but in 
> > compile line.
> >  And i can't whitelist it since there is no whitelist..
> >
> > PTAL.
>
>
> Doesn't the `AllowedIdentifiers` option work for you?


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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72378



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


[PATCH] D72378: [clang-tidy] Add `bugprone-reserved-identifier`

2020-01-25 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D72378#1840353 , @lebedev.ri wrote:

> So, i'm seeing an issue here:
>  https://godbolt.org/z/KM2qLa
>
> I can't `NOLINT` it, because it is defined not in the source file, but in 
> compile line.
>  And i can't whitelist it since there is no whitelist..
>
> PTAL.


Doesn't the `AllowedIdentifiers` option work for you?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72378



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


[PATCH] D73418: [WPD] Emit vcall_visibility metadata for MicrosoftCXXABI

2020-01-25 Thread Teresa Johnson via Phabricator via cfe-commits
tejohnson created this revision.
tejohnson added a reviewer: evgeny777.
Herald added a subscriber: Prazek.
Herald added a project: clang.

The MicrosoftCXXABI uses a separate mechanism for emitting vtable
type metadata, and thus didn't pick up the change from D71907 

to emit the vcall_visibility metadata under -fwhole-program-vtables.

I believe this is the cause of a Windows bot failure when I committed
follow on change D71913  that required a 
revert. The failure occurred
in a CFI test that was expecting to not abort because it expected a
devirtualization to occur, and without the necessary vcall_visibility
metadata we would not get devirtualization.

Note in the equivalent code in CodeGenModule::EmitVTableTypeMetadata
(used by the ItaniumCXXABI), we also emit the vcall_visibility metadata
when Virtual Function Elimination is enabled. Since I am not as familiar
with the details of that optimization, I have marked that as a TODO and
am only inserting under -fwhole-program-vtables.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D73418

Files:
  clang/lib/CodeGen/MicrosoftCXXABI.cpp
  clang/test/CodeGenCXX/vcall-visibility-metadata.cpp


Index: clang/test/CodeGenCXX/vcall-visibility-metadata.cpp
===
--- clang/test/CodeGenCXX/vcall-visibility-metadata.cpp
+++ clang/test/CodeGenCXX/vcall-visibility-metadata.cpp
@@ -1,5 +1,6 @@
 // RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -emit-llvm 
-fvirtual-function-elimination -fwhole-program-vtables -o - %s | FileCheck %s 
--check-prefix=CHECK --check-prefix=CHECK-VFE
 // RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -emit-llvm 
-fwhole-program-vtables -o - %s | FileCheck %s --check-prefix=CHECK 
--check-prefix=CHECK-NOVFE
+// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-pc-windows-msvc -emit-llvm 
-fwhole-program-vtables -o - %s | FileCheck %s --check-prefix=CHECK-MS 
--check-prefix=CHECK-NOVFE
 
 // Check that in ThinLTO we also get vcall_visibility summary entries in the 
bitcode
 // RUN: %clang_cc1 -flto=thin -flto-unit -triple x86_64-unknown-linux 
-emit-llvm-bc -fwhole-program-vtables -o - %s | llvm-dis -o - | FileCheck %s 
--check-prefix=CHECK --check-prefix=CHECK-NOVFE --check-prefix=CHECK-SUMMARY
@@ -87,6 +88,15 @@
   return new G();
 }
 
+// CHECK-MS: comdat($"??_7A@?A0xE24FC2D5@@6B@"){{.*}} !vcall_visibility 
[[VIS_TU:![0-9]+]]
+// CHECK-MS: comdat($"??_7B@@6B@"){{.*}} !vcall_visibility [[VIS_DSO:![0-9]+]]
+// CHECK-MS: comdat($"??_7C@@6B@"){{.*}} !vcall_visibility [[VIS_DSO]]
+// CHECK-MS: comdat($"??_7E@@6B@"){{.*}} !vcall_visibility [[VIS_DSO]]
+// CHECK-MS: comdat($"??_7F@?A0xE24FC2D5@@6B@"){{.*}} !vcall_visibility 
[[VIS_DSO]]
+// CHECK-MS: comdat($"??_7G@?A0xE24FC2D5@@6B@"){{.*}} !vcall_visibility 
[[VIS_DSO]]
+// CHECK-MS-DAG: [[VIS_TU]] = !{i64 2}
+// CHECK-MS-DAG: [[VIS_DSO]] = !{i64 1}
+
 // CHECK-DAG: [[VIS_DSO]] = !{i64 1}
 // CHECK-DAG: [[VIS_TU]] = !{i64 2}
 // CHECK-VFE-DAG: !{i32 1, !"Virtual Function Elim", i32 1}
Index: clang/lib/CodeGen/MicrosoftCXXABI.cpp
===
--- clang/lib/CodeGen/MicrosoftCXXABI.cpp
+++ clang/lib/CodeGen/MicrosoftCXXABI.cpp
@@ -1621,6 +1621,15 @@
   if (!CGM.getCodeGenOpts().LTOUnit)
 return;
 
+  // TODO: Should VirtualFunctionElimination also be supported here?
+  // See similar handling in CodeGenModule::EmitVTableTypeMetadata.
+  if (CGM.getCodeGenOpts().WholeProgramVTables) {
+llvm::GlobalObject::VCallVisibility TypeVis =
+CGM.GetVCallVisibilityLevel(RD);
+if (TypeVis != llvm::GlobalObject::VCallVisibilityPublic)
+  VTable->setVCallVisibilityMetadata(TypeVis);
+  }
+
   // The location of the first virtual function pointer in the virtual table,
   // aka the "address point" on Itanium. This is at offset 0 if RTTI is
   // disabled, or sizeof(void*) if RTTI is enabled.


Index: clang/test/CodeGenCXX/vcall-visibility-metadata.cpp
===
--- clang/test/CodeGenCXX/vcall-visibility-metadata.cpp
+++ clang/test/CodeGenCXX/vcall-visibility-metadata.cpp
@@ -1,5 +1,6 @@
 // RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -emit-llvm -fvirtual-function-elimination -fwhole-program-vtables -o - %s | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-VFE
 // RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -emit-llvm -fwhole-program-vtables -o - %s | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-NOVFE
+// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-pc-windows-msvc -emit-llvm -fwhole-program-vtables -o - %s | FileCheck %s --check-prefix=CHECK-MS --check-prefix=CHECK-NOVFE
 
 // Check that in ThinLTO we also get vcall_visibility summary entries in the bitcode
 // RUN: %clang_cc1 -flto=thin -flto-unit -triple x86_64-unknown-linux -emit-llvm-bc 

[PATCH] D72508: [clangd] Support pseudo-obj expr, opaque values, and property references in findExplicitReferences()

2020-01-25 Thread pre-merge checks [bot] via Phabricator via cfe-commits
merge_guards_bot added a comment.

{icon times-circle color=red} Unit tests: fail. 62194 tests passed, 1 failed 
and 815 were skipped.

  failed: 
libc++.std/thread/thread_mutex/thread_mutex_requirements/thread_mutex_requirements_mutex/thread_mutex_recursive/try_lock.pass.cpp

{icon check-circle color=green} clang-tidy: pass.

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 


//Pre-merge checks is in beta. Report issue 
.
 Please join beta  or enable 
it for your project 
.//


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72508



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


[PATCH] D72508: [clangd] Support pseudo-obj expr, opaque values, and property references in findExplicitReferences()

2020-01-25 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments.



Comment at: clang-tools-extra/clangd/FindTarget.cpp:624
+  /*IsDecl=*/false,
+  // Select the getter, setter, or @property depending on the call.
+  explicitReferenceTargets(DynTypedNode::create(*E), {})});

dgoldman wrote:
> Worth mentioning this is handled in `add()`'s `Visitor`'s 
> `VisitObjCPropertyRefExpr`?
I don't think we should echo the code structure here, it could change.



Comment at: clang-tools-extra/clangd/FindTarget.cpp:740
 
+  bool TraverseOpaqueValueExpr(OpaqueValueExpr *OVE) {
+visitNode(DynTypedNode::create(*OVE));

dgoldman wrote:
> Worth noting that these two functions are used currently for ObjC?
These functions are used for ObjC because these node types are used for objC - 
documenting that belongs on the AST class.
(I guess it's not currently done because someone currently/previously had an 
ambition to use these for other things, thus the generic names)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72508



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


[PATCH] D72634: [clangd] Improve ObjC property handling in SelectionTree.

2020-01-25 Thread pre-merge checks [bot] via Phabricator via cfe-commits
merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 62195 tests passed, 0 failed 
and 815 were skipped.

{icon check-circle color=green} clang-tidy: pass.

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 


//Pre-merge checks is in beta. Report issue 
.
 Please join beta  or enable 
it for your project 
.//


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72634



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


[PATCH] D72508: [clangd] Support pseudo-obj expr, opaque values, and property references in findExplicitReferences()

2020-01-25 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 240378.
sammccall marked 5 inline comments as done.
sammccall added a comment.

more tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72508

Files:
  clang-tools-extra/clangd/FindTarget.cpp
  clang-tools-extra/clangd/unittests/FindTargetTests.cpp

Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -589,6 +589,7 @@
 // parsing.
 TU.ExtraArgs.push_back("-fno-delayed-template-parsing");
 TU.ExtraArgs.push_back("-std=c++2a");
+TU.ExtraArgs.push_back("-xobjective-c++");
 
 auto AST = TU.build();
 auto *TestDecl = (AST, "foo");
@@ -696,7 +697,7 @@
  void foo() {
$0^Struct $1^x;
$2^Typedef $3^y;
-   static_cast<$4^Struct*>(0);
+   (void) static_cast<$4^Struct*>(0);
  }
)cpp",
 "0: targets = {Struct}\n"
@@ -707,10 +708,10 @@
// Name qualifiers.
{R"cpp(
  namespace a { namespace b { struct S { typedef int type; }; } }
- void foo() {
+ int foo() {
$0^a::$1^b::$2^S $3^x;
using namespace $4^a::$5^b;
-   $6^S::$7^type $8^y;
+   return $6^S::$7^type(5);
  }
 )cpp",
 "0: targets = {a}\n"
@@ -720,8 +721,7 @@
 "4: targets = {a}\n"
 "5: targets = {a::b}, qualifier = 'a::'\n"
 "6: targets = {a::b::S}\n"
-"7: targets = {a::b::S::type}, qualifier = 'struct S::'\n"
-"8: targets = {y}, decl\n"},
+"7: targets = {a::b::S::type}, qualifier = 'struct S::'\n"},
// Simple templates.
{R"cpp(
   template  struct vector { using value_type = T; };
@@ -791,8 +791,8 @@
 #define FOO a
 #define BAR b
 
-void foo(int a, int b) {
-  $0^FOO+$1^BAR;
+int foo(int a, int b) {
+  return $0^FOO+$1^BAR;
 }
 )cpp",
 "0: targets = {a}\n"
@@ -989,7 +989,7 @@
};
// delegating initializer
class $10^Foo {
- $11^Foo(int);
+ $11^Foo(int) {}
  $12^Foo(): $13^Foo(111) {}
};
  }
@@ -1123,7 +1123,37 @@
"3: targets = {foo::bar}, decl\n"
"4: targets = {T}\n"
"5: targets = {t}, decl\n"
-   "6: targets = {t}\n"}};
+   "6: targets = {t}\n"},
+   // Objective-C: properties
+   {
+   R"cpp(
+@interface I {}
+@property(retain) I* x;
+@property(retain) I* y;
+@end
+I *f;
+void foo() {
+  $0^f.$1^x.$2^y = 0;
+}
+  )cpp",
+   "0: targets = {f}\n"
+   "1: targets = {I::x}\n"
+   "2: targets = {I::y}\n"},
+   // Objective-C: implicit properties
+   {
+   R"cpp(
+@interface I {}
+-(I*)x;
+-(void)setY:(I*)y;
+@end
+I *f;
+void foo() {
+  $0^f.$1^x.$2^y = 0;
+}
+  )cpp",
+   "0: targets = {f}\n"
+   "1: targets = {I::x}\n"
+   "2: targets = {I::setY:}\n"}};
 
   for (const auto  : Cases) {
 llvm::StringRef ExpectedCode = C.first;
Index: clang-tools-extra/clangd/FindTarget.cpp
===
--- clang-tools-extra/clangd/FindTarget.cpp
+++ clang-tools-extra/clangd/FindTarget.cpp
@@ -619,7 +619,7 @@
 
 llvm::SmallVector refInExpr(const Expr *E) {
   struct Visitor : ConstStmtVisitor {
-// FIXME: handle more complicated cases, e.g. ObjC, designated initializers.
+// FIXME: handle more complicated cases: more ObjC, designated initializers.
 llvm::SmallVector Refs;
 
 void VisitConceptSpecializationExpr(const ConceptSpecializationExpr *E) {
@@ -660,6 +660,14 @@
   /*IsDecl=*/false,
   {E->getPack()}});
 }
+
+void VisitObjCPropertyRefExpr(const ObjCPropertyRefExpr *E) {
+  Refs.push_back(ReferenceLoc{
+  NestedNameSpecifierLoc(), E->getLocation(),
+  /*IsDecl=*/false,
+  // Select the getter, setter, or @property depending on the call.
+  explicitReferenceTargets(DynTypedNode::create(*E), {})});
+}
   };
 
   Visitor V;
@@ -780,6 +788,19 @@
 return true;
   }
 
+  bool TraverseOpaqueValueExpr(OpaqueValueExpr *OVE) {
+visitNode(DynTypedNode::create(*OVE));
+// Not clear why the source expression is skipped by default...
+return RecursiveASTVisitor::TraverseStmt(OVE->getSourceExpr());
+  }
+
+  bool TraversePseudoObjectExpr(PseudoObjectExpr *POE) 

[PATCH] D72634: [clangd] Improve ObjC property handling in SelectionTree.

2020-01-25 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 240377.
sammccall added a comment.

more tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72634

Files:
  clang-tools-extra/clangd/Selection.cpp
  clang-tools-extra/clangd/unittests/FindTargetTests.cpp
  clang-tools-extra/clangd/unittests/SelectionTests.cpp


Index: clang-tools-extra/clangd/unittests/SelectionTests.cpp
===
--- clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -323,12 +323,52 @@
 Foo x = [[^12_ud]];
   )cpp",
   "UserDefinedLiteral"},
+
   {
   R"cpp(
 int a;
 decltype([[^a]] + a) b;
 )cpp",
   "DeclRefExpr"},
+
+  // Objective-C OpaqueValueExpr/PseudoObjectExpr has weird ASTs.
+  // Need to traverse the contents of the OpaqueValueExpr to the POE,
+  // and ensure we traverse only the syntactic form of the 
PseudoObjectExpr.
+  {
+  R"cpp(
+@interface I{}
+@property(retain) I*x;
+@property(retain) I*y;
+@end
+void test(I *f) { [[^f]].x.y = 0; }
+  )cpp",
+  "DeclRefExpr"},
+  {
+  R"cpp(
+@interface I{}
+@property(retain) I*x;
+@property(retain) I*y;
+@end
+void test(I *f) { [[f.^x]].y = 0; }
+  )cpp",
+  "ObjCPropertyRefExpr"},
+  // Examples with implicit properties.
+  {
+  R"cpp(
+@interface I{}
+-(int)foo;
+@end
+int test(I *f) { return 42 + [[^f]].foo; }
+  )cpp",
+  "DeclRefExpr"},
+  {
+  R"cpp(
+@interface I{}
+-(int)foo;
+@end
+int test(I *f) { return 42 + [[f.^foo]]; }
+  )cpp",
+  "ObjCPropertyRefExpr"},
   };
   for (const Case  : Cases) {
 Annotations Test(C.Code);
@@ -339,6 +379,7 @@
 // FIXME: Auto-completion in a template requires disabling delayed template
 // parsing.
 TU.ExtraArgs.push_back("-fno-delayed-template-parsing");
+TU.ExtraArgs.push_back("-xobjective-c++");
 
 auto AST = TU.build();
 auto T = makeSelectionTree(C.Code, AST);
Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -536,7 +536,8 @@
   [[f.x]].y = 0;
 }
   )cpp";
-  EXPECT_DECLS("OpaqueValueExpr", "@property(atomic, retain, readwrite) I *x");
+  EXPECT_DECLS("ObjCPropertyRefExpr",
+   "@property(atomic, retain, readwrite) I *x");
 
   Code = R"cpp(
 @protocol Foo
Index: clang-tools-extra/clangd/Selection.cpp
===
--- clang-tools-extra/clangd/Selection.cpp
+++ clang-tools-extra/clangd/Selection.cpp
@@ -462,6 +462,14 @@
  TraverseStmt(S->getRangeInit()) && TraverseStmt(S->getBody());
 });
   }
+  // OpaqueValueExpr blocks traversal, we must explicitly traverse it.
+  bool TraverseOpaqueValueExpr(OpaqueValueExpr *E) {
+return traverseNode(E, [&] { return TraverseStmt(E->getSourceExpr()); });
+  }
+  // We only want to traverse the *syntactic form* to understand the selection.
+  bool TraversePseudoObjectExpr(PseudoObjectExpr *E) {
+return traverseNode(E, [&] { return TraverseStmt(E->getSyntacticForm()); 
});
+  }
 
 private:
   using Base = RecursiveASTVisitor;


Index: clang-tools-extra/clangd/unittests/SelectionTests.cpp
===
--- clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -323,12 +323,52 @@
 Foo x = [[^12_ud]];
   )cpp",
   "UserDefinedLiteral"},
+
   {
   R"cpp(
 int a;
 decltype([[^a]] + a) b;
 )cpp",
   "DeclRefExpr"},
+
+  // Objective-C OpaqueValueExpr/PseudoObjectExpr has weird ASTs.
+  // Need to traverse the contents of the OpaqueValueExpr to the POE,
+  // and ensure we traverse only the syntactic form of the PseudoObjectExpr.
+  {
+  R"cpp(
+@interface I{}
+@property(retain) I*x;
+@property(retain) I*y;
+@end
+void test(I *f) { [[^f]].x.y = 0; }
+  )cpp",
+  "DeclRefExpr"},
+  {
+  R"cpp(
+@interface I{}
+@property(retain) I*x;
+@property(retain) I*y;
+@end
+void test(I *f) { [[f.^x]].y = 0; }
+  )cpp",
+  "ObjCPropertyRefExpr"},
+  // Examples with implicit properties.
+  {
+  R"cpp(
+   

[PATCH] D73413: [clang-tidy] Add check to detect external definitions with no header declaration

2020-01-25 Thread pre-merge checks [bot] via Phabricator via cfe-commits
merge_guards_bot added a comment.

{icon times-circle color=red} Unit tests: fail. 62128 tests passed, 5 failed 
and 807 were skipped.

  failed: libc++.std/language_support/cmp/cmp_partialord/partialord.pass.cpp
  failed: libc++.std/language_support/cmp/cmp_strongeq/cmp.strongeq.pass.cpp
  failed: libc++.std/language_support/cmp/cmp_strongord/strongord.pass.cpp
  failed: libc++.std/language_support/cmp/cmp_weakeq/cmp.weakeq.pass.cpp
  failed: libc++.std/language_support/cmp/cmp_weakord/weakord.pass.cpp

{icon times-circle color=red} clang-tidy: fail. clang-tidy found 0 errors and 1 
warnings 
.
 0 of them are added as review comments below (why? 
).

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 


//Pre-merge checks is in beta. Report issue 
.
 Please join beta  or enable 
it for your project 
.//


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73413



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


[PATCH] D73413: [clang-tidy] Add check to detect external definitions with no header declaration

2020-01-25 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 240375.
njames93 marked 3 inline comments as done.
njames93 added a comment.

- Address nits


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73413

Files:
  clang-tools-extra/clang-tidy/misc/CMakeLists.txt
  clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
  clang-tools-extra/clang-tidy/misc/MissingHeaderFileDeclarationCheck.cpp
  clang-tools-extra/clang-tidy/misc/MissingHeaderFileDeclarationCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/docs/clang-tidy/checks/misc-missing-header-file-declaration.rst
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/misc-missing-header-file-declaration/misc-missing-header-file-declaration.cpp.tmp.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/misc-missing-header-file-declaration/wrong_header.h
  
clang-tools-extra/test/clang-tidy/checkers/misc-missing-header-file-declaration-any-header.cpp
  
clang-tools-extra/test/clang-tidy/checkers/misc-missing-header-file-declaration.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/misc-missing-header-file-declaration.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc-missing-header-file-declaration.cpp
@@ -0,0 +1,87 @@
+// RUN: %check_clang_tidy %s misc-missing-header-file-declaration %t -- \
+// RUN: -- -I%S/Inputs/misc-missing-header-file-declaration
+
+#include "misc-missing-header-file-declaration.cpp.tmp.h"
+// Include file has to be named *.cpp.tmp.h to account for the script renaming
+// this file to *.cpp.tmp.cpp.
+// Outside this test environment it will work normally
+//  -> 
+#include "wrong_header.h"
+
+// These declarations should be ignored by the check as they are in the same
+// file.
+extern bool DeclInSource;
+extern void declInSource();
+
+// These declarations should be ignored by the check as they are in the same
+// file, however there is a corresponding decl in the header that will prevent
+// a failing check.
+extern bool DeclInBoth;
+extern void declInBoth();
+
+// No external linkage so no warning
+static bool StaticOK = false;
+constexpr bool ConstexprOK = false;
+inline void inlineOK() {}
+static void staticOK() {}
+constexpr bool constexprOK() { return true; }
+
+// External linkage but decl in header so no warning
+bool DeclInHeader = false;
+bool DeclInBoth = false;
+void declInHeader() {}
+void declInBoth() {}
+
+//Decls don't appear in corresponding header so issue a warning
+bool DeclInSource = false;
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Variable 'DeclInSource' is defined with external linkage
+bool NoDecl = false;
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Variable 'NoDecl' is defined with external linkage
+void declInSource() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Function 'declInSource' is defined with external linkage
+void noDecl() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Function 'noDecl' is defined with external linkage
+
+bool DeclInWrongHeader = false;
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Variable 'DeclInWrongHeader' is defined with external linkage
+void declInWrongHeader() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Function 'declInWrongHeader' is defined with external linkage
+
+// Decls in an anonymous namespace don't have external linkage, so no warning
+// should be emitted
+namespace {
+bool AnonNS = false;
+void anonNS() {}
+} // namespace
+
+// Ensure in namespace definitions are correctly resolved
+namespace ns1 {
+bool NS = false;
+void nS() {}
+} // namespace ns1
+
+// Ensure out of namespace definitions are correctly resolved
+bool /*namespace*/ ns2::NS = false;
+void /*namespace*/ ns2::nS() {}
+
+// Static class members declared in the header shouldn't be warned on.
+int /*struct*/ Foo::Bar = 0;
+
+// main is special, don't warn for it.
+int main() {
+}
+
+template 
+void templateFuncNoHeader() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Function 'templateFuncNoHeader' is defined with external linkage
+
+// Warn on explicit instantiations
+template <>
+void templateFuncNoHeader() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Function 'templateFuncNoHeader' is defined with external linkage
+
+static void foo() {
+  // We don't want warnings for these implicit instantations
+  templateFuncNoHeader();
+  templateFuncNoHeader();
+}
Index: clang-tools-extra/test/clang-tidy/checkers/misc-missing-header-file-declaration-any-header.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc-missing-header-file-declaration-any-header.cpp
@@ -0,0 +1,16 @@
+// RUN: %check_clang_tidy %s misc-missing-header-file-declaration %t -- \
+// RUN: -config='{CheckOptions: \
+// RUN: [{key: misc-missing-header-file-declaration.CheckAnyHeader, value: 1}]}' \
+// RUN: -- 

[clang-tools-extra] d085634 - [clangd] Make Notification a little safer.

2020-01-25 Thread Sam McCall via cfe-commits

Author: Sam McCall
Date: 2020-01-25T15:31:55+01:00
New Revision: d08563486e06df3ddb4d7c1018d1e1e762690ee8

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

LOG: [clangd] Make Notification a little safer.

I just fixed a test involving a similar Notification class: 18e6a65bae93a

The pattern (notify() on one thread, wait() and then destroy the Notification
on the other) seems innocuous enough. I'm not sure we actually use it in clangd,
but better safe than sorry.

Added: 


Modified: 
clang-tools-extra/clangd/Threading.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/Threading.cpp 
b/clang-tools-extra/clangd/Threading.cpp
index 016a90297c32..47c91f449d3f 100644
--- a/clang-tools-extra/clangd/Threading.cpp
+++ b/clang-tools-extra/clangd/Threading.cpp
@@ -20,8 +20,10 @@ void Notification::notify() {
   {
 std::lock_guard Lock(Mu);
 Notified = true;
+// Broadcast with the lock held. This ensures that it's safe to destroy
+// a Notification after wait() returns, even from another thread.
+CV.notify_all();
   }
-  CV.notify_all();
 }
 
 void Notification::wait() const {



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


[PATCH] D73413: [clang-tidy] Add check to detect external definitions with no header declaration

2020-01-25 Thread pre-merge checks [bot] via Phabricator via cfe-commits
merge_guards_bot added a comment.

{icon times-circle color=red} Unit tests: fail. 62128 tests passed, 5 failed 
and 807 were skipped.

  failed: libc++.std/language_support/cmp/cmp_partialord/partialord.pass.cpp
  failed: libc++.std/language_support/cmp/cmp_strongeq/cmp.strongeq.pass.cpp
  failed: libc++.std/language_support/cmp/cmp_strongord/strongord.pass.cpp
  failed: libc++.std/language_support/cmp/cmp_weakeq/cmp.weakeq.pass.cpp
  failed: libc++.std/language_support/cmp/cmp_weakord/weakord.pass.cpp

{icon times-circle color=red} clang-tidy: fail. clang-tidy found 0 errors and 1 
warnings 
.
 0 of them are added as review comments below (why? 
).

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 


//Pre-merge checks is in beta. Report issue 
.
 Please join beta  or enable 
it for your project 
.//


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73413



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


[PATCH] D73413: [clang-tidy] Add check to detect external definitions with no header declaration

2020-01-25 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/misc/MissingHeaderFileDeclarationCheck.cpp:14
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/StringSwitch.h"
+#include 

Please also include StringRef.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/Inputs/misc-missing-header-file-declaration/wrong_header.h:8
+#endif
\ No newline at end of file


Please add newline.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/misc-missing-header-file-declaration-any-header.cpp:17
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Variable 'Failure' is defined with 
external linkage
\ No newline at end of file


Please add newline.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73413



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


[PATCH] D72378: [clang-tidy] Add `bugprone-reserved-identifier`

2020-01-25 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment.

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

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

PTAL.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72378



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


[PATCH] D73413: [clang-tidy] Add check to detect external definitions with no header declaration

2020-01-25 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 240373.
njames93 added a comment.

- Ignores implicit template instantiations


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73413

Files:
  clang-tools-extra/clang-tidy/misc/CMakeLists.txt
  clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
  clang-tools-extra/clang-tidy/misc/MissingHeaderFileDeclarationCheck.cpp
  clang-tools-extra/clang-tidy/misc/MissingHeaderFileDeclarationCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/docs/clang-tidy/checks/misc-missing-header-file-declaration.rst
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/misc-missing-header-file-declaration/misc-missing-header-file-declaration.cpp.tmp.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/misc-missing-header-file-declaration/wrong_header.h
  
clang-tools-extra/test/clang-tidy/checkers/misc-missing-header-file-declaration-any-header.cpp
  
clang-tools-extra/test/clang-tidy/checkers/misc-missing-header-file-declaration.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/misc-missing-header-file-declaration.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc-missing-header-file-declaration.cpp
@@ -0,0 +1,87 @@
+// RUN: %check_clang_tidy %s misc-missing-header-file-declaration %t -- \
+// RUN: -- -I%S/Inputs/misc-missing-header-file-declaration
+
+#include "misc-missing-header-file-declaration.cpp.tmp.h"
+// Include file has to be named *.cpp.tmp.h to account for the script renaming
+// this file to *.cpp.tmp.cpp.
+// Outside this test environment it will work normally
+//  -> 
+#include "wrong_header.h"
+
+// These declarations should be ignored by the check as they are in the same
+// file.
+extern bool DeclInSource;
+extern void declInSource();
+
+// These declarations should be ignored by the check as they are in the same
+// file, however there is a corresponding decl in the header that will prevent
+// a failing check.
+extern bool DeclInBoth;
+extern void declInBoth();
+
+// No external linkage so no warning
+static bool StaticOK = false;
+constexpr bool ConstexprOK = false;
+inline void inlineOK() {}
+static void staticOK() {}
+constexpr bool constexprOK() { return true; }
+
+// External linkage but decl in header so no warning
+bool DeclInHeader = false;
+bool DeclInBoth = false;
+void declInHeader() {}
+void declInBoth() {}
+
+//Decls don't appear in corresponding header so issue a warning
+bool DeclInSource = false;
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Variable 'DeclInSource' is defined with external linkage
+bool NoDecl = false;
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Variable 'NoDecl' is defined with external linkage
+void declInSource() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Function 'declInSource' is defined with external linkage
+void noDecl() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Function 'noDecl' is defined with external linkage
+
+bool DeclInWrongHeader = false;
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Variable 'DeclInWrongHeader' is defined with external linkage
+void declInWrongHeader() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Function 'declInWrongHeader' is defined with external linkage
+
+// Decls in an anonymous namespace don't have external linkage, so no warning
+// should be emitted
+namespace {
+bool AnonNS = false;
+void anonNS() {}
+} // namespace
+
+// Ensure in namespace definitions are correctly resolved
+namespace ns1 {
+bool NS = false;
+void nS() {}
+} // namespace ns1
+
+// Ensure out of namespace definitions are correctly resolved
+bool /*namespace*/ ns2::NS = false;
+void /*namespace*/ ns2::nS() {}
+
+// Static class members declared in the header shouldn't be warned on.
+int /*struct*/ Foo::Bar = 0;
+
+// main is special, don't warn for it.
+int main() {
+}
+
+template 
+void templateFuncNoHeader() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Function 'templateFuncNoHeader' is defined with external linkage
+
+// Warn on explicit instantiations
+template <>
+void templateFuncNoHeader() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Function 'templateFuncNoHeader' is defined with external linkage
+
+static void foo() {
+  // We don't want warnings for these implicit instantations
+  templateFuncNoHeader();
+  templateFuncNoHeader();
+}
Index: clang-tools-extra/test/clang-tidy/checkers/misc-missing-header-file-declaration-any-header.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc-missing-header-file-declaration-any-header.cpp
@@ -0,0 +1,16 @@
+// RUN: %check_clang_tidy %s misc-missing-header-file-declaration %t -- \
+// RUN: -config='{CheckOptions: \
+// RUN: [{key: misc-missing-header-file-declaration.CheckAnyHeader, value: 1}]}' \
+// RUN: -- 

[PATCH] D72331: OpaquePtr: add type to inalloca attribute.

2020-01-25 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment.

@rnk mind taking a look at this when you get a chance just to sanity check from 
the inalloca  design side of things?


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

https://reviews.llvm.org/D72331



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


[PATCH] D72362: [clang-tidy] misc-no-recursion: a new check

2020-01-25 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment.
Herald added a subscriber: Charusso.

In D72362#1819243 , @bader wrote:

> In D72362#1817682 , @lebedev.ri 
> wrote:
>
> > In D72362#1817599 , @bader wrote:
> >
> > > Does it make sense to implement such diagnostics in clang Sema, 
> > > considering that OpenCL does not allow recursion?
> > >  We implemented similar diagnostics for SYCL programming model and would 
> > > be like to upstream it to clang later 
> > > (https://github.com/intel/llvm/commit/4efe9fcf2dc6f6150b5b477b0f8320ea13a7f596).
> > >  Can we somehow leverage this work for the compiler?
> >
> >
> > Implementing it elsewhere will be more restrictive in the future - somehow 
> > i suspect
> >  it will be easier to make clang-tidy CTU-aware rather than clang sema.
> >
> > That being said, is SYCL inherently single-TU, does it not support
> >  linking multiple separately compiled object files together?
>
>
> SYCL doesn't require multi-TU support. AFAIK, ComputeCPP implementation is 
> signle-TU. +@Naghasan to confirm/clarify.
>  The open source implementation I referred to, does support linking 
> separately compiled object files, but still I think detecting single-TU 
> recursion in clang is very useful.
>  Is it possible to have both: intra-TU diagnostics in clang and inter-TU 
> diagnostics in clang-tidy tool? Share any infrastructure (e.g. recursion 
> detection)?


Feel free to post that as an RFC to cfe-dev. If the feedback is favorable, i 
suspect i can rework this code somehow.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72362



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


[PATCH] D73413: [clang-tidy] Add check to detect external definitions with no header declaration

2020-01-25 Thread pre-merge checks [bot] via Phabricator via cfe-commits
merge_guards_bot added a comment.

{icon times-circle color=red} Unit tests: fail. 62128 tests passed, 5 failed 
and 807 were skipped.

  failed: libc++.std/language_support/cmp/cmp_partialord/partialord.pass.cpp
  failed: libc++.std/language_support/cmp/cmp_strongeq/cmp.strongeq.pass.cpp
  failed: libc++.std/language_support/cmp/cmp_strongord/strongord.pass.cpp
  failed: libc++.std/language_support/cmp/cmp_weakeq/cmp.weakeq.pass.cpp
  failed: libc++.std/language_support/cmp/cmp_weakord/weakord.pass.cpp

{icon times-circle color=red} clang-tidy: fail. clang-tidy found 0 errors and 1 
warnings 
.
 0 of them are added as review comments below (why? 
).

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 


//Pre-merge checks is in beta. Report issue 
.
 Please join beta  or enable 
it for your project 
.//


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73413



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


[PATCH] D73413: [clang-tidy] Add check to detect external definitions with no header declaration

2020-01-25 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 240372.
njames93 added a comment.

- fix formatting


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73413

Files:
  clang-tools-extra/clang-tidy/misc/CMakeLists.txt
  clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
  clang-tools-extra/clang-tidy/misc/MissingHeaderFileDeclarationCheck.cpp
  clang-tools-extra/clang-tidy/misc/MissingHeaderFileDeclarationCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/docs/clang-tidy/checks/misc-missing-header-file-declaration.rst
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/misc-missing-header-file-declaration/misc-missing-header-file-declaration.cpp.tmp.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/misc-missing-header-file-declaration/wrong_header.h
  
clang-tools-extra/test/clang-tidy/checkers/misc-missing-header-file-declaration-any-header.cpp
  
clang-tools-extra/test/clang-tidy/checkers/misc-missing-header-file-declaration.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/misc-missing-header-file-declaration.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc-missing-header-file-declaration.cpp
@@ -0,0 +1,72 @@
+// RUN: %check_clang_tidy %s misc-missing-header-file-declaration %t -- \
+// RUN: -- -I%S/Inputs/misc-missing-header-file-declaration
+
+#include "misc-missing-header-file-declaration.cpp.tmp.h"
+// Include file has to be named *.cpp.tmp.h to account for the script renaming
+// this file to *.cpp.tmp.cpp.
+// Outside this test environment it will work normally
+//  -> 
+#include "wrong_header.h"
+
+// These declarations should be ignored by the check as they are in the same
+// file.
+extern bool DeclInSource;
+extern void declInSource();
+
+// These declarations should be ignored by the check as they are in the same
+// file, however there is a corresponding decl in the header that will prevent
+// a failing check.
+extern bool DeclInBoth;
+extern void declInBoth();
+
+// No external linkage so no warning
+static bool StaticOK = false;
+constexpr bool ConstexprOK = false;
+inline void inlineOK() {}
+static void staticOK() {}
+constexpr bool constexprOK() { return true; }
+
+// External linkage but decl in header so no warning
+bool DeclInHeader = false;
+bool DeclInBoth = false;
+void declInHeader() {}
+void declInBoth() {}
+
+//Decls don't appear in corresponding header so issue a warning
+bool DeclInSource = false;
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Variable 'DeclInSource' is defined with external linkage
+bool NoDecl = false;
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Variable 'NoDecl' is defined with external linkage
+void declInSource() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Function 'declInSource' is defined with external linkage
+void noDecl() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Function 'noDecl' is defined with external linkage
+
+bool DeclInWrongHeader = false;
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Variable 'DeclInWrongHeader' is defined with external linkage
+void declInWrongHeader() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Function 'declInWrongHeader' is defined with external linkage
+
+// Decls in an anonymous namespace don't have external linkage, so no warning
+// should be emitted
+namespace {
+bool AnonNS = false;
+void anonNS() {}
+} // namespace
+
+// Ensure in namespace definitions are correctly resolved
+namespace ns1 {
+bool NS = false;
+void nS() {}
+} // namespace ns1
+
+// Ensure out of namespace definitions are correctly resolved
+bool /*namespace*/ ns2::NS = false;
+void /*namespace*/ ns2::nS() {}
+
+// Static class members declared in the header shouldn't be warned on.
+int /*struct*/ Foo::Bar = 0;
+
+// main is special, don't warn for it.
+int main() {
+}
\ No newline at end of file
Index: clang-tools-extra/test/clang-tidy/checkers/misc-missing-header-file-declaration-any-header.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc-missing-header-file-declaration-any-header.cpp
@@ -0,0 +1,77 @@
+// RUN: %check_clang_tidy %s misc-missing-header-file-declaration %t -- \
+// RUN: -config='{CheckOptions: \
+// RUN: [{key: misc-missing-header-file-declaration.CheckAnyHeader, value: 1}]}' \
+// RUN: -- -I%S/Inputs/misc-missing-header-file-declaration
+
+// NOTE in this file everything is actually in the wrong header file, as the
+// corresponding header is called
+// 'misc-missing-header-file-declaration-any-header.cpp.tmp.h'
+
+#include "misc-missing-header-file-declaration.cpp.tmp.h"
+// Include file has to be named *.cpp.tmp.h to account for the script renaming
+// this file to *.cpp.tmp.cpp.
+// Outside this test environment it will work normally
+//  -> 
+#include "wrong_header.h"
+
+// These 

[PATCH] D73413: [clang-tidy] Add check to detect external definitions with no header declaration

2020-01-25 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment.

This is what gets flagged when ran on clang/lib with CheckAnyHeader enabled 
(with it disabled it just goes crazy as a lot of clang headers files seem to 
share a source file for implementation)
clang/lib 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73413



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


[PATCH] D73413: [clang-tidy] Add check to detect external definitions with no header declaration

2020-01-25 Thread pre-merge checks [bot] via Phabricator via cfe-commits
merge_guards_bot added a comment.

{icon times-circle color=red} Unit tests: fail. 62128 tests passed, 5 failed 
and 807 were skipped.

  failed: libc++.std/language_support/cmp/cmp_partialord/partialord.pass.cpp
  failed: libc++.std/language_support/cmp/cmp_strongeq/cmp.strongeq.pass.cpp
  failed: libc++.std/language_support/cmp/cmp_strongord/strongord.pass.cpp
  failed: libc++.std/language_support/cmp/cmp_weakeq/cmp.weakeq.pass.cpp
  failed: libc++.std/language_support/cmp/cmp_weakord/weakord.pass.cpp

{icon times-circle color=red} clang-tidy: fail. clang-tidy found 0 errors and 1 
warnings 
.
 0 of them are added as review comments below (why? 
).

{icon times-circle color=red} clang-format: fail. Please format your changes 
with clang-format by running `git-clang-format HEAD^` or applying this patch 
.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 


//Pre-merge checks is in beta. Report issue 
.
 Please join beta  or enable 
it for your project 
.//


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73413



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


[PATCH] D73413: [clang-tidy] Add check to detect external definitions with no header declaration

2020-01-25 Thread pre-merge checks [bot] via Phabricator via cfe-commits
merge_guards_bot added a comment.

{icon times-circle color=red} Unit tests: fail. 62128 tests passed, 5 failed 
and 807 were skipped.

  failed: libc++.std/language_support/cmp/cmp_partialord/partialord.pass.cpp
  failed: libc++.std/language_support/cmp/cmp_strongeq/cmp.strongeq.pass.cpp
  failed: libc++.std/language_support/cmp/cmp_strongord/strongord.pass.cpp
  failed: libc++.std/language_support/cmp/cmp_weakeq/cmp.weakeq.pass.cpp
  failed: libc++.std/language_support/cmp/cmp_weakord/weakord.pass.cpp

{icon times-circle color=red} clang-tidy: fail. clang-tidy found 0 errors and 1 
warnings 
.
 0 of them are added as review comments below (why? 
).

{icon times-circle color=red} clang-format: fail. Please format your changes 
with clang-format by running `git-clang-format HEAD^` or applying this patch 
.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 


//Pre-merge checks is in beta. Report issue 
.
 Please join beta  or enable 
it for your project 
.//


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73413



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


[PATCH] D73413: [clang-tidy] Add check to detect external definitions with no header declaration

2020-01-25 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 240371.
njames93 added a comment.

- Remove unnecessary duplicated code


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73413

Files:
  clang-tools-extra/clang-tidy/misc/CMakeLists.txt
  clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
  clang-tools-extra/clang-tidy/misc/MissingHeaderFileDeclarationCheck.cpp
  clang-tools-extra/clang-tidy/misc/MissingHeaderFileDeclarationCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/docs/clang-tidy/checks/misc-missing-header-file-declaration.rst
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/misc-missing-header-file-declaration/misc-missing-header-file-declaration.cpp.tmp.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/misc-missing-header-file-declaration/wrong_header.h
  
clang-tools-extra/test/clang-tidy/checkers/misc-missing-header-file-declaration-any-header.cpp
  
clang-tools-extra/test/clang-tidy/checkers/misc-missing-header-file-declaration.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/misc-missing-header-file-declaration.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc-missing-header-file-declaration.cpp
@@ -0,0 +1,74 @@
+// RUN: %check_clang_tidy %s misc-missing-header-file-declaration %t -- \
+// RUN: -- -I%S/Inputs/misc-missing-header-file-declaration
+
+#include "misc-missing-header-file-declaration.cpp.tmp.h"
+// Include file has to be named *.cpp.tmp.h to account for the script renaming
+// this file to *.cpp.tmp.cpp.
+// Outside this test environment it will work normally
+//  -> 
+#include "wrong_header.h"
+
+// These declarations should be ignored by the check as they are in the same
+// file.
+extern bool DeclInSource;
+extern void declInSource();
+
+// These declarations should be ignored by the check as they are in the same
+// file, however there is a corresponding decl in the header that will prevent
+// a failing check.
+extern bool DeclInBoth;
+extern void declInBoth();
+
+// No external linkage so no warning
+static bool StaticOK = false;
+constexpr bool ConstexprOK = false;
+inline void inlineOK() {}
+static void staticOK() {}
+constexpr bool constexprOK() { return true; }
+
+// External linkage but decl in header so no warning
+bool DeclInHeader = false;
+bool DeclInBoth = false;
+void declInHeader() {}
+void declInBoth() {}
+
+
+//Decls don't appear in corresponding header so issue a warning
+bool DeclInSource = false;
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Variable 'DeclInSource' is defined with external linkage
+bool NoDecl = false;
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Variable 'NoDecl' is defined with external linkage
+void declInSource() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Function 'declInSource' is defined with external linkage
+void noDecl() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Function 'noDecl' is defined with external linkage
+
+bool DeclInWrongHeader = false;
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Variable 'DeclInWrongHeader' is defined with external linkage
+void declInWrongHeader(){}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Function 'declInWrongHeader' is defined with external linkage
+
+
+// Decls in an anonymous namespace don't have external linkage, so no warning
+// should be emitted
+namespace {
+bool AnonNS = false;
+void anonNS() {}
+} // namespace
+
+// Ensure in namespace definitions are correctly resolved
+namespace ns1 {
+bool NS = false;
+void nS() {}
+} // namespace ns1
+
+// Ensure out of namespace definitions are correctly resolved
+bool /*namespace*/ns2::NS = false;
+void /*namespace*/ns2::nS() {}
+
+// Static class members declared in the header shouldn't be warned on.
+int /*struct*/Foo::Bar = 0;
+
+// main is special, don't warn for it.
+int main() {
+}
\ No newline at end of file
Index: clang-tools-extra/test/clang-tidy/checkers/misc-missing-header-file-declaration-any-header.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc-missing-header-file-declaration-any-header.cpp
@@ -0,0 +1,79 @@
+// RUN: %check_clang_tidy %s misc-missing-header-file-declaration %t -- \
+// RUN: -config='{CheckOptions: \
+// RUN: [{key: misc-missing-header-file-declaration.CheckAnyHeader, value: 1}]}' \
+// RUN: -- -I%S/Inputs/misc-missing-header-file-declaration
+
+// NOTE in this file everything is actually in the wrong header file, as the
+// corresponding header is called
+// 'misc-missing-header-file-declaration-any-header.cpp.tmp.h'
+
+#include "misc-missing-header-file-declaration.cpp.tmp.h"
+// Include file has to be named *.cpp.tmp.h to account for the script renaming
+// this file to *.cpp.tmp.cpp.
+// Outside this test environment it will work normally
+//  -> 
+#include 

[PATCH] D73413: [clang-tidy] Add check to detect external definitions with no header declaration

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

Flags variables and functions with external linkage that don't have a 
declaration in the corresponding header file.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D73413

Files:
  clang-tools-extra/clang-tidy/misc/CMakeLists.txt
  clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
  clang-tools-extra/clang-tidy/misc/MissingHeaderFileDeclarationCheck.cpp
  clang-tools-extra/clang-tidy/misc/MissingHeaderFileDeclarationCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/docs/clang-tidy/checks/misc-missing-header-file-declaration.rst
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/misc-missing-header-file-declaration/misc-missing-header-file-declaration.cpp.tmp.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/misc-missing-header-file-declaration/wrong_header.h
  
clang-tools-extra/test/clang-tidy/checkers/misc-missing-header-file-declaration-any-header.cpp
  
clang-tools-extra/test/clang-tidy/checkers/misc-missing-header-file-declaration.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/misc-missing-header-file-declaration.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc-missing-header-file-declaration.cpp
@@ -0,0 +1,74 @@
+// RUN: %check_clang_tidy %s misc-missing-header-file-declaration %t -- \
+// RUN: -- -I%S/Inputs/misc-missing-header-file-declaration
+
+#include "misc-missing-header-file-declaration.cpp.tmp.h"
+// Include file has to be named *.cpp.tmp.h to account for the script renaming
+// this file to *.cpp.tmp.cpp.
+// Outside this test environment it will work normally
+//  -> 
+#include "wrong_header.h"
+
+// These declarations should be ignored by the check as they are in the same
+// file.
+extern bool DeclInSource;
+extern void declInSource();
+
+// These declarations should be ignored by the check as they are in the same
+// file, however there is a corresponding decl in the header that will prevent
+// a failing check.
+extern bool DeclInBoth;
+extern void declInBoth();
+
+// No external linkage so no warning
+static bool StaticOK = false;
+constexpr bool ConstexprOK = false;
+inline void inlineOK() {}
+static void staticOK() {}
+constexpr bool constexprOK() { return true; }
+
+// External linkage but decl in header so no warning
+bool DeclInHeader = false;
+bool DeclInBoth = false;
+void declInHeader() {}
+void declInBoth() {}
+
+
+//Decls don't appear in corresponding header so issue a warning
+bool DeclInSource = false;
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Variable 'DeclInSource' is defined with external linkage
+bool NoDecl = false;
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Variable 'NoDecl' is defined with external linkage
+void declInSource() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Function 'declInSource' is defined with external linkage
+void noDecl() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Function 'noDecl' is defined with external linkage
+
+bool DeclInWrongHeader = false;
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Variable 'DeclInWrongHeader' is defined with external linkage
+void declInWrongHeader(){}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Function 'declInWrongHeader' is defined with external linkage
+
+
+// Decls in an anonymous namespace don't have external linkage, so no warning
+// should be emitted
+namespace {
+bool AnonNS = false;
+void anonNS() {}
+} // namespace
+
+// Ensure in namespace definitions are correctly resolved
+namespace ns1 {
+bool NS = false;
+void nS() {}
+} // namespace ns1
+
+// Ensure out of namespace definitions are correctly resolved
+bool /*namespace*/ns2::NS = false;
+void /*namespace*/ns2::nS() {}
+
+// Static class members declared in the header shouldn't be warned on.
+int /*struct*/Foo::Bar = 0;
+
+// main is special, don't warn for it.
+int main() {
+}
\ No newline at end of file
Index: clang-tools-extra/test/clang-tidy/checkers/misc-missing-header-file-declaration-any-header.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc-missing-header-file-declaration-any-header.cpp
@@ -0,0 +1,79 @@
+// RUN: %check_clang_tidy %s misc-missing-header-file-declaration %t -- \
+// RUN: -config='{CheckOptions: \
+// RUN: [{key: misc-missing-header-file-declaration.CheckAnyHeader, value: 1}]}' \
+// RUN: -- -I%S/Inputs/misc-missing-header-file-declaration
+
+// NOTE in this file everything is actually in the wrong header file, as the
+// corresponding header is called
+// 'misc-missing-header-file-declaration-any-header.cpp.tmp.h'
+
+#include "misc-missing-header-file-declaration.cpp.tmp.h"
+// Include file has to be named *.cpp.tmp.h to account for the script renaming
+// this file to *.cpp.tmp.cpp.
+// Outside this test 

[PATCH] D73388: NFC: Implement AST node skipping in ParentMapContext

2020-01-25 Thread Stephen Kelly via Phabricator via cfe-commits
steveire added a comment.

In D73388#1840100 , @rsmith wrote:

> This seems reasonable to me.
>
> If you want to unify something about this and the `Ignore*` functions in 
> `Expr`, maybe we could add a "classify" mechanism, so you could ask "what 
> kind of node is this?" and get back an enumerated value that indicates 
> whether it's semantic / syntactic / whatever. Then we could implement both 
> this and the downward-skipping in terms of that.


That seems like a reasonable future refactoring yes. I agree we can progress 
with this patch for now though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73388



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


[PATCH] D73380: [clang] Annotating C++'s `operator new` with more attributes

2020-01-25 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri marked 2 inline comments as done.
lebedev.ri added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:14361
+/// attributes are applied to declarations.
+void Sema::AddKnownFunctionAttributesForReplaceableGlobalAllocationFunction(
+FunctionDecl *FD) {

rsmith wrote:
> This should all be done by CodeGen, not by injecting source-level attributes.
I don't agree, can you explain why this should be done in codegen?



Comment at: clang/lib/Sema/SemaDecl.cpp:14414-14432
+  // C++2a [basic.stc.dynamic.allocation]p3:
+  //   For an allocation function [...], the pointer returned on a successful
+  //   call shall represent the address of storage that is aligned as follows:
+  //   (3.2),(3.3) Otherwise, [...] the storage is aligned for any object
+  //   that does not have new-extended alignment [...].
+  //
+  // NOTE: we intentionally always manifest this basic alignment, because it is

rsmith wrote:
> This is incorrect. The pointer returned by `operator new` is only suitably 
> aligned for any object that does not have new-extended alignment **and is of 
> the requested size**. And the pointer returned by `operator new[]` is 
> suitably aligned for any object **that is no larger than the requested 
> size**. (These are both different from the rule for `malloc`, which does 
> behave as you're suggesting here.) For example:
> 
> Suppose the default new alignment and the largest fundamental alignment are 
> both 16, and we try to allocate 12 bytes. Then:
> 
>  * `operator new` need only return storage that is 4-byte aligned (because 
> that is the largest alignment that can be required by a type `T` with 
> `sizeof(T) == 12`)
>  * `operator new` need only return storage that is 8-byte aligned (because 
> that is the largest alignment that can be required by a type `T` with 
> `sizeof(T) <= 12`)
>  * `malloc` must return storage that is 16-byte aligned (because that is the 
> largest fundamental alignment)
So essentially, if we can't evaluate the requested byte count as a 
constant/constant range,
we must simply give up here, on the most interesting case of variable 
allocation size?
That is surprisingly extremely pessimizing from C++ :)




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73380



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


[PATCH] D73408: [Clang][Bundler] Add 'exclude' flag to target objects sections

2020-01-25 Thread Sergey Dmitriev via Phabricator via cfe-commits
sdmitriev created this revision.
sdmitriev added a reviewer: ABataev.
Herald added a reviewer: alexshap.
Herald added a reviewer: jdoerfert.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This flag tells link editor to exclude section from linker inputs when linking 
executable or shared library.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D73408

Files:
  clang/test/Driver/clang-offload-bundler.c
  clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp

Index: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
===
--- clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
+++ clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
@@ -30,6 +30,7 @@
 #include "llvm/Support/Error.h"
 #include "llvm/Support/ErrorOr.h"
 #include "llvm/Support/FileSystem.h"
+#include "llvm/Support/FileUtilities.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Program.h"
@@ -463,6 +464,15 @@
 if (NumberOfProcessedInputs != NumberOfInputs)
   return Error::success();
 
+// We will use llvm-objcopy to add target objects’ sections to the output
+// fat object. These sections should have ‘exclude’ flag set which tells
+// link editor to remove them from linker inputs when linking executable or
+// shared library. llvm-objcopy currently does not support adding new
+// section and changing flags for the added section in one invocation, and
+// because of that we have to run it two times. First run adds sections and
+// the second changes flags. TODO: change it to one run once llvm-objcopy
+// starts supporting that.
+
 // Find llvm-objcopy in order to create the bundle binary.
 ErrorOr Objcopy = sys::findProgramByName(
 "llvm-objcopy", sys::path::parent_path(BundlerExecutable));
@@ -476,7 +486,15 @@
 // to pass down to llvm-objcopy.
 OS.close();
 
-// Compose command line for the objcopy tool.
+// Create an intermediate temporary file to save object after the first
+// llvm-objcopy run.
+SmallString<128u> IntermediateObj;
+if (std::error_code EC = sys::fs::createTemporaryFile(
+"clang-offload-bundler", "tmp", IntermediateObj))
+  return createFileError(IntermediateObj, EC);
+FileRemover IntermediateObjRemover(IntermediateObj);
+
+// Compose llvm-objcopy command line for add target objects' sections.
 BumpPtrAllocator Alloc;
 StringSaver SS{Alloc};
 SmallVector ObjcopyArgs{"llvm-objcopy"};
@@ -485,20 +503,38 @@
 OFFLOAD_BUNDLER_MAGIC_STR + TargetNames[I] +
 "=" + InputFileNames[I]));
 ObjcopyArgs.push_back(InputFileNames[HostInputIndex]);
+ObjcopyArgs.push_back(IntermediateObj);
+
+auto ExecuteObjcopy = [](ArrayRef ObjcopyArgs) -> Error {
+  // If the user asked for the commands to be printed out, we do that
+  // instead of executing it.
+  if (PrintExternalCommands) {
+errs() << "\"" << *Objcopy << "\"";
+for (StringRef Arg : drop_begin(ObjcopyArgs, 1))
+  errs() << " \"" << Arg << "\"";
+errs() << "\n";
+  } else {
+if (sys::ExecuteAndWait(*Objcopy, ObjcopyArgs))
+  return createStringError(inconvertibleErrorCode(),
+   "'llvm-objcopy' tool failed");
+  }
+  return Error::success();
+};
+
+if (Error Err = ExecuteObjcopy(ObjcopyArgs))
+  return Err;
+
+// And run llvm-objcopy fro the second time to update section flags.
+ObjcopyArgs.resize(1);
+for (unsigned I = 0; I < NumberOfInputs; ++I)
+  ObjcopyArgs.push_back(SS.save(Twine("--set-section-flags=") +
+OFFLOAD_BUNDLER_MAGIC_STR + TargetNames[I] +
+"=readonly,exclude"));
+ObjcopyArgs.push_back(IntermediateObj);
 ObjcopyArgs.push_back(OutputFileNames.front());
 
-// If the user asked for the commands to be printed out, we do that instead
-// of executing it.
-if (PrintExternalCommands) {
-  errs() << "\"" << *Objcopy << "\"";
-  for (StringRef Arg : drop_begin(ObjcopyArgs, 1))
-errs() << " \"" << Arg << "\"";
-  errs() << "\n";
-} else {
-  if (sys::ExecuteAndWait(*Objcopy, ObjcopyArgs))
-return createStringError(inconvertibleErrorCode(),
- "'llvm-objcopy' tool failed");
-}
+if (Error Err = ExecuteObjcopy(ObjcopyArgs))
+  return Err;
 
 return Error::success();
   }
Index: clang/test/Driver/clang-offload-bundler.c
===
--- clang/test/Driver/clang-offload-bundler.c
+++ clang/test/Driver/clang-offload-bundler.c
@@ -253,7 +253,8 @@
 
 // RUN: clang-offload-bundler -type=o