[PATCH] D68914: [clang-format] Remove duplciate code from Invalid BOM detection

2019-10-20 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 225779.
MyDeveloperDay marked 2 inline comments as done.
MyDeveloperDay added a comment.

I think we agree it should be (StringRef ), no const,no reference


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

https://reviews.llvm.org/D68914

Files:
  clang/include/clang/Basic/SourceManager.h
  clang/lib/Basic/SourceManager.cpp
  clang/tools/clang-format/ClangFormat.cpp
  clang/unittests/Basic/SourceManagerTest.cpp

Index: clang/unittests/Basic/SourceManagerTest.cpp
===
--- clang/unittests/Basic/SourceManagerTest.cpp
+++ clang/unittests/Basic/SourceManagerTest.cpp
@@ -200,6 +200,47 @@
 "");
 }
 
+TEST_F(SourceManagerTest, getInvalidBOM) {
+  ASSERT_EQ(SrcMgr::ContentCache::getInvalidBOM(""), nullptr);
+  ASSERT_EQ(SrcMgr::ContentCache::getInvalidBOM("\x00\x00\x00"), nullptr);
+  ASSERT_EQ(SrcMgr::ContentCache::getInvalidBOM("\xFF\xFF\xFF"), nullptr);
+  ASSERT_EQ(SrcMgr::ContentCache::getInvalidBOM("#include "),
+nullptr);
+
+  ASSERT_EQ(StringRef(SrcMgr::ContentCache::getInvalidBOM(
+"\xFE\xFF#include ")),
+"UTF-16 (BE)");
+  ASSERT_EQ(StringRef(SrcMgr::ContentCache::getInvalidBOM(
+"\xFF\xFE#include ")),
+"UTF-16 (LE)");
+  ASSERT_EQ(StringRef(SrcMgr::ContentCache::getInvalidBOM(
+"\x2B\x2F\x76#include ")),
+"UTF-7");
+  ASSERT_EQ(StringRef(SrcMgr::ContentCache::getInvalidBOM(
+"\xF7\x64\x4C#include ")),
+"UTF-1");
+  ASSERT_EQ(StringRef(SrcMgr::ContentCache::getInvalidBOM(
+"\xDD\x73\x66\x73#include ")),
+"UTF-EBCDIC");
+  ASSERT_EQ(StringRef(SrcMgr::ContentCache::getInvalidBOM(
+"\x0E\xFE\xFF#include ")),
+"SCSU");
+  ASSERT_EQ(StringRef(SrcMgr::ContentCache::getInvalidBOM(
+"\xFB\xEE\x28#include ")),
+"BOCU-1");
+  ASSERT_EQ(StringRef(SrcMgr::ContentCache::getInvalidBOM(
+"\x84\x31\x95\x33#include ")),
+"GB-18030");
+  ASSERT_EQ(StringRef(SrcMgr::ContentCache::getInvalidBOM(
+llvm::StringLiteral::withInnerNUL(
+"\x00\x00\xFE\xFF#include "))),
+"UTF-32 (BE)");
+  ASSERT_EQ(StringRef(SrcMgr::ContentCache::getInvalidBOM(
+llvm::StringLiteral::withInnerNUL(
+"\xFF\xFE\x00\x00#include "))),
+"UTF-32 (LE)");
+}
+
 #if defined(LLVM_ON_UNIX)
 
 TEST_F(SourceManagerTest, getMacroArgExpandedLocation) {
Index: clang/tools/clang-format/ClangFormat.cpp
===
--- clang/tools/clang-format/ClangFormat.cpp
+++ clang/tools/clang-format/ClangFormat.cpp
@@ -290,31 +290,6 @@
   }
 }
 
-// If BufStr has an invalid BOM, returns the BOM name; otherwise, returns
-// nullptr.
-static const char *getInValidBOM(StringRef BufStr) {
-  // Check to see if the buffer has a UTF Byte Order Mark (BOM).
-  // We only support UTF-8 with and without a BOM right now.  See
-  // https://en.wikipedia.org/wiki/Byte_order_mark#Byte_order_marks_by_encoding
-  // for more information.
-  const char *InvalidBOM =
-  llvm::StringSwitch(BufStr)
-  .StartsWith(llvm::StringLiteral::withInnerNUL("\x00\x00\xFE\xFF"),
-  "UTF-32 (BE)")
-  .StartsWith(llvm::StringLiteral::withInnerNUL("\xFF\xFE\x00\x00"),
-  "UTF-32 (LE)")
-  .StartsWith("\xFE\xFF", "UTF-16 (BE)")
-  .StartsWith("\xFF\xFE", "UTF-16 (LE)")
-  .StartsWith("\x2B\x2F\x76", "UTF-7")
-  .StartsWith("\xF7\x64\x4C", "UTF-1")
-  .StartsWith("\xDD\x73\x66\x73", "UTF-EBCDIC")
-  .StartsWith("\x0E\xFE\xFF", "SCSU")
-  .StartsWith("\xFB\xEE\x28", "BOCU-1")
-  .StartsWith("\x84\x31\x95\x33", "GB-18030")
-  .Default(nullptr);
-  return InvalidBOM;
-}
-
 static bool
 emitReplacementWarnings(const Replacements &Replaces, StringRef AssumedFileName,
 const std::unique_ptr &Code) {
@@ -400,7 +375,7 @@
 
   StringRef BufStr = Code->getBuffer();
 
-  const char *InvalidBOM = getInValidBOM(BufStr);
+  const char *InvalidBOM = SrcMgr::ContentCache::getInvalidBOM(BufStr);
 
   if (InvalidBOM) {
 errs() << "error: encoding with unsupported byte order mark \""
Index: clang/lib/Basic/SourceManager.cpp
===
--- clang/lib/Basic/SourceManager.cpp
+++ clang/lib/Basic/SourceManager.cpp
@@ -95,6 +95,29 @@
   Buffer.setInt((B && DoNotFree) ? DoNotFreeFlag : 0);
 }
 
+const char *ContentCache::getInvalidBOM(StringRef BufStr) {
+  // If the buffer is valid, check to see if it has a UTF Byte Order Mark
+  // (BOM).  We only support UTF-8 with and without a BOM right now.  See
+  // http://en.wikipedia.org/wiki/Byte_order_mark for more information.
+  const char *InvalidBOM =
+  llvm:

[PATCH] D68448: [clang-tools-extra] [cmake] Link against libclang-cpp whenever possible

2019-10-20 Thread Sylvestre Ledru via Phabricator via cfe-commits
sylvestre.ledru added a comment.

> More dynamic, less static isn't always a good thing. It saves disk space at 
> the expense of performance. The option that controls this is 
> `CLANG_LINK_CLANG_DYLIB`, which is mentioned in the clang 9.0 release notes.

Sure :)
For example, for a full build of llvm-toolchain, all the packages size drop 
from 510M to 278M


Repository:
  rL LLVM

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

https://reviews.llvm.org/D68448



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


[PATCH] D69129: [AMDGPU] Fix assertion due to initializer list

2019-10-20 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl marked 2 inline comments as done.
yaxunl added inline comments.



Comment at: lib/CodeGen/CodeGenModule.cpp:3898
+Entry = CE->stripPointerCasts();
   }
 

rjmccall wrote:
> You can just make this whole thing `Entry = Entry->stripPointerCasts()`.
will do when committing


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

https://reviews.llvm.org/D69129



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


[PATCH] D68913: Adds fixit hints to the Wrange-loop-analysis

2019-10-20 Thread Mark de Wever via Phabricator via cfe-commits
Mordante updated this revision to Diff 225782.
Mordante added a comment.

Undo the changes to the caret position as requested by @aaron.ballman .


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

https://reviews.llvm.org/D68913

Files:
  clang/include/clang/AST/Decl.h
  clang/lib/AST/Decl.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/test/SemaCXX/warn-range-loop-analysis.cpp

