[PATCH] D69475: [clang] Provide better fix-it on exception spec error

[Jeremy Demeule via Phabricator via cfe-commits]

jdemeule created this revision.
jdemeule added a reviewer: rsmith.
jdemeule added a project: clang.

Clang will report an error when the exception specification is not respected 
with a fix-it.
However, this fix-it does not fully qualify the type in case of function 
template specialization which lead to "incomplete" transformation (resulting 
code does not compile).
This patch attempts to create a more realiable in the first place by reusing 
the pretty printer helper defined in `SemaTemplate` which expand template 
arguments.


Repository:
  rC Clang

https://reviews.llvm.org/D69475

Files:
  clang/include/clang/AST/PrettyPrinter.h
  clang/lib/AST/StmtPrinter.cpp
  clang/lib/Sema/SemaExceptionSpec.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/SemaCXX/exception-spec.cpp

Index: clang/test/SemaCXX/exception-spec.cpp
===
--- clang/test/SemaCXX/exception-spec.cpp
+++ clang/test/SemaCXX/exception-spec.cpp
@@ -52,3 +52,21 @@
 D2 &operator=(const D2&); // expected-error {{more lax}}
   };
 }
+
+namespace MissingOnTemplate {
+  template
+  struct type_dependant_noexcept {
+static constexpr const bool value = (sizeof(T) < 4);
+  };
+
+  template void a_fct(T) noexcept(type_dependant_noexcept::value);  // expected-note {{previous}}
+  template void a_fct(T);  // expected-error {{missing exception specification 'noexcept(type_dependant_noexcept::value)'}}
+}
+
+namespace abc {
+  struct A {};
+}  // namespace abc
+
+namespace MissingOnTemplate {
+  template<>void a_fct(abc::A);  // expected-error {{missing exception specification 'noexcept(type_dependant_noexcept::value)'}} expected-note {{previous}}
+}
\ No newline at end of file
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -3115,40 +3115,6 @@
   return Cond;
 }
 
-namespace {
-
-// A PrinterHelper that prints more helpful diagnostics for some sub-expressions
-// within failing boolean expression, such as substituting template parameters
-// for actual types.
-class FailedBooleanConditionPrinterHelper : public PrinterHelper {
-public:
-  explicit FailedBooleanConditionPrinterHelper(const PrintingPolicy &P)
-  : Policy(P) {}
-
-  bool handledStmt(Stmt *E, raw_ostream &OS) override {
-const auto *DR = dyn_cast(E);
-if (DR && DR->getQualifier()) {
-  // If this is a qualified name, expand the template arguments in nested
-  // qualifiers.
-  DR->getQualifier()->print(OS, Policy, true);
-  // Then print the decl itself.
-  const ValueDecl *VD = DR->getDecl();
-  OS << VD->getName();
-  if (const auto *IV = dyn_cast(VD)) {
-// This is a template variable, print the expanded template arguments.
-printTemplateArgumentList(OS, IV->getTemplateArgs().asArray(), Policy);
-  }
-  return true;
-}
-return false;
-  }
-
-private:
-  const PrintingPolicy Policy;
-};
-
-} // end anonymous namespace
-
 std::pair
 Sema::findFailedBooleanCondition(Expr *Cond) {
   Cond = lookThroughRangesV3Condition(PP, Cond);
@@ -3187,7 +3153,7 @@
 llvm::raw_string_ostream Out(Description);
 PrintingPolicy Policy = getPrintingPolicy();
 Policy.PrintCanonicalTypes = true;
-FailedBooleanConditionPrinterHelper Helper(Policy);
+ExpandTemplateArgumentsPrinterHelper Helper(Policy);
 FailedCond->printPretty(Out, &Helper, Policy, 0, "\n", nullptr);
   }
   return { FailedCond, Description };
Index: clang/lib/Sema/SemaExceptionSpec.cpp
===
--- clang/lib/Sema/SemaExceptionSpec.cpp
+++ clang/lib/Sema/SemaExceptionSpec.cpp
@@ -423,7 +423,11 @@
   case EST_NoexceptTrue:
 OS << "noexcept(";
 assert(OldProto->getNoexceptExpr() != nullptr && "Expected non-null Expr");
-OldProto->getNoexceptExpr()->printPretty(OS, nullptr, getPrintingPolicy());
+{
+  PrintingPolicy Policy = getPrintingPolicy();
+  ExpandTemplateArgumentsPrinterHelper Helper(Policy);
+  OldProto->getNoexceptExpr()->printPretty(OS, &Helper, Policy);
+}
 OS << ")";
 break;
   case EST_NoThrow:
Index: clang/lib/AST/StmtPrinter.cpp
===
--- clang/lib/AST/StmtPrinter.cpp
+++ clang/lib/AST/StmtPrinter.cpp
@@ -2472,3 +2472,26 @@
 
 // Implement virtual destructor.
 PrinterHelper::~PrinterHelper() = default;
+
+//===--===//
+// ExpandTemplateArgumentsPrinterHelper
+//===--===//
+
+bool ExpandTemplateArgumentsPrinterHelper::handledStmt(Stmt *E,
+   raw_ostream &OS) {
+  const auto *DR = dyn_cast(E);
+  if (DR && DR->getQualifier()) {
+// If this is a qualified name, expand the template arguments in nest

[clang] d4758d4 - Fix a spelling mistake in a couple of intrinsic description comments. NFC

[Greg Bedwell via cfe-commits]

Author: Greg Bedwell
Date: 2019-10-27T09:42:14Z
New Revision: d4758d4a8d842275f4319ae278808be0bcd2ede3

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

LOG: Fix a spelling mistake in a couple of intrinsic description comments. NFC

Added: 


Modified: 
clang/lib/Headers/emmintrin.h

Removed: 




diff  --git a/clang/lib/Headers/emmintrin.h b/clang/lib/Headers/emmintrin.h
index c8fefdfc792a..f98b7e75c220 100644
--- a/clang/lib/Headers/emmintrin.h
+++ b/clang/lib/Headers/emmintrin.h
@@ -2288,7 +2288,7 @@ _mm_adds_epu16(__m128i __a, __m128i __b)
   return (__m128i)__builtin_ia32_paddusw128((__v8hi)__a, (__v8hi)__b);
 }
 
-/// Computes the rounded avarages of corresponding elements of two
+/// Computes the rounded averages of corresponding elements of two
 ///128-bit unsigned [16 x i8] vectors, saving each result in the
 ///corresponding element of a 128-bit result vector of [16 x i8].
 ///
@@ -2308,7 +2308,7 @@ _mm_avg_epu8(__m128i __a, __m128i __b)
   return (__m128i)__builtin_ia32_pavgb128((__v16qi)__a, (__v16qi)__b);
 }
 
-/// Computes the rounded avarages of corresponding elements of two
+/// Computes the rounded averages of corresponding elements of two
 ///128-bit unsigned [8 x i16] vectors, saving each result in the
 ///corresponding element of a 128-bit result vector of [8 x i16].
 ///



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

[PATCH] D69433: [clang-format] [NFC] update the documentation in Format.h to allow dump_format_style.py to get a little closer to being correct. (part 2)

[Duncan Ogilvie via Phabricator via cfe-commits]

mrexodia added a comment.

I don't really have a say in this, but I was trying to run the python script to 
generate the rst file and it didn't work. I tried fixing it, but it's 
definitely very hacky. Perhaps some kind of test can be introduced that runs 
this python script and checks if it succeeds, so that people are aware that 
their changes to Format.h break the documentation generation. I also need this 
to continue with https://reviews.llvm.org/D31635.


Repository:
  rC Clang

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

https://reviews.llvm.org/D69433



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

[PATCH] D69433: [clang-format] [NFC] update the documentation in Format.h to allow dump_format_style.py to get a little closer to being correct. (part 2)

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

In D69433#1722463 , @mrexodia wrote:

> I don't really have a say in this, but I was trying to run the python script 
> to generate the rst file and it didn't work. I tried fixing it, but it's 
> definitely very hacky. Perhaps some kind of test can be introduced that runs 
> this python script and checks if it succeeds, so that people are aware that 
> their changes to Format.h break the documentation generation. I also need 
> this to continue with https://reviews.llvm.org/D31635.


I agree, but first let us get them aligned again first, this revision goes some 
way to making that possible.

The remaining item from @mitchell-stellar's change I had an idea that we could 
put comments regarding the enums into a global dictionary of options, then if 
we see the enum used in the struct we could write the documentation out then, 
and then I think we'd be good, but I'd like to work on that change in 
isolation, and I don't want to do that with this one inflight still

Bottom line lets work out how we want to move forward after we are back in 
shape, at present we are out of sync and the script can't be used without 
wiping out previous documentation.


Repository:
  rC Clang

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

https://reviews.llvm.org/D69433



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

[PATCH] D31839: make -Winteger-overflow find overflows in function arguments

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added a comment.

Are you gonna commit this patch?


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

https://reviews.llvm.org/D31839



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

[PATCH] D68377: [Builtins] Teach Clang about memccpy

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 updated this revision to Diff 226570.
xbolva00 added a comment.

Addressed review note.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68377

Files:
  clang/include/clang/Basic/Builtins.def
  clang/test/CodeGen/builtin-memfns.c


Index: clang/test/CodeGen/builtin-memfns.c
===
--- clang/test/CodeGen/builtin-memfns.c
+++ clang/test/CodeGen/builtin-memfns.c
@@ -4,6 +4,7 @@
 typedef __SIZE_TYPE__ size_t;
 
 void *memcpy(void *, void const *, size_t);
+void *memccpy(void *, void const *, int, size_t);
 
 // CHECK: @test1
 // CHECK: call void @llvm.memset.p0i8.i32
@@ -118,3 +119,9 @@
   // CHECK: call void @llvm.memcpy{{.*}}(
   memcpy(&dest_array, &dest_array, 2);
 }
+
+// CHECK-LABEL: @test13
+void test13(char *d, char *s, int c, size_t n) {
+  // CHECK: call i8* @memccpy
+  memccpy(d, s, c, n);
+}
Index: clang/include/clang/Basic/Builtins.def
===
--- clang/include/clang/Basic/Builtins.def
+++ clang/include/clang/Basic/Builtins.def
@@ -979,6 +979,7 @@
 // when these functions are used in non-GNU mode. PR16138.
 LIBBUILTIN(alloca, "v*z", "f", "stdlib.h", ALL_GNU_LANGUAGES)
 // POSIX string.h
+LIBBUILTIN(memccpy, "v*v*vC*iz",  "f", "string.h", ALL_GNU_LANGUAGES)
 LIBBUILTIN(stpcpy, "c*c*cC*", "f", "string.h", ALL_GNU_LANGUAGES)
 LIBBUILTIN(stpncpy, "c*c*cC*z",   "f", "string.h", ALL_GNU_LANGUAGES)
 LIBBUILTIN(strdup, "c*cC*",   "f", "string.h", ALL_GNU_LANGUAGES)


Index: clang/test/CodeGen/builtin-memfns.c
===
--- clang/test/CodeGen/builtin-memfns.c
+++ clang/test/CodeGen/builtin-memfns.c
@@ -4,6 +4,7 @@
 typedef __SIZE_TYPE__ size_t;
 
 void *memcpy(void *, void const *, size_t);
+void *memccpy(void *, void const *, int, size_t);
 
 // CHECK: @test1
 // CHECK: call void @llvm.memset.p0i8.i32
@@ -118,3 +119,9 @@
   // CHECK: call void @llvm.memcpy{{.*}}(
   memcpy(&dest_array, &dest_array, 2);
 }
+
+// CHECK-LABEL: @test13
+void test13(char *d, char *s, int c, size_t n) {
+  // CHECK: call i8* @memccpy
+  memccpy(d, s, c, n);
+}
Index: clang/include/clang/Basic/Builtins.def
===
--- clang/include/clang/Basic/Builtins.def
+++ clang/include/clang/Basic/Builtins.def
@@ -979,6 +979,7 @@
 // when these functions are used in non-GNU mode. PR16138.
 LIBBUILTIN(alloca, "v*z", "f", "stdlib.h", ALL_GNU_LANGUAGES)
 // POSIX string.h
+LIBBUILTIN(memccpy, "v*v*vC*iz",  "f", "string.h", ALL_GNU_LANGUAGES)
 LIBBUILTIN(stpcpy, "c*c*cC*", "f", "string.h", ALL_GNU_LANGUAGES)
 LIBBUILTIN(stpncpy, "c*c*cC*z",   "f", "string.h", ALL_GNU_LANGUAGES)
 LIBBUILTIN(strdup, "c*cC*",   "f", "string.h", ALL_GNU_LANGUAGES)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68377: [Builtins] Teach Clang about memccpy

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 updated this revision to Diff 226571.
xbolva00 added a comment.

Added new testcase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68377

Files:
  clang/include/clang/Basic/Builtins.def
  clang/test/CodeGen/builtin-memfns.c
  clang/test/CodeGen/memccpy-libcall.c


Index: clang/test/CodeGen/memccpy-libcall.c
===
--- /dev/null
+++ clang/test/CodeGen/memccpy-libcall.c
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -fno-builtin-memccpy -emit-llvm < %s| FileCheck %s
+
+typedef __SIZE_TYPE__ size_t;
+
+void *memccpy(void *, void const *, int, size_t);
+
+void test13(char *d, char *s, int c, size_t n) {
+  // CHECK: call i8* @memccpy{{.*}} #2
+  memccpy(d, s, c, n);
+}
+
+// CHECK: attributes #2 = { nobuiltin }
\ No newline at end of file
Index: clang/test/CodeGen/builtin-memfns.c
===
--- clang/test/CodeGen/builtin-memfns.c
+++ clang/test/CodeGen/builtin-memfns.c
@@ -4,6 +4,7 @@
 typedef __SIZE_TYPE__ size_t;
 
 void *memcpy(void *, void const *, size_t);
+void *memccpy(void *, void const *, int, size_t);
 
 // CHECK: @test1
 // CHECK: call void @llvm.memset.p0i8.i32
@@ -118,3 +119,9 @@
   // CHECK: call void @llvm.memcpy{{.*}}(
   memcpy(&dest_array, &dest_array, 2);
 }
+
+// CHECK-LABEL: @test13
+void test13(char *d, char *s, int c, size_t n) {
+  // CHECK: call i8* @memccpy
+  memccpy(d, s, c, n);
+}
Index: clang/include/clang/Basic/Builtins.def
===
--- clang/include/clang/Basic/Builtins.def
+++ clang/include/clang/Basic/Builtins.def
@@ -979,6 +979,7 @@
 // when these functions are used in non-GNU mode. PR16138.
 LIBBUILTIN(alloca, "v*z", "f", "stdlib.h", ALL_GNU_LANGUAGES)
 // POSIX string.h
+LIBBUILTIN(memccpy, "v*v*vC*iz",  "f", "string.h", ALL_GNU_LANGUAGES)
 LIBBUILTIN(stpcpy, "c*c*cC*", "f", "string.h", ALL_GNU_LANGUAGES)
 LIBBUILTIN(stpncpy, "c*c*cC*z",   "f", "string.h", ALL_GNU_LANGUAGES)
 LIBBUILTIN(strdup, "c*cC*",   "f", "string.h", ALL_GNU_LANGUAGES)


Index: clang/test/CodeGen/memccpy-libcall.c
===
--- /dev/null
+++ clang/test/CodeGen/memccpy-libcall.c
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -fno-builtin-memccpy -emit-llvm < %s| FileCheck %s
+
+typedef __SIZE_TYPE__ size_t;
+
+void *memccpy(void *, void const *, int, size_t);
+
+void test13(char *d, char *s, int c, size_t n) {
+  // CHECK: call i8* @memccpy{{.*}} #2
+  memccpy(d, s, c, n);
+}
+
+// CHECK: attributes #2 = { nobuiltin }
\ No newline at end of file
Index: clang/test/CodeGen/builtin-memfns.c
===
--- clang/test/CodeGen/builtin-memfns.c
+++ clang/test/CodeGen/builtin-memfns.c
@@ -4,6 +4,7 @@
 typedef __SIZE_TYPE__ size_t;
 
 void *memcpy(void *, void const *, size_t);
+void *memccpy(void *, void const *, int, size_t);
 
 // CHECK: @test1
 // CHECK: call void @llvm.memset.p0i8.i32
@@ -118,3 +119,9 @@
   // CHECK: call void @llvm.memcpy{{.*}}(
   memcpy(&dest_array, &dest_array, 2);
 }
+
+// CHECK-LABEL: @test13
+void test13(char *d, char *s, int c, size_t n) {
+  // CHECK: call i8* @memccpy
+  memccpy(d, s, c, n);
+}
Index: clang/include/clang/Basic/Builtins.def
===
--- clang/include/clang/Basic/Builtins.def
+++ clang/include/clang/Basic/Builtins.def
@@ -979,6 +979,7 @@
 // when these functions are used in non-GNU mode. PR16138.
 LIBBUILTIN(alloca, "v*z", "f", "stdlib.h", ALL_GNU_LANGUAGES)
 // POSIX string.h
+LIBBUILTIN(memccpy, "v*v*vC*iz",  "f", "string.h", ALL_GNU_LANGUAGES)
 LIBBUILTIN(stpcpy, "c*c*cC*", "f", "string.h", ALL_GNU_LANGUAGES)
 LIBBUILTIN(stpncpy, "c*c*cC*z",   "f", "string.h", ALL_GNU_LANGUAGES)
 LIBBUILTIN(strdup, "c*cC*",   "f", "string.h", ALL_GNU_LANGUAGES)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69478: [Sema] Allow warnStackExhausted to show more info

[Mark de Wever via Phabricator via cfe-commits]

Mordante created this revision.
Mordante added reviewers: rnk, rsmith, aaron.ballman.
Mordante added a project: clang.

This allows the caller of the function to add a note about how to fix the stack 
exhaustion. This can be useful to aid the user. For example it can be used to 
show a help message for https://bugs.llvm.org/show_bug.cgi?id=14030


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69478

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/Sema.cpp


Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -386,17 +386,21 @@
   SemaPPCallbackHandler->reset();
 }
 
-void Sema::warnStackExhausted(SourceLocation Loc) {
+void Sema::warnStackExhausted(SourceLocation Loc, unsigned NoteDiagID) {
   // Only warn about this once.
   if (!WarnedStackExhausted) {
 Diag(Loc, diag::warn_stack_exhausted);
+if (NoteDiagID)
+  Diag(Loc, NoteDiagID);
 WarnedStackExhausted = true;
   }
 }
 
 void Sema::runWithSufficientStackSpace(SourceLocation Loc,
-   llvm::function_ref Fn) {
-  clang::runWithSufficientStackSpace([&] { warnStackExhausted(Loc); }, Fn);
+   llvm::function_ref Fn,
+   unsigned NoteDiagID) {
+  clang::runWithSufficientStackSpace(
+  [&] { warnStackExhausted(Loc, NoteDiagID); }, Fn);
 }
 
 /// makeUnavailableInSystemHeader - There is an error in the current
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -1307,14 +1307,17 @@
   void PrintStats() const;
 
   /// Warn that the stack is nearly exhausted.
-  void warnStackExhausted(SourceLocation Loc);
+  /// \param NoteDiagID Extra information of the user how to resolve the issue.
+  void warnStackExhausted(SourceLocation Loc, unsigned NoteDiagID = 0);
 
   /// Run some code with "sufficient" stack space. (Currently, at least 256K is
   /// guaranteed). Produces a warning if we're low on stack space and allocates
   /// more in that case. Use this in code that may recurse deeply (for example,
   /// in template instantiation) to avoid stack overflow.
+  /// \param NoteDiagID Extra information of the user how to resolve the issue.
   void runWithSufficientStackSpace(SourceLocation Loc,
-   llvm::function_ref Fn);
+   llvm::function_ref Fn,
+   unsigned NoteDiagID = 0);
 
   /// Helper class that creates diagnostics with optional
   /// template instantiation stacks.


Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -386,17 +386,21 @@
   SemaPPCallbackHandler->reset();
 }
 
-void Sema::warnStackExhausted(SourceLocation Loc) {
+void Sema::warnStackExhausted(SourceLocation Loc, unsigned NoteDiagID) {
   // Only warn about this once.
   if (!WarnedStackExhausted) {
 Diag(Loc, diag::warn_stack_exhausted);
+if (NoteDiagID)
+  Diag(Loc, NoteDiagID);
 WarnedStackExhausted = true;
   }
 }
 
 void Sema::runWithSufficientStackSpace(SourceLocation Loc,
-   llvm::function_ref Fn) {
-  clang::runWithSufficientStackSpace([&] { warnStackExhausted(Loc); }, Fn);
+   llvm::function_ref Fn,
+   unsigned NoteDiagID) {
+  clang::runWithSufficientStackSpace(
+  [&] { warnStackExhausted(Loc, NoteDiagID); }, Fn);
 }
 
 /// makeUnavailableInSystemHeader - There is an error in the current
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -1307,14 +1307,17 @@
   void PrintStats() const;
 
   /// Warn that the stack is nearly exhausted.
-  void warnStackExhausted(SourceLocation Loc);
+  /// \param NoteDiagID Extra information of the user how to resolve the issue.
+  void warnStackExhausted(SourceLocation Loc, unsigned NoteDiagID = 0);
 
   /// Run some code with "sufficient" stack space. (Currently, at least 256K is
   /// guaranteed). Produces a warning if we're low on stack space and allocates
   /// more in that case. Use this in code that may recurse deeply (for example,
   /// in template instantiation) to avoid stack overflow.
+  /// \param NoteDiagID Extra information of the user how to resolve the issue.
   void runWithSufficientStackSpace(SourceLocation Loc,
-   llvm::function_ref Fn);
+   llvm::function_ref Fn,
+   unsigned NoteDiagID = 0);
 
   /// Helper class that creates diagnostics with optional
   /// template instantiation stacks.
__

[PATCH] D69479: [Sema] Warn about possible stack exhaution

[Mark de Wever via Phabricator via cfe-commits]

Mordante created this revision.
Mordante added reviewers: rnk, rsmith, aaron.ballman.
Mordante added a project: clang.

When a long set of expressions is chained it may overflow the stack. This warns 
about the issue.

Note I'm not sure whether `AnalyzeImplicitConversions` is the best place to add 
this test, but it was the best I could find. Also not sure what the naming 
convention for these helpers is. Another option is to put the original code in 
a lambda in `AnalyzeImplicitConversions` and thus remove the extra function.

Fixes https://bugs.llvm.org/show_bug.cgi?id=14030

Depends on D69478  (if this patch is unwanted 
it is possible to remove the dependency.)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69479

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/stack-overflow-expr-bool.cpp
  clang/test/Sema/stack-overflow-expr-double.c
  clang/test/Sema/stack-overflow-expr-int.c