Index: clang/test/SemaCXX/warn-range-loop-analysis.cpp
===
--- clang/test/SemaCXX/warn-range-loop-analysis.cpp
+++ clang/test/SemaCXX/warn-range-loop-analysis.cpp
@@ -1,5 +1,6 @@
 // RUN: %clang_cc1 -fsyntax-only -std=c++11 -Wloop-analysis -verify %s
 // RUN: %clang_cc1 -fsyntax-only -std=c++11 -Wrange-loop-analysis -verify %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++11 -Wloop-analysis -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
 
 template 
 struct Iterator {
@@ -67,14 +68,17 @@
   for (const int &x : int_non_ref_container) {}
   // expected-warning@-1 {{loop variable 'x' is always a copy because the range of type 'Container' does not return a reference}}
   // expected-note@-2 {{use non-reference type 'int'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:19}:""
 
   for (const double &x : int_container) {}
   // expected-warning@-1 {{loop variable 'x' has type 'const double &' but is initialized with type 'int' resulting in a copy}}
   // expected-note@-2 {{use non-reference type 'double' to keep the copy or type 'const int &' to prevent copying}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:21-[[@LINE-3]]:22}:""
 
   for (const Bar x : bar_container) {}
   // expected-warning@-1 {{loop variable 'x' of type 'const Bar' creates a copy from type 'const Bar'}}
   // expected-note@-2 {{use reference type 'const Bar &' to prevent copying}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:18}:"&"
 }
 
 void test1() {
@@ -83,6 +87,7 @@
   for (const int &x : A) {}
   // expected-warning@-1 {{always a copy}}
   // expected-note@-2 {{'int'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:19}:""
   for (const int x : A) {}
   // No warning, non-reference type indicates copy is made
   //for (int &x : A) {}
@@ -93,6 +98,7 @@
   for (const double &x : A) {}
   // expected-warning@-1 {{always a copy}}
   // expected-note@-2 {{'double'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:21-[[@LINE-3]]:22}:""
   for (const double x : A) {}
   // No warning, non-reference type indicates copy is made
   //for (double &x : A) {}
@@ -103,6 +109,7 @@
   for (const Bar &x : A) {}
   // expected-warning@-1 {{always a copy}}
   // expected-note@-2 {{'Bar'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:19}:""
   for (const Bar x : A) {}
   // No warning, non-reference type indicates copy is made
   //for (Bar &x : A) {}
@@ -126,6 +133,7 @@
   for (const double &x : B) {}
   // expected-warning@-1 {{resulting in a copy}}
   // expected-note-re@-2 {{'double'{{.*}}'const int &'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:21-[[@LINE-3]]:22}:""
   for (const double x : B) {}
   //for (double &x : B) {}
   // Binding error
@@ -135,6 +143,7 @@
   for (const Bar &x : B) {}
   // expected-warning@-1 {{resulting in a copy}}
   // expected-note@-2 {{'Bar'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:19}:""
   for (const Bar x : B) {}
   //for (Bar &x : B) {}
   // Binding error
@@ -148,6 +157,7 @@
   for (const Bar &x : C) {}
   // expected-warning@-1 {{always a copy}}
   // expected-note@-2 {{'Bar'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:19}:""
   for (const Bar x : C) {}
   // No warning, non-reference type indicates copy is made
   //for (Bar &x : C) {}
@@ -158,6 +168,7 @@
   for (const int &x : C) {}
   // expected-warning@-1 {{always a copy}}
   // expected-note@-2 {{'int'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:19}:""
   for (const int x : C) {}
   // No warning, copy made
   //for (int &x : C) {}
@@ -174,6 +185,7 @@
   for (const Bar x : D) {}
   // expected-warning@-1 {{creates a copy}}
   // expected-note@-2 {{'const Bar &'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:18}:"&"
   for (Bar &x : D) {}
   // No warning
   for (Bar x : D) {}
@@ -182,6 +194,7 @@
   for (const int &x : D) {}
   // expected-warning@-1 {{resulting in a copy}}
   // expected-note-re@-2 {{'int'{{.*}}'const Bar &'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:19}:""
   for (const int x : D) {}
   // No warning
   //for (int &x : D) {}
@@ -196,6 +209,7 @@
   for (const Bar &x : E) {}
   // expected-warning@-1 {{always a copy}}
   // expected-note@-2 {{'Bar'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:19}:""
   for (const Bar x : E) {}
   // No warning, non-reference type indicates copy is made
   //for (Bar &x : E) {}
@@ -210,6 +224,7 @@
   for (const Bar &x : F) {}
   // expected-warning@-1 {{resulting in a copy}}
   // expected-note-re@-2 {{'Bar'{{.*}}'const Foo &'}}
+  // CHE

[PATCH] D69218: [clang][AST] Add `CXXBaseSpecifier` matcher support

2019-10-20 Thread Nikita Kniazev via Phabricator via cfe-commits
nick updated this revision to Diff 225783.
nick added a comment.

Fixed typo.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69218

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/AST/ASTTypeTraits.h
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/include/clang/ASTMatchers/ASTMatchersInternal.h
  clang/lib/AST/ASTTypeTraits.cpp
  clang/unittests/AST/ASTTypeTraitsTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -320,6 +320,14 @@
 varDecl(hasType(pointsTo(ClassX);
 }
 
+TEST(HasType, TakesQualTypeMatcherAndMatchesCXXBaseSpecifier) {
+  TypeMatcher ClassX = hasDeclaration(recordDecl(hasName("X")));
+  CXXBaseSpecifierMatcher BaseClassX = cxxBaseSpecifier(hasType(ClassX));
+  DeclarationMatcher ClassHasBaseClassX = cxxRecordDecl(hasBase(BaseClassX));
+  EXPECT_TRUE(matches("class X {}; class Y : X {};", ClassHasBaseClassX));
+  EXPECT_TRUE(notMatches("class Z {}; class Y : Z {};", ClassHasBaseClassX));
+}
+
 TEST(HasType, TakesDeclMatcherAndMatchesExpr) {
   DeclarationMatcher ClassX = recordDecl(hasName("X"));
   EXPECT_TRUE(
@@ -337,6 +345,14 @@
 notMatches("class X {}; void y() { X *x; }", varDecl(hasType(ClassX;
 }
 
+TEST(HasType, TakesDeclMatcherAndMatchesCXXBaseSpecifier) {
+  DeclarationMatcher ClassX = recordDecl(hasName("X"));
+  CXXBaseSpecifierMatcher BaseClassX = cxxBaseSpecifier(hasType(ClassX));
+  DeclarationMatcher ClassHasBaseClassX = cxxRecordDecl(hasBase(BaseClassX));
+  EXPECT_TRUE(matches("class X {}; class Y : X {};", ClassHasBaseClassX));
+  EXPECT_TRUE(notMatches("class Z {}; class Y : Z {};", ClassHasBaseClassX));
+}
+
 TEST(HasType, MatchesTypedefDecl) {
   EXPECT_TRUE(matches("typedef int X;", typedefDecl(hasType(asString("int");
   EXPECT_TRUE(matches("typedef const int T;",
Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -678,6 +678,29 @@
   "@interface Z : A @end", ZIsDirectlyDerivedFromX));
 }
 
+TEST(DeclarationMatcher, ClassHasBase) {
+  DeclarationMatcher ClassHasAnyBase =
+  cxxRecordDecl(hasBase(cxxBaseSpecifier()));
+  EXPECT_TRUE(notMatches("class X {};", ClassHasAnyBase));
+  EXPECT_TRUE(matches("class X {}; class Y : X {};", ClassHasAnyBase));
+
+  TypeMatcher ClassX = asString("class X");
+  CXXBaseSpecifierMatcher BaseClassX = cxxBaseSpecifier(hasType(ClassX));
+  DeclarationMatcher ClassHasBaseClassX = cxxRecordDecl(hasBase(BaseClassX));
+  EXPECT_TRUE(matches("class X {}; class Y {}; class Z : X, Y {};",
+  ClassHasBaseClassX));
+  EXPECT_TRUE(matches("class X {}; class Y {}; class Z : Y, X {};",
+  ClassHasBaseClassX));
+  EXPECT_TRUE(notMatches("class W {}; class Y {}; class Z : W, Y {};",
+ ClassHasBaseClassX));
+  DeclarationMatcher ClassZHasBaseClassX =
+  cxxRecordDecl(hasName("Z"), hasBase(BaseClassX));
+  EXPECT_TRUE(matches("class X {}; class Y : X {}; class Z : X {};",
+  ClassZHasBaseClassX));
+  EXPECT_TRUE(notMatches("class X {}; class Y : X {}; class Z : Y {};",
+ ClassZHasBaseClassX));
+}
+
 TEST(DeclarationMatcher, IsLambda) {
   const auto IsLambda = cxxMethodDecl(ofClass(cxxRecordDecl(isLambda(;
   EXPECT_TRUE(matches("auto x = []{};", IsLambda));
Index: clang/unittests/AST/ASTTypeTraitsTest.cpp
===
--- clang/unittests/AST/ASTTypeTraitsTest.cpp
+++ clang/unittests/AST/ASTTypeTraitsTest.cpp
@@ -112,6 +112,7 @@
   VERIFY_NAME(NestedNameSpecifierLoc);
   VERIFY_NAME(QualType);
   VERIFY_NAME(TypeLoc);
+  VERIFY_NAME(CXXBaseSpecifier);
   VERIFY_NAME(CXXCtorInitializer);
   VERIFY_NAME(NestedNameSpecifier);
   VERIFY_NAME(Decl);
@@ -141,6 +142,15 @@
   EXPECT_TRUE(Verifier.match("void f() {}", typeLoc(loc(functionType();
 }
 
+#if 0 // Cannot be used as a top level matcher at the time
+TEST(DynTypedNode, CXXBaseSpecifierSourceRange) {
+  RangeVerifier Verifier;
+  Verifier.expectRange(1, 23, 1, 39);
+  EXPECT_TRUE(Verifier.match("class B {}; class C : public virtual B {};",
+ cxxBaseSpecifier()));
+}
+#endif
+
 TEST(DynTypedNode, NNSLocSourceRange) {
   RangeVerifier Verifier;
   Verifier.expectRange(1, 33, 1, 34);
Index: clang/lib/AST/ASTTypeTraits.cpp
===
--- clang/lib/AST/ASTTypeTraits.cpp
+++ 

[PATCH] D69181: [clang-tidy] Adding misc-signal-terminated-thread check

2019-10-20 Thread Kocsis Ábel via Phabricator via cfe-commits
abelkocsis updated this revision to Diff 225784.
abelkocsis marked an inline comment as done.
abelkocsis added a comment.

Header documentation and Release Notes synchronised. Documentation of 
checker updated. Type changed to auto type when necessary.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D69181

Files:
  clang-tools-extra/clang-tidy/misc/BadSignalToKillThreadCheck.cpp
  clang-tools-extra/clang-tidy/misc/BadSignalToKillThreadCheck.h
  clang-tools-extra/clang-tidy/misc/CMakeLists.txt
  clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/misc-bad-signal-to-kill-thread.rst
  clang-tools-extra/test/clang-tidy/misc-bad-signal-to-kill-thread.cpp

Index: clang-tools-extra/test/clang-tidy/misc-bad-signal-to-kill-thread.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/misc-bad-signal-to-kill-thread.cpp
@@ -0,0 +1,39 @@
+// RUN: %check_clang_tidy %s misc-bad-signal-to-kill-thread %t
+
+#define SIGTERM 15
+#define SIGINT 2
+using pthread_t = int;
+using pthread_attr_t = int;
+
+int pthread_create(pthread_t *thread, const pthread_attr_t *attr,
+   void *(*start_routine)(void *), void *arg);
+
+int pthread_kill(pthread_t thread, int sig);
+
+int pthread_cancel(pthread_t thread);
+
+void *test_func_return_a_pointer(void *foo);
+
+int main() {
+  int result;
+  pthread_t thread;
+
+  if ((result = pthread_create(&thread, nullptr, test_func_return_a_pointer, 0)) != 0) {
+  }
+  if ((result = pthread_kill(thread, SIGTERM)) != 0) {
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: Thread should not be terminated by SIGTERM signal. [misc-bad-signal-to-kill-thread]
+  }
+
+  //compliant solution
+  if ((result = pthread_cancel(thread)) != 0) {
+  }
+
+  if ((result = pthread_kill(thread, SIGINT)) != 0) {
+  }
+  if ((result = pthread_kill(thread, 0xF)) != 0) {
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: Thread should not be terminated by SIGTERM signal. [misc-bad-signal-to-kill-thread]
+  }
+  
+
+  return 0;
+}
Index: clang-tools-extra/docs/clang-tidy/checks/misc-bad-signal-to-kill-thread.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/misc-bad-signal-to-kill-thread.rst
@@ -0,0 +1,14 @@
+.. title:: clang-tidy - misc-bad-signal-to-kill-thread
+
+misc-bad-signal-to-kill-thread
+==
+
+Finds ``pthread_kill`` function calls when thread is terminated by uncaught
+``SIGTERM`` signal and the signal kills the entire process, not just the
+individual thread. Use any signal except ``SIGTERM`` or ``SIGKILL``.
+To learn more about this rule please visit the following page:
+https://wiki.sei.cmu.edu/confluence/display/c/POS44-C.+Do+not+use+signals+to+terminate+threads
+
+.. code-block: c++
+
+pthread_kill(thread, SIGTERM);
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -282,6 +282,7 @@
llvm-prefer-isa-or-dyn-cast-in-conditionals
llvm-prefer-register-over-unsigned
llvm-twine-local
+   misc-bad-signal-to-kill-thread
misc-definitions-in-headers
misc-misplaced-const
misc-new-delete-overloads
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -115,6 +115,12 @@
   Finds historical use of ``unsigned`` to hold vregs and physregs and rewrites
   them to use ``Register``
 
+- New :doc:`misc-bad-signal-to-kill-thread
+  ` check.
+
+  Finds ``pthread_kill`` function calls when thread is terminated by
+  uncaught ``SIGTERM`` signal.
+
 - New :doc:`objc-missing-hash
   ` check.
 
Index: clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
===
--- clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
+++ clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
@@ -9,6 +9,7 @@
 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
+#include "BadSignalToKillThreadCheck.h"
 #include "DefinitionsInHeadersCheck.h"
 #include "MisplacedConstCheck.h"
 #include "NewDeleteOverloadsCheck.h"
@@ -30,6 +31,8 @@
 class MiscModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
+CheckFactories.registerCheck(
+"misc-bad-signal-to-kill-thread");
 CheckFactories.registerCheck(
 "misc-definitions-in-headers");
 CheckFactories.registerCheck("misc-misplaced-const");
Index: clang-tool

[PATCH] D69181: [clang-tidy] Adding misc-signal-terminated-thread check

2019-10-20 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/misc-bad-signal-to-kill-thread.rst:9
+individual thread. Use any signal except ``SIGTERM`` or ``SIGKILL``.
+To learn more about this rule please visit the following page:
+https://wiki.sei.cmu.edu/confluence/display/c/POS44-C.+Do+not+use+signals+to+terminate+threads

Usually links to documentation are located at the end of documentation. See 
other checks documentation as examples.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D69181



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


r375362 - [AMDGPU] Fix assertion due to initializer list

2019-10-20 Thread Yaxun Liu via cfe-commits
Author: yaxunl
Date: Sun Oct 20 08:02:22 2019
New Revision: 375362

URL: http://llvm.org/viewvc/llvm-project?rev=375362&view=rev
Log:
[AMDGPU] Fix assertion due to initializer list

Sometimes a global var is replaced by a different llvm value. clang use 
GetAddrOfGlobalVar to get the original llvm global variable.
For most targets, GetAddrOfGlobalVar returns either the llvm global variable or 
a bitcast of the llvm global variable.
However, for AMDGPU target, GetAddrOfGlobalVar returns the addrspace cast or 
addrspace cast plus bitcast of the llvm global variable.
To get the llvm global variable, these casts need to be stripped, otherwise 
there is assertion.

This patch fixes that.

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

Modified:
cfe/trunk/lib/CodeGen/CodeGenModule.cpp
cfe/trunk/test/CodeGenCXX/cxx11-extern-constexpr.cpp

Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=375362&r1=375361&r2=375362&view=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Sun Oct 20 08:02:22 2019
@@ -3546,7 +3546,8 @@ CodeGenModule::GetOrCreateLLVMGlobal(Str
   // Make a new global with the correct type, this is now 
guaranteed
   // to work.
   auto *NewGV = cast(
-  GetAddrOfGlobalVar(D, InitType, IsForDefinition));
+  GetAddrOfGlobalVar(D, InitType, IsForDefinition)
+  ->stripPointerCasts());
 
   // Erase the old global, since it is no longer used.
   GV->eraseFromParent();
@@ -3928,14 +3929,8 @@ void CodeGenModule::EmitGlobalVarDefinit
   llvm::Constant *Entry =
   GetAddrOfGlobalVar(D, InitType, ForDefinition_t(!IsTentative));
 
-  // Strip off a bitcast if we got one back.
-  if (auto *CE = dyn_cast(Entry)) {
-assert(CE->getOpcode() == llvm::Instruction::BitCast ||
-   CE->getOpcode() == llvm::Instruction::AddrSpaceCast ||
-   // All zero index gep.
-   CE->getOpcode() == llvm::Instruction::GetElementPtr);
-Entry = CE->getOperand(0);
-  }
+  // Strip off pointer casts if we got them.
+  Entry = Entry->stripPointerCasts();
 
   // Entry is now either a Function or GlobalVariable.
   auto *GV = dyn_cast(Entry);
@@ -3958,7 +3953,8 @@ void CodeGenModule::EmitGlobalVarDefinit
 
 // Make a new global with the correct type, this is now guaranteed to work.
 GV = cast(
-GetAddrOfGlobalVar(D, InitType, ForDefinition_t(!IsTentative)));
+GetAddrOfGlobalVar(D, InitType, ForDefinition_t(!IsTentative))
+->stripPointerCasts());
 
 // Replace all uses of the old global with the new global
 llvm::Constant *NewPtrForOldDecl =

Modified: cfe/trunk/test/CodeGenCXX/cxx11-extern-constexpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/cxx11-extern-constexpr.cpp?rev=375362&r1=375361&r2=375362&view=diff
==
--- cfe/trunk/test/CodeGenCXX/cxx11-extern-constexpr.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/cxx11-extern-constexpr.cpp Sun Oct 20 08:02:22 
2019
@@ -1,10 +1,13 @@
-// RUN: %clang_cc1 -std=c++11 %s -emit-llvm -o - -triple x86_64-linux-gnu | 
FileCheck %s --check-prefix=CHECK --check-prefix=CXX11
-// RUN: %clang_cc1 -std=c++1z %s -emit-llvm -o - -triple x86_64-linux-gnu | 
FileCheck %s --check-prefix=CHECK --check-prefix=CXX17
+// RUN: %clang_cc1 -std=c++11 %s -emit-llvm -o - -triple x86_64-linux-gnu | 
FileCheck %s --check-prefixes=X86,CXX11X86
+// RUN: %clang_cc1 -std=c++1z %s -emit-llvm -o - -triple x86_64-linux-gnu | 
FileCheck %s --check-prefixes=X86,CXX17X86
+// RUN: %clang_cc1 -std=c++11 %s -emit-llvm -o - -triple amdgcn-amd-amdhsa | 
FileCheck %s --check-prefixes=AMD,CXX11AMD
+// RUN: %clang_cc1 -std=c++1z %s -emit-llvm -o - -triple amdgcn-amd-amdhsa | 
FileCheck %s --check-prefixes=AMD,CXX17AMD
 
 struct A {
   static const int Foo = 123;
 };
-// CHECK: @_ZN1A3FooE = constant i32 123, align 4
+// X86: @_ZN1A3FooE = constant i32 123, align 4
+// AMD: @_ZN1A3FooE = addrspace(4) constant i32 123, align 4
 const int *p = &A::Foo; // emit available_externally
 const int A::Foo;   // convert to full definition
 
@@ -16,7 +19,8 @@ struct CreatePOD {
   // Deferred initialization of the structure here requires changing
   // the type of the global variable: the initializer list does not include
   // the tail padding.
-  // CXX11: @_ZN9CreatePOD3podE = available_externally constant { i32, i8 } { 
i32 42, i8 43 },
+  // CXX11X86: @_ZN9CreatePOD3podE = available_externally constant { i32, i8 } 
{ i32 42, i8 43 },
+  // CXX11AMD: @_ZN9CreatePOD3podE = available_externally addrspace(1) 
constant { i32, i8 } { i32 42, i8 43 },
   static constexpr PODWithInit pod{};
 };
 const int *p_pod = &CreatePOD::pod.g;
@@

[PATCH] D69181: [clang-tidy] Adding misc-signal-terminated-thread check

2019-10-20 Thread Kocsis Ábel via Phabricator via cfe-commits
abelkocsis updated this revision to Diff 225786.
abelkocsis added a comment.

Test clang-formatted. Uncaught word removed from documentations.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D69181

Files:
  clang-tools-extra/clang-tidy/misc/BadSignalToKillThreadCheck.cpp
  clang-tools-extra/clang-tidy/misc/BadSignalToKillThreadCheck.h
  clang-tools-extra/clang-tidy/misc/CMakeLists.txt
  clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/misc-bad-signal-to-kill-thread.rst
  clang-tools-extra/test/clang-tidy/misc-bad-signal-to-kill-thread.cpp

Index: clang-tools-extra/test/clang-tidy/misc-bad-signal-to-kill-thread.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/misc-bad-signal-to-kill-thread.cpp
@@ -0,0 +1,39 @@
+// RUN: %check_clang_tidy %s misc-bad-signal-to-kill-thread %t
+
+#define SIGTERM 15
+#define SIGINT 2
+using pthread_t = int;
+using pthread_attr_t = int;
+
+int pthread_create(pthread_t *thread, const pthread_attr_t *attr,
+   void *(*start_routine)(void *), void *arg);
+
+int pthread_kill(pthread_t thread, int sig);
+
+int pthread_cancel(pthread_t thread);
+
+void *test_func_return_a_pointer(void *foo);
+
+int main() {
+  int result;
+  pthread_t thread;
+
+  if ((result = pthread_create(&thread, nullptr, test_func_return_a_pointer, 0)) != 0) {
+  }
+  if ((result = pthread_kill(thread, SIGTERM)) != 0) {
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: Thread should not be terminated by SIGTERM signal. [misc-bad-signal-to-kill-thread]
+  }
+
+  //compliant solution
+  if ((result = pthread_cancel(thread)) != 0) {
+  }
+
+  if ((result = pthread_kill(thread, SIGINT)) != 0) {
+  }
+  if ((result = pthread_kill(thread, 0xF)) != 0) {
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: Thread should not be terminated by SIGTERM signal. [misc-bad-signal-to-kill-thread]
+  }
+  
+
+  return 0;
+}
Index: clang-tools-extra/docs/clang-tidy/checks/misc-bad-signal-to-kill-thread.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/misc-bad-signal-to-kill-thread.rst
@@ -0,0 +1,14 @@
+.. title:: clang-tidy - misc-bad-signal-to-kill-thread
+
+misc-bad-signal-to-kill-thread
+==
+
+Finds ``pthread_kill`` function calls when thread is terminated by uncaught
+``SIGTERM`` signal and the signal kills the entire process, not just the
+individual thread. Use any signal except ``SIGTERM`` or ``SIGKILL``.
+To learn more about this rule please visit the following page:
+https://wiki.sei.cmu.edu/confluence/display/c/POS44-C.+Do+not+use+signals+to+terminate+threads
+
+.. code-block: c++
+
+pthread_kill(thread, SIGTERM);
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -282,6 +282,7 @@
llvm-prefer-isa-or-dyn-cast-in-conditionals
llvm-prefer-register-over-unsigned
llvm-twine-local
+   misc-bad-signal-to-kill-thread
misc-definitions-in-headers
misc-misplaced-const
misc-new-delete-overloads
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -115,6 +115,12 @@
   Finds historical use of ``unsigned`` to hold vregs and physregs and rewrites
   them to use ``Register``
 
+- New :doc:`misc-bad-signal-to-kill-thread
+  ` check.
+
+  Finds ``pthread_kill`` function calls when thread is terminated by
+  uncaught ``SIGTERM`` signal.
+
 - New :doc:`objc-missing-hash
   ` check.
 
Index: clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
===
--- clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
+++ clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
@@ -9,6 +9,7 @@
 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
+#include "BadSignalToKillThreadCheck.h"
 #include "DefinitionsInHeadersCheck.h"
 #include "MisplacedConstCheck.h"
 #include "NewDeleteOverloadsCheck.h"
@@ -30,6 +31,8 @@
 class MiscModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
+CheckFactories.registerCheck(
+"misc-bad-signal-to-kill-thread");
 CheckFactories.registerCheck(
 "misc-definitions-in-headers");
 CheckFactories.registerCheck("misc-misplaced-const");
Index: clang-tools-extra/clang-tidy/misc/CMakeLists.txt
===
---

[PATCH] D69129: [AMDGPU] Fix assertion due to initializer list

2019-10-20 Thread Yaxun Liu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
yaxunl marked an inline comment as done.
Closed by commit rGe6125fc0ec34: [AMDGPU] Fix assertion due to initializer list 
(authored by yaxunl).
Herald added a project: clang.

Changed prior to commit:
  https://reviews.llvm.org/D69129?vs=225486&id=225787#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69129

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGenCXX/cxx11-extern-constexpr.cpp

Index: clang/test/CodeGenCXX/cxx11-extern-constexpr.cpp
===
--- clang/test/CodeGenCXX/cxx11-extern-constexpr.cpp
+++ clang/test/CodeGenCXX/cxx11-extern-constexpr.cpp
@@ -1,10 +1,13 @@
-// RUN: %clang_cc1 -std=c++11 %s -emit-llvm -o - -triple x86_64-linux-gnu | FileCheck %s --check-prefix=CHECK --check-prefix=CXX11
-// RUN: %clang_cc1 -std=c++1z %s -emit-llvm -o - -triple x86_64-linux-gnu | FileCheck %s --check-prefix=CHECK --check-prefix=CXX17
+// RUN: %clang_cc1 -std=c++11 %s -emit-llvm -o - -triple x86_64-linux-gnu | FileCheck %s --check-prefixes=X86,CXX11X86
+// RUN: %clang_cc1 -std=c++1z %s -emit-llvm -o - -triple x86_64-linux-gnu | FileCheck %s --check-prefixes=X86,CXX17X86
+// RUN: %clang_cc1 -std=c++11 %s -emit-llvm -o - -triple amdgcn-amd-amdhsa | FileCheck %s --check-prefixes=AMD,CXX11AMD
+// RUN: %clang_cc1 -std=c++1z %s -emit-llvm -o - -triple amdgcn-amd-amdhsa | FileCheck %s --check-prefixes=AMD,CXX17AMD
 
 struct A {
   static const int Foo = 123;
 };
-// CHECK: @_ZN1A3FooE = constant i32 123, align 4
+// X86: @_ZN1A3FooE = constant i32 123, align 4
+// AMD: @_ZN1A3FooE = addrspace(4) constant i32 123, align 4
 const int *p = &A::Foo; // emit available_externally
 const int A::Foo;   // convert to full definition
 
@@ -16,7 +19,8 @@
   // Deferred initialization of the structure here requires changing
   // the type of the global variable: the initializer list does not include
   // the tail padding.
-  // CXX11: @_ZN9CreatePOD3podE = available_externally constant { i32, i8 } { i32 42, i8 43 },
+  // CXX11X86: @_ZN9CreatePOD3podE = available_externally constant { i32, i8 } { i32 42, i8 43 },
+  // CXX11AMD: @_ZN9CreatePOD3podE = available_externally addrspace(1) constant { i32, i8 } { i32 42, i8 43 },
   static constexpr PODWithInit pod{};
 };
 const int *p_pod = &CreatePOD::pod.g;
@@ -30,29 +34,40 @@
 };
 
 struct Foo {
-  // CXX11: @_ZN3Foo21ConstexprStaticMemberE = available_externally constant i32 42,
-  // CXX17: @_ZN3Foo21ConstexprStaticMemberE = linkonce_odr constant i32 42,
+  // CXX11X86: @_ZN3Foo21ConstexprStaticMemberE = available_externally constant i32 42,
+  // CXX17X86: @_ZN3Foo21ConstexprStaticMemberE = linkonce_odr constant i32 42,
+  // CXX11AMD: @_ZN3Foo21ConstexprStaticMemberE = available_externally addrspace(4) constant i32 42,
+  // CXX17AMD: @_ZN3Foo21ConstexprStaticMemberE = linkonce_odr addrspace(4) constant i32 42,
   static constexpr int ConstexprStaticMember = 42;
-  // CHECK: @_ZN3Foo17ConstStaticMemberE = available_externally constant i32 43,
+  // X86: @_ZN3Foo17ConstStaticMemberE = available_externally constant i32 43,
+  // AMD: @_ZN3Foo17ConstStaticMemberE = available_externally addrspace(4) constant i32 43,
   static const int ConstStaticMember = 43;
 
-  // CXX11: @_ZN3Foo23ConstStaticStructMemberE = available_externally constant %struct.Bar { i32 44 },
-  // CXX17: @_ZN3Foo23ConstStaticStructMemberE = linkonce_odr constant %struct.Bar { i32 44 },
+  // CXX11X86: @_ZN3Foo23ConstStaticStructMemberE = available_externally constant %struct.Bar { i32 44 },
+  // CXX17X86: @_ZN3Foo23ConstStaticStructMemberE = linkonce_odr constant %struct.Bar { i32 44 },
+  // CXX11AMD: @_ZN3Foo23ConstStaticStructMemberE = available_externally addrspace(1) constant %struct.Bar { i32 44 },
+  // CXX17AMD: @_ZN3Foo23ConstStaticStructMemberE = linkonce_odr addrspace(1) constant %struct.Bar { i32 44 },
   static constexpr Bar ConstStaticStructMember = {44};
 
-  // CXX11: @_ZN3Foo34ConstexprStaticMutableStructMemberE = external global %struct.MutableBar,
-  // CXX17: @_ZN3Foo34ConstexprStaticMutableStructMemberE = linkonce_odr global %struct.MutableBar { i32 45 },
+  // CXX11X86: @_ZN3Foo34ConstexprStaticMutableStructMemberE = external global %struct.MutableBar,
+  // CXX17X86: @_ZN3Foo34ConstexprStaticMutableStructMemberE = linkonce_odr global %struct.MutableBar { i32 45 },
+  // CXX11AMD: @_ZN3Foo34ConstexprStaticMutableStructMemberE = external addrspace(1) global %struct.MutableBar,
+  // CXX17AMD: @_ZN3Foo34ConstexprStaticMutableStructMemberE = linkonce_odr addrspace(1) global %struct.MutableBar { i32 45 },
   static constexpr MutableBar ConstexprStaticMutableStructMember = {45};
 };
-// CHECK: @_ZL15ConstStaticexpr = internal constant i32 46,
+// X86: @_ZL15ConstStaticexpr = internal constant i32 46,
+// AMD: @_ZL15ConstStaticexpr = internal addrspace(4)

[PATCH] D69181: [clang-tidy] Adding misc-signal-terminated-thread check

2019-10-20 Thread Kocsis Ábel via Phabricator via cfe-commits
abelkocsis updated this revision to Diff 225788.
abelkocsis added a comment.

The documentation of the checker updated.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D69181

Files:
  clang-tools-extra/clang-tidy/misc/BadSignalToKillThreadCheck.cpp
  clang-tools-extra/clang-tidy/misc/BadSignalToKillThreadCheck.h
  clang-tools-extra/clang-tidy/misc/CMakeLists.txt
  clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/misc-bad-signal-to-kill-thread.rst
  clang-tools-extra/test/clang-tidy/misc-bad-signal-to-kill-thread.cpp

Index: clang-tools-extra/test/clang-tidy/misc-bad-signal-to-kill-thread.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/misc-bad-signal-to-kill-thread.cpp
@@ -0,0 +1,39 @@
+// RUN: %check_clang_tidy %s misc-bad-signal-to-kill-thread %t
+
+#define SIGTERM 15
+#define SIGINT 2
+using pthread_t = int;
+using pthread_attr_t = int;
+
+int pthread_create(pthread_t *thread, const pthread_attr_t *attr,
+   void *(*start_routine)(void *), void *arg);
+
+int pthread_kill(pthread_t thread, int sig);
+
+int pthread_cancel(pthread_t thread);
+
+void *test_func_return_a_pointer(void *foo);
+
+int main() {
+  int result;
+  pthread_t thread;
+
+  if ((result = pthread_create(&thread, nullptr, test_func_return_a_pointer, 0)) != 0) {
+  }
+  if ((result = pthread_kill(thread, SIGTERM)) != 0) {
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: Thread should not be terminated by SIGTERM signal. [misc-bad-signal-to-kill-thread]
+  }
+
+  //compliant solution
+  if ((result = pthread_cancel(thread)) != 0) {
+  }
+
+  if ((result = pthread_kill(thread, SIGINT)) != 0) {
+  }
+  if ((result = pthread_kill(thread, 0xF)) != 0) {
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: Thread should not be terminated by SIGTERM signal. [misc-bad-signal-to-kill-thread]
+  }
+  
+
+  return 0;
+}
Index: clang-tools-extra/docs/clang-tidy/checks/misc-bad-signal-to-kill-thread.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/misc-bad-signal-to-kill-thread.rst
@@ -0,0 +1,14 @@
+.. title:: clang-tidy - misc-bad-signal-to-kill-thread
+
+misc-bad-signal-to-kill-thread
+==
+
+Finds ``pthread_kill`` function calls when thread is terminated by uncaught
+``SIGTERM`` signal and the signal kills the entire process, not just the
+individual thread. Use any signal except ``SIGTERM`` or ``SIGKILL``.
+To learn more about this rule please visit the following page:
+https://wiki.sei.cmu.edu/confluence/display/c/POS44-C.+Do+not+use+signals+to+terminate+threads
+
+.. code-block: c++
+
+pthread_kill(thread, SIGTERM);
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -282,6 +282,7 @@
llvm-prefer-isa-or-dyn-cast-in-conditionals
llvm-prefer-register-over-unsigned
llvm-twine-local
+   misc-bad-signal-to-kill-thread
misc-definitions-in-headers
misc-misplaced-const
misc-new-delete-overloads
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -115,6 +115,12 @@
   Finds historical use of ``unsigned`` to hold vregs and physregs and rewrites
   them to use ``Register``
 
+- New :doc:`misc-bad-signal-to-kill-thread
+  ` check.
+
+  Finds ``pthread_kill`` function calls when thread is terminated by
+  uncaught ``SIGTERM`` signal.
+
 - New :doc:`objc-missing-hash
   ` check.
 
Index: clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
===
--- clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
+++ clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
@@ -9,6 +9,7 @@
 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
+#include "BadSignalToKillThreadCheck.h"
 #include "DefinitionsInHeadersCheck.h"
 #include "MisplacedConstCheck.h"
 #include "NewDeleteOverloadsCheck.h"
@@ -30,6 +31,8 @@
 class MiscModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
+CheckFactories.registerCheck(
+"misc-bad-signal-to-kill-thread");
 CheckFactories.registerCheck(
 "misc-definitions-in-headers");
 CheckFactories.registerCheck("misc-misplaced-const");
Index: clang-tools-extra/clang-tidy/misc/CMakeLists.txt
===
--- clang-tools-extra/clan

[PATCH] D69223: WDocumentation: Implement the \anchor.

2019-10-20 Thread Mark de Wever via Phabricator via cfe-commits
Mordante created this revision.
Mordante added a reviewer: gribozavr.
Mordante added a project: clang.
Herald added a subscriber: arphaman.

I'm not sure it should be added to the `InlineComment` group. It's not entirely 
a markup. Do you think it should be a in a separate group?

(I also have not yet posted code for `\emoji` which is also in the 
`InlineComment` group.)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69223

Files:
  clang/bindings/xml/comment-xml-schema.rng
  clang/include/clang-c/Documentation.h
  clang/include/clang/AST/Comment.h
  clang/include/clang/AST/CommentCommands.td
  clang/lib/AST/CommentSema.cpp
  clang/lib/AST/JSONNodeDumper.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/Index/CommentToXML.cpp
  clang/test/Index/Inputs/CommentXML/valid-function-02.xml
  clang/test/Index/comment-to-html-xml-conversion.cpp
  clang/test/Sema/warn-documentation.cpp
  clang/tools/c-index-test/c-index-test.c
  clang/tools/libclang/CXComment.cpp

Index: clang/tools/libclang/CXComment.cpp
===
--- clang/tools/libclang/CXComment.cpp
+++ clang/tools/libclang/CXComment.cpp
@@ -159,6 +159,9 @@
 
   case InlineCommandComment::RenderEmphasized:
 return CXCommentInlineCommandRenderKind_Emphasized;
+
+  case InlineCommandComment::RenderAnchor:
+return CXCommentInlineCommandRenderKind_Anchor;
   }
   llvm_unreachable("unknown InlineCommandComment::RenderKind");
 }
Index: clang/tools/c-index-test/c-index-test.c
===
--- clang/tools/c-index-test/c-index-test.c
+++ clang/tools/c-index-test/c-index-test.c
@@ -497,6 +497,9 @@
 case CXCommentInlineCommandRenderKind_Emphasized:
   printf(" RenderEmphasized");
   break;
+case CXCommentInlineCommandRenderKind_Anchor:
+  printf(" RenderAnchor");
+  break;
 }
 for (i = 0, e = clang_InlineCommandComment_getNumArgs(Comment);
  i != e; ++i) {
Index: clang/test/Sema/warn-documentation.cpp
===
--- clang/test/Sema/warn-documentation.cpp
+++ clang/test/Sema/warn-documentation.cpp
@@ -1057,6 +1057,13 @@
 /// \a A
 int test_inline_no_argument_a_good(int);
 
+// expected-warning@+1 {{'\anchor' command does not have a valid word argument}}
+/// \anchor
+int test_inline_no_argument_anchor_bad(int);
+
+/// \anchor A
+int test_inline_no_argument_anchor_good(int);
+
 // expected-warning@+1 {{'@b' command does not have a valid word argument}}
 /// @b
 int test_inline_no_argument_b_bad(int);
Index: clang/test/Index/comment-to-html-xml-conversion.cpp
===
--- clang/test/Index/comment-to-html-xml-conversion.cpp
+++ clang/test/Index/comment-to-html-xml-conversion.cpp
@@ -734,6 +734,16 @@
 // CHECK-NEXT: (CXComment_Text Text=[Aaa])
 // CHECK-NEXT: (CXComment_HTMLEndTag Name=[h1])))]
 
+/// \anchor A
+void comment_to_html_conversion_37();
+
+// CHECK: comment-to-html-xml-conversion.cpp:[[@LINE-2]]:6: FunctionDecl=comment_to_html_conversion_37:{{.*}} FullCommentAsHTML=[ ] FullCommentAsXML=[comment_to_html_conversion_37c:@F@comment_to_html_conversion_37#void comment_to_html_conversion_37() A]
+// CHECK-NEXT:  CommentAST=[
+// CHECK-NEXT:(CXComment_FullComment
+// CHECK-NEXT:   (CXComment_Paragraph
+// CHECK-NEXT: (CXComment_Text Text=[ ] IsWhitespace)
+// CHECK-NEXT: (CXComment_InlineCommand CommandName=[anchor] RenderAnchor Arg[0]=A)))]
+
 
 /// Aaa.
 class comment_to_xml_conversion_01 {
Index: clang/test/Index/Inputs/CommentXML/valid-function-02.xml
===
--- clang/test/Index/Inputs/CommentXML/valid-function-02.xml
+++ clang/test/Index/Inputs/CommentXML/valid-function-02.xml
@@ -9,6 +9,7 @@
 
 
 .
+hhh
   
 
 
Index: clang/lib/Index/CommentToXML.cpp
===
--- clang/lib/Index/CommentToXML.cpp
+++ clang/lib/Index/CommentToXML.cpp
@@ -297,6 +297,12 @@
 appendToResultWithHTMLEscaping(Arg0);
 Result << "";
 return;
+  case InlineCommandComment::RenderAnchor:
+assert(C->getNumArgs() == 1);
+Result << "";
+return;
   }
 }
 
@@ -641,6 +647,12 @@
 appendToResultWithXMLEscaping(Arg0);
 Result << "";
 return;
+  case InlineCommandComment::RenderAnchor:
+assert(C->getNumArgs() == 1);
+Result << "";
+appendToResultWithXMLEscaping(Arg0);
+Result << "";
+return;
   }
 }
 
Index: clang/lib/AST/TextNodeDumper.cpp
===
--- clang/lib/AST/TextNodeDumper.cpp
+++ clang/lib/AST/TextNodeDumper.cpp
@@ -489,6 +489,9 @@
   case comments::InlineCommandComment::RenderEmphasized:
 OS << " RenderEmphasized";
 break;
+  case comments::InlineCommandComment::RenderAnchor:
+OS << " Render

[PATCH] D67052: Add reference type transformation builtins

2019-10-20 Thread Zoe Carver via Phabricator via cfe-commits
zoecarver added a comment.
Herald added a reviewer: mclow.lists.

Friendly ping. Other than type mangling, does anything need to be fixed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67052



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


[PATCH] D69225: Sema: Fixes a crash with a templated destructor

2019-10-20 Thread Mark de Wever via Phabricator via cfe-commits
Mordante created this revision.
Mordante added reviewers: rcraik, hubert.reinterpretcast, aaron.ballman, rsmith.
Mordante added a project: clang.

This fixes PR38671.
The issue was introduced by D33833  which 
fixed PR33189.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69225

Files:
  clang/lib/Sema/SemaLookup.cpp
  clang/test/SemaTemplate/destructor-template.cpp


Index: clang/test/SemaTemplate/destructor-template.cpp
===
--- clang/test/SemaTemplate/destructor-template.cpp
+++ clang/test/SemaTemplate/destructor-template.cpp
@@ -92,3 +92,13 @@
   template 
   ~PR33189() { } // expected-error{{destructor cannot be declared as a 
template}}
 };
+
+namespace PR38671 {
+struct S {
+  template 
+  ~S(); // expected-error{{destructor cannot be declared as a template}}
+};
+struct T : S {// expected-note{{destructor of 'T' is implicitly deleted 
because base class 'PR38671::S' has no destructor}}
+  ~T() = default; // expected-warning{{explicitly defaulted destructor is 
implicitly deleted}}
+};
+} // namespace PR38671
Index: clang/lib/Sema/SemaLookup.cpp
===
--- clang/lib/Sema/SemaLookup.cpp
+++ clang/lib/Sema/SemaLookup.cpp
@@ -3101,11 +3101,10 @@
   });
 }
 CXXDestructorDecl *DD = RD->getDestructor();
-assert(DD && "record without a destructor");
 Result->setMethod(DD);
-Result->setKind(DD->isDeleted() ?
-SpecialMemberOverloadResult::NoMemberOrDeleted :
-SpecialMemberOverloadResult::Success);
+Result->setKind(DD && !DD->isDeleted()
+? SpecialMemberOverloadResult::Success
+: SpecialMemberOverloadResult::NoMemberOrDeleted);
 return *Result;
   }
 


Index: clang/test/SemaTemplate/destructor-template.cpp
===
--- clang/test/SemaTemplate/destructor-template.cpp
+++ clang/test/SemaTemplate/destructor-template.cpp
@@ -92,3 +92,13 @@
   template 
   ~PR33189() { } // expected-error{{destructor cannot be declared as a template}}
 };
+
+namespace PR38671 {
+struct S {
+  template 
+  ~S(); // expected-error{{destructor cannot be declared as a template}}
+};
+struct T : S {// expected-note{{destructor of 'T' is implicitly deleted because base class 'PR38671::S' has no destructor}}
+  ~T() = default; // expected-warning{{explicitly defaulted destructor is implicitly deleted}}
+};
+} // namespace PR38671
Index: clang/lib/Sema/SemaLookup.cpp
===
--- clang/lib/Sema/SemaLookup.cpp
+++ clang/lib/Sema/SemaLookup.cpp
@@ -3101,11 +3101,10 @@
   });
 }
 CXXDestructorDecl *DD = RD->getDestructor();