Index: clang/test/Sema/stack-overflow-expr-int.c
===
--- /dev/null
+++ clang/test/Sema/stack-overflow-expr-int.c
@@ -0,0 +1,21 @@
+// The segmentation fault gives exit code 139
+// RUN: not not %clang_cc1 %s -fsyntax-only 2>&1 | FileCheck %s
+// REQUIRES: thread_support
+
+#define A10 ARG ARG ARG ARG ARG ARG ARG ARG ARG ARG
+#define A100 A10 A10 A10 A10 A10 A10 A10 A10 A10 A10
+#define A1000 A10 A10 A10 A10 A10 A10 A10 A10 A10 A10
+#define A1 A100 A100 A100 A100 A100 A100 A100 A100 A100 A100
+#define A10 A1000 A1000 A1000 A1000 A1000 A1000 A1000 A1000 A1000 A1000
+#define A100 A1 A1 A1 A1 A1 A1 A1 A1 A1 A1
+#define A1000 A10 A10 A10 A10 A10 A10 A10 A10 A10 A10
+#define A1 A100 A100 A100 A100 A100 A100 A100 A100 A100 A100
+
+#define ARG 1 +
+void foo() {
+  int i = A1 1;
+}
+
+// CHECK: warning: stack nearly exhausted; compilation time may suffer, and crashes due to stack overflow are likely
+// CHECK: note: Reduce the number of chained expressions, using helper variables or parens
+// CHECK: Stack dump:
Index: clang/test/Sema/stack-overflow-expr-double.c
===
--- /dev/null
+++ clang/test/Sema/stack-overflow-expr-double.c
@@ -0,0 +1,21 @@
+// The segmentation fault gives exit code 139
+// RUN: not not %clang_cc1 %s -fsyntax-only 2>&1 | FileCheck %s
+// REQUIRES: thread_support
+
+#define A10 ARG ARG ARG ARG ARG ARG ARG ARG ARG ARG
+#define A100 A10 A10 A10 A10 A10 A10 A10 A10 A10 A10
+#define A1000 A10 A10 A10 A10 A10 A10 A10 A10 A10 A10
+#define A1 A100 A100 A100 A100 A100 A100 A100 A100 A100 A100
+#define A10 A1000 A1000 A1000 A1000 A1000 A1000 A1000 A1000 A1000 A1000
+#define A100 A1 A1 A1 A1 A1 A1 A1 A1 A1 A1
+#define A1000 A10 A10 A10 A10 A10 A10 A10 A10 A10 A10
+#define A1 A100 A100 A100 A100 A100 A100 A100 A100 A100 A100
+
+#define ARG 1. +
+void foo() {
+  double f = A1 1.;
+}
+
+// CHECK: warning: stack nearly exhausted; compilation time may suffer, and crashes due to stack overflow are likely
+// CHECK: note: Reduce the number of chained expressions, using helper variables or parens
+// CHECK: Stack dump:
Index: clang/test/Sema/stack-overflow-expr-bool.cpp
===
--- /dev/null
+++ clang/test/Sema/stack-overflow-expr-bool.cpp
@@ -0,0 +1,21 @@
+// The segmentation fault gives exit code 139
+// RUN: not not %clang_cc1 %s -fsyntax-only 2>&1 | FileCheck %s
+// REQUIRES: thread_support
+
+#define A10 ARG ARG ARG ARG ARG ARG ARG ARG ARG ARG
+#define A100 A10 A10 A10 A10 A10 A10 A10 A10 A10 A10
+#define A1000 A10 A10 A10 A10 A10 A10 A10 A10 A10 A10
+#define A1 A100 A100 A100 A100 A100 A100 A100 A100 A100 A100
+#define A10 A1000 A1000 A1000 A1000 A1000 A1000 A1000 A1000 A1000 A1000
+#define A100 A1 A1 A1 A1 A1 A1 A1 A1 A1 A1
+#define A1000 A10 A10 A10 A10 A10 A10 A10 A10 A10 A10
+#define A1 A100 A100 A100 A100 A100 A100 A100 A100 A100 A100
+
+#define ARG true &&
+void foo() {
+  bool b = A1 true;
+}
+
+// CHECK: warning: stack nearly exhausted; compilation time may suffer, and crashes due to stack overflow are likely
+// CHECK: note: Reduce the number of chained expressions, using helper variables or parens
+// CHECK: Stack dump:
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -10368,8 +10368,8 @@
   IsSameFloatAfterCast(value.getComplexFloatI

[PATCH] D69478: [Sema] Allow warnStackExhausted to show more info

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added a comment.

Exact use case? Test?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69478



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

[PATCH] D69479: [Sema] Warn about possible stack exhaution

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added inline comments.



Comment at: clang/lib/Sema/SemaChecking.cpp:12349
   // This is not the right CC for (e.g.) a variable initialization.
-  AnalyzeImplicitConversions(*this, E, CC);
+  analyzeImplicitConversionsWithSufficientStackSpace(*this, E, CC);
 }

Please do not change function names.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69479



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

[PATCH] D69478: [Sema] Allow warnStackExhausted to show more info

[Mark de Wever via Phabricator via cfe-commits]

Mordante added a comment.

The use case and its test are in D69479 . Is 
that sufficient?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69478



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

[PATCH] D69478: [Sema] Allow warnStackExhausted to show more info

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added a comment.

Yes, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69478



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

[PATCH] D69479: [Sema] Warn about possible stack exhaution

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added inline comments.



Comment at: clang/lib/Sema/SemaChecking.cpp:12349
   // This is not the right CC for (e.g.) a variable initialization.
-  AnalyzeImplicitConversions(*this, E, CC);
+  analyzeImplicitConversionsWithSufficientStackSpace(*this, E, CC);
 }

xbolva00 wrote:
> Please do not change function names.
Ah, you had to change it.

Can this be solved maybe via new bool argument for 
AnalyzeImplicitConversions(.., arg)? If arg is false, we could just call 
S.runWithSufficientStackSpace(AnalyzeImplicitConversions(, true)), 
otherwise continue in normal flow.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69479



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

[PATCH] D69481: [Sema] Fixes templated friend member assertion

[Mark de Wever via Phabricator via cfe-commits]

Mordante created this revision.
Mordante added reviewers: rjmccall, rsmith, aaron.ballman.
Mordante added a project: clang.
Mordante added a subscriber: rtrieu.

Fixes https://bugs.llvm.org/show_bug.cgi?id=41792


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69481

Files:
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/CXX/temp/temp.spec/temp.expl.spec/p20.cpp


Index: clang/test/CXX/temp/temp.spec/temp.expl.spec/p20.cpp
===
--- clang/test/CXX/temp/temp.spec/temp.expl.spec/p20.cpp
+++ clang/test/CXX/temp/temp.spec/temp.expl.spec/p20.cpp
@@ -3,7 +3,7 @@
 void f(T);
 
 template
-struct A { };
+struct A { }; // expected-note{{template is declared here}}
 
 struct X {
   template<> friend void f(int); // expected-error{{in a friend}}
@@ -12,3 +12,12 @@
   friend void f(float); // okay
   friend class A; // okay
 };
+
+struct PR41792 {
+  // expected-error@+1{{cannot declare an explicit specialization in a friend}}
+  template <> friend void f<>(int);
+
+  // expected-error@+2{{template specialization declaration cannot be a 
friend}}
+  // expected-error@+1{{too few template arguments for class template 'A'}}
+  template <> friend class A<>;
+};
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -2549,6 +2549,9 @@
 /// list.
 static bool
 DependsOnTemplateParameters(QualType T, TemplateParameterList *Params) {
+  if (!Params->size())
+return false;
+
   DependencyChecker Checker(Params, /*IgnoreNonTypeDependent*/false);
   Checker.TraverseType(T);
   return Checker.Match;


Index: clang/test/CXX/temp/temp.spec/temp.expl.spec/p20.cpp
===
--- clang/test/CXX/temp/temp.spec/temp.expl.spec/p20.cpp
+++ clang/test/CXX/temp/temp.spec/temp.expl.spec/p20.cpp
@@ -3,7 +3,7 @@
 void f(T);
 
 template
-struct A { };
+struct A { }; // expected-note{{template is declared here}}
 
 struct X {
   template<> friend void f(int); // expected-error{{in a friend}}
@@ -12,3 +12,12 @@
   friend void f(float); // okay
   friend class A; // okay
 };
+
+struct PR41792 {
+  // expected-error@+1{{cannot declare an explicit specialization in a friend}}
+  template <> friend void f<>(int);
+
+  // expected-error@+2{{template specialization declaration cannot be a friend}}
+  // expected-error@+1{{too few template arguments for class template 'A'}}
+  template <> friend class A<>;
+};
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -2549,6 +2549,9 @@
 /// list.
 static bool
 DependsOnTemplateParameters(QualType T, TemplateParameterList *Params) {
+  if (!Params->size())
+return false;
+
   DependencyChecker Checker(Params, /*IgnoreNonTypeDependent*/false);
   Checker.TraverseType(T);
   return Checker.Match;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] faee39b - PR43762: when implicitly changing the active union member for an

[Richard Smith via cfe-commits]

Author: Richard Smith
Date: 2019-10-27T12:31:16-07:00
New Revision: faee39baa87e43f4b746dd77e479268391163658

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

LOG: PR43762: when implicitly changing the active union member for an
assignment during constant evaluation, only start the lifetime of
trivially-default-constructible union members.

Added: 


Modified: 
clang/lib/AST/ExprConstant.cpp
clang/test/SemaCXX/constant-expression-cxx2a.cpp

Removed: 




diff  --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 42c746e60285..7ed082185670 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -5333,9 +5333,16 @@ static bool HandleUnionActiveMemberChange(EvalInfo 
&Info, const Expr *LHSExpr,
   if (!FD || FD->getType()->isReferenceType())
 break;
 
-  //... and also contains A.B if B names a union member
-  if (FD->getParent()->isUnion())
-UnionPathLengths.push_back({PathLength - 1, FD});
+  //... and also contains A.B if B names a union member ...
+  if (FD->getParent()->isUnion()) {
+//... of a non-class, non-array type, or of a class type with a
+//trivial default constructor that is not deleted, or an array of
+//such types.
+auto *RD =
+FD->getType()->getBaseElementTypeUnsafe()->getAsCXXRecordDecl();
+if (!RD || RD->hasTrivialDefaultConstructor())
+  UnionPathLengths.push_back({PathLength - 1, FD});
+  }
 
   E = ME->getBase();
   --PathLength;

diff  --git a/clang/test/SemaCXX/constant-expression-cxx2a.cpp 
b/clang/test/SemaCXX/constant-expression-cxx2a.cpp
index 7c1718cebe44..8db705dcdc67 100644
--- a/clang/test/SemaCXX/constant-expression-cxx2a.cpp
+++ b/clang/test/SemaCXX/constant-expression-cxx2a.cpp
@@ -584,6 +584,48 @@ namespace Union {
 r.b.r.b = 2; // expected-note {{read of member 'b' of union with active 
member 'a'}}
 return r.b.r.b;
   }
+
+  namespace PR43762 {
+struct A { int x = 1; constexpr int f() { return 1; } };
+struct B : A { int y = 1; constexpr int g() { return 2; } };
+struct C {
+  int x;
+  constexpr virtual int f() = 0;
+};
+struct D : C {
+  int y;
+  constexpr virtual int f() override { return 3; }
+};
+
+union U {
+  int n;
+  B b;
+  D d;
+};
+
+constexpr int test(int which) {
+  U u{.n = 5};
+  switch (which) {
+  case 0:
+u.b.x = 10; // expected-note {{active member 'n'}}
+return u.b.f();
+  case 1:
+u.b.y = 10; // expected-note {{active member 'n'}}
+return u.b.g();
+  case 2:
+u.d.x = 10; // expected-note {{active member 'n'}}
+return u.d.f();
+  case 3:
+u.d.y = 10; // expected-note {{active member 'n'}}
+return u.d.f();
+  }
+}
+
+static_assert(test(0)); // expected-error {{}} expected-note {{in call}}
+static_assert(test(1)); // expected-error {{}} expected-note {{in call}}
+static_assert(test(2)); // expected-error {{}} expected-note {{in call}}
+static_assert(test(3)); // expected-error {{}} expected-note {{in call}}
+  }
 }
 
 namespace TwosComplementShifts {



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

[PATCH] D67787: Add 8548 CPU definition and attributes

[Justin Hibbits via Phabricator via cfe-commits]

jhibbits updated this revision to Diff 226583.
jhibbits added a comment.

Don't double-check 8548 CPU for setting LLVM CPU type.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67787

Files:
  clang/lib/Basic/Targets/PPC.cpp
  clang/lib/Basic/Targets/PPC.h
  clang/lib/Driver/ToolChains/Arch/PPC.cpp
  clang/test/Misc/target-invalid-cpu-note.c
  clang/test/Preprocessor/init.c

Index: clang/test/Preprocessor/init.c
===
--- clang/test/Preprocessor/init.c
+++ clang/test/Preprocessor/init.c
@@ -7584,6 +7584,12 @@
 //
 // PPC32-SPE:#define __NO_FPRS__ 1
 // PPC32-SPE:#define __SPE__ 1
+// 
+// RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc-unknown-linux-gnu -target-cpu 8548 < /dev/null | FileCheck -match-full-lines -check-prefix PPC8548 %s
+//
+// PPC8548:#define __NO_FPRS__ 1
+// PPC8548:#define __NO_LWSYNC__ 1
+// PPC8548:#define __SPE__ 1
 //
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc-apple-darwin8 < /dev/null | FileCheck -match-full-lines -check-prefix PPC-DARWIN %s
 //
Index: clang/test/Misc/target-invalid-cpu-note.c
===
--- clang/test/Misc/target-invalid-cpu-note.c
+++ clang/test/Misc/target-invalid-cpu-note.c
@@ -79,10 +79,10 @@
 // PPC: error: unknown target CPU 'not-a-cpu'
 // PPC: note: valid target CPU values are: generic, 440, 450, 601, 602, 603,
 // PPC-SAME: 603e, 603ev, 604, 604e, 620, 630, g3, 7400, g4, 7450, g4+, 750,
-// PPC-SAME: 970, g5, a2, a2q, e500mc, e5500, power3, pwr3, power4, pwr4,
-// PPC-SAME: power5, pwr5, power5x, pwr5x, power6, pwr6, power6x, pwr6x, power7,
-// PPC-SAME: pwr7, power8, pwr8, power9, pwr9, powerpc, ppc, powerpc64, ppc64,
-// PPC-SAME: powerpc64le, ppc64le
+// PPC-SAME: 8548, 970, g5, a2, a2q, e500, e500mc, e5500, power3, pwr3, power4,
+// PPC-SAME: pwr4, power5, pwr5, power5x, pwr5x, power6, pwr6, power6x, pwr6x,
+// PPC-SAME: power7, pwr7, power8, pwr8, power9, pwr9, powerpc, ppc, powerpc64,
+// PPC-SAME: ppc64, powerpc64le, ppc64le
 
 // RUN: not %clang_cc1 -triple mips--- -target-cpu not-a-cpu -fsyntax-only %s 2>&1 | FileCheck %s --check-prefix MIPS
 // MIPS: error: unknown target CPU 'not-a-cpu'
Index: clang/lib/Driver/ToolChains/Arch/PPC.cpp
===
--- clang/lib/Driver/ToolChains/Arch/PPC.cpp
+++ clang/lib/Driver/ToolChains/Arch/PPC.cpp
@@ -53,10 +53,12 @@
 .Case("7450", "7450")
 .Case("G4+", "g4+")
 .Case("750", "750")
+.Case("8548", "e500")
 .Case("970", "970")
 .Case("G5", "g5")
 .Case("a2", "a2")
 .Case("a2q", "a2q")
+.Case("e500", "e500")
 .Case("e500mc", "e500mc")
 .Case("e5500", "e5500")
 .Case("power3", "pwr3")
Index: clang/lib/Basic/Targets/PPC.h
===
--- clang/lib/Basic/Targets/PPC.h
+++ clang/lib/Basic/Targets/PPC.h
@@ -44,7 +44,8 @@
 ArchDefinePwr8 = 1 << 12,
 ArchDefinePwr9 = 1 << 13,
 ArchDefineA2 = 1 << 14,
-ArchDefineA2q = 1 << 15
+ArchDefineA2q = 1 << 15,
+ArchDefineE500 = 1 << 16
   } ArchDefineTypes;
 
 