-assert(DD && "record without a destructor");
 Result->setMethod(DD);
-Result->setKind(DD->isDeleted() ?
-SpecialMemberOverloadResult::NoMemberOrDeleted :
-SpecialMemberOverloadResult::Success);
+Result->setKind(DD && !DD->isDeleted()
+? SpecialMemberOverloadResult::Success
+: SpecialMemberOverloadResult::NoMemberOrDeleted);
 return *Result;
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D68074: [clang-tidy] Add readability-make-member-function-const

2019-10-20 Thread Matthias Gehre via Phabricator via cfe-commits
mgehre updated this revision to Diff 225804.
mgehre marked 6 inline comments as done.
mgehre added a comment.

- Update documentation


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68074

Files:
  clang-tools-extra/clang-tidy/readability/CMakeLists.txt
  clang-tools-extra/clang-tidy/readability/MakeMemberFunctionConstCheck.cpp
  clang-tools-extra/clang-tidy/readability/MakeMemberFunctionConstCheck.h
  clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/docs/clang-tidy/checks/readability-make-member-function-const.rst
  clang-tools-extra/test/clang-tidy/readability-make-member-function-const.cpp

Index: clang-tools-extra/test/clang-tidy/readability-make-member-function-const.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/readability-make-member-function-const.cpp
@@ -0,0 +1,321 @@
+// RUN: %check_clang_tidy %s readability-make-member-function-const %t
+
+struct Str {
+  void const_method() const;
+  void non_const_method();
+};
+
+namespace Diagnose {
+struct A;
+
+void free_const_use(const A *);
+void free_const_use(const A &);
+
+struct A {
+  int M;
+  const int ConstM;
+  struct {
+int M;
+  } Struct;
+  Str S;
+  Str Sref;
+
+  void already_const() const;
+
+  int read_field() {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: method 'read_field' can be made const
+// CHECK-FIXES: {{^}}  int read_field() const {
+return M;
+  }
+
+  int read_struct_field() {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: method 'read_struct_field' can be made const
+// CHECK-FIXES: {{^}}  int read_struct_field() const {
+return Struct.M;
+  }
+
+  int read_const_field() {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: method 'read_const_field' can be made const
+// CHECK-FIXES: {{^}}  int read_const_field() const {
+return ConstM;
+  }
+
+  void call_const_member() {
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: method 'call_const_member' can be made const
+// CHECK-FIXES: {{^}}  void call_const_member() const {
+already_const();
+  }
+
+  void call_const_member_on_public_field() {
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: method 'call_const_member_on_public_field' can be made const
+// CHECK-FIXES: {{^}}  void call_const_member_on_public_field() const {
+S.const_method();
+  }
+
+  void call_const_member_on_public_field_ref() {
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: method 'call_const_member_on_public_field_ref' can be made const
+// CHECK-FIXES: {{^}}  void call_const_member_on_public_field_ref() const {
+Sref.const_method();
+  }
+
+  const Str &return_public_field_ref() {
+// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: method 'return_public_field_ref' can be made const
+// CHECK-FIXES: {{^}}  const Str &return_public_field_ref() const {
+return S;
+  }
+
+  const A *return_this_const() {
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: method 'return_this_const' can be made const
+// CHECK-FIXES: {{^}}  const A *return_this_const() const {
+return this;
+  }
+
+  const A &return_this_const_ref() {
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: method 'return_this_const_ref' can be made const
+// CHECK-FIXES: {{^}}  const A &return_this_const_ref() const {
+return *this;
+  }
+
+  void const_use() {
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: method 'const_use' can be made const
+// CHECK-FIXES: {{^}}  void const_use() const {
+free_const_use(this);
+  }
+
+  void const_use_ref() {
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: method 'const_use_ref' can be made const
+// CHECK-FIXES: {{^}}  void const_use_ref() const {
+free_const_use(*this);
+  }
+
+  auto trailingReturn() -> int {
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: method 'trailingReturn' can be made const
+// CHECK-FIXES: {{^}}  auto trailingReturn() const -> int {
+return M;
+  }
+
+  int volatileFunction() volatile {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: method 'volatileFunction' can be made const
+// CHECK-FIXES: {{^}}  int volatileFunction() const volatile {
+return M;
+  }
+
+  int restrictFunction() __restrict {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: method 'restrictFunction' can be made const
+// CHECK-FIXES: {{^}}  int restrictFunction() const __restrict {
+return M;
+  }
+
+  int refFunction() & {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: method 'refFunction' can be made const
+// CHECK-FIXES: {{^}}  int refFunction() const & {
+return M;
+  }
+
+  void out_of_line_call_const();
+  // CHECK-FIXES: {{^}}  void out_of_line_call_const() const;
+};
+
+void A::out_of_line_call_const() {
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: method 'out_of_line_call_const' can be 

[PATCH] D66046: Add new tautological compare warning for bitwise-or with a non-zero constant

2019-10-20 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment.

Any reason why this is off by default?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66046



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


[PATCH] D66046: Add new tautological compare warning for bitwise-or with a non-zero constant

2019-10-20 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment.

Probably it is not very cheap in terms of compile time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66046



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


[PATCH] D62731: Add support for options -frounding-math, ftrapping-math, -fp-model=, and -fp-exception-behavior=, : Specify floating point behavior

2019-10-20 Thread Melanie Blower via Phabricator via cfe-commits
mibintc added inline comments.



Comment at: clang/docs/UsersManual.rst:1318
+   mode informs the compiler that it must not assume any particular
+   rounding mode.
+

rjmccall wrote:
> rjmccall wrote:
> > "represent *the* corresponding IEEE rounding rules"
> A few points about this documentation that occurred to me since the last time 
> I looked at it:
> 
> - It's weird to talk about LLVM here, since this is the Clang documentation.  
> Clang's behavior is not specified in terms of the IR it generates; it's 
> specified in terms of the formal behavior of the source code.  Therefore this 
> documentation should talk about things using concepts from an appropriate 
> language standard whenever possible; in this case, C99 works.
> 
> - It's weird to bring up all these different rounding modes when this option 
> doesn't actually let you do anything with them.  If you want to talk about 
> rounding modes in general that's fine as a way of informing the programmer, 
> but we shouldn't give them information they can't use.
> 
> - I don't think `-fno-rounding-math` is actually equivalent to forcing the 
> use of the `tonearest` rounding mode; I think it *assumes* that the rounding 
> mode is set to `tonearest`.  (Or am I wrong and this is actually guaranteed 
> by ABI?)
> 
> - I don't think we want to *define* `-frounding-math` as exactly equivalent 
> to `-ffp-model=strict`.  That might be a convenient implementation for now, 
> but it seems to me that `-frounding-math` still allows some optimizations 
> that `-ffp-model=strict` wouldn't.
> 
> With that in mind, I'd suggest something like this:
> 
> > Force floating-point operations to honor the dynamically-set rounding mode 
> > by default.
> >
> > The result of a floating-point operation often cannot be exactly 
> > represented in the result type and therefore must be rounded.  IEEE 754 
> > describes different rounding modes that control how to perform this 
> > rounding, not all of which are supported by all implementations.  C 
> > provides interfaces (``fesetround`` and ``fesetenv``) for dynamically 
> > controlling the rounding mode, and while it also recommends certain 
> > conventions for changing the rounding mode, these conventions are not 
> > typically enforced in the ABI.  Since the rounding mode changes the 
> > numerical result of operations, the compiler must understand something 
> > about it in order to optimize floating point operations.
> >
> > Note that floating-point operations performed as part of constant 
> > initialization are formally performed prior to the start of the program and 
> > are therefore not subject to the current rounding mode.  This includes the 
> > initialization of global variables and local ``static`` variables.  
> > Floating-point operations in these contexts will be rounded using 
> > ``FE_TONEAREST``.
> >
> > - The option ``-fno-rounding-math`` allows the compiler to assume that the 
> > rounding mode is set to ``FE_TONEAREST``.  This is the default.
> > - The option ``-frounding-math`` forces the compiler to honor the 
> > dynamically-set rounding mode.  This prevents optimizations which might 
> > affect results if the rounding mode changes or is different from the 
> > default; for example, it prevents floating-point operations from being 
> > reordered across most calls and prevents constant-folding when the result 
> > is not exactly representable.
Thank you, I will work on another patch 


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62731



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


[PATCH] D69233: [OpenCL] Support -fdeclare-opencl-builtins in C++ mode

2019-10-20 Thread Sven van Haastregt via Phabricator via cfe-commits
svenvh created this revision.
svenvh added a reviewer: Anastasia.
Herald added subscribers: cfe-commits, kristina, yaxunl.
Herald added a project: clang.

Support for C++ mode was accidentally lacking due to not checking the
OpenCLCPlusPlus LangOpts version.


Repository:
  rC Clang

https://reviews.llvm.org/D69233

Files:
  clang/lib/Sema/SemaLookup.cpp
  clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl


Index: clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl
===
--- clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl
+++ clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl
@@ -4,8 +4,10 @@
 // RUN: %clang_cc1 %s -triple spir -verify -pedantic -Wconversion -Werror 
-fsyntax-only -cl-std=CL1.2 -fdeclare-opencl-builtins -finclude-default-header
 // RUN: %clang_cc1 %s -triple spir -verify -pedantic -Wconversion -Werror 
-fsyntax-only -cl-std=CL2.0 -fdeclare-opencl-builtins -DNO_HEADER
 // RUN: %clang_cc1 %s -triple spir -verify -pedantic -Wconversion -Werror 
-fsyntax-only -cl-std=CL2.0 -fdeclare-opencl-builtins -finclude-default-header
+// RUN: %clang_cc1 %s -triple spir -verify -pedantic -Wconversion -Werror 
-fsyntax-only -cl-std=CLC++ -fdeclare-opencl-builtins -DNO_HEADER
+// RUN: %clang_cc1 %s -triple spir -verify -pedantic -Wconversion -Werror 
-fsyntax-only -cl-std=CLC++ -fdeclare-opencl-builtins -finclude-default-header
 
-#if __OPENCL_C_VERSION__ >= CL_VERSION_2_0
+#if defined(__OPENCL_CPP_VERSION__) || __OPENCL_C_VERSION__ >= CL_VERSION_2_0
 // expected-no-diagnostics
 #endif
 
@@ -97,7 +99,7 @@
 
 kernel void basic_subgroup(global uint *out) {
   out[0] = get_sub_group_size();
-#if __OPENCL_C_VERSION__ < CL_VERSION_2_0
+#if !defined(__OPENCL_CPP_VERSION__) && __OPENCL_C_VERSION__ < CL_VERSION_2_0
 // expected-error@-2{{implicit declaration of function 'get_sub_group_size' is 
invalid in OpenCL}}
 // expected-error@-3{{implicit conversion changes signedness: 'int' to 'uint' 
(aka 'unsigned int')}}
 #endif