@@ -145,6 +146,7 @@
  ArchDefinePwr9 | ArchDefinePwr8 | ArchDefinePwr7 |
  ArchDefinePwr6 | ArchDefinePwr5x | ArchDefinePwr5 |
  ArchDefinePwr4 | ArchDefinePpcgr | ArchDefinePpcsq)
+  .Cases("8548", "e500", ArchDefineE500)
   .Default(ArchDefineNone);
 }
 return CPUKnown;
Index: clang/lib/Basic/Targets/PPC.cpp
===
--- clang/lib/Basic/Targets/PPC.cpp
+++ clang/lib/Basic/Targets/PPC.cpp
@@ -157,6 +157,8 @@
 Builder.defineMacro("_ARCH_A2Q");
 Builder.defineMacro("_ARCH_QP");
   }
+  if (ArchDefs & ArchDefineE500)
+Builder.defineMacro("__NO_LWSYNC__");
 
   if (getTriple().getVendor() == llvm::Triple::BGQ) {
 Builder.defineMacro("__bg__");
@@ -312,6 +314,11 @@
 .Case("pwr8", true)
 .Default(false);
 
+  Features["spe"] = llvm::StringSwitch(CPU)
+.Case("8548", true)
+.Case("e500", true)
+.Default(false);
+
   if (!ppcUserFeaturesCheck(Diags, FeaturesVec))
 return false;
 
@@ -449,16 +456,16 @@
 }
 
 static constexpr llvm::StringLiteral ValidCPUNames[] = {
-{"generic"}, {"440"}, {"450"}, {"601"},{"602"},
-{"603"}, {"603e"},{"603ev"},   {"604"},{"604e"},
-{"620"}, {"630"}, {"g3"},  {"7400"},   {"g4"},
-{"7450"},{"g4+"}, {"750"}, {"970"},{"g5"},
-{"a2"},  {"a2q"}, {"e500mc"},  {"e5500"},  {"power3"},
-{"pwr3"},{"power4"},  {"pwr4"},{"power5"}, {"pwr5"},
-{"power5x"}

[PATCH] D67787: Add 8548 CPU definition and attributes

[Justin Hibbits via Phabricator via cfe-commits]

jhibbits marked an inline comment as done.
jhibbits added inline comments.



Comment at: clang/lib/Driver/ToolChains/Arch/PPC.cpp:61
 .Case("a2q", "a2q")
+.Case("8548", "e500")
+.Case("e500", "e500")

vit9696 wrote:
> That looks like a typo to me. There is one more 8548 case above.
You're right, I missed that I added it above already.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67787



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

[clang] 06d3fce - PR43400: Add test that we can instantiate a friend function that is

[Richard Smith via cfe-commits]

Author: Richard Smith
Date: 2019-10-27T12:41:53-07:00
New Revision: 06d3fce01d4f4dfd743e24e1f553afbb6709610f

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

LOG: PR43400: Add test that we can instantiate a friend function that is
defined as deleted.

The actual bug was fixed in commit d052a578.

Added: 


Modified: 
clang/test/SemaTemplate/instantiate-friend-function.cpp

Removed: 




diff  --git a/clang/test/SemaTemplate/instantiate-friend-function.cpp 
b/clang/test/SemaTemplate/instantiate-friend-function.cpp
index 08602d522df0..1a923a2e92e0 100644
--- a/clang/test/SemaTemplate/instantiate-friend-function.cpp
+++ b/clang/test/SemaTemplate/instantiate-friend-function.cpp
@@ -47,3 +47,10 @@ namespace PR10856_Root {
 
 // bool PR10856_Root::g, 
void>(PR10856_Root::MyClass)
 // CHECK: call {{.*}} @_ZN12PR10856_Root1gINS_7MyClassIiEEvEEbT_
+
+namespace PR43400 {
+  template struct X {
+friend void f() = delete;
+  };
+  X xi;
+}



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

[PATCH] D69479: [Sema] Warn about possible stack exhaution

[Mark de Wever via Phabricator via cfe-commits]

Mordante marked 3 inline comments as done.
Mordante added inline comments.



Comment at: clang/lib/Sema/SemaChecking.cpp:12349
   // This is not the right CC for (e.g.) a variable initialization.
-  AnalyzeImplicitConversions(*this, E, CC);
+  analyzeImplicitConversionsWithSufficientStackSpace(*this, E, CC);
 }

xbolva00 wrote:
> xbolva00 wrote:
> > Please do not change function names.
> Ah, you had to change it.
> 
> Can this be solved maybe via new bool argument for 
> AnalyzeImplicitConversions(.., arg)? If arg is false, we could just call 
> S.runWithSufficientStackSpace(AnalyzeImplicitConversions(, true)), 
> otherwise continue in normal flow.
I like this idea better than my original approach. Thanks for the suggestion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69479



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

[PATCH] D69479: [Sema] Warn about possible stack exhaution

[Mark de Wever via Phabricator via cfe-commits]

Mordante updated this revision to Diff 226590.
Mordante marked an inline comment as done.
Mordante added a comment.

Add an extra argument to `AnalyzeImplicitConversions` as suggested by @xbolva00 
.


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

https://reviews.llvm.org/D69479

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/stack-overflow-expr-bool.cpp
  clang/test/Sema/stack-overflow-expr-double.c
  clang/test/Sema/stack-overflow-expr-int.c

Index: clang/test/Sema/stack-overflow-expr-int.c
===
--- /dev/null
+++ clang/test/Sema/stack-overflow-expr-int.c
@@ -0,0 +1,21 @@
+// The segmentation fault gives exit code 139
+// RUN: not not %clang_cc1 %s -fsyntax-only 2>&1 | FileCheck %s
+// REQUIRES: thread_support
+
+#define A10 ARG ARG ARG ARG ARG ARG ARG ARG ARG ARG
+#define A100 A10 A10 A10 A10 A10 A10 A10 A10 A10 A10
+#define A1000 A10 A10 A10 A10 A10 A10 A10 A10 A10 A10
+#define A1 A100 A100 A100 A100 A100 A100 A100 A100 A100 A100
+#define A10 A1000 A1000 A1000 A1000 A1000 A1000 A1000 A1000 A1000 A1000
+#define A100 A1 A1 A1 A1 A1 A1 A1 A1 A1 A1
+#define A1000 A10 A10 A10 A10 A10 A10 A10 A10 A10 A10
+#define A1 A100 A100 A100 A100 A100 A100 A100 A100 A100 A100
+
+#define ARG 1 +
+void foo() {
+  int i = A1 1;
+}
+
+// CHECK: warning: stack nearly exhausted; compilation time may suffer, and crashes due to stack overflow are likely
+// CHECK: note: Reduce the number of chained expressions, using helper variables or parens
+// CHECK: Stack dump:
Index: clang/test/Sema/stack-overflow-expr-double.c
===
--- /dev/null
+++ clang/test/Sema/stack-overflow-expr-double.c
@@ -0,0 +1,21 @@
+// The segmentation fault gives exit code 139
+// RUN: not not %clang_cc1 %s -fsyntax-only 2>&1 | FileCheck %s
+// REQUIRES: thread_support
+
+#define A10 ARG ARG ARG ARG ARG ARG ARG ARG ARG ARG
+#define A100 A10 A10 A10 A10 A10 A10 A10 A10 A10 A10
+#define A1000 A10 A10 A10 A10 A10 A10 A10 A10 A10 A10
+#define A1 A100 A100 A100 A100 A100 A100 A100 A100 A100 A100
+#define A10 A1000 A1000 A1000 A1000 A1000 A1000 A1000 A1000 A1000 A1000
+#define A100 A1 A1 A1 A1 A1 A1 A1 A1 A1 A1
+#define A1000 A10 A10 A10 A10 A10 A10 A10 A10 A10 A10
+#define A1 A100 A100 A100 A100 A100 A100 A100 A100 A100 A100
+
+#define ARG 1. +
+void foo() {
+  double f = A1 1.;
+}
+
+// CHECK: warning: stack nearly exhausted; compilation time may suffer, and crashes due to stack overflow are likely
+// CHECK: note: Reduce the number of chained expressions, using helper variables or parens
+// CHECK: Stack dump:
Index: clang/test/Sema/stack-overflow-expr-bool.cpp
===
--- /dev/null
+++ clang/test/Sema/stack-overflow-expr-bool.cpp
@@ -0,0 +1,21 @@
+// The segmentation fault gives exit code 139
+// RUN: not not %clang_cc1 %s -fsyntax-only 2>&1 | FileCheck %s
+// REQUIRES: thread_support
+
+#define A10 ARG ARG ARG ARG ARG ARG ARG ARG ARG ARG
+#define A100 A10 A10 A10 A10 A10 A10 A10 A10 A10 A10
+#define A1000 A10 A10 A10 A10 A10 A10 A10 A10 A10 A10
+#define A1 A100 A100 A100 A100 A100 A100 A100 A100 A100 A100
+#define A10 A1000 A1000 A1000 A1000 A1000 A1000 A1000 A1000 A1000 A1000
+#define A100 A1 A1 A1 A1 A1 A1 A1 A1 A1 A1
+#define A1000 A10 A10 A10 A10 A10 A10 A10 A10 A10 A10
+#define A1 A100 A100 A100 A100 A100 A100 A100 A100 A100 A100
+
+#define ARG true &&
+void foo() {
+  bool b = A1 true;
+}
+
+// CHECK: warning: stack nearly exhausted; compilation time may suffer, and crashes due to stack overflow are likely
+// CHECK: note: Reduce the number of chained expressions, using helper variables or parens
+// CHECK: Stack dump:
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -10368,8 +10368,10 @@
   IsSameFloatAfterCast(value.getComplexFloatImag(), Src, Tgt));
 }
 