@@ -130,7 +132,7 @@
   uint ui;
 
   get_enqueued_local_size(ui);
-#if __OPENCL_C_VERSION__ < CL_VERSION_2_0
+#if !defined(__OPENCL_CPP_VERSION__) && __OPENCL_C_VERSION__ < CL_VERSION_2_0
 // expected-error@-2{{implicit declaration of function 
'get_enqueued_local_size' is invalid in OpenCL}}
 #endif
 }
Index: clang/lib/Sema/SemaLookup.cpp
===
--- clang/lib/Sema/SemaLookup.cpp
+++ clang/lib/Sema/SemaLookup.cpp
@@ -765,10 +765,13 @@
 ASTContext &Context = S.Context;
 
 // Ignore this BIF if its version does not match the language options.
-if (Context.getLangOpts().OpenCLVersion < OpenCLBuiltin.MinVersion)
+unsigned OpenCLVersion = Context.getLangOpts().OpenCLVersion;
+if (Context.getLangOpts().OpenCLCPlusPlus)
+  OpenCLVersion = 200;
+if (OpenCLVersion < OpenCLBuiltin.MinVersion)
   continue;
 if ((OpenCLBuiltin.MaxVersion != 0) &&
-(Context.getLangOpts().OpenCLVersion >= OpenCLBuiltin.MaxVersion))
+(OpenCLVersion >= OpenCLBuiltin.MaxVersion))
   continue;
 
 SmallVector RetTypes;


Index: clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl
===
--- clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl
+++ clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl
@@ -4,8 +4,10 @@
 // RUN: %clang_cc1 %s -triple spir -verify -pedantic -Wconversion -Werror -fsyntax-only -cl-std=CL1.2 -fdeclare-opencl-builtins -finclude-default-header
 // RUN: %clang_cc1 %s -triple spir -verify -pedantic -Wconversion -Werror -fsyntax-only -cl-std=CL2.0 -fdeclare-opencl-builtins -DNO_HEADER
 // RUN: %clang_cc1 %s -triple spir -verify -pedantic -Wconversion -Werror -fsyntax-only -cl-std=CL2.0 -fdeclare-opencl-builtins -finclude-default-header
+// RUN: %clang_cc1 %s -triple spir -verify -pedantic -Wconversion -Werror -fsyntax-only -cl-std=CLC++ -fdeclare-opencl-builtins -DNO_HEADER
+// RUN: %clang_cc1 %s -triple spir -verify -pedantic -Wconversion -Werror -fsyntax-only -cl-std=CLC++ -fdeclare-opencl-builtins -finclude-default-header
 
-#if __OPENCL_C_VERSION__ >= CL_VERSION_2_0
+#if defined(__OPENCL_CPP_VERSION__) || __OPENCL_C_VERSION__ >= CL_VERSION_2_0
 // expected-no-diagnostics
 #endif
 