-static void AnalyzeImplicitConversions(Sema &S, Expr *E, SourceLocation CC,
-   bool IsListInit = false);
+static void
+AnalyzeImplicitConversions(Sema &S, Expr *E, SourceLocation CC,
+   bool IsListInit = false,
+   bool CalledWithSufficientStackSpace = false);
 
 static bool IsEnumConstOrFromMacro(Sema &S, Expr *E) {
   // Suppress cases where we are comparing again

[PATCH] D69479: [Sema] Warn about possible stack exhaution

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

We should resist using `runWithSufficientStackSpace` where possible. Have you 
tried making this process data-recursive instead? It looks fairly 
straightforward to form a worklist of expressions to which we want to apply 
`AnalyseImplicitConversions`, and run through that list until no new entries 
are added.




Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:3481
+def note_reduce_expression_chaining : Note<
+  "Reduce the number of chained expressions, using helper variables or 
parens">;
+

The note should explain what the problem is. If you want to suggest a solution, 
that could be OK, but should be secondary and non-obvious from the description 
of the problem. In this case, a note saying the expression is too complex would 
cover both explaining what the problem is, and implying the solution (make the 
expression less complex).

Also, Clang diagnostics start with lowercase letters.



Comment at: clang/lib/Sema/SemaChecking.cpp:12349
   // This is not the right CC for (e.g.) a variable initialization.
-  AnalyzeImplicitConversions(*this, E, CC);
+  analyzeImplicitConversionsWithSufficientStackSpace(*this, E, CC);
 }

Mordante wrote:
> xbolva00 wrote:
> > xbolva00 wrote:
> > > Please do not change function names.
> > Ah, you had to change it.
> > 
> > Can this be solved maybe via new bool argument for 
> > AnalyzeImplicitConversions(.., arg)? If arg is false, we could just call 
> > S.runWithSufficientStackSpace(AnalyzeImplicitConversions(, true)), 
> > otherwise continue in normal flow.
> I like this idea better than my original approach. Thanks for the suggestion.
How about renaming the original function to `AnalyzeImplicitConversionsImpl` 
and using the original name for the `runWithSufficientStackSpace` version?



Comment at: clang/test/Sema/stack-overflow-expr-int.c:2
+// The segmentation fault gives exit code 139
+// RUN: not not %clang_cc1 %s -fsyntax-only 2>&1 | FileCheck %s
+// REQUIRES: thread_support

You can use `not --crash` if you want to allow crashes. But testing that we do 
crash seems inappropriate here; if we keep using `runWithSufficientStackSpace`, 
it would seem preferable to test that we produce the diagnostic, and make sure 
the test passes regardless of whether we crash.


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

https://reviews.llvm.org/D69479



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

LLVM buildmaster will be updated and restarted soon

[Galina Kistanova via cfe-commits]

Hello everyone,

LLVM buildmaster will be updated and restarted in the nearest hour.

Thanks

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

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

[Kocsis Ábel via Phabricator via cfe-commits]

abelkocsis marked 3 inline comments as done.
abelkocsis added inline comments.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/misc-bad-signal-to-kill-thread.rst:8
+``SIGTERM`` signal and the signal kills the entire process, not just the
+individual thread. Use any signal except ``SIGTERM`` or ``SIGKILL``.
+

aaron.ballman wrote:
> Why does the check not look for `SIGKILL` as well as `SIGTERM`?
In my opinion `SIGKILL` is used only in cases when the programmer wants the 
entire process to terminate. I updated the docs not mentioning `SIGKILL`.


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

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

[Kocsis Ábel via Phabricator via cfe-commits]

abelkocsis updated this revision to Diff 226596.
abelkocsis marked 4 inline comments as done.
abelkocsis added a comment.

Checker moved to bugprone module, cert alias added. Small fixes in checker
file, warning message 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/bugprone/BadSignalToKillThreadCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.h
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/bugprone-bad-signal-to-kill-thread.rst
  clang-tools-extra/docs/clang-tidy/checks/cert-pos44-c.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/bugprone-bad-signal-to-kill-thread.cpp

Index: clang-tools-extra/test/clang-tidy/bugprone-bad-signal-to-kill-thread.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/bugprone-bad-signal-to-kill-thread.cpp
@@ -0,0 +1,38 @@
+// RUN: %check_clang_tidy %s bugprone-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 raising the 'SIGTERM' signal [bugprone-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 raising the 'SIGTERM' signal [bugprone-bad-signal-to-kill-thread]
+  }
+
+  return 0;
+}
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
@@ -40,6 +40,7 @@
boost-use-to-string
bugprone-argument-comment
bugprone-assert-side-effect
+   bugprone-bad-signal-to-kill-thread
bugprone-bool-pointer-implicit-conversion
bugprone-branch-clone
bugprone-copy-constructor-init
@@ -106,6 +107,7 @@
cert-msc51-cpp
cert-oop11-cpp (redirects to performance-move-constructor-init) 
cert-oop54-cpp (redirects to bugprone-unhandled-self-assignment) 
+   cert-pos44-c (redirects to bugprone-bad-signal-to-kill-thread) 
clang-analyzer-core.CallAndMessage
clang-analyzer-core.DivideZero
clang-analyzer-core.DynamicTypePropagation
Index: clang-tools-extra/docs/clang-tidy/checks/cert-pos44-c.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/cert-pos44-c.rst
@@ -0,0 +1,9 @@
+.. title:: clang-tidy - cert-pos44-c
+.. meta::
+   :http-equiv=refresh: 5;URL=bugprone-bad-signal-to-kill-thread.html
+
+cert-pos44-c
+
+
+The cert-pos44-c check is an alias, please see
+`bugprone-bad-signal-to-kill-thread `_ for more information.
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone-bad-signal-to-kill-thread.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone-bad-signal-to-kill-thread.rst
@@ -0,0 +1,16 @@
+.. title:: clang-tidy - bugprone-bad-signal-to-kill-thread
+
+bugprone-bad-signal-to-kill-thread
+==
+
+Finds ``pthread_kill`` function calls when a thread is terminated by 
+raising ``SIGTERM`` signal and the signal kills the entire process, not 
+just the individual thread. Use any signal except ``SIGTERM``.
+
+.. code-block: c++
+
+pthread_kill(thread, SIGTERM);
+
+This check corresponds to the CERT C Coding Standard rule
+`POS44-C. Do not use signals to terminate threads
+`_.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -67,6 +67,12 @@
 Improvements to clang-tidy
 --
 
+- New :doc:`bugprone-bad-signal-to-kill-thread
+  ` check.
+
+  Finds ``pthread_kill`` function calls when a thread is terminated by 
+

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

[Kocsis Ábel via Phabricator via cfe-commits]

abelkocsis updated this revision to Diff 226598.
abelkocsis marked an inline comment as done.
abelkocsis added a comment.

Small fixes


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/bugprone/BadSignalToKillThreadCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.h
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/bugprone-bad-signal-to-kill-thread.rst
  clang-tools-extra/docs/clang-tidy/checks/cert-pos44-c.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/bugprone-bad-signal-to-kill-thread.cpp

Index: clang-tools-extra/test/clang-tidy/bugprone-bad-signal-to-kill-thread.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/bugprone-bad-signal-to-kill-thread.cpp
@@ -0,0 +1,38 @@
+// RUN: %check_clang_tidy %s bugprone-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 raising the 'SIGTERM' signal [bugprone-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 raising the 'SIGTERM' signal [bugprone-bad-signal-to-kill-thread]
+  }
+
+  return 0;
+}
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
@@ -40,6 +40,7 @@
boost-use-to-string
bugprone-argument-comment
bugprone-assert-side-effect
+   bugprone-bad-signal-to-kill-thread
bugprone-bool-pointer-implicit-conversion
bugprone-branch-clone
bugprone-copy-constructor-init
@@ -106,6 +107,7 @@
cert-msc51-cpp
cert-oop11-cpp (redirects to performance-move-constructor-init) 
cert-oop54-cpp (redirects to bugprone-unhandled-self-assignment) 
+   cert-pos44-c (redirects to bugprone-bad-signal-to-kill-thread) 
clang-analyzer-core.CallAndMessage
clang-analyzer-core.DivideZero
clang-analyzer-core.DynamicTypePropagation
Index: clang-tools-extra/docs/clang-tidy/checks/cert-pos44-c.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/cert-pos44-c.rst
@@ -0,0 +1,9 @@
+.. title:: clang-tidy - cert-pos44-c
+.. meta::
+   :http-equiv=refresh: 5;URL=bugprone-bad-signal-to-kill-thread.html
+
+cert-pos44-c
+
+
+The cert-pos44-c check is an alias, please see
+`bugprone-bad-signal-to-kill-thread `_ for more information.
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone-bad-signal-to-kill-thread.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone-bad-signal-to-kill-thread.rst
@@ -0,0 +1,16 @@
+.. title:: clang-tidy - bugprone-bad-signal-to-kill-thread
+
+bugprone-bad-signal-to-kill-thread
+==
+
+Finds ``pthread_kill`` function calls when a thread is terminated by 
+raising ``SIGTERM`` signal and the signal kills the entire process, not 
+just the individual thread. Use any signal except ``SIGTERM``.
+
+.. code-block: c++
+
+pthread_kill(thread, SIGTERM);
+
+This check corresponds to the CERT C Coding Standard rule
+`POS44-C. Do not use signals to terminate threads
+`_.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -67,6 +67,12 @@
 Improvements to clang-tidy
 --
 
+- New :doc:`bugprone-bad-signal-to-kill-thread
+  ` check.
+
+  Finds ``pthread_kill`` function calls when a thread is terminated by 
+  raising ``SIGTERM`` signal.
+
 - New :doc:`bugprone-dynamic-static-initializers
   ` check.

[PATCH] D69494: OpenMP: Add helper function for convergent runtime calls

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a reviewer: grokos.
JonChesterfield accepted this revision.
JonChesterfield added a comment.
This revision is now accepted and ready to land.

LGTM.


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

https://reviews.llvm.org/D69494



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

[PATCH] D69494: OpenMP: Add helper function for convergent runtime calls

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added a comment.

In D69494#1722853 , @ABataev wrote:

> Tests?




In D69494#1722853 , @ABataev wrote:

> Tests?


This is NFC and doesn't change any behavior. The test for the convergent 
attribute is already handled in nvptx_parallel_codegen.cpp


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

https://reviews.llvm.org/D69494



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

[PATCH] D69494: OpenMP: Add helper function for convergent runtime calls

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D69494#1722867 , @arsenm wrote:

> In D69494#1722853 , @ABataev wrote:
>
> > Tests?
>
>
>
>
> In D69494#1722853 , @ABataev wrote:
>
> > Tests?
>
>
> This is NFC and doesn't change any behavior. The test for the convergent 
> attribute is already handled in nvptx_parallel_codegen.cpp


Nope, you're narking some calls withe the new attribute. It is not NFC in this 
case.


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

https://reviews.llvm.org/D69494



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

[PATCH] D69494: OpenMP: Add helper function for convergent runtime calls

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added a comment.

In D69494#1722868 , @ABataev wrote:

> In D69494#1722867 , @arsenm wrote:
>
> > In D69494#1722853 , @ABataev wrote:
> >
> > > Tests?
> >
> >
> >
> >
> > In D69494#1722853 , @ABataev wrote:
> >
> > > Tests?
> >
> >
> > This is NFC and doesn't change any behavior. The test for the convergent 
> > attribute is already handled in nvptx_parallel_codegen.cpp
>
>
> Nope, you're narking some calls withe the new attribute. It is not NFC in 
> this case.


More precisely, this is marking the function declarations with the attribute. 
These were already marked with the attribute. This just moves it into the 
helper function. D69495  adds new instances of 
the attribute


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

https://reviews.llvm.org/D69494



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

[PATCH] D69494: OpenMP: Add helper function for convergent runtime calls

[Alexey Bataev via Phabricator via cfe-commits]

ABataev accepted this revision.
ABataev added a comment.
This revision is now accepted and ready to land.

Ah, yes, missed this. Then just add a comment.


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

https://reviews.llvm.org/D69494



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

[PATCH] D69493: Add -fconvergent-functions flag

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added a comment.

In D69493#1722894 , @jlebar wrote:

> I'm confused about "The CUDA builtin library is apparently compiled in C++ 
> mode".  By...whom, and for what purposes?
>
> No objections from me, this seems reasonable regardless, but I'd also like 
> tra to weigh in, since it's been a while for me.


That's what the comment says in propagate-metadata.cu:

> // Build the bitcode library.  This is not built in CUDA mode, otherwise it
>  // might have incompatible attributes.  This mirrors how libdevice is built.


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

https://reviews.llvm.org/D69493



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

[clang] 40ab8ae - OpenMP: Add helper function for convergent runtime calls

[Matt Arsenault via cfe-commits]

Author: Matt Arsenault
Date: 2019-10-27T21:26:55-07:00
New Revision: 40ab8ae9fb70f1550815bf0f867148b5101a4f66

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

LOG: OpenMP: Add helper function for convergent runtime calls

Most of the functions emitted here should probably be convergent, but
only barriers are currently marked. Introduce this helper before
adding convergent to more functions.

Added: 


Modified: 
clang/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
clang/lib/CodeGen/CodeGenModule.cpp
clang/lib/CodeGen/CodeGenModule.h

Removed: 




diff  --git a/clang/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp 
b/clang/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
index 708260429f68..910992e76d0a 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
@@ -1799,9 +1799,8 @@ CGOpenMPRuntimeNVPTX::createNVPTXRuntimeFunction(unsigned 
Function) {
 llvm::Type *TypeParams[] = {getIdentTyPointerTy(), CGM.Int32Ty};
 auto *FnTy =
 llvm::FunctionType::get(CGM.VoidTy, TypeParams, /*isVarArg*/ false);
-RTLFn = CGM.CreateRuntimeFunction(FnTy, /*Name*/ "__kmpc_barrier");
-cast(RTLFn.getCallee())
-->addFnAttr(llvm::Attribute::Convergent);
+RTLFn =
+CGM.CreateConvergentRuntimeFunction(FnTy, /*Name*/ "__kmpc_barrier");
 break;
   }
   case OMPRTL__kmpc_barrier_simple_spmd: {
@@ -1810,10 +1809,8 @@ 
CGOpenMPRuntimeNVPTX::createNVPTXRuntimeFunction(unsigned Function) {
 llvm::Type *TypeParams[] = {getIdentTyPointerTy(), CGM.Int32Ty};
 auto *FnTy =
 llvm::FunctionType::get(CGM.VoidTy, TypeParams, /*isVarArg*/ false);
-RTLFn =
-CGM.CreateRuntimeFunction(FnTy, /*Name*/ "__kmpc_barrier_simple_spmd");
-cast(RTLFn.getCallee())
-->addFnAttr(llvm::Attribute::Convergent);
+RTLFn = CGM.CreateConvergentRuntimeFunction(
+FnTy, /*Name*/ "__kmpc_barrier_simple_spmd");
 break;
   }
   case OMPRTL_NVPTX__kmpc_warp_active_thread_mask: {

diff  --git a/clang/lib/CodeGen/CodeGenModule.cpp 
b/clang/lib/CodeGen/CodeGenModule.cpp
index b05a58848e82..75708d6e4966 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -3332,8 +3332,14 @@ GetRuntimeFunctionDecl(ASTContext &C, StringRef Name) {
 /// type and name.
 llvm::FunctionCallee
 CodeGenModule::CreateRuntimeFunction(llvm::FunctionType *FTy, StringRef Name,
- llvm::AttributeList ExtraAttrs,
- bool Local) {
+ llvm::AttributeList ExtraAttrs, bool 
Local,
+ bool AssumeConvergent) {
+  if (AssumeConvergent) {
+ExtraAttrs =
+ExtraAttrs.addAttribute(VMContext, llvm::AttributeList::FunctionIndex,
+llvm::Attribute::Convergent);
+  }
+
   llvm::Constant *C =
   GetOrCreateLLVMFunction(Name, FTy, GlobalDecl(), /*ForVTable=*/false,
   /*DontDefer=*/false, /*IsThunk=*/false,

diff  --git a/clang/lib/CodeGen/CodeGenModule.h 
b/clang/lib/CodeGen/CodeGenModule.h
index 73f81adae35f..f5014c05b067 100644
--- a/clang/lib/CodeGen/CodeGenModule.h
+++ b/clang/lib/CodeGen/CodeGenModule.h
@@ -1027,11 +1027,22 @@ class CodeGenModule : public CodeGenTypeCache {
   }
 
   /// Create or return a runtime function declaration with the specified type
-  /// and name.
+  /// and name. If \p AssumeConvergent is true, the call will have the
+  /// convergent attribute added.
   llvm::FunctionCallee
   CreateRuntimeFunction(llvm::FunctionType *Ty, StringRef Name,
 llvm::AttributeList ExtraAttrs = llvm::AttributeList(),
-bool Local = false);
+bool Local = false, bool AssumeConvergent = false);
+
+  /// Create or return a runtime function declaration with the specified type
+  /// and name. This will automatically add the convergent attribute to the
+  /// function declaration.
+  llvm::FunctionCallee CreateConvergentRuntimeFunction(
+  llvm::FunctionType *Ty, StringRef Name,
+  llvm::AttributeList ExtraAttrs = llvm::AttributeList(),
+  bool Local = false) {
+return CreateRuntimeFunction(Ty, Name, ExtraAttrs, Local, true);
+  }
 
   /// Create a new runtime global variable with the specified type and name.
   llvm::Constant *CreateRuntimeVariable(llvm::Type *Ty,



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

[clang] 56a503b - OpenMP: Add convergent to more runtime functions

[Matt Arsenault via cfe-commits]

Author: Matt Arsenault
Date: 2019-10-27T21:26:55-07:00
New Revision: 56a503bdba9c33fce4d8fe86494cfd9c0b62c88a

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

LOG: OpenMP: Add convergent to more runtime functions

Several of these other functions are probably also convergent, but
these two seem obviously convergent.

Added: 


Modified: 
clang/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
clang/test/OpenMP/nvptx_parallel_codegen.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp 
b/clang/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
index 910992e76d0a..b7808e0c4f69 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
@@ -1817,14 +1817,14 @@ 
CGOpenMPRuntimeNVPTX::createNVPTXRuntimeFunction(unsigned Function) {
 // Build int32_t __kmpc_warp_active_thread_mask(void);
 auto *FnTy =
 llvm::FunctionType::get(CGM.Int32Ty, llvm::None, /*isVarArg=*/false);
-RTLFn = CGM.CreateRuntimeFunction(FnTy, "__kmpc_warp_active_thread_mask");
+RTLFn = CGM.CreateConvergentRuntimeFunction(FnTy, 
"__kmpc_warp_active_thread_mask");
 break;
   }
   case OMPRTL_NVPTX__kmpc_syncwarp: {
 // Build void __kmpc_syncwarp(kmp_int32 Mask);
 auto *FnTy =
 llvm::FunctionType::get(CGM.VoidTy, CGM.Int32Ty, /*isVarArg=*/false);
-RTLFn = CGM.CreateRuntimeFunction(FnTy, "__kmpc_syncwarp");
+RTLFn = CGM.CreateConvergentRuntimeFunction(FnTy, "__kmpc_syncwarp");
 break;
   }
   }

diff  --git a/clang/test/OpenMP/nvptx_parallel_codegen.cpp 
b/clang/test/OpenMP/nvptx_parallel_codegen.cpp
index 32061bf7386c..2fc06350c380 100644
--- a/clang/test/OpenMP/nvptx_parallel_codegen.cpp
+++ b/clang/test/OpenMP/nvptx_parallel_codegen.cpp
@@ -88,7 +88,7 @@ int bar(int n){
 // CHECK: br label {{%?}}[[AWAIT_WORK:.+]]
 //
 // CHECK: [[AWAIT_WORK]]
-// CHECK: call void @__kmpc_barrier_simple_spmd(%struct.ident_t* null, i32 0) 
#[[#BARRIER_ATTRS:]]
+// CHECK: call void @__kmpc_barrier_simple_spmd(%struct.ident_t* null, i32 0) 
#[[#CONVERGENT:]]
 // CHECK: [[KPR:%.+]] = call i1 @__kmpc_kernel_parallel(i8** [[OMP_WORK_FN]]
 // CHECK: [[KPRB:%.+]] = zext i1 [[KPR]] to i8
 // store i8 [[KPRB]], i8* [[OMP_EXEC_STATUS]], align 1
@@ -318,10 +318,10 @@ int bar(int n){
 // CHECK: define internal void [[PARALLEL_FN4]](
 // CHECK: [[A:%.+]] = alloca i[[SZ:32|64]],
 // CHECK: store i[[SZ]] 45, i[[SZ]]* %a,
-// CHECK: call void @__kmpc_barrier(%struct.ident_t* @{{.+}}, i32 %{{.+}}) 
#[[#BARRIER_ATTRS]]
+// CHECK: call void @__kmpc_barrier(%struct.ident_t* @{{.+}}, i32 %{{.+}}) 
#[[#CONVERGENT:]]
 // CHECK: ret void
 
-// CHECK: declare void @__kmpc_barrier(%struct.ident_t*, i32) 
#[[#BARRIER_ATTRS]]
+// CHECK: declare void @__kmpc_barrier(%struct.ident_t*, i32) #[[#CONVERGENT]]
 
 // CHECK-LABEL: define {{.*}}void 
{{@__omp_offloading_.+template.+l55}}_worker()
 // CHECK-LABEL: define {{.*}}void {{@__omp_offloading_.+template.+l55}}(
@@ -343,7 +343,7 @@ int bar(int n){
 
 // CHECK-LABEL: define internal void @{{.+}}(i32* noalias %{{.+}}, i32* 
noalias %{{.+}}, i32* dereferenceable{{.*}})
 // CHECK:  [[CC:%.+]] = alloca i32,
-// CHECK:  [[MASK:%.+]] = call i32 @__kmpc_warp_active_thread_mask()
+// CHECK:  [[MASK:%.+]] = call i32 @__kmpc_warp_active_thread_mask(){{$}}
 // CHECK:  [[TID:%.+]] = call i32 @llvm.nvvm.read.ptx.sreg.tid.x()
 // CHECK:  [[NUM_THREADS:%.+]] = call i32 @llvm.nvvm.read.ptx.sreg.ntid.x()
 // CHECK:  store i32 0, i32* [[CC]],
@@ -363,11 +363,15 @@ int bar(int n){
 // CHECK:  store i32
 // CHECK:  call void @__kmpc_end_critical(
 
-// CHECK:  call void @__kmpc_syncwarp(i32 [[MASK]])
+// CHECK:  call void @__kmpc_syncwarp(i32 [[MASK]]){{$}}
 // CHECK:  [[NEW_CC_VAL:%.+]] = add nsw i32 [[CC_VAL]], 1
 // CHECK:  store i32 [[NEW_CC_VAL]], i32* [[CC]],
 // CHECK:  br label
 
-// CHECK: attributes #[[#BARRIER_ATTRS]] = {{.*}} convergent {{.*}}
+
+// CHECK: declare i32 @__kmpc_warp_active_thread_mask() #[[#CONVERGENT:]]
+// CHECK: declare void @__kmpc_syncwarp(i32) #[[#CONVERGENT:]]
+
+// CHECK: attributes #[[#CONVERGENT]] = {{.*}} convergent {{.*}}
 
 #endif



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

[PATCH] D69494: OpenMP: Add helper function for convergent runtime calls

[Matt Arsenault via Phabricator via cfe-commits]

arsenm closed this revision.
arsenm added a comment.

r40ab8ae9fb70f1550815bf0f867148b5101a4f66


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

https://reviews.llvm.org/D69494



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

[PATCH] D69495: OpenMP: Add convergent to more runtime functions

[Matt Arsenault via Phabricator via cfe-commits]

arsenm closed this revision.
arsenm added a comment.

56a503bdba9c33fce4d8fe86494cfd9c0b62c88a


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

https://reviews.llvm.org/D69495



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

[clang] 39eef2c - PR43775: don't produce a bogus 'auto' -Wc++98-compat warning for CTAD

[Richard Smith via cfe-commits]

Author: Richard Smith
Date: 2019-10-27T21:42:58-07:00
New Revision: 39eef2cbb6ab1fda2e0beb8789cd49a3ce276b16

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

LOG: PR43775: don't produce a bogus 'auto' -Wc++98-compat warning for CTAD

Added: 


Modified: 
clang/lib/Sema/SemaType.cpp
clang/test/SemaCXX/cxx98-compat.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
index 02fc81105037..aa795e7b0656 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -3139,7 +3139,7 @@ static QualType 
GetDeclSpecTypeForDeclarator(TypeProcessingState &state,
 
   T = SemaRef.Context.IntTy;
   D.setInvalidType(true);
-} else if (!HaveTrailing &&
+} else if (Auto && !HaveTrailing &&
D.getContext() != DeclaratorContext::LambdaExprContext) {
   // If there was a trailing return type, we already got
   // warn_cxx98_compat_trailing_return_type in the parser.

diff  --git a/clang/test/SemaCXX/cxx98-compat.cpp 
b/clang/test/SemaCXX/cxx98-compat.cpp
index 885ab110ab1f..e221259859bc 100644
--- a/clang/test/SemaCXX/cxx98-compat.cpp
+++ b/clang/test/SemaCXX/cxx98-compat.cpp
@@ -1,5 +1,6 @@
 // RUN: %clang_cc1 -fsyntax-only -std=c++11 -Wc++98-compat -verify %s
 // RUN: %clang_cc1 -fsyntax-only -std=c++14 -Wc++98-compat -verify %s 
-DCXX14COMPAT
+// RUN: %clang_cc1 -fsyntax-only -std=c++17 -Wc++98-compat -verify %s 
-DCXX14COMPAT -DCXX17COMPAT
 
 namespace std {
   struct type_info;
@@ -180,12 +181,16 @@ void LocalTemplateArg() {
 struct {} obj_of_unnamed_type; // expected-note {{here}}
 int UnnamedTemplateArg = TemplateFn(obj_of_unnamed_type); // expected-warning 
{{unnamed type as template argument is incompatible with C++98}}
 
+// FIXME: We do not implement C++98 compatibility warnings for the C++17
+// template argument evaluation rules.
+#ifndef CXX17COMPAT
 namespace RedundantParensInAddressTemplateParam {
   int n;
   template struct S {};
   S<(&n)> s; // expected-warning {{redundant parentheses surrounding address 
non-type template argument are incompatible with C++98}}
   S<(((&n)))> t; // expected-warning {{redundant parentheses surrounding 
address non-type template argument are incompatible with C++98}}
 }
+#endif
 
 namespace TemplateSpecOutOfScopeNs {
   template struct S {};
@@ -299,6 +304,9 @@ namespace LiteralUCNs {
   const wchar_t *s2 = L"bar\u0085"; // expected-warning {{universal character 
name referring to a control character is incompatible with C++98}}
 }
 
+// FIXME: We do not implement C++98 compatibility warnings for the C++17
+// template argument evaluation rules.
+#ifndef CXX17COMPAT
 namespace NonTypeTemplateArgs {
   template struct S {};
   const int k = 5; // expected-note {{here}}
@@ -314,6 +322,7 @@ namespace NullPointerTemplateArg {
   X<(int*)0> x; // expected-warning {{use of null pointer as non-type template 
argument is incompatible with C++98}}
   Y<(int A::*)0> y; // expected-warning {{use of null pointer as non-type 
template argument is incompatible with C++98}}
 }
+#endif
 
 namespace PR13480 {
   struct basic_iterator {
@@ -400,3 +409,11 @@ float fsvar = B::v;
 #ifdef CXX14COMPAT
 int digit_seps = 123'456; // expected-warning {{digit separators are 
incompatible with C++ standards before C++14}}
 #endif
+
+#ifdef CXX17COMPAT
+template struct CTAD {};
+void ctad_test() {
+  CTAD s;
+  CTAD t = s; // expected-warning {{class template argument deduction is 
incompatible with C++ standards before C++17}}
+}
+#endif



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