@@ -97,7 +99,7 @@
 
 kernel void basic_subgroup(global uint *out) {
   out[0] = get_sub_group_size();
-#if __OPENCL_C_VERSION__ < CL_VERSION_2_0
+#if !defined(__OPENCL_CPP_VERSION__) && __OPENCL_C_VERSION__ < CL_VERSION_2_0
 // expected-error@-2{{implicit declaration of function 'get_sub_group_size' is invalid in OpenCL}}
 // expected-error@-3{{implicit conversion changes signedness: 'int' to 'uint' (aka 'unsigned int')}}
 #endif
@@ -130,7 +132,7 @@
   uint ui;
 
   get_enqueued_local_size(ui);
-#if __OPENCL_C_VERSION__ < CL_VERSION_2_0
+#if !defined(__OPENCL_CPP_VERSION__) && __OPENCL_C_VERSION__ < CL_VERSION_2_0
 // expected-error@-2{{implicit declaration of function 'get_enqueued_local_size' is inva

[PATCH] D69237: Refactor getDeclAtPosition() to use SelectionTree + targetDecl()

2019-10-20 Thread Nathan Ridge via Phabricator via cfe-commits
nridge created this revision.
nridge added a reviewer: sammccall.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, mgrang, 
jkorous.
Herald added a project: clang.

This fixes issue #163, among other improvements to go-to-definition.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69237

Files:
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/unittests/SymbolInfoTests.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp

Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -517,9 +517,10 @@
   EXPECT_THAT(locateSymbolAt(AST, T.point("4")), ElementsAre(Sym("g")));
   EXPECT_THAT(locateSymbolAt(AST, T.point("5")), ElementsAre(Sym("f")));
   EXPECT_THAT(locateSymbolAt(AST, T.point("6")), ElementsAre(Sym("str")));
-  EXPECT_THAT(locateSymbolAt(AST, T.point("7")), ElementsAre(Sym("abc")));
+  EXPECT_THAT(locateSymbolAt(AST, T.point("7")),
+  ElementsAre(Sym("abc"), Sym("Foo")));
   EXPECT_THAT(locateSymbolAt(AST, T.point("8")),
-  ElementsAre(Sym("Foo"), Sym("abcd")));
+  ElementsAre(Sym("abcd"), Sym("Foo")));
   EXPECT_THAT(locateSymbolAt(AST, T.point("9")),
   // First one is class definition, second is the constructor.
   ElementsAre(Sym("Foo"), Sym("Foo")));