[clang-tools-extra] 1a9c01c - [clangd] Pass ExtraArgs to TestTU in TweakIsAvailable matcher

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2019-10-28T07:18:26+01:00
New Revision: 1a9c01c7f6c7e88676440869bbbe9f43fa45b109

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

LOG: [clangd] Pass ExtraArgs to TestTU in TweakIsAvailable matcher

Added: 


Modified: 
clang-tools-extra/clangd/unittests/TweakTesting.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/unittests/TweakTesting.cpp 
b/clang-tools-extra/clangd/unittests/TweakTesting.cpp
index 63ecd7b4d4d5..b67b41368329 100644
--- a/clang-tools-extra/clangd/unittests/TweakTesting.cpp
+++ b/clang-tools-extra/clangd/unittests/TweakTesting.cpp
@@ -13,6 +13,7 @@
 #include "refactor/Tweak.h"
 #include "clang/Tooling/Core/Replacement.h"
 #include "llvm/Support/Error.h"
+#include "gmock/gmock.h"
 
 namespace clang {
 namespace clangd {
@@ -59,7 +60,7 @@ std::pair rangeOrPoint(const Annotations 
&A) {
   cantFail(positionToOffset(A.code(), SelectionRng.end))};
 }
 
-MATCHER_P3(TweakIsAvailable, TweakID, Ctx, Header,
+MATCHER_P4(TweakIsAvailable, TweakID, Ctx, Header, ExtraArgs,
(TweakID + (negation ? " is unavailable" : " is available")).str()) 
{
   std::string WrappedCode = wrap(Ctx, arg);
   Annotations Input(WrappedCode);
@@ -67,6 +68,7 @@ MATCHER_P3(TweakIsAvailable, TweakID, Ctx, Header,
   TestTU TU;
   TU.HeaderCode = Header;
   TU.Code = Input.code();
+  TU.ExtraArgs = ExtraArgs;
   ParsedAST AST = TU.build();
   Tweak::Selection S(AST, Selection.first, Selection.second);
   auto PrepareResult = prepareTweak(TweakID, S);
@@ -113,7 +115,7 @@ std::string TweakTest::apply(llvm::StringRef MarkedCode) 
const {
 }
 
 ::testing::Matcher TweakTest::isAvailable() const {
-  return TweakIsAvailable(llvm::StringRef(TweakID), Context, Header); 
+  return TweakIsAvailable(llvm::StringRef(TweakID), Context, Header, 
ExtraArgs);
 }
 
 std::vector TweakTest::expandCases(llvm::StringRef MarkedCode) {



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

[clang] a4facd3 - [c++20] Enforce rule that a union-like class or class with reference

[Richard Smith via cfe-commits]

Author: Richard Smith
Date: 2019-10-27T23:26:44-07:00
New Revision: a4facd355dc36bc83d5c2402856f5a3741890c9a

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

LOG: [c++20] Enforce rule that a union-like class or class with reference
members cannot have defaulted comparisons.

Added: 
clang/test/CXX/class/class.compare/class.compare.default/p2.cpp

Modified: 
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaDeclCXX.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticGroups.td 
b/clang/include/clang/Basic/DiagnosticGroups.td
index 11218ccaeee7..29d27ec681f5 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -113,6 +113,7 @@ def UndefinedVarTemplate : 
DiagGroup<"undefined-var-template">;
 def UndefinedFuncTemplate : DiagGroup<"undefined-func-template">;
 def MissingNoEscape : DiagGroup<"missing-noescape">;
 
+def DefaultedComparison : DiagGroup<"defaulted-comparison">;
 def DeleteIncomplete : DiagGroup<"delete-incomplete">;
 def DeleteNonAbstractNonVirtualDtor : 
DiagGroup<"delete-non-abstract-non-virtual-dtor">;
 def DeleteAbstractNonVirtualDtor : 
DiagGroup<"delete-abstract-non-virtual-dtor">;

diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 385be4b44923..db877a46c300 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -8155,6 +8155,21 @@ def err_defaulted_comparison_non_const : Error<
 def err_defaulted_comparison_return_type_not_bool : Error<
   "return type for defaulted %sub{select_defaulted_comparison_kind}0 "
   "must be 'bool', not %1">;
+def err_defaulted_comparison_reference_member : Error<
+  "cannot default %0 in class %1 with reference member">;
+def ext_defaulted_comparison_reference_member : ExtWarn<
+  "ISO C++2a does not allow defaulting %0 in class %1 with reference member">,
+  InGroup;
+def note_reference_member : Note<"reference member %0 declared here">;
+def err_defaulted_comparison_union : Error<
+  "cannot default %0 in %select{union-like class|union}1 %2">;
+def ext_defaulted_comparison_union : ExtWarn<
+  "ISO C++2a does not allow defaulting %0 in "
+  "%select{union-like class|union}1 %2">, InGroup;
+def ext_defaulted_comparison_empty_union : ExtWarn<
+  "ISO C++2a does not allow defaulting %0 in "
+  "%select{union-like class|union}1 %2 despite it having no variant members">,
+  InGroup;
 
 def ext_implicit_exception_spec_mismatch : ExtWarn<
   "function previously declared with an %select{explicit|implicit}0 exception "

diff  --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index ccf6c0a604b2..6a718db6c553 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -7096,8 +7096,40 @@ bool 
Sema::CheckExplicitlyDefaultedComparison(FunctionDecl *FD,
   //   A defaulted comparison operator function for class C is defined as
   //   deleted if any non-static data member of C is of reference type or C is
   //   a union-like class.
-  // FIXME: Applying this to cases other than == and <=> is unreasonable.
-  // FIXME: Implement.
+  llvm::SmallVector Classes(1, RD);
+  FieldDecl *ReferenceMember = nullptr;
+  bool UnionLike = RD->isUnion();
+  while (!Classes.empty()) {
+if (Classes.back()->isUnion())
+  UnionLike = true;
+for (FieldDecl *FD : Classes.pop_back_val()->fields()) {
+  if (FD->getType()->isReferenceType())
+ReferenceMember = FD;
+  if (FD->isAnonymousStructOrUnion())
+Classes.push_back(FD->getType()->getAsCXXRecordDecl());
+}
+  }
+  // For non-memberwise comparisons, this rule is unjustified, so we permit
+  // those cases as an extension.
+  bool Memberwise = DCK == DefaultedComparisonKind::Equal ||
+DCK == DefaultedComparisonKind::ThreeWay;
+  if (ReferenceMember) {
+Diag(FD->getLocation(),
+ Memberwise ? diag::err_defaulted_comparison_reference_member
+: diag::ext_defaulted_comparison_reference_member)
+<< FD << RD;
+Diag(ReferenceMember->getLocation(), diag::note_reference_member)
+<< ReferenceMember;
+  } else if (UnionLike) {
+// If the class actually has no variant members, this rule similarly
+// is unjustified, so we permit those cases too.
+Diag(FD->getLocation(),
+ !Memberwise ? diag::ext_defaulted_comparison_union
+ : !RD->hasVariantMembers()
+   ? diag::ext_defaulted_comparison_empty_union
+   : diag::err_defaulted_comparison_union)
+<< FD << RD->isUnion() << RD;
+  }
 
   // C++2a [class

[clang-tools-extra] 5ab9a85 - [clangd] Reland DefineInline action availability checks

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2019-10-28T07:28:21+01:00
New Revision: 5ab9a850f6bde53974798ee285a06335fb788ae5

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

LOG: [clangd] Reland DefineInline action availability checks

Summary:
Introduces DefineInline action and initial version of availability
checks.

Reviewers: sammccall, ilya-biryukov, hokein

Reviewed By: hokein

Subscribers: thakis, usaxena95, mgorny, ilya-biryukov, MaskRay, jkorous, 
arphaman, cfe-commits

Tags: #clang

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

Added: 
clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp

Modified: 
clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
clang-tools-extra/clangd/unittests/TweakTesting.cpp
clang-tools-extra/clangd/unittests/TweakTesting.h
clang-tools-extra/clangd/unittests/TweakTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt 
b/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
index f43f132304c9..ddf10a2ca2ba 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
+++ b/clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
@@ -14,6 +14,7 @@ set(LLVM_LINK_COMPONENTS
 add_clang_library(clangDaemonTweaks OBJECT
   AnnotateHighlightings.cpp
   DumpAST.cpp
+  DefineInline.cpp
   ExpandAutoType.cpp
   ExpandMacro.cpp
   ExtractFunction.cpp

diff  --git a/clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp 
b/clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
new file mode 100644
index ..c2806d756983
--- /dev/null
+++ b/clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
@@ -0,0 +1,214 @@
+//===--- DefineInline.cpp *- 
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "AST.h"
+#include "Logger.h"
+#include "Selection.h"
+#include "SourceCode.h"
+#include "XRefs.h"
+#include "refactor/Tweak.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/ASTTypeTraits.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/DeclBase.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/DeclTemplate.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/ExprCXX.h"
+#include "clang/AST/NestedNameSpecifier.h"
+#include "clang/AST/PrettyPrinter.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/Stmt.h"
+#include "clang/AST/TemplateBase.h"
+#include "clang/AST/Type.h"
+#include "clang/AST/TypeLoc.h"
+#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Basic/TokenKinds.h"
+#include "clang/Index/IndexDataConsumer.h"
+#include "clang/Index/IndexSymbol.h"
+#include "clang/Index/IndexingAction.h"
+#include "clang/Lex/Lexer.h"
+#include "clang/Lex/Preprocessor.h"
+#include "clang/Lex/Token.h"
+#include "clang/Sema/Lookup.h"
+#include "clang/Sema/Sema.h"
+#include "clang/Tooling/Core/Replacement.h"
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Casting.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/Signals.h"
+#include "llvm/Support/raw_ostream.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+
+namespace clang {
+namespace clangd {
+namespace {
+
+// Deduces the FunctionDecl from a selection. Requires either the function body
+// or the function decl to be selected. Returns null if none of the above
+// criteria is met.
+const FunctionDecl *getSelectedFunction(const SelectionTree::Node *SelNode) {
+  const ast_type_traits::DynTypedNode &AstNode = SelNode->ASTNode;
+  if (const FunctionDecl *FD = AstNode.get())
+return FD;
+  if (AstNode.get() &&
+  SelNode->Selected == SelectionTree::Complete) {
+if (const SelectionTree::Node *P = SelNode->Parent)
+  return P->ASTNode.get();
+  }
+  return nullptr;
+}
+
+// Checks the decls mentioned in Source are visible in the context of Target.
+// Achives that by checking declarations occur before target location in
+// translation unit or declared in the same class.
+bool checkDeclsAreVisible(const llvm::DenseSet &DeclRefs,
+  const FunctionDecl *Target, const SourceManager &SM) 
{
+  SourceLocation TargetLoc = Target->getLocation();
+  // To be used in visibility check below, decls in a class are visible
+  // independent of order.
+  const RecordDecl *Class = nullptr;
+  if (const auto *MD = llvm::dyn_cast(Target))
+Class = MD-

[clang-tools-extra] 3d65def - [clangd] Reland DefineInline action apply logic with fully qualified names

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2019-10-28T07:45:38+01:00
New Revision: 3d65def1fd2febe5c1748de6f5ce009712e88f31

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

LOG: [clangd] Reland DefineInline action apply logic with fully qualified names

Summary:
Initial version of DefineInline action that will fully qualify every
name inside function body.

Reviewers: sammccall, ilya-biryukov, hokein

Tags: #clang

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

Added: 


Modified: 
clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
clang-tools-extra/clangd/unittests/TweakTesting.cpp
clang-tools-extra/clangd/unittests/TweakTesting.h
clang-tools-extra/clangd/unittests/TweakTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp 
b/clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
index c2806d756983..8785db1a292b 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
@@ -7,6 +7,7 @@
 
//===--===//
 
 #include "AST.h"
+#include "FindTarget.h"
 #include "Logger.h"
 #include "Selection.h"
 #include "SourceCode.h"
@@ -42,23 +43,42 @@
 #include "clang/Tooling/Core/Replacement.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/None.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/Error.h"
+#include "llvm/Support/FormatAdapters.h"
+#include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
 #include 
 #include 
 #include 
+#include 
 #include 
 
 namespace clang {
 namespace clangd {
 namespace {
 
+// Returns semicolon location for the given FD. Since AST doesn't contain that
+// information, searches for a semicolon by lexing from end of function decl
+// while skipping comments.
+llvm::Optional getSemicolonForDecl(const FunctionDecl *FD) {
+  const SourceManager &SM = FD->getASTContext().getSourceManager();
+  const LangOptions &LangOpts = FD->getASTContext().getLangOpts();
+
+  SourceLocation CurLoc = FD->getEndLoc();
+  auto NextTok = Lexer::findNextToken(CurLoc, SM, LangOpts);
+  if (!NextTok || !NextTok->is(tok::semi))
+return llvm::None;
+  return NextTok->getLocation();
+}
+
 // Deduces the FunctionDecl from a selection. Requires either the function body
 // or the function decl to be selected. Returns null if none of the above
 // criteria is met.
@@ -75,7 +95,7 @@ const FunctionDecl *getSelectedFunction(const 
SelectionTree::Node *SelNode) {
 }
 
 // Checks the decls mentioned in Source are visible in the context of Target.
-// Achives that by checking declarations occur before target location in
+// Achives that by checking declaraions occur before target location in
 // translation unit or declared in the same class.
 bool checkDeclsAreVisible(const llvm::DenseSet &DeclRefs,
   const FunctionDecl *Target, const SourceManager &SM) 
{
@@ -115,6 +135,97 @@ bool checkDeclsAreVisible(const llvm::DenseSet &DeclRefs,
   return true;
 }
 
+// Rewrites body of FD to fully-qualify all of the decls inside.
+llvm::Expected qualifyAllDecls(const FunctionDecl *FD) {
+  // There are three types of spellings that needs to be qualified in a 
function
+  // body:
+  // - Types:   Foo -> ns::Foo
+  // - DeclRefExpr: ns2::foo()  -> ns1::ns2::foo();
+  // - UsingDecls:
+  //using ns2::foo  -> using ns1::ns2::foo
+  //using namespace ns2 -> using namespace ns1::ns2
+  //using ns3 = ns2 -> using ns3 = ns1::ns2
+  //
+  // Go over all references inside a function body to generate replacements 
that
+  // will fully qualify those. So that body can be moved into an arbitrary 
file.
+  // We perform the qualification by qualyfying the first type/decl in a
+  // (un)qualified name. e.g:
+  //namespace a { namespace b { class Bar{}; void foo(); } }
+  //b::Bar x; -> a::b::Bar x;
+  //foo(); -> a::b::foo();
+  // FIXME: Instead of fully qualyfying we should try deducing visible scopes 
at
+  // target location and generate minimal edits.
+
+  const SourceManager &SM = FD->getASTContext().getSourceManager();
+  tooling::Replacements Replacements;
+  bool HadErrors = false;
+  findExplicitReferences(FD->getBody(), [&](ReferenceLoc Ref) {
+// Since we want to qualify only the first qualifier, skip names with a
+// qualifier.
+if (Ref.Qualifier)
+  return;
+// There might be no decl in dependent contexts, there's nothing much we 
can
+// do in s

[clang-tools-extra] f4d7caf - [clangd] Reland Store Index in Tweak::Selection

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2019-10-28T07:55:58+01:00
New Revision: f4d7caf6df819d18b4d5b460d21f7f8fe1e1b705

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

LOG: [clangd] Reland Store Index in Tweak::Selection

Summary:
Incoming define out-of-line tweak requires access to index.

This patch simply propogates the index in ClangdServer to Tweak::Selection while
passing the AST. Also updates TweakTest to accommodate this change.

Reviewers: ilya-biryukov

Subscribers: MaskRay, jkorous, arphaman, usaxena95, cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/refactor/Tweak.cpp
clang-tools-extra/clangd/refactor/Tweak.h
clang-tools-extra/clangd/unittests/TweakTesting.cpp
clang-tools-extra/clangd/unittests/TweakTesting.h

Removed: 




diff  --git a/clang-tools-extra/clangd/ClangdServer.cpp 
b/clang-tools-extra/clangd/ClangdServer.cpp
index 4f1fe8f5b08b..64ccbe417c24 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -367,7 +367,7 @@ tweakSelection(const Range &Sel, const InputsAndAST &AST) {
   auto End = positionToOffset(AST.Inputs.Contents, Sel.end);
   if (!End)
 return End.takeError();
-  return Tweak::Selection(AST.AST, *Begin, *End);
+  return Tweak::Selection(AST.Inputs.Index, AST.AST, *Begin, *End);
 }
 
 void ClangdServer::enumerateTweaks(PathRef File, Range Sel,

diff  --git a/clang-tools-extra/clangd/refactor/Tweak.cpp 
b/clang-tools-extra/clangd/refactor/Tweak.cpp
index 4f3c40d1eb13..2dc091ed762a 100644
--- a/clang-tools-extra/clangd/refactor/Tweak.cpp
+++ b/clang-tools-extra/clangd/refactor/Tweak.cpp
@@ -9,6 +9,7 @@
 #include "Logger.h"
 #include "Path.h"
 #include "SourceCode.h"
+#include "index/Index.h"
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/STLExtras.h"
@@ -44,9 +45,10 @@ void validateRegistry() {
 }
 } // namespace
 
-Tweak::Selection::Selection(ParsedAST &AST, unsigned RangeBegin,
-unsigned RangeEnd)
-: AST(AST), SelectionBegin(RangeBegin), SelectionEnd(RangeEnd),
+Tweak::Selection::Selection(const SymbolIndex *Index, ParsedAST &AST,
+unsigned RangeBegin, unsigned RangeEnd)
+: Index(Index), AST(AST), SelectionBegin(RangeBegin),
+  SelectionEnd(RangeEnd),
   ASTSelection(AST.getASTContext(), AST.getTokens(), RangeBegin, RangeEnd) 
{
   auto &SM = AST.getSourceManager();
   Code = SM.getBufferData(SM.getMainFileID());

diff  --git a/clang-tools-extra/clangd/refactor/Tweak.h 
b/clang-tools-extra/clangd/refactor/Tweak.h
index 42b7fb0684e8..de655abd98c7 100644
--- a/clang-tools-extra/clangd/refactor/Tweak.h
+++ b/clang-tools-extra/clangd/refactor/Tweak.h
@@ -24,6 +24,7 @@
 #include "Protocol.h"
 #include "Selection.h"
 #include "SourceCode.h"
+#include "index/Index.h"
 #include "clang/Tooling/Core/Replacement.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringMap.h"
@@ -46,9 +47,12 @@ class Tweak {
 public:
   /// Input to prepare and apply tweaks.
   struct Selection {
-Selection(ParsedAST &AST, unsigned RangeBegin, unsigned RangeEnd);
+Selection(const SymbolIndex *Index, ParsedAST &AST, unsigned RangeBegin,
+  unsigned RangeEnd);
 /// The text of the active document.
 llvm::StringRef Code;
+/// The Index for handling codebase related queries.
+const SymbolIndex *Index = nullptr;
 /// Parsed AST of the active file.
 ParsedAST *
 /// A location of the cursor in the editor.

diff  --git a/clang-tools-extra/clangd/unittests/TweakTesting.cpp 
b/clang-tools-extra/clangd/unittests/TweakTesting.cpp
index 39d5e899b4dc..3331a3d93715 100644
--- a/clang-tools-extra/clangd/unittests/TweakTesting.cpp
+++ b/clang-tools-extra/clangd/unittests/TweakTesting.cpp
@@ -63,7 +63,7 @@ std::pair rangeOrPoint(const Annotations 
&A) {
   cantFail(positionToOffset(A.code(), SelectionRng.end))};
 }
 
-MATCHER_P5(TweakIsAvailable, TweakID, Ctx, Header, ExtraArgs, ExtraFiles,
+MATCHER_P6(TweakIsAvailable, TweakID, Ctx, Header, ExtraArgs, ExtraFiles, 
Index,
(TweakID + (negation ? " is unavailable" : " is available")).str()) 
{
   std::string WrappedCode = wrap(Ctx, arg);
   Annotations Input(WrappedCode);
@@ -74,7 +74,7 @@ MATCHER_P5(TweakIsAvailable, TweakID, Ctx, Header, ExtraArgs, 
ExtraFiles,
   TU.ExtraArgs = ExtraArgs;
   TU.AdditionalFiles = std::move(ExtraFiles);
   ParsedAST AST = TU.build();
-  Tweak::Selection S(AST, Selection.first, Selection.second);
+  Tweak::Selection S(Index, AST, Selection.first, Selection.second);
   auto PrepareResult = prepareTweak(TweakID, S);
   if (PrepareResult)
 return