@@ -2190,10 +2191,7 @@
   struct Test {
 StringRef AnnotatedCode;
 const char *DeducedType;
-  } Tests[] = {
-  {"^auto i = 0;", "int"},
-  {"^auto f(){ return 1;};", "int"}
-  };
+  } Tests[] = {{"^auto i = 0;", "int"}, {"^auto f(){ return 1;};", "int"}};
   for (Test T : Tests) {
 Annotations File(T.AnnotatedCode);
 auto AST = TestTU::withCode(File.code()).build();
Index: clang-tools-extra/clangd/unittests/SymbolInfoTests.cpp
===
--- clang-tools-extra/clangd/unittests/SymbolInfoTests.cpp
+++ clang-tools-extra/clangd/unittests/SymbolInfoTests.cpp
@@ -24,7 +24,7 @@
 namespace clangd {
 namespace {
 
-using ::testing::ElementsAreArray;
+using ::testing::UnorderedElementsAreArray;
 
 auto CreateExpectedSymbolDetails = [](const std::string &name,
   const std::string &container,
@@ -329,7 +329,7 @@
 auto AST = TestTU::withCode(TestInput.code()).build();
 
 EXPECT_THAT(getSymbolInfo(AST, TestInput.point()),
-ElementsAreArray(T.second))
+UnorderedElementsAreArray(T.second))
 << T.first;
   }
 }
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -14,6 +14,7 @@
 #include "Logger.h"
 #include "ParsedAST.h"
 #include "Protocol.h"
+#include "Selection.h"
 #include "SourceCode.h"
 #include "URI.h"
 #include "index/Index.h"
@@ -133,83 +134,43 @@
   return Merged.CanonicalDeclaration;
 }
 
-/// Finds declarations locations that a given source location refers to.
-class DeclarationFinder : public index::IndexDataConsumer {
-  llvm::DenseSet Decls;
-  const SourceLocation &SearchedLocation;
-
-public:
-  DeclarationFinder(const SourceLocation &SearchedLocation)
-  : SearchedLocation(SearchedLocation) {}
-
-  // The results are sorted by declaration location.
-  std::vector getFoundDecls() const {
-std::vector Result;
-for (const Decl *D : Decls)
-  Result.push_back(D);
-
-llvm::sort(Result, [](const Decl *L, const Decl *R) {
-  return L->getBeginLoc() < R->getBeginLoc();
-});
-return Result;
+// If a constructor is being called at the location indicated by N, return
+// that constructor. FIXME(nridge): this should probably be handled in
+// targetDecl() itself.
+const CXXConstructorDecl *findCalledConstructor(const SelectionTree::Node *N) {
+  if (const VarDecl *VD = N->ASTNode.get()) {
+if (const Expr *Init = VD->getInit()) {
+  if (const CXXConstructExpr *CE = dyn_cast(Init))
+return CE->getConstructor();
+}
   }
+  while ((N = N->Parent)) {
+if (N->ASTNode.get() || N->ASTNode.get() ||
+N->ASTNode.get())
+  break;
+if (const CXXConstructExpr *CE = N->ASTNode.get())
+  return CE->getConstructor();
+  }
+  return nullptr;
+}
 
-  bool
-  handleDeclOccurence(const Decl *D, index::SymbolRoleSet Roles,
-  llvm::ArrayRef Relations,
-  SourceLocation Loc,
-  index::IndexDataConsumer::ASTNodeInfo ASTNode) override {
-// Skip non-semantic references.
-if (Roles & static_cast(index::SymbolRole::NameReference))
-  return true;
-
-if (Loc == SearchedLocation) {
-  auto IsImplicitExpr = [](const Expr *E) {
-if (!E)
-  return false;
-// We assume that a constructor expression is implict (was inserted b

[PATCH] D69237: Refactor getDeclAtPosition() to use SelectionTree + targetDecl()

2019-10-20 Thread Nathan Ridge via Phabricator via cfe-commits
nridge marked 3 inline comments as done.
nridge added inline comments.



Comment at: clang-tools-extra/clangd/XRefs.cpp:139
+// that constructor. FIXME(nridge): this should probably be handled in
+// targetDecl() itself.
+const CXXConstructorDecl *findCalledConstructor(const SelectionTree::Node *N) {

I would appreciate some tips on how we could handle this in `targetDecl()`. 
Please see more specific comments below.



Comment at: clang-tools-extra/clangd/XRefs.cpp:141
+const CXXConstructorDecl *findCalledConstructor(const SelectionTree::Node *N) {
+  if (const VarDecl *VD = N->ASTNode.get()) {
+if (const Expr *Init = VD->getInit()) {

This part is intended to handle case #8 in `LocateSymbol.Ambiguous`, where you 
have a declaration of the form `Foo abcd("asdf");` which invokes a constructor.

A naive attempt to add this logic to `TargetFinder::add(const Decl*)` resulted 
in the constructor being a target even for //references// to the variable, not 
just its declaration.



Comment at: clang-tools-extra/clangd/XRefs.cpp:147
   }
+  while ((N = N->Parent)) {
+if (N->ASTNode.get() || N->ASTNode.get() ||

This part is a super hacky way to keep case #9 in `LocateSymbol.Ambiguous` 
passing while not regressing other tests.

In that case, the selection tree looks like this:

```
   DeclStmt Foo foox = Foo("asdf");

 VarDecl Foo foox = Foo("asdf")
   ExprWithCleanups Foo("asdf")
 CXXConstructExpr Foo("asdf")
   MaterializeTemporaryExpr Foo("asdf")
 CXXFunctionalCastExpr Foo("asdf")
  .RecordTypeLoc Foo
```

The common ancestor is the `RecordTypeLoc`, but we'd like to find the 
constructor referred to by the `CXXConstructExpr` as a target as well. To 
achieve that, my current code walks up the parent tree until we encounter a 
`CXXConstructExpr`, but aborts the walk if certain node types are encountered 
to avoid certain other scenarios where we have a `CXXConstructExpr` ancestor.

This is almost certainly wrong, as there are likely many other cases where we 
have a `CXXConstructExpr` ancestor but don't want to find the constructor as a 
target which are not being handled correctly.

Is there a better way to identify the syntactic form at issue in this testcase?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69237



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