[PATCH] D44552: [Coroutines] Find custom allocators in class scope

2018-03-15 Thread Brian Gesiak via Phabricator via cfe-commits
modocache created this revision.
modocache added reviewers: GorNishanov, eric_niebler, lewissbaker.
Herald added a subscriber: EricWF.

https://reviews.llvm.org/rL325291 implemented Coroutines TS N4723
section [dcl.fct.def.coroutine]/7, but it performed lookup of allocator
functions within both the global and class scope, whereas the specified
behavior is to perform lookup for custom allocators within just the
class scope.

To fix, add parameters to the `Sema::FindAllocationFunctions` function
such that it can be used to lookup allocators in global scope,
class scope, or both (instead of just being able to look up in just global
scope or in both global and class scope). Then, use those parameters
from within the coroutine Sema.

This incorrect behavior had the unfortunate side-effect of causing the
bug https://bugs.llvm.org/show_bug.cgi?id=36578 (or at least the reports
of that bug in C++ programs). That bug would occur for any C++ user with
a coroutine frame that took a single pointer argument, since it would
then find the global placement form `operator new`, described in the
C++ standard 18.6.1.3.1. This patch prevents Clang from generating code
that triggers the LLVM assert described in that bug report.

Test Plan: `check-clang`


Repository:
  rC Clang

https://reviews.llvm.org/D44552

Files:
  include/clang/Sema/Sema.h
  lib/Sema/SemaCoroutine.cpp
  lib/Sema/SemaExprCXX.cpp
  test/CodeGenCoroutines/coro-alloc.cpp
  test/SemaCXX/coroutines.cpp

Index: test/SemaCXX/coroutines.cpp
===
--- test/SemaCXX/coroutines.cpp
+++ test/SemaCXX/coroutines.cpp
@@ -806,62 +806,6 @@
   void *operator new(SizeT, coroutine_nonstatic_member_struct &, double);
 };
 
-struct bad_promise_nonstatic_member_mismatched_custom_new_operator {
-  coro get_return_object();
-  suspend_always initial_suspend();
-  suspend_always final_suspend();
-  void return_void();
-  void unhandled_exception();
-  // expected-note@+1 {{candidate function not viable: requires 2 arguments, but 1 was provided}}
-  void *operator new(SizeT, double);
-};
-
-struct coroutine_nonstatic_member_struct {
-  coro
-  good_coroutine_calls_nonstatic_member_custom_new_operator(double) {
-co_return;
-  }
-
-  coro
-  bad_coroutine_calls_nonstatic_member_mistmatched_custom_new_operator(double) {
-// expected-error@-1 {{no matching function for call to 'operator new'}}
-co_return;
-  }
-};
-
-struct bad_promise_mismatched_custom_new_operator {
-  coro get_return_object();
-  suspend_always initial_suspend();
-  suspend_always final_suspend();
-  void return_void();
-  void unhandled_exception();
-  // expected-note@+1 {{candidate function not viable: requires 4 arguments, but 1 was provided}}
-  void *operator new(SizeT, double, float, int);
-};
-
-coro
-bad_coroutine_calls_mismatched_custom_new_operator(double) {
-  // expected-error@-1 {{no matching function for call to 'operator new'}}
-  co_return;
-}
-
-struct bad_promise_throwing_custom_new_operator {
-  static coro get_return_object_on_allocation_failure();
-  coro get_return_object();
-  suspend_always initial_suspend();
-  suspend_always final_suspend();
-  void return_void();
-  void unhandled_exception();
-  // expected-error@+1 {{'operator new' is required to have a non-throwing noexcept specification when the promise type declares 'get_return_object_on_allocation_failure()'}}
-  void *operator new(SizeT, double, float, int);
-};
-
-coro
-bad_coroutine_calls_throwing_custom_new_operator(double, float, int) {
-  // expected-note@-1 {{call to 'operator new' implicitly required by coroutine function here}}
-  co_return;
-}
-
 struct good_promise_noexcept_custom_new_operator {
   static coro get_return_object_on_allocation_failure();
   coro get_return_object();
Index: test/CodeGenCoroutines/coro-alloc.cpp
===
--- test/CodeGenCoroutines/coro-alloc.cpp
+++ test/CodeGenCoroutines/coro-alloc.cpp
@@ -134,6 +134,32 @@
   co_return;
 }
 
+// Declare a placement form operator new, such as the one described in
+// C++ 18.6.1.3.1, which takes a void* argument.
+void* operator new(SizeT __sz, void *__p) noexcept;
+
+struct promise_matching_global_placement_new_tag {};
+struct dummy {};
+template<>
+struct std::experimental::coroutine_traits {
+  struct promise_type {
+void get_return_object() {}
+suspend_always initial_suspend() { return {}; }
+suspend_always final_suspend() { return {}; }
+void return_void() {}
+  };
+};
+
+// A coroutine that takes a single pointer argument should not invoke this
+// placement form operator. [dcl.fct.def.coroutine]/7 dictates that lookup for
+// allocation functions matching the coroutine function's signature be done
+// within the scope of the promise type's class.
+// CHECK-LABEL: f1b(
+extern "C" void f1b(promise_matching_global_placement_new_tag, dummy *) {
+  // CHECK: call i8* @_Znwm(i64
+  co_return;
+}
+
 struct promise

[libcxxabi] r327690 - [demangler] Support for s in generic lambdas.

2018-03-15 Thread Erik Pilkington via cfe-commits
Author: epilk
Date: Thu Mar 15 20:06:30 2018
New Revision: 327690

URL: http://llvm.org/viewvc/llvm-project?rev=327690&view=rev
Log:
[demangler] Support for s in generic lambdas.

These s refer to "artifical" s that don't appear
in the mangled name, so we just print them as "auto".

Modified:
libcxxabi/trunk/src/cxa_demangle.cpp
libcxxabi/trunk/test/test_demangle.pass.cpp

Modified: libcxxabi/trunk/src/cxa_demangle.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxxabi/trunk/src/cxa_demangle.cpp?rev=327690&r1=327689&r2=327690&view=diff
==
--- libcxxabi/trunk/src/cxa_demangle.cpp (original)
+++ libcxxabi/trunk/src/cxa_demangle.cpp Thu Mar 15 20:06:30 2018
@@ -2013,6 +2013,7 @@ struct Db {
   bool TagTemplates = true;
   bool FixForwardReferences = false;
   bool TryToParseTemplateArgs = true;
+  bool ParsingLambdaParams = false;
 
   BumpPointerAllocator ASTAllocator;
 
@@ -2270,6 +2271,7 @@ Node *Db::parseUnnamedTypeName(NameState
   }
   if (consumeIf("Ul")) {
 NodeArray Params;
+SwapAndRestore SwapParams(ParsingLambdaParams, true);
 if (!consumeIf("vE")) {
   size_t ParamsBegin = Names.size();
   do {
@@ -4658,20 +4660,20 @@ Node *Db::parseTemplateParam() {
   if (!consumeIf('T'))
 return nullptr;
 
-  if (consumeIf('_')) {
-if (TemplateParams.empty()) {
-  FixForwardReferences = true;
-  return make("FORWARD_REFERENCE");
-}
-return TemplateParams[0];
+  size_t Index = 0;
+  if (!consumeIf('_')) {
+if (parsePositiveInteger(&Index))
+  return nullptr;
+++Index;
+if (!consumeIf('_'))
+  return nullptr;
   }
 
-  size_t Index;
-  if (parsePositiveInteger(&Index))
-return nullptr;
-  ++Index;
-  if (!consumeIf('_'))
-return nullptr;
+  // Itanium ABI 5.1.8: In a generic lambda, uses of auto in the parameter list
+  // are mangled as the corresponding artificial template type parameter.
+  if (ParsingLambdaParams)
+return make("auto");
+
   if (Index >= TemplateParams.size()) {
 FixForwardReferences = true;
 return make("FORWARD_REFERENCE");

Modified: libcxxabi/trunk/test/test_demangle.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxxabi/trunk/test/test_demangle.pass.cpp?rev=327690&r1=327689&r2=327690&view=diff
==
--- libcxxabi/trunk/test/test_demangle.pass.cpp (original)
+++ libcxxabi/trunk/test/test_demangle.pass.cpp Thu Mar 15 20:06:30 2018
@@ -29604,7 +29604,7 @@ const char* cases[][2] =
 {"PFvRmOE", "void (*)(unsigned long&) &&"},
 {"_ZTW1x", "thread-local wrapper routine for x"},
 {"_ZTHN3fooE", "thread-local initialization routine for foo"},
-{"_Z4algoIJiiiEEvZ1gEUlDpT_E_", "void algo(g::'lambda'(int, 
int, int))"},
+
 // attribute abi_tag
 {"_Z1fB3foov", "f[abi:foo]()"},
 {"_Z1fB3fooB3barv", "f[abi:foo][abi:bar]()"},
@@ -29725,6 +29725,8 @@ const char* cases[][2] =
 
 {"_ZGRDC1x1yE_", "reference temporary for [x, y]"},
 {"_ZGR1bIvE2_", "reference temporary for b"},
+
+
{"_ZZ18test_assign_throwsI20small_throws_on_copyLb0EEvvENKUlRNSt3__13anyEOT_E_clIRS0_EEDaS3_S5_",
 "auto void test_assign_throws()::'lambda'(std::__1::any&, 
auto&&)::operator()(std::__1::any&, auto&&) const"},
 };
 
 const unsigned N = sizeof(cases) / sizeof(cases[0]);


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


[PATCH] D44505: [MS] Always use base dtors in place of complete/vbase dtors when possible

2018-03-15 Thread John McCall via Phabricator via cfe-commits
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.

LGTM.


https://reviews.llvm.org/D44505



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


r327687 - [Analysis] Fix some Clang-tidy modernize and Include What You Use warnings; other minor fixes (NFC).

2018-03-15 Thread Eugene Zelenko via cfe-commits
Author: eugenezelenko
Date: Thu Mar 15 17:37:51 2018
New Revision: 327687

URL: http://llvm.org/viewvc/llvm-project?rev=327687&view=rev
Log:
[Analysis] Fix some Clang-tidy modernize and Include What You Use warnings; 
other minor fixes (NFC).

Modified:
cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyTIL.h
cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyUtil.h
cfe/trunk/lib/Analysis/ThreadSafetyTIL.cpp

Modified: cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyTIL.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyTIL.h?rev=327687&r1=327686&r2=327687&view=diff
==
--- cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyTIL.h (original)
+++ cfe/trunk/include/clang/Analysis/Analyses/ThreadSafetyTIL.h Thu Mar 15 
17:37:51 2018
@@ -1,4 +1,4 @@
-//===- ThreadSafetyTIL.h ---*- C++ 
--*-===//
+//===- ThreadSafetyTIL.h *- C++ 
-*-===//
 //
 // The LLVM Compiler Infrastructure
 //
@@ -47,20 +47,33 @@
 #ifndef LLVM_CLANG_ANALYSIS_ANALYSES_THREADSAFETYTIL_H
 #define LLVM_CLANG_ANALYSIS_ANALYSES_THREADSAFETYTIL_H
 
-// All clang include dependencies for this file must be put in
-// ThreadSafetyUtil.h.
-#include "ThreadSafetyUtil.h"
+#include "clang/AST/Decl.h"
+#include "clang/Analysis/Analyses/ThreadSafetyUtil.h"
+#include "clang/Basic/LLVM.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/None.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Casting.h"
+#include "llvm/Support/raw_ostream.h"
 #include 
 #include 
 #include 
-#include 
+#include 
+#include 
+#include 
 #include 
 
-
 namespace clang {
+
+class CallExpr;
+class Expr;
+class Stmt;
+
 namespace threadSafety {
 namespace til {
 
+class BasicBlock;
 
 /// Enum for the different distinct classes of SExpr
 enum TIL_Opcode {
@@ -100,11 +113,21 @@ enum TIL_BinaryOpcode : unsigned char {
 /// Opcode for cast operations.
 enum TIL_CastOpcode : unsigned char {
   CAST_none = 0,
-  CAST_extendNum,   // extend precision of numeric type
-  CAST_truncNum,// truncate precision of numeric type
-  CAST_toFloat, // convert to floating point type
-  CAST_toInt,   // convert to integer type
-  CAST_objToPtr // convert smart pointer to pointer  (C++ only)
+
+  // Extend precision of numeric type
+  CAST_extendNum,
+
+  // Truncate precision of numeric type
+  CAST_truncNum,
+
+  // Convert to floating point type
+  CAST_toFloat,
+
+  // Convert to integer type
+  CAST_toInt,
+
+  // Convert smart pointer to pointer (C++ only)
+  CAST_objToPtr
 };
 
 const TIL_Opcode   COP_Min  = COP_Future;
@@ -122,7 +145,6 @@ StringRef getUnaryOpcodeString(TIL_Unary
 /// Return the name of a binary opcode.
 StringRef getBinaryOpcodeString(TIL_BinaryOpcode Op);
 
-
 /// ValueTypes are data types that can actually be held in registers.
 /// All variables and expressions must have a value type.
 /// Pointer types are further subdivided into the various heap-allocated
@@ -150,22 +172,22 @@ struct ValueType {
 ST_128
   };
 
+  ValueType(BaseType B, SizeType Sz, bool S, unsigned char VS)
+  : Base(B), Size(Sz), Signed(S), VectSize(VS) {}
+
   inline static SizeType getSizeType(unsigned nbytes);
 
   template 
   inline static ValueType getValueType();
 
-  ValueType(BaseType B, SizeType Sz, bool S, unsigned char VS)
-  : Base(B), Size(Sz), Signed(S), VectSize(VS)
-  { }
+  BaseType Base;
+  SizeType Size;
+  bool Signed;
 
-  BaseType  Base;
-  SizeType  Size;
-  bool  Signed;
-  unsigned char VectSize;  // 0 for scalar, otherwise num elements in vector
+  // 0 for scalar, otherwise num elements in vector
+  unsigned char VectSize;
 };
 
-
 inline ValueType::SizeType ValueType::getSizeType(unsigned nbytes) {
   switch (nbytes) {
 case 1: return ST_8;
@@ -177,7 +199,6 @@ inline ValueType::SizeType ValueType::ge
   }
 }
 
-
 template<>
 inline ValueType ValueType::getValueType() {
   return ValueType(BT_Void, ST_0, false, 0);
@@ -253,13 +274,11 @@ inline ValueType ValueType::getValueType
   return ValueType(BT_Pointer, getSizeType(sizeof(void*)), false, 0);
 }
 
-
-class BasicBlock;
-
-
 /// Base class for AST nodes in the typed intermediate language.
 class SExpr {
 public:
+  SExpr() = delete;
+
   TIL_Opcode opcode() const { return static_cast(Opcode); }
 
   // Subclasses of SExpr must define the following:
@@ -280,6 +299,9 @@ public:
 return ::operator new(S, R);
   }
 
+  /// SExpr objects must be created in an arena.
+  void *operator new(size_t) = delete;
+
   /// SExpr objects cannot be deleted.
   // This declaration is public to workaround a gcc bug that breaks building
   // with REQUIRES_EH=1.
@@ -291,45 +313,33 @@ public:
 
   /// Returns the block, if this is an instruction in a basic block,
   /// otherwise returns null.
-  BasicBloc

Re: r327593 - [CFG] Allow CallExpr's to be looked up in CFG's

2018-03-15 Thread Artem Dergachev via cfe-commits

Thank you for fixing this one! I must have missed it.

On 14/03/2018 5:09 PM, Richard Trieu via cfe-commits wrote:

Author: rtrieu
Date: Wed Mar 14 17:09:26 2018
New Revision: 327593

URL: http://llvm.org/viewvc/llvm-project?rev=327593&view=rev
Log:
[CFG] Allow CallExpr's to be looked up in CFG's

r327343 changed the handling for CallExpr in a CFG, which prevented lookups for
CallExpr while other Stmt kinds still worked.  This change carries over the
necessary bits from Stmt function to CallExpr function.

Added:
 cfe/trunk/test/SemaCXX/constant-conversion.cpp
Modified:
 cfe/trunk/lib/Analysis/CFG.cpp

Modified: cfe/trunk/lib/Analysis/CFG.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CFG.cpp?rev=327593&r1=327592&r2=327593&view=diff
==
--- cfe/trunk/lib/Analysis/CFG.cpp (original)
+++ cfe/trunk/lib/Analysis/CFG.cpp Wed Mar 14 17:09:26 2018
@@ -750,6 +750,9 @@ private:
}
  
void appendCall(CFGBlock *B, CallExpr *CE) {

+if (alwaysAdd(CE) && cachedEntry)
+  cachedEntry->second = B;
+
  if (BuildOpts.AddRichCXXConstructors) {
if (CFGCXXRecordTypedCall::isCXXRecordTypedCall(CE, *Context)) {
  if (const ConstructionContextLayer *Layer =

Added: cfe/trunk/test/SemaCXX/constant-conversion.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/constant-conversion.cpp?rev=327593&view=auto
==
--- cfe/trunk/test/SemaCXX/constant-conversion.cpp (added)
+++ cfe/trunk/test/SemaCXX/constant-conversion.cpp Wed Mar 14 17:09:26 2018
@@ -0,0 +1,24 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -triple x86_64-apple-darwin %s
+
+// This file tests -Wconstant-conversion, a subcategory of -Wconversion
+// which is on by default.
+
+constexpr int nines() { return 9; }
+
+void too_big_for_char(int param) {
+  char warn1 = false ? 0 : 9;
+  // expected-warning@-1 {{implicit conversion from 'int' to 'char' changes 
value from 9 to -97}}
+  char warn2 = false ? 0 : nines();
+  // expected-warning@-1 {{implicit conversion from 'int' to 'char' changes 
value from 9 to -97}}
+
+  char warn3 = param > 0 ? 0 : 9;
+  // expected-warning@-1 {{implicit conversion from 'int' to 'char' changes 
value from 9 to -97}}
+  char warn4 = param > 0 ? 0 : nines();
+  // expected-warning@-1 {{implicit conversion from 'int' to 'char' changes 
value from 9 to -97}}
+
+  char ok1 = true ? 0 : 9;
+  char ok2 = true ? 0 : nines();
+
+  char ok3 = true ? 0 : 9 + 1;
+  char ok4 = true ? 0 : nines() + 1;
+}


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


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


[PATCH] D17893: Sema: Add semantic analysis for the C++ ABI stability attributes and whitelist.

2018-03-15 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

I think this is okay.  We can review further if we see other problems.


https://reviews.llvm.org/D17893



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


[PATCH] D44508: Remove unnecessary include from Driver.cpp

2018-03-15 Thread Frederich Munch via Phabricator via cfe-commits
marsupial added a comment.

Shouldn't the Translation Unit be responsible for knowing what it needs rather 
than hoping it's already been used somewhere up the chain.
Perhaps std::unique_ptr and std::make_pair are too low level and one can 
assume, but https://reviews.llvm.org/D44509 seems like a possibly overzealous 
application?


Repository:
  rL LLVM

https://reviews.llvm.org/D44508



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


[PATCH] D43184: [WIP] [ItaniunCXXABI] Add an option for loading the type info vtable with dllimport

2018-03-15 Thread Martin Storsjö via Phabricator via cfe-commits
mstorsjo added a comment.

In https://reviews.llvm.org/D43184#1039385, @rnk wrote:

> In https://reviews.llvm.org/D43184#1039354, @mstorsjo wrote:
>
> > Ok - I'll put it on the backburner, and maybe see if I'd try implementing 
> > the linker part of fixing unannotated data imports at some point.
>
>
> I'm not sure that's feasible.


If we'd just fix the RTTI vtable case, this still would need to be implemented 
in the linker, right? That's what I meant originally - implementing enough of 
that to handle RTTI, optionally giving the larger usecase a shot.

> At least for x86, global addresses can be folded in ways that mean the linker 
> cannot relax them. You can go the other way, though. If we start by assuming 
> all data is imported, you can often relax an __imp_sym load to a LEA 
> sym(%rip). This increases register pressure and generates worse code, but it 
> has conceivable use cases.

Hmm, I think it seems like GCC does something like that. When accessing data 
symbols that aren't known to be DSO local, it seems to produce code like this: 
"movq .refptr.a(%rip), %rax; mov (%rax), %rax". My x86-fu is rather weak but I 
guess that's pretty much what you meant?

Anyway, good to know about that case.


Repository:
  rC Clang

https://reviews.llvm.org/D43184



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


[PATCH] D44541: [OpenMP][Clang] Move device global stack init before master-workers split

2018-03-15 Thread Gheorghe-Teodor Bercea via Phabricator via cfe-commits
gtbercea created this revision.
gtbercea added reviewers: ABataev, carlo.bertolli, grokos, caomhin.
Herald added subscribers: cfe-commits, guansong, jholewinski.

This patch moves the call to the stack init data sharing function before the 
splitting of threads into master and workers. This ensures that the 
initialization happens for all active warp master threads. Test is modified 
appropriately.


Repository:
  rC Clang

https://reviews.llvm.org/D44541

Files:
  lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
  test/OpenMP/nvptx_data_sharing.cpp


Index: test/OpenMP/nvptx_data_sharing.cpp
===
--- test/OpenMP/nvptx_data_sharing.cpp
+++ test/OpenMP/nvptx_data_sharing.cpp
@@ -30,8 +30,9 @@
 // CK1: {{.*}}define void @__omp_offloading{{.*}}test_ds{{.*}}()
 // CK1: [[SHAREDARGS1:%.+]] = alloca i8**
 // CK1: [[SHAREDARGS2:%.+]] = alloca i8**
-// CK1: call void @__kmpc_kernel_init
 // CK1: call void @__kmpc_data_sharing_init_stack
+// CK1: call void @__omp_offloading{{.*}}_worker()
+// CK1: call void @__kmpc_kernel_init
 // CK1: [[GLOBALSTACK:%.+]] = call i8* @__kmpc_data_sharing_push_stack(i64 8, 
i16 0)
 // CK1: [[GLOBALSTACK2:%.+]] = bitcast i8* [[GLOBALSTACK]] to 
%struct._globalized_locals_ty*
 // CK1: [[A:%.+]] = getelementptr inbounds %struct._globalized_locals_ty, 
%struct._globalized_locals_ty* [[GLOBALSTACK2]], i32 0, i32 0
Index: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
===
--- lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
+++ lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
@@ -598,6 +598,11 @@
   WorkerFunctionState &WST) {
   CGBuilderTy &Bld = CGF.Builder;
 
+  // For data sharing, initialize the stack for master and workers.
+  CGF.EmitRuntimeCall(
+  createNVPTXRuntimeFunction(
+  OMPRTL_NVPTX__kmpc_data_sharing_init_stack));
+
   llvm::BasicBlock *WorkerBB = CGF.createBasicBlock(".worker");
   llvm::BasicBlock *MasterCheckBB = CGF.createBasicBlock(".mastercheck");
   llvm::BasicBlock *MasterBB = CGF.createBasicBlock(".master");
@@ -626,11 +631,6 @@
   CGF.EmitRuntimeCall(
   createNVPTXRuntimeFunction(OMPRTL_NVPTX__kmpc_kernel_init), Args);
 
-  // For data sharing, we need to initialize the stack.
-  CGF.EmitRuntimeCall(
-  createNVPTXRuntimeFunction(
-  OMPRTL_NVPTX__kmpc_data_sharing_init_stack));
-
   emitGenericVarsProlog(CGF, WST.Loc);
 }
 


Index: test/OpenMP/nvptx_data_sharing.cpp
===
--- test/OpenMP/nvptx_data_sharing.cpp
+++ test/OpenMP/nvptx_data_sharing.cpp
@@ -30,8 +30,9 @@
 // CK1: {{.*}}define void @__omp_offloading{{.*}}test_ds{{.*}}()
 // CK1: [[SHAREDARGS1:%.+]] = alloca i8**
 // CK1: [[SHAREDARGS2:%.+]] = alloca i8**
-// CK1: call void @__kmpc_kernel_init
 // CK1: call void @__kmpc_data_sharing_init_stack
+// CK1: call void @__omp_offloading{{.*}}_worker()
+// CK1: call void @__kmpc_kernel_init
 // CK1: [[GLOBALSTACK:%.+]] = call i8* @__kmpc_data_sharing_push_stack(i64 8, i16 0)
 // CK1: [[GLOBALSTACK2:%.+]] = bitcast i8* [[GLOBALSTACK]] to %struct._globalized_locals_ty*
 // CK1: [[A:%.+]] = getelementptr inbounds %struct._globalized_locals_ty, %struct._globalized_locals_ty* [[GLOBALSTACK2]], i32 0, i32 0
Index: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
===
--- lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
+++ lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
@@ -598,6 +598,11 @@
   WorkerFunctionState &WST) {
   CGBuilderTy &Bld = CGF.Builder;
 
+  // For data sharing, initialize the stack for master and workers.
+  CGF.EmitRuntimeCall(
+  createNVPTXRuntimeFunction(
+  OMPRTL_NVPTX__kmpc_data_sharing_init_stack));
+
   llvm::BasicBlock *WorkerBB = CGF.createBasicBlock(".worker");
   llvm::BasicBlock *MasterCheckBB = CGF.createBasicBlock(".mastercheck");
   llvm::BasicBlock *MasterBB = CGF.createBasicBlock(".master");
@@ -626,11 +631,6 @@
   CGF.EmitRuntimeCall(
   createNVPTXRuntimeFunction(OMPRTL_NVPTX__kmpc_kernel_init), Args);
 
-  // For data sharing, we need to initialize the stack.
-  CGF.EmitRuntimeCall(
-  createNVPTXRuntimeFunction(
-  OMPRTL_NVPTX__kmpc_data_sharing_init_stack));
-
   emitGenericVarsProlog(CGF, WST.Loc);
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D44539: [Sema][Objective-C] Add check to warn when property of objc type has assign attribute

2018-03-15 Thread Alfred Zien via Phabricator via cfe-commits
QF5690 created this revision.
QF5690 added a reviewer: rsmith.
QF5690 added a project: clang.
Herald added a subscriber: cfe-commits.

There is a problem, that `assign` attribute very often getting out of 
attention. For example, consider this code:

  @property(nonatomic, strong, readonly, nullable) SomeObjcClassType1* foo;
  @property(nonatomic, strong, readwrite) SomeObjcClassType2* bar;
  @property(nonatomic, assign, readonly) SomeObjcClassType* property;
  @property(nonatomic, assign, readonly) SomeCStructType state;

It is very easy to miss that `assign` keyword, and it leads to hard to find and 
reproduce bugs. Most of the time, we found such bugs in crash reports from 
already in production code.

Now, consider this code:

  @property(nonatomic, strong, readonly, nullable) SomeObjcClassType1* foo;
  @property(nonatomic, strong, readwrite) SomeObjcClassType2* bar;
  @property(nonatomic, unsafe_unretained, readonly) SomeObjcClassType* property;
  @property(nonatomic, assign, readonly) SomeCStructType state;

It is now much harder to even make that mistake and it will be much obvious 
during code review.

As there is no difference in behaviour between `assign` and `unsafe_unretained` 
attribute, but second is much more verbose, saying "think twice when doing 
this", I suggest to have, at least, optional warning, that will catch such 
constructs.

This is my first revision in llvm, so any help would be very much appreciated.


Repository:
  rC Clang

https://reviews.llvm.org/D44539

Files:
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaObjCProperty.cpp
  test/SemaObjC/property-assign-on-object-type.m
  test/SemaObjC/property-in-class-extension-1.m


Index: test/SemaObjC/property-in-class-extension-1.m
===
--- test/SemaObjC/property-in-class-extension-1.m
+++ test/SemaObjC/property-in-class-extension-1.m
@@ -15,12 +15,12 @@
 @property (readonly) NSString* none;
 @property (readonly) NSString* none1;
 
-@property (assign, readonly) NSString* changeMemoryModel; // expected-note 
{{property declared here}}
+@property (unsafe_unretained, readonly) NSString* changeMemoryModel; // 
expected-note {{property declared here}}
 
 @property (readonly) __weak id weak_prop;
 @property (readonly) __weak id weak_prop1;
 
-@property (assign, readonly) NSString* assignProperty;
+@property (unsafe_unretained, readonly) NSString* unsafeUnretainedProperty;
 
 @property (readonly) NSString* readonlyProp;
 
@@ -43,8 +43,8 @@
 @property () __weak id weak_prop;
 @property (readwrite) __weak id weak_prop1;
 
-@property (assign, readwrite) NSString* assignProperty;
-@property (assign) NSString* readonlyProp;
+@property (unsafe_unretained, readwrite) NSString* unsafeUnretainedProperty;
+@property (unsafe_unretained) NSString* readonlyProp;
 @end
 
 // rdar://12214070
Index: test/SemaObjC/property-assign-on-object-type.m
===
--- /dev/null
+++ test/SemaObjC/property-assign-on-object-type.m
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wobjc-property-assign-on-object-type
+
+@interface Foo @end
+
+@interface Bar
+@property(assign, readonly) Foo* o1; // expected-warning {{'assign' attribute 
must not be of object type, use 'unsafe_unretained' instead}}
+@property(unsafe_unretained, readonly) Foo* o2;
+
+@property(assign) int s1;
+@property(assign) int* s2;
+@end
+
+@interface Bar ()
+@property(readwrite) Foo* o1;
+@property(readwrite) Foo* o2;
+@end
Index: lib/Sema/SemaObjCProperty.cpp
===
--- lib/Sema/SemaObjCProperty.cpp
+++ lib/Sema/SemaObjCProperty.cpp
@@ -2547,6 +2547,13 @@
 PropertyDecl->setInvalidDecl();
   }
 
+  // Check for assign on object types.
+  if ((Attributes & ObjCDeclSpec::DQ_PR_assign) &&
+  !(Attributes & ObjCDeclSpec::DQ_PR_unsafe_unretained) &&
+  PropertyTy->isObjCRetainableType()) {
+Diag(Loc, diag::warn_objc_property_assign_on_object);
+  }
+
   // Check for more than one of { assign, copy, retain }.
   if (Attributes & ObjCDeclSpec::DQ_PR_assign) {
 if (Attributes & ObjCDeclSpec::DQ_PR_copy) {
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -1014,6 +1014,9 @@
   "property attributes '%0' and '%1' are mutually exclusive">;
 def err_objc_property_requires_object : Error<
   "property with '%0' attribute must be of object type">;
+def warn_objc_property_assign_on_object : Warning<
+  "'assign' attribute must not be of object type, use 'unsafe_unretained' 
instead">,
+  InGroup, DefaultIgnore;
 def warn_objc_property_no_assignment_attribute : Warning<
   "no 'assign', 'retain', or 'copy' attribute is specified - "
   "'assign' is assumed">,
Index: include/clang/Basic/Diagn

[PATCH] D44536: Avoid segfault when destructor is not yet known

2018-03-15 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

In https://reviews.llvm.org/D44536#1039428, @rjmccall wrote:

> Hmm.  Sema is lazy about actually creating implicit destructor declarations, 
> but it's supposed to only do it whenever the destructor is actually used for 
> something.  I suspect that Sema just thinks that nothing is using c::~c, 
> because the only thing that does use it is d::~d, which is inline and has no 
> apparent users.  But from the stack trace, it seems that IRGen is in fact 
> emitting d::~d.  So the question is, why is IRGen emitting d::~d?  Because 
> AFAIK a non-array new-expression never requires the destructor to exist.


Oh, because it's used in d's v-table, of course, which is needed by the 
implicit constructor for d.


Repository:
  rC Clang

https://reviews.llvm.org/D44536



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


[PATCH] D44536: Avoid segfault when destructor is not yet known

2018-03-15 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

Hmm.  Sema is lazy about actually creating implicit destructor declarations, 
but it's supposed to only do it whenever the destructor is actually used for 
something.  I suspect that Sema just thinks that nothing is using c::~c, 
because the only thing that does use it is d::~d, which is inline and has no 
apparent users.  But from the stack trace, it seems that IRGen is in fact 
emitting d::~d.  So the question is, why is IRGen emitting d::~d?  Because 
AFAIK a non-array new-expression never requires the destructor to exist.


Repository:
  rC Clang

https://reviews.llvm.org/D44536



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


[PATCH] D44536: Avoid segfault when destructor is not yet known

2018-03-15 Thread Dimitry Andric via Phabricator via cfe-commits
dim added a comment.

In https://reviews.llvm.org/D44536#1039420, @rjmccall wrote:

> I'm not sure it's supposed to be a valid state to get into IRGen with a 
> non-trivial destructor that isn't yet declared.  Richard?


As a side note, clang also emits a warning about it (but then crashes :) ):

  VMMapsMain-minimized.cpp:10:18: warning: deleting pointer to incomplete type 
'c' may cause undefined behavior
virtual ~d() { delete e; }
   ^  ~
  VMMapsMain-minimized.cpp:7:7: note: forward declaration of 'c'
  class c;
^
  Segmentation fault


Repository:
  rC Clang

https://reviews.llvm.org/D44536



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


[PATCH] D44536: Avoid segfault when destructor is not yet known

2018-03-15 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

I'm not sure it's supposed to be a valid state to get into IRGen with a 
non-trivial destructor that isn't yet declared.  Richard?


Repository:
  rC Clang

https://reviews.llvm.org/D44536



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


[PATCH] D43184: [WIP] [ItaniunCXXABI] Add an option for loading the type info vtable with dllimport

2018-03-15 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment.

In https://reviews.llvm.org/D43184#1039354, @mstorsjo wrote:

> Ok - I'll put it on the backburner, and maybe see if I'd try implementing the 
> linker part of fixing unannotated data imports at some point.


I'm not sure that's feasible. At least for x86, global addresses can be folded 
in ways that mean the linker cannot relax them. You can go the other way, 
though. If we start by assuming all data is imported, you can often relax an 
__imp_sym load to a LEA sym(%rip). This increases register pressure and 
generates worse code, but it has conceivable use cases.


Repository:
  rC Clang

https://reviews.llvm.org/D43184



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


[PATCH] D44536: Avoid segfault when destructor is not yet known

2018-03-15 Thread Dimitry Andric via Phabricator via cfe-commits
dim created this revision.
dim added reviewers: rjmccall, rsmith, majnemer, efriedma.

In some cases, a class type can have a definition, but its destructor
may not yet be known.  In that case, a segfault can occur in
`EmitObjectDelete`.

Avoid it by checking the return value of `CXXRecordDecl::getDestructor`,
and add a minimized test case.

Fixes PR36749 .


Repository:
  rC Clang

https://reviews.llvm.org/D44536

Files:
  lib/CodeGen/CGExprCXX.cpp
  test/CodeGenCXX/pr36749.cpp


Index: test/CodeGenCXX/pr36749.cpp
===
--- /dev/null
+++ test/CodeGenCXX/pr36749.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -cc1 -triple x86_64-- -emit-llvm-only %s
+class a {
+protected:
+  ~a();
+};
+struct b : a {};
+struct c;
+struct d {
+  c *e;
+  virtual ~d() { delete e; } // expected-warning {{deleting pointer to 
incomplete type 'c' may cause undefined behavior}}
+};
+struct c {
+  b f;
+};
+void g() { new d; }
Index: lib/CodeGen/CGExprCXX.cpp
===
--- lib/CodeGen/CGExprCXX.cpp
+++ lib/CodeGen/CGExprCXX.cpp
@@ -1862,7 +1862,7 @@
 if (RD->hasDefinition() && !RD->hasTrivialDestructor()) {
   Dtor = RD->getDestructor();
 
-  if (Dtor->isVirtual()) {
+  if (Dtor && Dtor->isVirtual()) {
 CGF.CGM.getCXXABI().emitVirtualObjectDelete(CGF, DE, Ptr, ElementType,
 Dtor);
 return;


Index: test/CodeGenCXX/pr36749.cpp
===
--- /dev/null
+++ test/CodeGenCXX/pr36749.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -cc1 -triple x86_64-- -emit-llvm-only %s
+class a {
+protected:
+  ~a();
+};
+struct b : a {};
+struct c;
+struct d {
+  c *e;
+  virtual ~d() { delete e; } // expected-warning {{deleting pointer to incomplete type 'c' may cause undefined behavior}}
+};
+struct c {
+  b f;
+};
+void g() { new d; }
Index: lib/CodeGen/CGExprCXX.cpp
===
--- lib/CodeGen/CGExprCXX.cpp
+++ lib/CodeGen/CGExprCXX.cpp
@@ -1862,7 +1862,7 @@
 if (RD->hasDefinition() && !RD->hasTrivialDestructor()) {
   Dtor = RD->getDestructor();
 
-  if (Dtor->isVirtual()) {
+  if (Dtor && Dtor->isVirtual()) {
 CGF.CGM.getCXXABI().emitVirtualObjectDelete(CGF, DE, Ptr, ElementType,
 Dtor);
 return;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D43184: [WIP] [ItaniunCXXABI] Add an option for loading the type info vtable with dllimport

2018-03-15 Thread Martin Storsjö via Phabricator via cfe-commits
mstorsjo added a comment.

In https://reviews.llvm.org/D43184#1039278, @rnk wrote:

> In https://reviews.llvm.org/D43184#1038396, @mstorsjo wrote:
>
> > (Accidentally pressed submit too soon)
> >
> > This was my conclusion at some point as well - but there's also a few 
> > issues with that approach which makes me quite hesitant:
> >
> > - doing fixups in the code segment requires making it writable temporarily 
> > at runtime, which is disallowed in UWP
> > - the current set of runtime relocations only allows adding addresses in 1, 
> > 2, 4 and 8 byte sized addresses. On arm/arm64, absolute addresses can be 
> > present in instructions with a much more complicated opcode format, and 
> > tweaking addresses there requires defining more runtime relocation formats.
>
>
> I wouldn't consider relocating non-dllimport references to dllimport symbols 
> as being in scope. As long the symbol is annotated as dllimport, the compiler 
> will generate code that does a load from the __imp_ pointer in the IAT.
>
> For unannotated symbols, the linker already generates thunks for function 
> references, so that just leaves data imports.


Yes - it's only unannotated data imports that is the issue.

> Still, the RTTI will be in the .rdata section, so it will require 
> unprotecting readonly memory, which won't work in UWP. We could move it to 
> .data, I guess.

True, that would avoid the really bad cases - and since a reference to that 
isn't embedded in instructions in the text segments, the existing plain pointer 
fixups should suffice.

>> So given all this - I only see bad options. I could just postpone this as 
>> well - I can live with only linking libc++ statically for now.
> 
> Might be the thing to do.

Ok - I'll put it on the backburner, and maybe see if I'd try implementing the 
linker part of fixing unannotated data imports at some point.

>> The other case you mentioned, with multiple inheritance for statically 
>> allocated objects, needs to be handled in one way or another though (unless 
>> one forbids C++ interfaces over DLL boundaries); how's that handled with the 
>> MSVC C++ ABI?
> 
> I think local vftables handle it.

Ok, thanks.


Repository:
  rC Clang

https://reviews.llvm.org/D43184



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


[PATCH] D44534: Fix codegen for structured binding binding in conditions

2018-03-15 Thread Nicolas Lesser via Phabricator via cfe-commits
Rakete created this revision.
Rakete added reviewers: lichray, rsmith.
Rakete added a project: clang.

The codegen for conditions assumes that a normal variable declaration is used 
in a condition, but this is not the case when a structured binding is used.

This fixes PR36747 .


Repository:
  rC Clang

https://reviews.llvm.org/D44534

Files:
  lib/CodeGen/CGStmt.cpp
  test/Parser/decomposed-condition.cpp

Index: test/Parser/decomposed-condition.cpp
===
--- test/Parser/decomposed-condition.cpp
+++ test/Parser/decomposed-condition.cpp
@@ -1,5 +1,20 @@
 // RUN: %clang_cc1 -std=c++1z %s -verify
 
+namespace std {
+  template struct tuple_size;
+  template struct tuple_element;
+}
+
+struct Get {
+  template int get() { return 0; }
+  operator bool() { return true; }
+};
+
+namespace std {
+  template<> struct tuple_size { static constexpr int value = 1; };
+  template<> struct tuple_element<0, Get> { using type = int; };
+}
+
 struct Na {
   bool flag;
   float data;
@@ -17,29 +32,35 @@
 Na g();
 
 namespace CondInIf {
-void h() {
+int h() {
   if (auto [ok, d] = f()) // expected-warning {{ISO C++17 does not permit structured binding declaration in a condition}}
 ;
   if (auto [ok, d] = g()) // expected-warning {{ISO C++17 does not permit structured binding declaration in a condition}} expected-error {{value of type 'Na' is not contextually convertible to 'bool'}}
 ;
+  if (auto [value] = Get()) // expected-warning {{ISO C++17 does not permit structured binding declaration in a condition}}
+return value;
 }
 } // namespace CondInIf
 
 namespace CondInWhile {
-void h() {
+int h() {
   while (auto [ok, d] = f()) // expected-warning {{ISO C++17 does not permit structured binding declaration in a condition}}
 ;
   while (auto [ok, d] = g()) // expected-warning {{ISO C++17 does not permit structured binding declaration in a condition}} expected-error {{value of type 'Na' is not contextually convertible to 'bool'}}
 ;
+  while (auto [value] = Get()) // expected-warning{{ISO C++17 does not permit structured binding declaration in a condition}}
+return value;
 }
 } // namespace CondInWhile
 
 namespace CondInFor {
-void h() {
+int h() {
   for (; auto [ok, d] = f();) // expected-warning {{ISO C++17 does not permit structured binding declaration in a condition}}
 ;
   for (; auto [ok, d] = g();) // expected-warning {{ISO C++17 does not permit structured binding declaration in a condition}} expected-error {{value of type 'Na' is not contextually convertible to 'bool'}}
 ;
+  for (; auto [value] = Get();) // expected-warning {{ISO C++17 does not permit structured binding declaration in a condition}}
+return value;
 }
 } // namespace CondInFor
 
@@ -52,10 +73,15 @@
 };
 
 namespace CondInSwitch {
-void h(IntegerLike x) {
+int h(IntegerLike x) {
   switch (auto [ok, d] = x) // expected-warning {{ISO C++17 does not permit structured binding declaration in a condition}}
 ;
   switch (auto [ok, d] = g()) // expected-warning {{ISO C++17 does not permit structured binding declaration in a condition}} expected-error {{statement requires expression of integer type ('Na' invalid)}}
 ;
+  switch (auto [value] = Get()) {// expected-warning {{ISO C++17 does not permit structured binding declaration in a condition}}
+  // expected-warning@-1{{switch condition has boolean value}}
+  case 1:
+return value;
+  }
 }
 } // namespace CondInSwitch
Index: lib/CodeGen/CGStmt.cpp
===
--- lib/CodeGen/CGStmt.cpp
+++ lib/CodeGen/CGStmt.cpp
@@ -608,7 +608,7 @@
 EmitStmt(S.getInit());
 
   if (S.getConditionVariable())
-EmitAutoVarDecl(*S.getConditionVariable());
+EmitDecl(*S.getConditionVariable());
 
   // If the condition constant folds and can be elided, try to avoid emitting
   // the condition and the dead arm of the if/else.
@@ -705,7 +705,7 @@
   RunCleanupsScope ConditionScope(*this);
 
   if (S.getConditionVariable())
-EmitAutoVarDecl(*S.getConditionVariable());
+EmitDecl(*S.getConditionVariable());
 
   // Evaluate the conditional in the while header.  C99 6.8.5.1: The
   // evaluation of the controlling expression takes place before each
@@ -865,7 +865,7 @@
 // If the for statement has a condition scope, emit the local variable
 // declaration.
 if (S.getConditionVariable()) {
-  EmitAutoVarDecl(*S.getConditionVariable());
+  EmitDecl(*S.getConditionVariable());
 }
 
 llvm::BasicBlock *ExitBlock = LoopExit.getBlock();
@@ -1574,7 +1574,7 @@
   // Emit the condition variable if needed inside the entire cleanup scope
   // used by this special case for constant folded switches.
   if (S.getConditionVariable())
-EmitAutoVarDecl(*S.getConditionVariable());
+EmitDecl(*S.getConditionVariable());
 
   // At this point, we are no longer "within" a switch in

[PATCH] D44533: [AMDGPU] Fix codegen for inline assembly

2018-03-15 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl created this revision.
yaxunl added reviewers: arsenm, rampitec, b-sumner.
Herald added subscribers: eraman, t-tye, tpr, dstuttard, nhaehnle, wdng, 
kzhuravl.

Need to override convertConstraint to recognise amdgpu specific register names.


https://reviews.llvm.org/D44533

Files:
  lib/Basic/Targets/AMDGPU.h
  lib/CodeGen/CGStmt.cpp
  test/CodeGenOpenCL/inline-asm-amdgcn.cl
  test/Sema/inline-asm-validate-amdgpu.cl


Index: test/Sema/inline-asm-validate-amdgpu.cl
===
--- test/Sema/inline-asm-validate-amdgpu.cl
+++ test/Sema/inline-asm-validate-amdgpu.cl
@@ -74,3 +74,8 @@
 
 c[i] = ci;
 }
+
+void test_long(int arg0) {
+  long v15_16;
+  __asm volatile("v_lshlrev_b64 v[15:16], 0, %0" : "={v[15:16]}"(v15_16) : 
"v"(arg0));
+}
Index: test/CodeGenOpenCL/inline-asm-amdgcn.cl
===
--- /dev/null
+++ test/CodeGenOpenCL/inline-asm-amdgcn.cl
@@ -0,0 +1,8 @@
+// REQUIRES: amdgpu-registered-target
+// RUN: %clang_cc1 -emit-llvm -o - -triple amdgcn %s | FileCheck %s
+
+kernel void test_long(int arg0) {
+  long v15_16;
+  // CHECK: tail call i64 asm sideeffect "v_lshlrev_b64 v[15:16], 0, $0", 
"={v[15:16]},v"(i32 %arg0)
+  __asm volatile("v_lshlrev_b64 v[15:16], 0, %0" : "={v[15:16]}"(v15_16) : 
"v"(arg0));
+}
Index: lib/CodeGen/CGStmt.cpp
===
--- lib/CodeGen/CGStmt.cpp
+++ lib/CodeGen/CGStmt.cpp
@@ -1926,7 +1926,7 @@
 // Simplify the output constraint.
 std::string OutputConstraint(S.getOutputConstraint(i));
 OutputConstraint = SimplifyConstraint(OutputConstraint.c_str() + 1,
-  getTarget());
+  getTarget(), &OutputConstraintInfos);
 
 const Expr *OutExpr = S.getOutputExpr(i);
 OutExpr = OutExpr->IgnoreParenNoopCasts(getContext());
Index: lib/Basic/Targets/AMDGPU.h
===
--- lib/Basic/Targets/AMDGPU.h
+++ lib/Basic/Targets/AMDGPU.h
@@ -285,6 +285,19 @@
 return true;
   }
 
+  // Constraint parm will be left pointing at the last character of
+  // the constraint.  In practice, it won't be changed unless the
+  // constraint is longer than one character.
+  std::string convertConstraint(const char *&Constraint) const override {
+const char *Begin = Constraint;
+TargetInfo::ConstraintInfo Info("", "");
+if (validateAsmConstraint(Constraint, Info))
+  return std::string(Begin).substr(0, Constraint - Begin + 1);
+
+Constraint = Begin;
+return std::string(1, *Constraint);
+  }
+
   bool
   initFeatureMap(llvm::StringMap &Features, DiagnosticsEngine &Diags,
  StringRef CPU,


Index: test/Sema/inline-asm-validate-amdgpu.cl
===
--- test/Sema/inline-asm-validate-amdgpu.cl
+++ test/Sema/inline-asm-validate-amdgpu.cl
@@ -74,3 +74,8 @@
 
 c[i] = ci;
 }
+
+void test_long(int arg0) {
+  long v15_16;
+  __asm volatile("v_lshlrev_b64 v[15:16], 0, %0" : "={v[15:16]}"(v15_16) : "v"(arg0));
+}
Index: test/CodeGenOpenCL/inline-asm-amdgcn.cl
===
--- /dev/null
+++ test/CodeGenOpenCL/inline-asm-amdgcn.cl
@@ -0,0 +1,8 @@
+// REQUIRES: amdgpu-registered-target
+// RUN: %clang_cc1 -emit-llvm -o - -triple amdgcn %s | FileCheck %s
+
+kernel void test_long(int arg0) {
+  long v15_16;
+  // CHECK: tail call i64 asm sideeffect "v_lshlrev_b64 v[15:16], 0, $0", "={v[15:16]},v"(i32 %arg0)
+  __asm volatile("v_lshlrev_b64 v[15:16], 0, %0" : "={v[15:16]}"(v15_16) : "v"(arg0));
+}
Index: lib/CodeGen/CGStmt.cpp
===
--- lib/CodeGen/CGStmt.cpp
+++ lib/CodeGen/CGStmt.cpp
@@ -1926,7 +1926,7 @@
 // Simplify the output constraint.
 std::string OutputConstraint(S.getOutputConstraint(i));
 OutputConstraint = SimplifyConstraint(OutputConstraint.c_str() + 1,
-  getTarget());
+  getTarget(), &OutputConstraintInfos);
 
 const Expr *OutExpr = S.getOutputExpr(i);
 OutExpr = OutExpr->IgnoreParenNoopCasts(getContext());
Index: lib/Basic/Targets/AMDGPU.h
===
--- lib/Basic/Targets/AMDGPU.h
+++ lib/Basic/Targets/AMDGPU.h
@@ -285,6 +285,19 @@
 return true;
   }
 
+  // Constraint parm will be left pointing at the last character of
+  // the constraint.  In practice, it won't be changed unless the
+  // constraint is longer than one character.
+  std::string convertConstraint(const char *&Constraint) const override {
+const char *Begin = Constraint;
+TargetInfo::ConstraintInfo Info("", "");
+if (validateAsmConstraint(Constraint, Info))
+  return std::string(Begin).substr(0, Constraint - Begin + 1);
+
+   

[PATCH] D44295: [clang-tidy] Detects and fixes calls to grand-...parent virtual methods instead of calls to parent's virtual methods

2018-03-15 Thread Zinovy Nis via Phabricator via cfe-commits
zinovy.nis updated this revision to Diff 138601.

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44295

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/ParentVirtualCallCheck.cpp
  clang-tidy/bugprone/ParentVirtualCallCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-parent-virtual-call.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-parent-virtual-call.cpp

Index: test/clang-tidy/bugprone-parent-virtual-call.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-parent-virtual-call.cpp
@@ -0,0 +1,139 @@
+// RUN: %check_clang_tidy %s bugprone-parent-virtual-call %t
+
+extern int foo();
+
+class A {
+public:
+  A() = default;
+  virtual ~A() = default;
+
+  virtual int virt_1() { return foo() + 1; }
+  virtual int virt_2() { return foo() + 2; }
+
+  int non_virt() { return foo() + 3; }
+  static int stat() { return foo() + 4; }
+};
+
+class B : public A {
+public:
+  B() = default;
+
+  // Nothing to fix: calls to parent.
+  int virt_1() override { return A::virt_1() + 3; }
+  int virt_2() override { return A::virt_2() + 4; }
+};
+
+class C : public B {
+public:
+  int virt_1() override { return A::virt_1() + B::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified function name A::virt_1 refers to a function not from a direct base class; did you mean 'B'? [bugprone-parent-virtual-call]
+  // CHECK-FIXES:  int virt_1() override { return B::virt_1() + B::virt_1(); }
+  int virt_2() override { return A::virt_1() + B::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified function name A::virt_1 refers to a function not from a direct base class; did you mean 'B'? [bugprone-parent-virtual-call]
+  // CHECK-FIXES:  int virt_2() override { return B::virt_1() + B::virt_1(); }
+
+  // Test that non-virtual and static methods are not affected by this cherker.
+  int method_c() { return A::stat() + A::non_virt(); }
+};
+
+// Test that the check affects grand-grand..-parent calls too.
+class D : public C {
+public:
+  int virt_1() override { return A::virt_1() + B::virt_1() + D::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified function name A::virt_1 refers to a function not from a direct base class; did you mean 'C'? [bugprone-parent-virtual-call]
+  // CHECK-MESSAGES: :[[@LINE-2]]:48: warning: qualified function name B::virt_1 refers to a function not from a direct base class; did you mean 'C'? [bugprone-parent-virtual-call]
+  // CHECK-FIXES:  int virt_1() override { return C::virt_1() + C::virt_1() + D::virt_1(); }
+  int virt_2() override { return A::virt_1() + B::virt_1() + D::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified function name A::virt_1 refers to a function not from a direct base class; did you mean 'C'? [bugprone-parent-virtual-call]
+  // CHECK-MESSAGES: :[[@LINE-2]]:48: warning: qualified function name B::virt_1 refers to a function not from a direct base class; did you mean 'C'? [bugprone-parent-virtual-call]
+  // CHECK-FIXES:  int virt_2() override { return C::virt_1() + C::virt_1() + D::virt_1(); }
+};
+
+// Test classes in anonymous namespaces.
+namespace {
+class BN : public A {
+public:
+  int virt_1() override { return A::virt_1() + 3; }
+  int virt_2() override { return A::virt_2() + 4; }
+};
+} // namespace N
+
+class CN : public BN {
+public:
+  int virt_1() override { return A::virt_1() + BN::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified function name A::virt_1 refers to a function not from a direct base class; did you mean 'BN'? [bugprone-parent-virtual-call]
+  // CHECK-FIXES:  int virt_1() override { return BN::virt_1() + BN::virt_1(); }
+  int virt_2() override { return A::virt_1() + BN::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified function name A::virt_1 refers to a function not from a direct base class; did you mean 'BN'? [bugprone-parent-virtual-call]
+  // CHECK-FIXES:  int virt_2() override { return BN::virt_1() + BN::virt_1(); }
+};
+
+// Test multiple inheritance fixes
+class AA {
+public:
+  AA() = default;
+  virtual ~AA() = default;
+
+  virtual int virt_1() { return foo() + 1; }
+  virtual int virt_2() { return foo() + 2; }
+
+  int non_virt() { return foo() + 3; }
+  static int stat() { return foo() + 4; }
+};
+
+class BB_1 : virtual public AA {
+public:
+  BB_1() = default;
+
+  // Nothing to fix: calls to parent.
+  int virt_1() override { return AA::virt_1() + 3; }
+  int virt_2() override { return AA::virt_2() + 4; }
+};
+
+class BB_2 : virtual public AA {
+public:
+BB_2() = default;
+};
+
+class CC : public BB_1, public BB_2 {
+public:
+  int virt_1() override { return AA::virt_1() + 3; }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified function name AA::virt_1 refers to a function not from a direct base class; did you mean 'BB_1' or 'BB_2'? [bugprone-parent-virt

[PATCH] D44295: [clang-tidy] Detects and fixes calls to grand-...parent virtual methods instead of calls to parent's virtual methods

2018-03-15 Thread Zinovy Nis via Phabricator via cfe-commits
zinovy.nis added inline comments.



Comment at: test/clang-tidy/bugprone-parent-virtual-call.cpp:127
+// Just to instantiate DF.
+int bar() { return (new DF())->virt_1(); }

aaron.ballman wrote:
> What should happen in this case?
> ```
> struct Base {
>   virtual void f() {}
> };
> 
> struct Intermediary : Base {
> };
> 
> struct Derived : Intermediary {
>   void f() override {
> Base::f(); // I would expect this not to diagnose
>   }
> };
> ```
> This would be a reasonable test case to add.
Please see `// Test virtual method is diagnosted although not overridden in 
parent.` section in the test file.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44295



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


[PATCH] D44295: [clang-tidy] Detects and fixes calls to grand-...parent virtual methods instead of calls to parent's virtual methods

2018-03-15 Thread Zinovy Nis via Phabricator via cfe-commits
zinovy.nis updated this revision to Diff 138599.

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44295

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/ParentVirtualCallCheck.cpp
  clang-tidy/bugprone/ParentVirtualCallCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-parent-virtual-call.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-parent-virtual-call.cpp

Index: test/clang-tidy/bugprone-parent-virtual-call.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-parent-virtual-call.cpp
@@ -0,0 +1,139 @@
+// RUN: %check_clang_tidy %s bugprone-parent-virtual-call %t
+
+extern int foo();
+
+class A {
+public:
+  A() = default;
+  virtual ~A() = default;
+
+  virtual int virt_1() { return foo() + 1; }
+  virtual int virt_2() { return foo() + 2; }
+
+  int non_virt() { return foo() + 3; }
+  static int stat() { return foo() + 4; }
+};
+
+class B : public A {
+public:
+  B() = default;
+
+  // Nothing to fix: calls to parent.
+  int virt_1() override { return A::virt_1() + 3; }
+  int virt_2() override { return A::virt_2() + 4; }
+};
+
+class C : public B {
+public:
+  int virt_1() override { return A::virt_1() + B::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified function name A::virt_1 refers to a function not from a direct base class; did you mean 'B'? [bugprone-parent-virtual-call]
+  // CHECK-FIXES:  int virt_1() override { return B::virt_1() + B::virt_1(); }
+  int virt_2() override { return A::virt_1() + B::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified function name A::virt_1 refers to a function not from a direct base class; did you mean 'B'? [bugprone-parent-virtual-call]
+  // CHECK-FIXES:  int virt_2() override { return B::virt_1() + B::virt_1(); }
+
+  // Test that non-virtual and static methods are not affected by this cherker.
+  int method_c() { return A::stat() + A::non_virt(); }
+};
+
+// Test that the check affects grand-grand..-parent calls too.
+class D : public C {
+public:
+  int virt_1() override { return A::virt_1() + B::virt_1() + D::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified function name A::virt_1 refers to a function not from a direct base class; did you mean 'C'? [bugprone-parent-virtual-call]
+  // CHECK-MESSAGES: :[[@LINE-2]]:48: warning: qualified function name B::virt_1 refers to a function not from a direct base class; did you mean 'C'? [bugprone-parent-virtual-call]
+  // CHECK-FIXES:  int virt_1() override { return C::virt_1() + C::virt_1() + D::virt_1(); }
+  int virt_2() override { return A::virt_1() + B::virt_1() + D::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified function name A::virt_1 refers to a function not from a direct base class; did you mean 'C'? [bugprone-parent-virtual-call]
+  // CHECK-MESSAGES: :[[@LINE-2]]:48: warning: qualified function name B::virt_1 refers to a function not from a direct base class; did you mean 'C'? [bugprone-parent-virtual-call]
+  // CHECK-FIXES:  int virt_2() override { return C::virt_1() + C::virt_1() + D::virt_1(); }
+};
+
+// Test classes in anonymous namespaces.
+namespace {
+class BN : public A {
+public:
+  int virt_1() override { return A::virt_1() + 3; }
+  int virt_2() override { return A::virt_2() + 4; }
+};
+} // namespace N
+
+class CN : public BN {
+public:
+  int virt_1() override { return A::virt_1() + BN::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified function name A::virt_1 refers to a function not from a direct base class; did you mean 'BN'? [bugprone-parent-virtual-call]
+  // CHECK-FIXES:  int virt_1() override { return BN::virt_1() + BN::virt_1(); }
+  int virt_2() override { return A::virt_1() + BN::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified function name A::virt_1 refers to a function not from a direct base class; did you mean 'BN'? [bugprone-parent-virtual-call]
+  // CHECK-FIXES:  int virt_2() override { return BN::virt_1() + BN::virt_1(); }
+};
+
+// Test multiple inheritance fixes
+class AA {
+public:
+  AA() = default;
+  virtual ~AA() = default;
+
+  virtual int virt_1() { return foo() + 1; }
+  virtual int virt_2() { return foo() + 2; }
+
+  int non_virt() { return foo() + 3; }
+  static int stat() { return foo() + 4; }
+};
+
+class BB_1 : virtual public AA {
+public:
+  BB_1() = default;
+
+  // Nothing to fix: calls to parent.
+  int virt_1() override { return AA::virt_1() + 3; }
+  int virt_2() override { return AA::virt_2() + 4; }
+};
+
+class BB_2 : virtual public AA {
+public:
+BB_2() = default;
+};
+
+class CC : public BB_1, public BB_2 {
+public:
+  int virt_1() override { return AA::virt_1() + 3; }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified function name AA::virt_1 refers to a function not from a direct base class; did you mean 'BB_1' or 'BB_2'? [bugprone-parent-virt

[PATCH] D44295: [clang-tidy] Detects and fixes calls to grand-...parent virtual methods instead of calls to parent's virtual methods

2018-03-15 Thread Zinovy Nis via Phabricator via cfe-commits
zinovy.nis updated this revision to Diff 138598.

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44295

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/ParentVirtualCallCheck.cpp
  clang-tidy/bugprone/ParentVirtualCallCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-parent-virtual-call.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-parent-virtual-call.cpp

Index: test/clang-tidy/bugprone-parent-virtual-call.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-parent-virtual-call.cpp
@@ -0,0 +1,139 @@
+// RUN: %check_clang_tidy %s bugprone-parent-virtual-call %t
+
+extern int foo();
+
+class A {
+public:
+  A() = default;
+  virtual ~A() = default;
+
+  virtual int virt_1() { return foo() + 1; }
+  virtual int virt_2() { return foo() + 2; }
+
+  int non_virt() { return foo() + 3; }
+  static int stat() { return foo() + 4; }
+};
+
+class B : public A {
+public:
+  B() = default;
+
+  // Nothing to fix: calls to parent.
+  int virt_1() override { return A::virt_1() + 3; }
+  int virt_2() override { return A::virt_2() + 4; }
+};
+
+class C : public B {
+public:
+  int virt_1() override { return A::virt_1() + B::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified function name A::virt_1 refers to a function not from a direct base class; did you mean 'B'? [bugprone-parent-virtual-call]
+  // CHECK-FIXES:  int virt_1() override { return B::virt_1() + B::virt_1(); }
+  int virt_2() override { return A::virt_1() + B::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified function name A::virt_1 refers to a function not from a direct base class; did you mean 'B'? [bugprone-parent-virtual-call]
+  // CHECK-FIXES:  int virt_2() override { return B::virt_1() + B::virt_1(); }
+
+  // Test that non-virtual and static methods are not affected by this cherker.
+  int method_c() { return A::stat() + A::non_virt(); }
+};
+
+// Test that the check affects grand-grand..-parent calls too.
+class D : public C {
+public:
+  int virt_1() override { return A::virt_1() + B::virt_1() + D::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified function name A::virt_1 refers to a function not from a direct base class; did you mean 'C'? [bugprone-parent-virtual-call]
+  // CHECK-MESSAGES: :[[@LINE-2]]:48: warning: qualified function name B::virt_1 refers to a function not from a direct base class; did you mean 'C'? [bugprone-parent-virtual-call]
+  // CHECK-FIXES:  int virt_1() override { return C::virt_1() + C::virt_1() + D::virt_1(); }
+  int virt_2() override { return A::virt_1() + B::virt_1() + D::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified function name A::virt_1 refers to a function not from a direct base class; did you mean 'C'? [bugprone-parent-virtual-call]
+  // CHECK-MESSAGES: :[[@LINE-2]]:48: warning: qualified function name B::virt_1 refers to a function not from a direct base class; did you mean 'C'? [bugprone-parent-virtual-call]
+  // CHECK-FIXES:  int virt_2() override { return C::virt_1() + C::virt_1() + D::virt_1(); }
+};
+
+// Test classes in anonymous namespaces.
+namespace {
+class BN : public A {
+public:
+  int virt_1() override { return A::virt_1() + 3; }
+  int virt_2() override { return A::virt_2() + 4; }
+};
+} // namespace N
+
+class CN : public BN {
+public:
+  int virt_1() override { return A::virt_1() + BN::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified function name A::virt_1 refers to a function not from a direct base class; did you mean 'BN'? [bugprone-parent-virtual-call]
+  // CHECK-FIXES:  int virt_1() override { return BN::virt_1() + BN::virt_1(); }
+  int virt_2() override { return A::virt_1() + BN::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified function name A::virt_1 refers to a function not from a direct base class; did you mean 'BN'? [bugprone-parent-virtual-call]
+  // CHECK-FIXES:  int virt_2() override { return BN::virt_1() + BN::virt_1(); }
+};
+
+// Test multiple inheritance fixes
+class AA {
+public:
+  AA() = default;
+  virtual ~AA() = default;
+
+  virtual int virt_1() { return foo() + 1; }
+  virtual int virt_2() { return foo() + 2; }
+
+  int non_virt() { return foo() + 3; }
+  static int stat() { return foo() + 4; }
+};
+
+class BB_1 : virtual public AA {
+public:
+  BB_1() = default;
+
+  // Nothing to fix: calls to parent.
+  int virt_1() override { return AA::virt_1() + 3; }
+  int virt_2() override { return AA::virt_2() + 4; }
+};
+
+class BB_2 : virtual public AA {
+public:
+BB_2() = default;
+};
+
+class CC : public BB_1, public BB_2 {
+public:
+  int virt_1() override { return AA::virt_1() + 3; }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified function name AA::virt_1 refers to a function not from a direct base class; did you mean 'BB_1' or 'BB_2'? [bugprone-parent-virt

[PATCH] D44295: [clang-tidy] Detects and fixes calls to grand-...parent virtual methods instead of calls to parent's virtual methods

2018-03-15 Thread Zinovy Nis via Phabricator via cfe-commits
zinovy.nis added inline comments.



Comment at: clang-tidy/bugprone/ParentVirtualCallCheck.cpp:128
+  diag(Member->getQualifierLoc().getSourceRange().getBegin(),
+   "'%0' is a grand-parent's method, not parent's. Did you mean %1?")
+  << CalleeName << ParentsStr;

aaron.ballman wrote:
> The diagnostic should not contain punctuation aside from the question mark.
> 
> Also, the diagnostic uses some novel terminology in "grandparent's method". 
> How about: "qualified function name %0 refers to a function that is not 
> defined by a direct base class; did you mean %1?"
"is not defined by a direct base class" is not a correct proposal. Issues that 
this check finds are all about calling a grand-parent's method instead of 
parent's whether base class defines this method or not.

How about 

> Qualified function name %0 refers to a function not from a direct base class; 
> did you mean %1?

?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44295



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


[PATCH] D44295: [clang-tidy] Detects and fixes calls to grand-...parent virtual methods instead of calls to parent's virtual methods

2018-03-15 Thread Zinovy Nis via Phabricator via cfe-commits
zinovy.nis updated this revision to Diff 138597.

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44295

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/ParentVirtualCallCheck.cpp
  clang-tidy/bugprone/ParentVirtualCallCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-parent-virtual-call.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-parent-virtual-call.cpp

Index: test/clang-tidy/bugprone-parent-virtual-call.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-parent-virtual-call.cpp
@@ -0,0 +1,139 @@
+// RUN: %check_clang_tidy %s bugprone-parent-virtual-call %t
+
+extern int foo();
+
+class A {
+public:
+  A() = default;
+  virtual ~A() = default;
+
+  virtual int virt_1() { return foo() + 1; }
+  virtual int virt_2() { return foo() + 2; }
+
+  int non_virt() { return foo() + 3; }
+  static int stat() { return foo() + 4; }
+};
+
+class B : public A {
+public:
+  B() = default;
+
+  // Nothing to fix: calls to parent.
+  int virt_1() override { return A::virt_1() + 3; }
+  int virt_2() override { return A::virt_2() + 4; }
+};
+
+class C : public B {
+public:
+  int virt_1() override { return A::virt_1() + B::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: Qualified function name A::virt_1 refers to a function not from a direct base class; did you mean 'B'? [bugprone-parent-virtual-call]
+  // CHECK-FIXES:  int virt_1() override { return B::virt_1() + B::virt_1(); }
+  int virt_2() override { return A::virt_1() + B::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: Qualified function name A::virt_1 refers to a function not from a direct base class; did you mean 'B'? [bugprone-parent-virtual-call]
+  // CHECK-FIXES:  int virt_2() override { return B::virt_1() + B::virt_1(); }
+
+  // Test that non-virtual and static methods are not affected by this cherker.
+  int method_c() { return A::stat() + A::non_virt(); }
+};
+
+// Test that the check affects grand-grand..-parent calls too.
+class D : public C {
+public:
+  int virt_1() override { return A::virt_1() + B::virt_1() + D::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: Qualified function name A::virt_1 refers to a function not from a direct base class; did you mean 'C'? [bugprone-parent-virtual-call]
+  // CHECK-MESSAGES: :[[@LINE-2]]:48: warning: Qualified function name B::virt_1 refers to a function not from a direct base class; did you mean 'C'? [bugprone-parent-virtual-call]
+  // CHECK-FIXES:  int virt_1() override { return C::virt_1() + C::virt_1() + D::virt_1(); }
+  int virt_2() override { return A::virt_1() + B::virt_1() + D::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: Qualified function name A::virt_1 refers to a function not from a direct base class; did you mean 'C'? [bugprone-parent-virtual-call]
+  // CHECK-MESSAGES: :[[@LINE-2]]:48: warning: Qualified function name B::virt_1 refers to a function not from a direct base class; did you mean 'C'? [bugprone-parent-virtual-call]
+  // CHECK-FIXES:  int virt_2() override { return C::virt_1() + C::virt_1() + D::virt_1(); }
+};
+
+// Test classes in anonymous namespaces.
+namespace {
+class BN : public A {
+public:
+  int virt_1() override { return A::virt_1() + 3; }
+  int virt_2() override { return A::virt_2() + 4; }
+};
+} // namespace N
+
+class CN : public BN {
+public:
+  int virt_1() override { return A::virt_1() + BN::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: Qualified function name A::virt_1 refers to a function not from a direct base class; did you mean 'BN'? [bugprone-parent-virtual-call]
+  // CHECK-FIXES:  int virt_1() override { return BN::virt_1() + BN::virt_1(); }
+  int virt_2() override { return A::virt_1() + BN::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: Qualified function name A::virt_1 refers to a function not from a direct base class; did you mean 'BN'? [bugprone-parent-virtual-call]
+  // CHECK-FIXES:  int virt_2() override { return BN::virt_1() + BN::virt_1(); }
+};
+
+// Test multiple inheritance fixes
+class AA {
+public:
+  AA() = default;
+  virtual ~AA() = default;
+
+  virtual int virt_1() { return foo() + 1; }
+  virtual int virt_2() { return foo() + 2; }
+
+  int non_virt() { return foo() + 3; }
+  static int stat() { return foo() + 4; }
+};
+
+class BB_1 : virtual public AA {
+public:
+  BB_1() = default;
+
+  // Nothing to fix: calls to parent.
+  int virt_1() override { return AA::virt_1() + 3; }
+  int virt_2() override { return AA::virt_2() + 4; }
+};
+
+class BB_2 : virtual public AA {
+public:
+BB_2() = default;
+};
+
+class CC : public BB_1, public BB_2 {
+public:
+  int virt_1() override { return AA::virt_1() + 3; }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: Qualified function name AA::virt_1 refers to a function not from a direct base class; did you mean 'BB_1' or 'BB_2'? [bugprone-parent-virt

[PATCH] D44295: [clang-tidy] Detects and fixes calls to grand-...parent virtual methods instead of calls to parent's virtual methods

2018-03-15 Thread Zinovy Nis via Phabricator via cfe-commits
zinovy.nis updated this revision to Diff 138596.
zinovy.nis marked 10 inline comments as done.

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44295

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/ParentVirtualCallCheck.cpp
  clang-tidy/bugprone/ParentVirtualCallCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-parent-virtual-call.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-parent-virtual-call.cpp

Index: test/clang-tidy/bugprone-parent-virtual-call.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-parent-virtual-call.cpp
@@ -0,0 +1,127 @@
+// RUN: %check_clang_tidy %s bugprone-parent-virtual-call %t
+
+extern int foo();
+
+class A {
+public:
+  A() = default;
+  virtual ~A() = default;
+
+  virtual int virt_1() { return foo() + 1; }
+  virtual int virt_2() { return foo() + 2; }
+
+  int non_virt() { return foo() + 3; }
+  static int stat() { return foo() + 4; }
+};
+
+class B : public A {
+public:
+  B() = default;
+
+  // Nothing to fix: calls to parent.
+  int virt_1() override { return A::virt_1() + 3; }
+  int virt_2() override { return A::virt_2() + 4; }
+};
+
+class C : public B {
+public:
+  int virt_1() override { return A::virt_1() + B::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: 'A::virt_1' is a grand-parent's method, not parent's. Did you mean 'B'?
+  // CHECK-FIXES:  int virt_1() override { return B::virt_1() + B::virt_1(); }
+  int virt_2() override { return A::virt_1() + B::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: 'A::virt_1' is a grand-parent's method, not parent's. Did you mean 'B'?
+  // CHECK-FIXES:  int virt_2() override { return B::virt_1() + B::virt_1(); }
+
+  // Test that non-virtual and static methods are not affected by this cherker.
+  int method_c() { return A::stat() + A::non_virt(); }
+};
+
+// Test that the check affects grand-grand..-parent calls too.
+class D : public C {
+public:
+  int virt_1() override { return A::virt_1() + B::virt_1() + D::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: 'A::virt_1' is a grand-parent's method, not parent's. Did you mean 'C'?
+  // CHECK-MESSAGES: :[[@LINE-2]]:48: warning: 'B::virt_1' is a grand-parent's method, not parent's. Did you mean 'C'?
+  // CHECK-FIXES:  int virt_1() override { return C::virt_1() + C::virt_1() + D::virt_1(); }
+  int virt_2() override { return A::virt_1() + B::virt_1() + D::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: 'A::virt_1' is a grand-parent's method, not parent's. Did you mean 'C'?
+  // CHECK-MESSAGES: :[[@LINE-2]]:48: warning: 'B::virt_1' is a grand-parent's method, not parent's. Did you mean 'C'?
+  // CHECK-FIXES:  int virt_2() override { return C::virt_1() + C::virt_1() + D::virt_1(); }
+};
+
+// Test classes in anonymous namespaces.
+namespace {
+class BN : public A {
+public:
+  int virt_1() override { return A::virt_1() + 3; }
+  int virt_2() override { return A::virt_2() + 4; }
+};
+} // namespace N
+
+class CN : public BN {
+public:
+  int virt_1() override { return A::virt_1() + BN::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: 'A::virt_1' is a grand-parent's method, not parent's. Did you mean 'BN'?
+  // CHECK-FIXES:  int virt_1() override { return BN::virt_1() + BN::virt_1(); }
+  int virt_2() override { return A::virt_1() + BN::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: 'A::virt_1' is a grand-parent's method, not parent's. Did you mean 'BN'?
+  // CHECK-FIXES:  int virt_2() override { return BN::virt_1() + BN::virt_1(); }
+};
+
+// Test multiple inheritance fixes
+class AA {
+public:
+  AA() = default;
+  virtual ~AA() = default;
+
+  virtual int virt_1() { return foo() + 1; }
+  virtual int virt_2() { return foo() + 2; }
+
+  int non_virt() { return foo() + 3; }
+  static int stat() { return foo() + 4; }
+};
+
+class BB_1 : virtual public AA {
+public:
+  BB_1() = default;
+
+  // Nothing to fix: calls to parent.
+  int virt_1() override { return AA::virt_1() + 3; }
+  int virt_2() override { return AA::virt_2() + 4; }
+};
+
+class BB_2 : virtual public AA {
+public:
+BB_2() = default;
+};
+
+class CC : public BB_1, public BB_2 {
+public:
+  int virt_1() override { return AA::virt_1() + 3; }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: 'AA::virt_1' is a grand-parent's method, not parent's. Did you mean 'BB_1' or 'BB_2'? [bugprone-parent-virtual-call]
+  // No fix available due to multiple choice of parent class.
+};
+
+// Test templated classes.
+template  class BF : public A {
+public:
+  int virt_1() override { return A::virt_1() + 3; }
+};
+
+// Test templated parent class.
+class CF : public BF {
+public:
+  int virt_1() override { return A::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: 'A::virt_1' is a grand-parent's method, not parent's. Did you mean 'BF'?
+};
+
+// Test both templated class a

[PATCH] D41655: [clang-tidy] New check bugprone-unused-return-value

2018-03-15 Thread Kalle Huttunen via Phabricator via cfe-commits
khuttun updated this revision to Diff 138595.
khuttun added a comment.

Rebased


https://reviews.llvm.org/D41655

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/UnusedReturnValueCheck.cpp
  clang-tidy/bugprone/UnusedReturnValueCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-unused-return-value.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-unused-return-value-custom.cpp
  test/clang-tidy/bugprone-unused-return-value.cpp

Index: test/clang-tidy/bugprone-unused-return-value.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-unused-return-value.cpp
@@ -0,0 +1,166 @@
+// RUN: %check_clang_tidy %s bugprone-unused-return-value %t
+
+namespace std {
+
+struct future {};
+
+enum class launch {
+  async,
+  deferred
+};
+
+template 
+future async(Function &&, Args &&...);
+
+template 
+future async(launch, Function &&, Args &&...);
+
+template 
+ForwardIt remove(ForwardIt, ForwardIt, const T &);
+
+template 
+ForwardIt remove_if(ForwardIt, ForwardIt, UnaryPredicate);
+
+template 
+ForwardIt unique(ForwardIt, ForwardIt);
+
+// the check should be able to match std lib calls even if the functions are
+// declared inside inline namespaces
+inline namespace v1 {
+
+template 
+T *launder(T *);
+
+} // namespace v1
+} // namespace std
+
+struct Foo {
+  void f();
+};
+
+int increment(int i) {
+  return i + 1;
+}
+
+void useFuture(const std::future &fut);
+
+void warning() {
+  std::async(increment, 42);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should be used [bugprone-unused-return-value]
+
+  std::async(std::launch::async, increment, 42);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should be used [bugprone-unused-return-value]
+
+  Foo F;
+  std::launder(&F);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should be used [bugprone-unused-return-value]
+
+  std::remove(nullptr, nullptr, 1);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should be used [bugprone-unused-return-value]
+
+  std::remove_if(nullptr, nullptr, nullptr);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should be used [bugprone-unused-return-value]
+
+  std::unique(nullptr, nullptr);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should be used [bugprone-unused-return-value]
+
+  // test discarding return values inside different kinds of statements
+
+  auto Lambda = [] { std::remove(nullptr, nullptr, 1); };
+  // CHECK-MESSAGES: [[@LINE-1]]:22: warning: the value returned by this function should be used [bugprone-unused-return-value]
+
+  if (true)
+std::remove(nullptr, nullptr, 1);
+  // CHECK-MESSAGES: [[@LINE-1]]:5: warning: the value returned by this function should be used [bugprone-unused-return-value]
+  else if (true)
+std::remove(nullptr, nullptr, 1);
+  // CHECK-MESSAGES: [[@LINE-1]]:5: warning: the value returned by this function should be used [bugprone-unused-return-value]
+  else
+std::remove(nullptr, nullptr, 1);
+  // CHECK-MESSAGES: [[@LINE-1]]:5: warning: the value returned by this function should be used [bugprone-unused-return-value]
+
+  while (true)
+std::remove(nullptr, nullptr, 1);
+  // CHECK-MESSAGES: [[@LINE-1]]:5: warning: the value returned by this function should be used [bugprone-unused-return-value]
+
+  do
+std::remove(nullptr, nullptr, 1);
+  // CHECK-MESSAGES: [[@LINE-1]]:5: warning: the value returned by this function should be used [bugprone-unused-return-value]
+  while (true);
+
+  for (;;)
+std::remove(nullptr, nullptr, 1);
+  // CHECK-MESSAGES: [[@LINE-1]]:5: warning: the value returned by this function should be used [bugprone-unused-return-value]
+
+  for (std::remove(nullptr, nullptr, 1);;)
+// CHECK-MESSAGES: [[@LINE-1]]:8: warning: the value returned by this function should be used [bugprone-unused-return-value]
+;
+
+  for (;; std::remove(nullptr, nullptr, 1))
+// CHECK-MESSAGES: [[@LINE-1]]:11: warning: the value returned by this function should be used [bugprone-unused-return-value]
+;
+
+  for (auto C : "foo")
+std::remove(nullptr, nullptr, 1);
+  // CHECK-MESSAGES: [[@LINE-1]]:5: warning: the value returned by this function should be used [bugprone-unused-return-value]
+
+  switch (1) {
+  case 1:
+std::remove(nullptr, nullptr, 1);
+// CHECK-MESSAGES: [[@LINE-1]]:5: warning: the value returned by this function should be used [bugprone-unused-return-value]
+break;
+  default:
+std::remove(nullptr, nullptr, 1);
+// CHECK-MESSAGES: [[@LINE-1]]:5: warning: the value returned by this function should be used [bugprone-unused-return-value]
+break;
+  }
+
+  try {
+std::remove(nullptr, nullptr, 1);
+// CHECK-MESSAGES: [[@LINE-1]]:5: warning: the value retu

[PATCH] D43184: [WIP] [ItaniunCXXABI] Add an option for loading the type info vtable with dllimport

2018-03-15 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment.

In https://reviews.llvm.org/D43184#1038396, @mstorsjo wrote:

> (Accidentally pressed submit too soon)
>
> This was my conclusion at some point as well - but there's also a few issues 
> with that approach which makes me quite hesitant:
>
> - doing fixups in the code segment requires making it writable temporarily at 
> runtime, which is disallowed in UWP
> - the current set of runtime relocations only allows adding addresses in 1, 
> 2, 4 and 8 byte sized addresses. On arm/arm64, absolute addresses can be 
> present in instructions with a much more complicated opcode format, and 
> tweaking addresses there requires defining more runtime relocation formats.


I wouldn't consider relocating non-dllimport references to dllimport symbols as 
being in scope. As long the symbol is annotated as dllimport, the compiler will 
generate code that does a load from the __imp_ pointer in the IAT.

For unannotated symbols, the linker already generates thunks for function 
references, so that just leaves data imports.

Still, the RTTI will be in the .rdata section, so it will require unprotecting 
readonly memory, which won't work in UWP. We could move it to .data, I guess.

> So given all this - I only see bad options. I could just postpone this as 
> well - I can live with only linking libc++ statically for now.

Might be the thing to do.

> The other case you mentioned, with multiple inheritance for statically 
> allocated objects, needs to be handled in one way or another though (unless 
> one forbids C++ interfaces over DLL boundaries); how's that handled with the 
> MSVC C++ ABI?

I think local vftables handle it.


Repository:
  rC Clang

https://reviews.llvm.org/D43184



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


[PATCH] D43322: Diagnose cases of "return x" that should be "return std::move(x)" for efficiency

2018-03-15 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone updated this revision to Diff 138594.
Quuxplusone added a comment.

Rebase on master, now that the refactoring https://reviews.llvm.org/D43898 has 
been pushed (thanks @rtrieu!)


Repository:
  rC Clang

https://reviews.llvm.org/D43322

Files:
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Sema.h
  lib/Sema/SemaStmt.cpp
  test/SemaCXX/warn-return-std-move.cpp

Index: test/SemaCXX/warn-return-std-move.cpp
===
--- /dev/null
+++ test/SemaCXX/warn-return-std-move.cpp
@@ -0,0 +1,334 @@
+// RUN: %clang_cc1 -fsyntax-only -Wreturn-std-move -Wreturn-std-move-in-cxx11 -std=c++11 -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wreturn-std-move -Wreturn-std-move-in-cxx11 -std=c++11 -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+
+// definitions for std::move
+namespace std {
+inline namespace foo {
+template  struct remove_reference { typedef T type; };
+template  struct remove_reference { typedef T type; };
+template  struct remove_reference { typedef T type; };
+
+template  typename remove_reference::type &&move(T &&t);
+} // namespace foo
+} // namespace std
+
+struct Instrument {
+Instrument() {}
+Instrument(Instrument&&) { /* MOVE */ }
+Instrument(const Instrument&) { /* COPY */ }
+};
+struct ConvertFromBase { Instrument i; };
+struct ConvertFromDerived { Instrument i; };
+struct Base {
+Instrument i;
+operator ConvertFromBase() const& { return ConvertFromBase{i}; }
+operator ConvertFromBase() && { return ConvertFromBase{std::move(i)}; }
+};
+struct Derived : public Base {
+operator ConvertFromDerived() const& { return ConvertFromDerived{i}; }
+operator ConvertFromDerived() && { return ConvertFromDerived{std::move(i)}; }
+};
+struct ConstructFromBase {
+Instrument i;
+ConstructFromBase(const Base& b): i(b.i) {}
+ConstructFromBase(Base&& b): i(std::move(b.i)) {}
+};
+struct ConstructFromDerived {
+Instrument i;
+ConstructFromDerived(const Derived& d): i(d.i) {}
+ConstructFromDerived(Derived&& d): i(std::move(d.i)) {}
+};
+
+struct TrivialInstrument {
+int i = 42;
+};
+struct ConvertFromTrivialBase { TrivialInstrument i; };
+struct ConvertFromTrivialDerived { TrivialInstrument i; };
+struct TrivialBase {
+TrivialInstrument i;
+operator ConvertFromTrivialBase() const& { return ConvertFromTrivialBase{i}; }
+operator ConvertFromTrivialBase() && { return ConvertFromTrivialBase{std::move(i)}; }
+};
+struct TrivialDerived : public TrivialBase {
+operator ConvertFromTrivialDerived() const& { return ConvertFromTrivialDerived{i}; }
+operator ConvertFromTrivialDerived() && { return ConvertFromTrivialDerived{std::move(i)}; }
+};
+struct ConstructFromTrivialBase {
+TrivialInstrument i;
+ConstructFromTrivialBase(const TrivialBase& b): i(b.i) {}
+ConstructFromTrivialBase(TrivialBase&& b): i(std::move(b.i)) {}
+};
+struct ConstructFromTrivialDerived {
+TrivialInstrument i;
+ConstructFromTrivialDerived(const TrivialDerived& d): i(d.i) {}
+ConstructFromTrivialDerived(TrivialDerived&& d): i(std::move(d.i)) {}
+};
+
+Derived test1() {
+Derived d1;
+return d1;  // ok
+}
+Base test2() {
+Derived d2;
+return d2;  // e1
+// expected-warning@-1{{will be copied despite being returned by name}}
+// expected-note@-2{{to avoid copying}}
+// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:14}:"std::move(d2)"
+}
+ConstructFromDerived test3() {
+Derived d3;
+return d3;  // e2-cxx11
+// expected-warning@-1{{would have been copied despite being returned by name}}
+// expected-note@-2{{to avoid copying on older compilers}}
+// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:14}:"std::move(d3)"
+}
+ConstructFromBase test4() {
+Derived d4;
+return d4;  // e3
+// expected-warning@-1{{will be copied despite being returned by name}}
+// expected-note@-2{{to avoid copying}}
+// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:14}:"std::move(d4)"
+}
+ConvertFromDerived test5() {
+Derived d5;
+return d5;  // e4
+// expected-warning@-1{{will be copied despite being returned by name}}
+// expected-note@-2{{to avoid copying}}
+// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:14}:"std::move(d5)"
+}
+ConvertFromBase test6() {
+Derived d6;
+return d6;  // e5
+// expected-warning@-1{{will be copied despite being returned by name}}
+// expected-note@-2{{to avoid copying}}
+// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:14}:"std::move(d6)"
+}
+
+// These test cases should not produce the warning.
+Derived ok1() { Derived d; return d; }
+Base ok2() { Derived d; return static_cast(d); }
+ConstructFromDerived ok3() { Derived d; return static_cast(d); }
+ConstructFromBase ok4() { Derived d; return static_cast(d); }
+ConvertFromDerived ok5() { Derived d; return static_cast(d); }
+ConvertFromBase ok6() 

[PATCH] D43322: Diagnose cases of "return x" that should be "return std::move(x)" for efficiency

2018-03-15 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added inline comments.



Comment at: include/clang/Basic/DiagnosticGroups.td:388
 def PessimizingMove : DiagGroup<"pessimizing-move">;
+def ReturnStdMoveInCXX11 : DiagGroup<"return-std-move-in-c++11">;
+def ReturnStdMove : DiagGroup<"return-std-move">;

>> The backward-compatibility-concerned diagnostic, -Wreturn-std-move-in-c++11, 
>> is *not* appropriate for -Wmove;
> Have you evaluated possibility of adding -Wreturn-std-move-in-c++11 to one of 
> CXX..Compat groups?

Hmm. I've considered it, but I don't know what the appropriate action is...
I suspect that it should *not* be "CXX11Compat" because technically the change 
was a DR against C++11 — sadly this is about portability to "older" compilers, 
not portability to "lesser" C++ standards.
Of course it can't be "CXX98Compat", for several reasons. :)
I'm not sure of the purpose of the "CXX..CompatPedantic" groups. They are all 
identical to the "CXX..Compat" groups, except that CXX98CompatPedantic includes 
one extra warning about binding to lifetime-extended temporaries which don't 
have copy constructors.


Repository:
  rC Clang

https://reviews.llvm.org/D43322



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


r327654 - [OPENMP, NVPTX] Improve globalization of the variables captured by value.

2018-03-15 Thread Alexey Bataev via cfe-commits
Author: abataev
Date: Thu Mar 15 11:10:54 2018
New Revision: 327654

URL: http://llvm.org/viewvc/llvm-project?rev=327654&view=rev
Log:
[OPENMP, NVPTX] Improve globalization of the variables captured by value.

If the variable is captured by value and the corresponding parameter in
the outlined function escapes its declaration context, this parameter
must be globalized. To globalize it we need to get the address of the
original parameter, load the value, store it to the global address and
use this global address instead of the original.

Patch improves globalization for parallel|teams regions + functions in
declare target regions.

Added:
cfe/trunk/test/OpenMP/declare_target_codegen_globalization.cpp
Modified:
cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.h
cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp
cfe/trunk/test/OpenMP/nvptx_teams_codegen.cpp

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp?rev=327654&r1=327653&r2=327654&view=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp Thu Mar 15 11:10:54 2018
@@ -171,16 +171,23 @@ class CheckVarsEscapingDeclContext final
 : public ConstStmtVisitor {
   CodeGenFunction &CGF;
   llvm::SetVector EscapedDecls;
+  llvm::SmallPtrSet EscapedParameters;
   llvm::SmallPtrSet IgnoredDecls;
   bool AllEscaped = false;
   RecordDecl *GlobalizedRD = nullptr;
   llvm::SmallDenseMap MappedDeclsFields;
 
   void markAsEscaped(const ValueDecl *VD) {
-if (IgnoredDecls.count(VD) ||
-(CGF.CapturedStmtInfo &&
- CGF.CapturedStmtInfo->lookup(cast(VD
+if (IgnoredDecls.count(VD))
   return;
+// Variables captured by value must be globalized.
+if (auto *CSI = CGF.CapturedStmtInfo) {
+  if (const FieldDecl *FD = 
CGF.CapturedStmtInfo->lookup(cast(VD))) {
+if (FD->getType()->isReferenceType())
+  return;
+EscapedParameters.insert(VD);
+  }
+}
 EscapedDecls.insert(VD);
   }
 
@@ -385,12 +392,15 @@ public:
 Visit(Child);
   }
 
+  /// Returns the record that handles all the escaped local variables and used
+  /// instead of their original storage.
   const RecordDecl *getGlobalizedRecord() {
 if (!GlobalizedRD)
   buildRecordForGlobalizedVars();
 return GlobalizedRD;
   }
 
+  /// Returns the field in the globalized record for the escaped variable.
   const FieldDecl *getFieldForGlobalizedVar(const ValueDecl *VD) const {
 assert(GlobalizedRD &&
"Record for globalized variables must be generated already.");
@@ -400,9 +410,16 @@ public:
 return I->getSecond();
   }
 
+  /// Returns the list of the escaped local variables/parameters.
   ArrayRef getEscapedDecls() const {
 return EscapedDecls.getArrayRef();
   }
+
+  /// Checks if the escaped local variable is actually a parameter passed by
+  /// value.
+  const llvm::SmallPtrSetImpl &getEscapedParameters() const {
+return EscapedParameters;
+  }
 };
 } // anonymous namespace
 
@@ -614,58 +631,14 @@ void CGOpenMPRuntimeNVPTX::emitGenericEn
   createNVPTXRuntimeFunction(
   OMPRTL_NVPTX__kmpc_data_sharing_init_stack));
 
-  const auto I = FunctionGlobalizedDecls.find(CGF.CurFn);
-  if (I == FunctionGlobalizedDecls.end())
-return;
-  const RecordDecl *GlobalizedVarsRecord = I->getSecond().first;
-  QualType RecTy = CGM.getContext().getRecordType(GlobalizedVarsRecord);
-
-  // Recover pointer to this function's global record. The runtime will
-  // handle the specifics of the allocation of the memory.
-  // Use actual memory size of the record including the padding
-  // for alignment purposes.
-  unsigned Alignment =
-  CGM.getContext().getTypeAlignInChars(RecTy).getQuantity();
-  unsigned GlobalRecordSize =
-  CGM.getContext().getTypeSizeInChars(RecTy).getQuantity();
-  GlobalRecordSize = llvm::alignTo(GlobalRecordSize, Alignment);
-  // TODO: allow the usage of shared memory to be controlled by
-  // the user, for now, default to global.
-  llvm::Value *GlobalRecordSizeArg[] = {
-  llvm::ConstantInt::get(CGM.SizeTy, GlobalRecordSize),
-  CGF.Builder.getInt16(/*UseSharedMemory=*/0)};
-  llvm::Value *GlobalRecValue = CGF.EmitRuntimeCall(
-  createNVPTXRuntimeFunction(OMPRTL_NVPTX__kmpc_data_sharing_push_stack),
-  GlobalRecordSizeArg);
-  llvm::Value *GlobalRecCastAddr = Bld.CreatePointerBitCastOrAddrSpaceCast(
-  GlobalRecValue, CGF.ConvertTypeForMem(RecTy)->getPointerTo());
-  FunctionToGlobalRecPtr.try_emplace(CGF.CurFn, GlobalRecValue);
-
-  // Emit the "global alloca" which is a GEP from the global declaration record
-  // using the pointer returned by the runtime.
-  LValue Base = CGF.MakeNaturalAlignPointeeAddrLValue(GlobalRecCastAddr, 
RecTy);
-  auto &Res = I->getSecon

[PATCH] D42978: Make march/target-cpu print a note with the list of valid values for ARM

2018-03-15 Thread Simi Pallipurath via Phabricator via cfe-commits
simpal01 added a comment.

@erichkeane Could you please tell me why this was done only for -cc1 and if 
there is any plan to add this to the driver in general ?


Repository:
  rL LLVM

https://reviews.llvm.org/D42978



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


[PATCH] D44435: Add the module name to __cuda_module_ctor and __cuda_module_dtor for unique function names

2018-03-15 Thread Artem Belevich via Phabricator via cfe-commits
tra added inline comments.



Comment at: unittests/CodeGen/IncrementalProcessingTest.cpp:176-178
+
+// In CUDA incremental processing, a CUDA ctor or dtor will be generated for 
+// every statement if a fatbinary file exists.

SimeonEhrig wrote:
> tra wrote:
> > SimeonEhrig wrote:
> > > tra wrote:
> > > > I don't understand the comment. What is 'CUDA incremental processing' 
> > > > and what exactly is meant by 'statement' here? I'd appreciate if you 
> > > > could give me more details. My understanding is that ctor/dtor are 
> > > > generated once per TU. I suspect "incremental processing" may change 
> > > > that, but I have no idea what exactly does it do.
> > > A CUDA ctor/dtor will generates for every llvm::module. The TU can also 
> > > composed of many modules. In our interpreter, we add new code to our AST 
> > > with new modules at runtime. 
> > > The ctor/dtor generation is depend on the fatbinary code. The CodeGen 
> > > checks, if a path to a fatbinary file is set. If it is, it generates an 
> > > ctor with at least a __cudaRegisterFatBinary() function call. So, the 
> > > generation is independent of the source code in the module and we can use 
> > > every statement. A statement can be an expression, a declaration, a 
> > > definition and so one.   
> > I still don't understand how it's going to work. Do you have some sort of 
> > design document outlining how the interpreter is going to work with CUDA?
> > 
> > The purpose of the ctor/dtor is to stitch together host-side kernel launch 
> > with the GPU-side kernel binary which resides in the GPU binary created by 
> > device-side compilation. 
> > 
> > So, the question #1 -- if you pass GPU-side binary to the compiler, where 
> > did you get it? Normally it's the result of device-side compilation of the 
> > same TU. In your case it's not quite clear what exactly would that be, if 
> > you feed the source to the compiler incrementally. I.e. do you somehow 
> > recompile everything we've seen on device side so far for each new chunk of 
> > host-side source you feed to the compiler? 
> > 
> > Next question is -- assuming that device side does have correct GPU-side 
> > binary, when do you call those ctors/dtors? JIT model does not quite fit 
> > the assumptions that drive regular CUDA compilation.
> > 
> > Let's consider this:
> > ```
> > __global__ void foo();
> > __global__ void bar();
> > 
> > // If that's all we've  fed to compiler so far, we have no GPU code yet, so 
> > there 
> > // should be no fatbin file. If we do have it, what's in it?
> > 
> > void launch() {
> >   foo<<<1,1>>>();
> >   bar<<<1,1>>>();
> > }
> > // If you've generated ctors/dtors at this point they would be 
> > // useless as no GPU code exists in the preceding code.
> > 
> > __global__ void foo() {}
> > // Now we'd have some GPU code, but how can we need to retrofit it into 
> > // all the ctors/dtors we've generated before. 
> > __global__ void bar() {}
> > // Does bar end up in its own fatbinary? Or is it combined into a new 
> > // fatbin which contains both boo and bar?
> > // If it's a new fatbin, you somehow need to update existing ctors/dtors, 
> > // unless you want to leak CUDA resources fast.
> > // If it's a separate fatbin, then you will need to at the very least 
> > change the way 
> > // ctors/dtors are generated by the 'launch' function, because now they 
> > need to 
> > // tie each kernel launch to a different fatbin.
> > 
> > ```
> > 
> > It looks to me that if you want to JIT CUDA code you will need to take over 
> > GPU-side kernel management.
> > ctors/dtors do that for full-TU compilation, but they rely on device-side 
> > code being compiled and available during host-side compilation. For JIT, 
> > the interpreter should be in charge of registering new kernels with the 
> > CUDA runtime and unregistering/unloading them when a kernel goes away. This 
> > makes ctors/dtors completely irrelevant.
> At the moment, there is no documentation, because we still develop the 
> feature. I try to describe how it works.
> 
> The device side compilation works with a second compiler (a normal clang), 
> which we start via syscall. In the interpreter, we check if the input line is 
> a kernel definition or a kernel launch. Then we write the source code to a 
> file and compile it with the clang to a PCH-file.  Then the PCH-file will be 
> compiled to PTX and then to a fatbin. If we add a new kernel, we will send 
> the source code with the existing PCH-file to clang compiler. So we easy 
> extend the AST and generate a PTX-file with all defined kernels. 
> 
> An implementation of this feature can you see at my prototype: 
> 
> 
> Running the ctor/dtor isn't hard. I search after the JITSymbol and generate 
> an function pointer. Than I can simply run it. This feature can you also see 
> in my prototype. So, we can run the ctor, if new fatbin code is generated and 
> 

[PATCH] D44408: Move DraftMgr from ClangdServer to ClangdLSPServer

2018-03-15 Thread Simon Marchi via Phabricator via cfe-commits
simark updated this revision to Diff 138581.
simark marked 2 inline comments as done.
simark added a comment.

Address latest review comments.

Also, update the comments in DraftStore.{h,cpp} that were outdated.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44408

Files:
  clangd/ClangdLSPServer.cpp
  clangd/ClangdLSPServer.h
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/DraftStore.cpp
  clangd/DraftStore.h
  unittests/clangd/ClangdTests.cpp

Index: unittests/clangd/ClangdTests.cpp
===
--- unittests/clangd/ClangdTests.cpp
+++ unittests/clangd/ClangdTests.cpp
@@ -478,7 +478,10 @@
   CDB.ExtraClangFlags.clear();
   DiagConsumer.clear();
   Server.removeDocument(BazCpp);
-  Server.reparseOpenedFiles();
+  Server.addDocument(FooCpp, FooSource.code(), WantDiagnostics::Auto,
+ /*SkipCache=*/true);
+  Server.addDocument(BarCpp, BarSource.code(), WantDiagnostics::Auto,
+ /*SkipCache=*/true);
   ASSERT_TRUE(Server.blockUntilIdleForTest());
 
   EXPECT_THAT(DiagConsumer.filesWithDiags(),
Index: clangd/DraftStore.h
===
--- clangd/DraftStore.h
+++ clangd/DraftStore.h
@@ -13,53 +13,33 @@
 #include "Path.h"
 #include "clang/Basic/LLVM.h"
 #include "llvm/ADT/StringMap.h"
-#include 
 #include 
 #include 
 #include 
 
 namespace clang {
 namespace clangd {
 
-/// Using unsigned int type here to avoid undefined behaviour on overflow.
-typedef uint64_t DocVersion;
-
-/// Document draft with a version of this draft.
-struct VersionedDraft {
-  DocVersion Version;
-  /// If the value of the field is None, draft is now deleted
-  llvm::Optional Draft;
-};
-
 /// A thread-safe container for files opened in a workspace, addressed by
-/// filenames. The contents are owned by the DraftStore. Versions are mantained
-/// for the all added documents, including removed ones. The document version is
-/// incremented on each update and removal of the document.
+/// filenames. The contents are owned by the DraftStore.
 class DraftStore {
 public:
-  /// \return version and contents of the stored document.
-  /// For untracked files, a (0, None) pair is returned.
-  VersionedDraft getDraft(PathRef File) const;
+  /// \return Contents of the stored document.
+  /// For untracked files, a llvm::None is returned.
+  llvm::Optional getDraft(PathRef File) const;
 
-  /// \return List of names of active drafts in this store.  Drafts that were
-  /// removed (which still have an entry in the Drafts map) are not returned by
-  /// this function.
+  /// \return List of names of the drafts in this store.
   std::vector getActiveFiles() const;
 
-  /// \return version of the tracked document.
-  /// For untracked files, 0 is returned.
-  DocVersion getVersion(PathRef File) const;
-
   /// Replace contents of the draft for \p File with \p Contents.
-  /// \return The new version of the draft for \p File.
-  DocVersion updateDraft(PathRef File, StringRef Contents);
-  /// Remove the contents of the draft
-  /// \return The new version of the draft for \p File.
-  DocVersion removeDraft(PathRef File);
+  void updateDraft(PathRef File, StringRef Contents);
+
+  /// Remove the draft from the store.
+  void removeDraft(PathRef File);
 
 private:
   mutable std::mutex Mutex;
-  llvm::StringMap Drafts;
+  llvm::StringMap Drafts;
 };
 
 } // namespace clangd
Index: clangd/DraftStore.cpp
===
--- clangd/DraftStore.cpp
+++ clangd/DraftStore.cpp
@@ -12,49 +12,34 @@
 using namespace clang;
 using namespace clang::clangd;
 
-VersionedDraft DraftStore::getDraft(PathRef File) const {
+llvm::Optional DraftStore::getDraft(PathRef File) const {
   std::lock_guard Lock(Mutex);
 
   auto It = Drafts.find(File);
   if (It == Drafts.end())
-return {0, llvm::None};
+return llvm::None;
+
   return It->second;
 }
 
 std::vector DraftStore::getActiveFiles() const {
   std::lock_guard Lock(Mutex);
   std::vector ResultVector;
 
   for (auto DraftIt = Drafts.begin(); DraftIt != Drafts.end(); DraftIt++)
-if (DraftIt->second.Draft)
-  ResultVector.push_back(DraftIt->getKey());
+ResultVector.push_back(DraftIt->getKey());
 
   return ResultVector;
 }
 
-DocVersion DraftStore::getVersion(PathRef File) const {
-  std::lock_guard Lock(Mutex);
-
-  auto It = Drafts.find(File);
-  if (It == Drafts.end())
-return 0;
-  return It->second.Version;
-}
-
-DocVersion DraftStore::updateDraft(PathRef File, StringRef Contents) {
+void DraftStore::updateDraft(PathRef File, StringRef Contents) {
   std::lock_guard Lock(Mutex);
 
-  auto &Entry = Drafts[File];
-  DocVersion NewVersion = ++Entry.Version;
-  Entry.Draft = Contents;
-  return NewVersion;
+  Drafts[File] = Contents;
 }
 
-DocVersion DraftStore::removeDraft(PathRef File) {
+void DraftStore::removeDraft(PathRef File) {
   std::lock_guard Lock(Mutex);
 
-  

[PATCH] D34796: upporting -f(no)-reorder-functions flag, clang side change

2018-03-15 Thread David Li via Phabricator via cfe-commits
davidxl added inline comments.



Comment at: docs/ClangCommandLineReference.rst:1750
+
+Add section prefixes for hot/cold functions
+

prefix or suffix? Or just leave the details out (also consider the interaction 
with -ffunction-sections)?

Consider documenting: 

1) what is the default behavior?
2) how does it affect function ordering?



https://reviews.llvm.org/D34796



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


[PATCH] D44408: Move DraftMgr from ClangdServer to ClangdLSPServer

2018-03-15 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added inline comments.



Comment at: clangd/ClangdLSPServer.h:78
 
+  /// Calls forceReparse() on all currently opened files.
+  /// As a result, this method may be very expensive.

NIT: there is no `forceReparse()` anymore, maybe remove its mention from the 
comment? 
I.e.
```
/// Reparses all tracked documents, invalidating compile commands cache.
/// As a result, this method may be very expensive.
/// This method is normally called when the compilation database is changed.
```



Comment at: clangd/ClangdLSPServer.h:111
+  // Store of the current versions of the open documents.
+  DraftStore DraftMgr;
 };

ClangdServer should be the last member! (see the comment above)



Comment at: clangd/DraftStore.h:48
   mutable std::mutex Mutex;
-  llvm::StringMap Drafts;
+  llvm::StringMap> Drafts;
 };

No need for `Optional`s here anymore. Maybe store `std::string` and `erase` on 
`removeDraft`?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44408



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


r327636 - [OPENMP] Codegen for `omp declare target` construct.

2018-03-15 Thread Alexey Bataev via cfe-commits
Author: abataev
Date: Thu Mar 15 08:47:20 2018
New Revision: 327636

URL: http://llvm.org/viewvc/llvm-project?rev=327636&view=rev
Log:
[OPENMP] Codegen for `omp declare target` construct.

Added initial codegen for device side of declarations inside `omp
declare target` construct + codegen for implicit `declare target`
functions, which are used in the target regions.

Added:
cfe/trunk/test/OpenMP/declare_target_codegen.cpp
Modified:
cfe/trunk/lib/AST/ASTContext.cpp
cfe/trunk/lib/CodeGen/CGDecl.cpp
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.h
cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp
cfe/trunk/lib/CodeGen/CodeGenModule.cpp
cfe/trunk/lib/Parse/ParseOpenMP.cpp
cfe/trunk/lib/Sema/SemaOpenMP.cpp

Modified: cfe/trunk/lib/AST/ASTContext.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=327636&r1=327635&r2=327636&view=diff
==
--- cfe/trunk/lib/AST/ASTContext.cpp (original)
+++ cfe/trunk/lib/AST/ASTContext.cpp Thu Mar 15 08:47:20 2018
@@ -9402,8 +9402,7 @@ bool ASTContext::DeclMustBeEmitted(const
   return false;
   } else if (isa(D))
 return true;
-  else if (isa(D) ||
-   D->hasAttr())
+  else if (isa(D))
 return true;
   else if (isa(D))
 return true;
@@ -9492,6 +9491,12 @@ bool ASTContext::DeclMustBeEmitted(const
 if (DeclMustBeEmitted(BindingVD))
   return true;
 
+  // If the decl is marked as `declare target`, it should be emitted.
+  for (const auto *Decl = D->getMostRecentDecl(); Decl;
+   Decl = Decl->getPreviousDecl())
+if (Decl->hasAttr())
+  return true;
+
   return false;
 }
 

Modified: cfe/trunk/lib/CodeGen/CGDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDecl.cpp?rev=327636&r1=327635&r2=327636&view=diff
==
--- cfe/trunk/lib/CodeGen/CGDecl.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDecl.cpp Thu Mar 15 08:47:20 2018
@@ -285,8 +285,11 @@ llvm::Constant *CodeGenModule::getOrCrea
 // never defer them.
 assert(isa(DC) && "unexpected parent code decl");
   }
-  if (GD.getDecl())
+  if (GD.getDecl()) {
+// Disable emission of the parent function for the OpenMP device codegen.
+CGOpenMPRuntime::DisableAutoDeclareTargetRAII NoDeclTarget(*this);
 (void)GetAddrOfGlobal(GD);
+  }
 
   return Addr;
 }

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp?rev=327636&r1=327635&r2=327636&view=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp Thu Mar 15 08:47:20 2018
@@ -7405,9 +7405,14 @@ bool CGOpenMPRuntime::emitTargetFunction
   // Try to detect target regions in the function.
   scanForTargetRegionsFunctions(FD.getBody(), CGM.getMangledName(GD));
 
-  // We should not emit any function other that the ones created during the
-  // scanning. Therefore, we signal that this function is completely dealt
-  // with.
+  // Do not to emit function if it is not marked as declare target.
+  if (!GD.getDecl()->hasAttrs())
+return true;
+
+  for (const auto *D = FD.getMostRecentDecl(); D; D = D->getPreviousDecl())
+if (D->hasAttr())
+  return false;
+
   return true;
 }
 
@@ -7433,8 +7438,15 @@ bool CGOpenMPRuntime::emitTargetGlobalVa
 }
   }
 
-  // If we are in target mode, we do not emit any global (declare target is not
-  // implemented yet). Therefore we signal that GD was processed in this case.
+  // Do not to emit variable if it is not marked as declare target.
+  if (!GD.getDecl()->hasAttrs())
+return true;
+
+  for (const Decl *D = GD.getDecl()->getMostRecentDecl(); D;
+   D = D->getPreviousDecl())
+if (D->hasAttr())
+  return false;
+
   return true;
 }
 
@@ -7446,6 +7458,38 @@ bool CGOpenMPRuntime::emitTargetGlobal(G
   return emitTargetGlobalVariable(GD);
 }
 
+CGOpenMPRuntime::DisableAutoDeclareTargetRAII::DisableAutoDeclareTargetRAII(
+CodeGenModule &CGM)
+: CGM(CGM) {
+  if (CGM.getLangOpts().OpenMPIsDevice) {
+SavedShouldMarkAsGlobal = CGM.getOpenMPRuntime().ShouldMarkAsGlobal;
+CGM.getOpenMPRuntime().ShouldMarkAsGlobal = false;
+  }
+}
+
+CGOpenMPRuntime::DisableAutoDeclareTargetRAII::~DisableAutoDeclareTargetRAII() 
{
+  if (CGM.getLangOpts().OpenMPIsDevice)
+CGM.getOpenMPRuntime().ShouldMarkAsGlobal = SavedShouldMarkAsGlobal;
+}
+
+bool CGOpenMPRuntime::markAsGlobalTarget(const FunctionDecl *D) {
+  if (!CGM.getLangOpts().OpenMPIsDevice || !ShouldMarkAsGlobal)
+return true;
+  // Do not to emit function if it is marked as declare target as it was 
already
+  // emitted.
+  for (const auto *FD = D->getMostRecentDecl(); FD; FD = FD->getPreviousDecl())
+if (FD->hasAttr())
+  ret

[PATCH] D44408: Move DraftMgr from ClangdServer to ClangdLSPServer

2018-03-15 Thread Simon Marchi via Phabricator via cfe-commits
simark updated this revision to Diff 138569.
simark marked 6 inline comments as done.
simark added a comment.

Changes based on review comments


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44408

Files:
  clangd/ClangdLSPServer.cpp
  clangd/ClangdLSPServer.h
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/DraftStore.cpp
  clangd/DraftStore.h
  unittests/clangd/ClangdTests.cpp

Index: unittests/clangd/ClangdTests.cpp
===
--- unittests/clangd/ClangdTests.cpp
+++ unittests/clangd/ClangdTests.cpp
@@ -478,7 +478,10 @@
   CDB.ExtraClangFlags.clear();
   DiagConsumer.clear();
   Server.removeDocument(BazCpp);
-  Server.reparseOpenedFiles();
+  Server.addDocument(FooCpp, FooSource.code(), WantDiagnostics::Auto,
+ /*SkipCache=*/true);
+  Server.addDocument(BarCpp, BarSource.code(), WantDiagnostics::Auto,
+ /*SkipCache=*/true);
   ASSERT_TRUE(Server.blockUntilIdleForTest());
 
   EXPECT_THAT(DiagConsumer.filesWithDiags(),
Index: clangd/DraftStore.h
===
--- clangd/DraftStore.h
+++ clangd/DraftStore.h
@@ -13,53 +13,39 @@
 #include "Path.h"
 #include "clang/Basic/LLVM.h"
 #include "llvm/ADT/StringMap.h"
-#include 
 #include 
 #include 
 #include 
 
 namespace clang {
 namespace clangd {
 
-/// Using unsigned int type here to avoid undefined behaviour on overflow.
-typedef uint64_t DocVersion;
-
-/// Document draft with a version of this draft.
-struct VersionedDraft {
-  DocVersion Version;
-  /// If the value of the field is None, draft is now deleted
-  llvm::Optional Draft;
-};
-
 /// A thread-safe container for files opened in a workspace, addressed by
 /// filenames. The contents are owned by the DraftStore. Versions are mantained
 /// for the all added documents, including removed ones. The document version is
 /// incremented on each update and removal of the document.
 class DraftStore {
 public:
   /// \return version and contents of the stored document.
   /// For untracked files, a (0, None) pair is returned.
-  VersionedDraft getDraft(PathRef File) const;
+  llvm::Optional getDraft(PathRef File) const;
 
   /// \return List of names of active drafts in this store.  Drafts that were
   /// removed (which still have an entry in the Drafts map) are not returned by
   /// this function.
   std::vector getActiveFiles() const;
 
-  /// \return version of the tracked document.
-  /// For untracked files, 0 is returned.
-  DocVersion getVersion(PathRef File) const;
-
   /// Replace contents of the draft for \p File with \p Contents.
   /// \return The new version of the draft for \p File.
-  DocVersion updateDraft(PathRef File, StringRef Contents);
+  void updateDraft(PathRef File, StringRef Contents);
+
   /// Remove the contents of the draft
   /// \return The new version of the draft for \p File.
-  DocVersion removeDraft(PathRef File);
+  void removeDraft(PathRef File);
 
 private:
   mutable std::mutex Mutex;
-  llvm::StringMap Drafts;
+  llvm::StringMap> Drafts;
 };
 
 } // namespace clangd
Index: clangd/DraftStore.cpp
===
--- clangd/DraftStore.cpp
+++ clangd/DraftStore.cpp
@@ -12,49 +12,35 @@
 using namespace clang;
 using namespace clang::clangd;
 
-VersionedDraft DraftStore::getDraft(PathRef File) const {
+llvm::Optional DraftStore::getDraft(PathRef File) const {
   std::lock_guard Lock(Mutex);
 
   auto It = Drafts.find(File);
   if (It == Drafts.end())
-return {0, llvm::None};
+return llvm::None;
+
   return It->second;
 }
 
 std::vector DraftStore::getActiveFiles() const {
   std::lock_guard Lock(Mutex);
   std::vector ResultVector;
 
   for (auto DraftIt = Drafts.begin(); DraftIt != Drafts.end(); DraftIt++)
-if (DraftIt->second.Draft)
+if (DraftIt->second)
   ResultVector.push_back(DraftIt->getKey());
 
   return ResultVector;
 }
 
-DocVersion DraftStore::getVersion(PathRef File) const {
-  std::lock_guard Lock(Mutex);
-
-  auto It = Drafts.find(File);
-  if (It == Drafts.end())
-return 0;
-  return It->second.Version;
-}
-
-DocVersion DraftStore::updateDraft(PathRef File, StringRef Contents) {
+void DraftStore::updateDraft(PathRef File, StringRef Contents) {
   std::lock_guard Lock(Mutex);
 
-  auto &Entry = Drafts[File];
-  DocVersion NewVersion = ++Entry.Version;
-  Entry.Draft = Contents;
-  return NewVersion;
+  Drafts[File] = Contents;
 }
 
-DocVersion DraftStore::removeDraft(PathRef File) {
+void DraftStore::removeDraft(PathRef File) {
   std::lock_guard Lock(Mutex);
 
-  auto &Entry = Drafts[File];
-  DocVersion NewVersion = ++Entry.Version;
-  Entry.Draft = llvm::None;
-  return NewVersion;
+  Drafts[File] = llvm::None;
 }
Index: clangd/ClangdServer.h
===
--- clangd/ClangdServer.h
+++ clangd/ClangdServer.h
@@ -13,7 +13,6 @@
 #include "ClangdUnit.h"

r327634 - Recommit r326946 after reducing CallArgList memory footprint

2018-03-15 Thread Yaxun Liu via cfe-commits
Author: yaxunl
Date: Thu Mar 15 08:25:19 2018
New Revision: 327634

URL: http://llvm.org/viewvc/llvm-project?rev=327634&view=rev
Log:
Recommit r326946 after reducing CallArgList memory footprint

Added:
cfe/trunk/test/CodeGenCXX/amdgcn-func-arg.cpp
Modified:
cfe/trunk/lib/CodeGen/CGAtomic.cpp
cfe/trunk/lib/CodeGen/CGCall.cpp
cfe/trunk/lib/CodeGen/CGCall.h
cfe/trunk/lib/CodeGen/CGClass.cpp
cfe/trunk/lib/CodeGen/CGDecl.cpp
cfe/trunk/lib/CodeGen/CGExprCXX.cpp
cfe/trunk/lib/CodeGen/CGGPUBuiltin.cpp
cfe/trunk/lib/CodeGen/CGObjCGNU.cpp
cfe/trunk/lib/CodeGen/CGObjCMac.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.h
cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp
cfe/trunk/test/CodeGenOpenCL/addr-space-struct-arg.cl
cfe/trunk/test/CodeGenOpenCL/byval.cl

Modified: cfe/trunk/lib/CodeGen/CGAtomic.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGAtomic.cpp?rev=327634&r1=327633&r2=327634&view=diff
==
--- cfe/trunk/lib/CodeGen/CGAtomic.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGAtomic.cpp Thu Mar 15 08:25:19 2018
@@ -1160,7 +1160,7 @@ RValue CodeGenFunction::EmitAtomicExpr(A
 if (UseOptimizedLibcall && Res.getScalarVal()) {
   llvm::Value *ResVal = Res.getScalarVal();
   if (PostOp) {
-llvm::Value *LoadVal1 = Args[1].RV.getScalarVal();
+llvm::Value *LoadVal1 = Args[1].getRValue(*this).getScalarVal();
 ResVal = Builder.CreateBinOp(PostOp, ResVal, LoadVal1);
   }
   if (E->getOp() == AtomicExpr::AO__atomic_nand_fetch)

Modified: cfe/trunk/lib/CodeGen/CGCall.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.cpp?rev=327634&r1=327633&r2=327634&view=diff
==
--- cfe/trunk/lib/CodeGen/CGCall.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGCall.cpp Thu Mar 15 08:25:19 2018
@@ -1040,42 +1040,49 @@ void CodeGenFunction::ExpandTypeFromArgs
 }
 
 void CodeGenFunction::ExpandTypeToArgs(
-QualType Ty, RValue RV, llvm::FunctionType *IRFuncTy,
+QualType Ty, CallArg Arg, llvm::FunctionType *IRFuncTy,
 SmallVectorImpl &IRCallArgs, unsigned &IRCallArgPos) {
   auto Exp = getTypeExpansion(Ty, getContext());
   if (auto CAExp = dyn_cast(Exp.get())) {
-forConstantArrayExpansion(*this, CAExp, RV.getAggregateAddress(),
-  [&](Address EltAddr) {
-  RValue EltRV =
-  convertTempToRValue(EltAddr, CAExp->EltTy, SourceLocation());
-  ExpandTypeToArgs(CAExp->EltTy, EltRV, IRFuncTy, IRCallArgs, 
IRCallArgPos);
-});
+Address Addr = Arg.hasLValue() ? Arg.getKnownLValue().getAddress()
+   : 
Arg.getKnownRValue().getAggregateAddress();
+forConstantArrayExpansion(
+*this, CAExp, Addr, [&](Address EltAddr) {
+  CallArg EltArg = CallArg(
+  convertTempToRValue(EltAddr, CAExp->EltTy, SourceLocation()),
+  CAExp->EltTy);
+  ExpandTypeToArgs(CAExp->EltTy, EltArg, IRFuncTy, IRCallArgs,
+   IRCallArgPos);
+});
   } else if (auto RExp = dyn_cast(Exp.get())) {
-Address This = RV.getAggregateAddress();
+Address This = Arg.hasLValue() ? Arg.getKnownLValue().getAddress()
+   : 
Arg.getKnownRValue().getAggregateAddress();
 for (const CXXBaseSpecifier *BS : RExp->Bases) {
   // Perform a single step derived-to-base conversion.
   Address Base =
   GetAddressOfBaseClass(This, Ty->getAsCXXRecordDecl(), &BS, &BS + 1,
 /*NullCheckValue=*/false, SourceLocation());
-  RValue BaseRV = RValue::getAggregate(Base);
+  CallArg BaseArg = CallArg(RValue::getAggregate(Base), BS->getType());
 
   // Recurse onto bases.
-  ExpandTypeToArgs(BS->getType(), BaseRV, IRFuncTy, IRCallArgs,
+  ExpandTypeToArgs(BS->getType(), BaseArg, IRFuncTy, IRCallArgs,
IRCallArgPos);
 }
 
 LValue LV = MakeAddrLValue(This, Ty);
 for (auto FD : RExp->Fields) {
-  RValue FldRV = EmitRValueForField(LV, FD, SourceLocation());
-  ExpandTypeToArgs(FD->getType(), FldRV, IRFuncTy, IRCallArgs,
+  CallArg FldArg =
+  CallArg(EmitRValueForField(LV, FD, SourceLocation()), FD->getType());
+  ExpandTypeToArgs(FD->getType(), FldArg, IRFuncTy, IRCallArgs,
IRCallArgPos);
 }
   } else if (isa(Exp.get())) {
-ComplexPairTy CV = RV.getComplexVal();
+ComplexPairTy CV = Arg.getKnownRValue().getComplexVal();
 IRCallArgs[IRCallArgPos++] = CV.first;
 IRCallArgs[IRCallArgPos++] = CV.second;
   } else {
 assert(isa(Exp.get()));
+auto RV = Arg.getKnownRValue();
 assert(RV.isScalar() &&
"Unexpected non-scalar rvalue during struct expansion.");
 
@@ -3417,13 +3424,17 @@ void CodeGenFunction::EmitCall

[PATCH] D44231: [clang-tidy] Check for sizeof that call functions

2018-03-15 Thread Paul Fultz II via Phabricator via cfe-commits
pfultz2 added a comment.

So I've updated the documentation on this. Are there any other updates needed 
for this to get it merged in?


https://reviews.llvm.org/D44231



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


[PATCH] D44517: [change-namespace] Don't match a function call/ref multiple times.

2018-03-15 Thread Eric Liu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL327629: [change-namespace] Don't match a function 
call/ref multiple times. (authored by ioeric, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D44517

Files:
  clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp
  clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp


Index: clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp
===
--- clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp
+++ clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp
@@ -478,13 +478,13 @@
 hasAncestor(namespaceDecl(isAnonymous())),
 hasAncestor(cxxRecordDecl(,
hasParent(namespaceDecl()));
-  Finder->addMatcher(decl(forEachDescendant(expr(anyOf(
-  callExpr(callee(FuncMatcher)).bind("call"),
-  declRefExpr(to(FuncMatcher.bind("func_decl")))
-  .bind("func_ref",
-  IsInMovedNs, unless(isImplicit()))
- .bind("dc"),
- this);
+  Finder->addMatcher(
+  expr(allOf(hasAncestor(decl().bind("dc")), IsInMovedNs,
+ unless(hasAncestor(isImplicit())),
+ anyOf(callExpr(callee(FuncMatcher)).bind("call"),
+   declRefExpr(to(FuncMatcher.bind("func_decl")))
+   .bind("func_ref",
+  this);
 
   auto GlobalVarMatcher = varDecl(
   hasGlobalStorage(), hasParent(namespaceDecl()),
Index: 
clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp
===
--- clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp
+++ clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp
@@ -850,22 +850,58 @@
 TEST_F(ChangeNamespaceTest, UsingShadowDeclInGlobal) {
   std::string Code = "namespace glob {\n"
  "class Glob {};\n"
+ "void GFunc() {}\n"
  "}\n"
  "using glob::Glob;\n"
+ "using glob::GFunc;\n"
  "namespace na {\n"
  "namespace nb {\n"
- "void f() { Glob g; }\n"
+ "void f() { Glob g; GFunc(); }\n"
  "} // namespace nb\n"
  "} // namespace na\n";
 
   std::string Expected = "namespace glob {\n"
  "class Glob {};\n"
+ "void GFunc() {}\n"
  "}\n"
  "using glob::Glob;\n"
+ "using glob::GFunc;\n"
  "\n"
  "namespace x {\n"
  "namespace y {\n"
- "void f() { Glob g; }\n"
+ "void f() { Glob g; GFunc(); }\n"
+ "} // namespace y\n"
+ "} // namespace x\n";
+  EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
+}
+
+TEST_F(ChangeNamespaceTest, UsingShadowDeclsInAnonymousNamespaces) {
+  std::string Code = "namespace util {\n"
+ "class Util {};\n"
+ "void func() {}\n"
+ "}\n"
+ "namespace na {\n"
+ "namespace nb {\n"
+ "namespace {\n"
+ "using ::util::Util;\n"
+ "using ::util::func;\n"
+ "void f() { Util u; func(); }\n"
+ "}\n"
+ "} // namespace nb\n"
+ "} // namespace na\n";
+
+  std::string Expected = "namespace util {\n"
+ "class Util {};\n"
+ "void func() {}\n"
+ "} // namespace util\n"
+ "\n"
+ "namespace x {\n"
+ "namespace y {\n"
+ "namespace {\n"
+ "using ::util::Util;\n"
+ "using ::util::func;\n"
+ "void f() { Util u; func(); }\n"
+ "}\n"
  "} // namespace y\n"
  "} // namespace x\n";
   EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));


Index: clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp
===
--- clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp
+++ clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp
@@ -478,13 +478,13 @@
 hasAncestor(namespaceDecl(isAnonymo

[PATCH] D44517: [change-namespace] Don't match a function call/ref multiple times.

2018-03-15 Thread Eric Liu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE327629: [change-namespace] Don't match a function 
call/ref multiple times. (authored by ioeric, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D44517?vs=138550&id=138556#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D44517

Files:
  change-namespace/ChangeNamespace.cpp
  unittests/change-namespace/ChangeNamespaceTests.cpp


Index: change-namespace/ChangeNamespace.cpp
===
--- change-namespace/ChangeNamespace.cpp
+++ change-namespace/ChangeNamespace.cpp
@@ -478,13 +478,13 @@
 hasAncestor(namespaceDecl(isAnonymous())),
 hasAncestor(cxxRecordDecl(,
hasParent(namespaceDecl()));
-  Finder->addMatcher(decl(forEachDescendant(expr(anyOf(
-  callExpr(callee(FuncMatcher)).bind("call"),
-  declRefExpr(to(FuncMatcher.bind("func_decl")))
-  .bind("func_ref",
-  IsInMovedNs, unless(isImplicit()))
- .bind("dc"),
- this);
+  Finder->addMatcher(
+  expr(allOf(hasAncestor(decl().bind("dc")), IsInMovedNs,
+ unless(hasAncestor(isImplicit())),
+ anyOf(callExpr(callee(FuncMatcher)).bind("call"),
+   declRefExpr(to(FuncMatcher.bind("func_decl")))
+   .bind("func_ref",
+  this);
 
   auto GlobalVarMatcher = varDecl(
   hasGlobalStorage(), hasParent(namespaceDecl()),
Index: unittests/change-namespace/ChangeNamespaceTests.cpp
===
--- unittests/change-namespace/ChangeNamespaceTests.cpp
+++ unittests/change-namespace/ChangeNamespaceTests.cpp
@@ -850,22 +850,58 @@
 TEST_F(ChangeNamespaceTest, UsingShadowDeclInGlobal) {
   std::string Code = "namespace glob {\n"
  "class Glob {};\n"
+ "void GFunc() {}\n"
  "}\n"
  "using glob::Glob;\n"
+ "using glob::GFunc;\n"
  "namespace na {\n"
  "namespace nb {\n"
- "void f() { Glob g; }\n"
+ "void f() { Glob g; GFunc(); }\n"
  "} // namespace nb\n"
  "} // namespace na\n";
 
   std::string Expected = "namespace glob {\n"
  "class Glob {};\n"
+ "void GFunc() {}\n"
  "}\n"
  "using glob::Glob;\n"
+ "using glob::GFunc;\n"
  "\n"
  "namespace x {\n"
  "namespace y {\n"
- "void f() { Glob g; }\n"
+ "void f() { Glob g; GFunc(); }\n"
+ "} // namespace y\n"
+ "} // namespace x\n";
+  EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
+}
+
+TEST_F(ChangeNamespaceTest, UsingShadowDeclsInAnonymousNamespaces) {
+  std::string Code = "namespace util {\n"
+ "class Util {};\n"
+ "void func() {}\n"
+ "}\n"
+ "namespace na {\n"
+ "namespace nb {\n"
+ "namespace {\n"
+ "using ::util::Util;\n"
+ "using ::util::func;\n"
+ "void f() { Util u; func(); }\n"
+ "}\n"
+ "} // namespace nb\n"
+ "} // namespace na\n";
+
+  std::string Expected = "namespace util {\n"
+ "class Util {};\n"
+ "void func() {}\n"
+ "} // namespace util\n"
+ "\n"
+ "namespace x {\n"
+ "namespace y {\n"
+ "namespace {\n"
+ "using ::util::Util;\n"
+ "using ::util::func;\n"
+ "void f() { Util u; func(); }\n"
+ "}\n"
  "} // namespace y\n"
  "} // namespace x\n";
   EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));


Index: change-namespace/ChangeNamespace.cpp
===
--- change-namespace/ChangeNamespace.cpp
+++ change-namespace/ChangeNamespace.cpp
@@ -478,13 +478,13 @@
 hasAncestor(namespaceDecl(isAnonymous())),
 hasAncestor(cxxRecordDecl(,
hasParent(namespaceDecl()));
-  Finder->addMatcher(decl(forEachDescendant(expr(anyOf(
-  callExpr(c

[clang-tools-extra] r327629 - [change-namespace] Don't match a function call/ref multiple times.

2018-03-15 Thread Eric Liu via cfe-commits
Author: ioeric
Date: Thu Mar 15 07:45:02 2018
New Revision: 327629

URL: http://llvm.org/viewvc/llvm-project?rev=327629&view=rev
Log:
[change-namespace] Don't match a function call/ref multiple times.

Summary:
Previously, the matcher matches a function call/ref multiple times, one
for each decl ancestor. This might cause problems. For example, in the following
case, `func()` would be matched once (with namespace context) before using decl 
is
seen and once after using decl is seeing, which would result in different 
conflicting
replacements as the first match would replace `func` with "ns::func" as it 
doesn't
know about the using decl.

```
namespace x {
namespace {
using ::ns::func;
void f() { func(); }
}
}
```

Switching from `hasDescendant` matching to `hasAncestor` matching solves the
problem.

Reviewers: hokein

Subscribers: klimek, cfe-commits

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

Modified:
clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp
clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp

Modified: clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp?rev=327629&r1=327628&r2=327629&view=diff
==
--- clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp (original)
+++ clang-tools-extra/trunk/change-namespace/ChangeNamespace.cpp Thu Mar 15 
07:45:02 2018
@@ -478,13 +478,13 @@ void ChangeNamespaceTool::registerMatche
 hasAncestor(namespaceDecl(isAnonymous())),
 hasAncestor(cxxRecordDecl(,
hasParent(namespaceDecl()));
-  Finder->addMatcher(decl(forEachDescendant(expr(anyOf(
-  callExpr(callee(FuncMatcher)).bind("call"),
-  declRefExpr(to(FuncMatcher.bind("func_decl")))
-  .bind("func_ref",
-  IsInMovedNs, unless(isImplicit()))
- .bind("dc"),
- this);
+  Finder->addMatcher(
+  expr(allOf(hasAncestor(decl().bind("dc")), IsInMovedNs,
+ unless(hasAncestor(isImplicit())),
+ anyOf(callExpr(callee(FuncMatcher)).bind("call"),
+   declRefExpr(to(FuncMatcher.bind("func_decl")))
+   .bind("func_ref",
+  this);
 
   auto GlobalVarMatcher = varDecl(
   hasGlobalStorage(), hasParent(namespaceDecl()),

Modified: 
clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp?rev=327629&r1=327628&r2=327629&view=diff
==
--- clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp 
(original)
+++ clang-tools-extra/trunk/unittests/change-namespace/ChangeNamespaceTests.cpp 
Thu Mar 15 07:45:02 2018
@@ -850,22 +850,58 @@ TEST_F(ChangeNamespaceTest, CommentsBefo
 TEST_F(ChangeNamespaceTest, UsingShadowDeclInGlobal) {
   std::string Code = "namespace glob {\n"
  "class Glob {};\n"
+ "void GFunc() {}\n"
  "}\n"
  "using glob::Glob;\n"
+ "using glob::GFunc;\n"
  "namespace na {\n"
  "namespace nb {\n"
- "void f() { Glob g; }\n"
+ "void f() { Glob g; GFunc(); }\n"
  "} // namespace nb\n"
  "} // namespace na\n";
 
   std::string Expected = "namespace glob {\n"
  "class Glob {};\n"
+ "void GFunc() {}\n"
  "}\n"
  "using glob::Glob;\n"
+ "using glob::GFunc;\n"
  "\n"
  "namespace x {\n"
  "namespace y {\n"
- "void f() { Glob g; }\n"
+ "void f() { Glob g; GFunc(); }\n"
+ "} // namespace y\n"
+ "} // namespace x\n";
+  EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
+}
+
+TEST_F(ChangeNamespaceTest, UsingShadowDeclsInAnonymousNamespaces) {
+  std::string Code = "namespace util {\n"
+ "class Util {};\n"
+ "void func() {}\n"
+ "}\n"
+ "namespace na {\n"
+ "namespace nb {\n"
+ "namespace {\n"
+ "using ::util::Util;\n"
+ "using ::util::func;\n"
+ "void f() { Util u; func(); }\n"
+ "}\n"
+ "} // namespa

[PATCH] D44517: [change-namespace] Don't match a function call/ref multiple times.

2018-03-15 Thread Haojian Wu via Phabricator via cfe-commits
hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.

LGTM.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44517



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


[PATCH] D44408: Move DraftMgr from ClangdServer to ClangdLSPServer

2018-03-15 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added inline comments.



Comment at: clangd/ClangdLSPServer.cpp:495
+llvm::Optional ClangdLSPServer::getDocument(PathRef File) {
+  llvm::Optional Contents = DraftMgr.getDraft(File);
+  if (!Contents)

This function is equivalent to `return DraftMgr.getDraft()`. Remove and replace 
all usages with `DraftMgr.getDraft()`?



Comment at: clangd/ClangdServer.cpp:154
  llvm::Expected IP) {
 assert(IP && "error when trying to read preamble for codeComplete");
 auto PreambleData = IP->Preamble;

Please replace this assert with:
```
if (!IP)
  return CB(IP.takeError());
```

(Otherwise the assert will fire when completion is called for non-tracked 
files, we really want this to be an error instead)



Comment at: clangd/ClangdServer.h:121
   /// compilation database even if they were cached in previous invocations.
-  void addDocument(PathRef File, StringRef Contents,
+  void addDocument(PathRef File, std::string Contents,
WantDiagnostics WD = WantDiagnostics::Auto,

Let's keep it a `StringRef`. It means an extra copy for now, `DraftMgr` return 
`StringRef`s instead if we remove mutexes from its interface.



Comment at: clangd/ClangdServer.h:225
+  /// Document contents with a version of this content.
+  struct VersionedContents {
+DocVersion Version;

We don't use `VersionedContents` anywhere. Maybe remove this struct?



Comment at: clangd/ClangdServer.h:236
   FileSystemProvider &FSProvider;
-  DraftStore DraftMgr;
+  llvm::StringMap InternalVersion;
   // The index used to look up symbols. This could be:

This is probably a good place for a small comment:
```
/// Used to synchronize diagnostic responses for added and removed files.
```



Comment at: clangd/DraftStore.h:36
+  void
+  forEachActiveDraft(std::function Callback) const;
 

Let's avoid calling `Callback` while holding a lock, it is very easy to come up 
with a code that deadlocks.

I suggest either keeping the `getActiveFiles()` function or  removing the mutex 
and the locks. IIRC, the `DraftStore` is never  actually used concurrently, so 
removing mutexes should be no-op change.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44408



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


[PATCH] D44517: [change-namespace] Don't match a function call/ref multiple times.

2018-03-15 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 138550.
ioeric added a comment.

- small fix.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44517

Files:
  change-namespace/ChangeNamespace.cpp
  unittests/change-namespace/ChangeNamespaceTests.cpp


Index: unittests/change-namespace/ChangeNamespaceTests.cpp
===
--- unittests/change-namespace/ChangeNamespaceTests.cpp
+++ unittests/change-namespace/ChangeNamespaceTests.cpp
@@ -850,22 +850,58 @@
 TEST_F(ChangeNamespaceTest, UsingShadowDeclInGlobal) {
   std::string Code = "namespace glob {\n"
  "class Glob {};\n"
+ "void GFunc() {}\n"
  "}\n"
  "using glob::Glob;\n"
+ "using glob::GFunc;\n"
  "namespace na {\n"
  "namespace nb {\n"
- "void f() { Glob g; }\n"
+ "void f() { Glob g; GFunc(); }\n"
  "} // namespace nb\n"
  "} // namespace na\n";
 
   std::string Expected = "namespace glob {\n"
  "class Glob {};\n"
+ "void GFunc() {}\n"
  "}\n"
  "using glob::Glob;\n"
+ "using glob::GFunc;\n"
  "\n"
  "namespace x {\n"
  "namespace y {\n"
- "void f() { Glob g; }\n"
+ "void f() { Glob g; GFunc(); }\n"
+ "} // namespace y\n"
+ "} // namespace x\n";
+  EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
+}
+
+TEST_F(ChangeNamespaceTest, UsingShadowDeclsInAnonymousNamespaces) {
+  std::string Code = "namespace util {\n"
+ "class Util {};\n"
+ "void func() {}\n"
+ "}\n"
+ "namespace na {\n"
+ "namespace nb {\n"
+ "namespace {\n"
+ "using ::util::Util;\n"
+ "using ::util::func;\n"
+ "void f() { Util u; func(); }\n"
+ "}\n"
+ "} // namespace nb\n"
+ "} // namespace na\n";
+
+  std::string Expected = "namespace util {\n"
+ "class Util {};\n"
+ "void func() {}\n"
+ "} // namespace util\n"
+ "\n"
+ "namespace x {\n"
+ "namespace y {\n"
+ "namespace {\n"
+ "using ::util::Util;\n"
+ "using ::util::func;\n"
+ "void f() { Util u; func(); }\n"
+ "}\n"
  "} // namespace y\n"
  "} // namespace x\n";
   EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
Index: change-namespace/ChangeNamespace.cpp
===
--- change-namespace/ChangeNamespace.cpp
+++ change-namespace/ChangeNamespace.cpp
@@ -478,13 +478,13 @@
 hasAncestor(namespaceDecl(isAnonymous())),
 hasAncestor(cxxRecordDecl(,
hasParent(namespaceDecl()));
-  Finder->addMatcher(decl(forEachDescendant(expr(anyOf(
-  callExpr(callee(FuncMatcher)).bind("call"),
-  declRefExpr(to(FuncMatcher.bind("func_decl")))
-  .bind("func_ref",
-  IsInMovedNs, unless(isImplicit()))
- .bind("dc"),
- this);
+  Finder->addMatcher(
+  expr(allOf(hasAncestor(decl().bind("dc")), IsInMovedNs,
+ unless(hasAncestor(isImplicit())),
+ anyOf(callExpr(callee(FuncMatcher)).bind("call"),
+   declRefExpr(to(FuncMatcher.bind("func_decl")))
+   .bind("func_ref",
+  this);
 
   auto GlobalVarMatcher = varDecl(
   hasGlobalStorage(), hasParent(namespaceDecl()),


Index: unittests/change-namespace/ChangeNamespaceTests.cpp
===
--- unittests/change-namespace/ChangeNamespaceTests.cpp
+++ unittests/change-namespace/ChangeNamespaceTests.cpp
@@ -850,22 +850,58 @@
 TEST_F(ChangeNamespaceTest, UsingShadowDeclInGlobal) {
   std::string Code = "namespace glob {\n"
  "class Glob {};\n"
+ "void GFunc() {}\n"
  "}\n"
  "using glob::Glob;\n"
+ "using glob::GFunc;\n"
  "namespace na {\n"
  "namespace nb {\n"
- "void f() { Glob g; }\n"
+   

[PATCH] D44517: [change-namespace] Don't match a function call/ref multiple times.

2018-03-15 Thread Eric Liu via Phabricator via cfe-commits
ioeric created this revision.
ioeric added a reviewer: hokein.
Herald added subscribers: cfe-commits, klimek.

Previously, the matcher matches a function call/ref multiple times, one
for each decl ancestor. This might cause problems. For example, in the following
case, `func()` would be matched once (with namespace context) before using decl 
is
seen and once after using decl is seeing, which would result in different 
conflicting
replacements as the first match would replace `func` with "ns::func" as it 
doesn't
know about the using decl.

  namespace x {
  namespace {
  using ::ns::func;
  void f() { func(); }
  }
  }

Switching from `hasDescendant` matching to `hasAncestor` matching solves the
problem.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44517

Files:
  change-namespace/ChangeNamespace.cpp
  unittests/change-namespace/ChangeNamespaceTests.cpp


Index: unittests/change-namespace/ChangeNamespaceTests.cpp
===
--- unittests/change-namespace/ChangeNamespaceTests.cpp
+++ unittests/change-namespace/ChangeNamespaceTests.cpp
@@ -850,22 +850,58 @@
 TEST_F(ChangeNamespaceTest, UsingShadowDeclInGlobal) {
   std::string Code = "namespace glob {\n"
  "class Glob {};\n"
+ "void GFunc() {}\n"
  "}\n"
  "using glob::Glob;\n"
+ "using glob::GFunc;\n"
  "namespace na {\n"
  "namespace nb {\n"
- "void f() { Glob g; }\n"
+ "void f() { Glob g; GFunc(); }\n"
  "} // namespace nb\n"
  "} // namespace na\n";
 
   std::string Expected = "namespace glob {\n"
  "class Glob {};\n"
+ "void GFunc() {}\n"
  "}\n"
  "using glob::Glob;\n"
+ "using glob::GFunc;\n"
  "\n"
  "namespace x {\n"
  "namespace y {\n"
- "void f() { Glob g; }\n"
+ "void f() { Glob g; GFunc(); }\n"
+ "} // namespace y\n"
+ "} // namespace x\n";
+  EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
+}
+
+TEST_F(ChangeNamespaceTest, UsingShadowDeclsInAnonymousNamespaces) {
+  std::string Code = "namespace util {\n"
+ "class Util {};\n"
+ "void func() {}\n"
+ "}\n"
+ "namespace na {\n"
+ "namespace nb {\n"
+ "namespace {\n"
+ "using ::util::Util;\n"
+ "using ::util::func;\n"
+ "void f() { Util u; func(); }\n"
+ "}\n"
+ "} // namespace nb\n"
+ "} // namespace na\n";
+
+  std::string Expected = "namespace util {\n"
+ "class Util {};\n"
+ "void func() {}\n"
+ "} // namespace util\n"
+ "\n"
+ "namespace x {\n"
+ "namespace y {\n"
+ "namespace {\n"
+ "using ::util::Util;\n"
+ "using ::util::func;\n"
+ "void f() { Util u; func(); }\n"
+ "}\n"
  "} // namespace y\n"
  "} // namespace x\n";
   EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
Index: change-namespace/ChangeNamespace.cpp
===
--- change-namespace/ChangeNamespace.cpp
+++ change-namespace/ChangeNamespace.cpp
@@ -478,13 +478,13 @@
 hasAncestor(namespaceDecl(isAnonymous())),
 hasAncestor(cxxRecordDecl(,
hasParent(namespaceDecl()));
-  Finder->addMatcher(decl(forEachDescendant(expr(anyOf(
-  callExpr(callee(FuncMatcher)).bind("call"),
-  declRefExpr(to(FuncMatcher.bind("func_decl")))
-  .bind("func_ref",
-  IsInMovedNs, unless(isImplicit()))
- .bind("dc"),
- this);
+  Finder->addMatcher(
+  expr(allOf(hasAncestor(decl().bind("dc")), IsInMovedNs,
+ unless(hasAncestor(isImplicit())),
+ anyOf(callExpr(callee(FuncMatcher)).bind("call"),
+   declRefExpr(to(FuncMatcher.bind("func_decl")))
+   .bind("func_ref",
+  this);
 
   auto GlobalVarMatcher = varDecl(
   hasGlobalStorage(), hasParent(namespaceDecl()),
@@ -509,6 +509,7 @@
 void ChangeNamespaceTool::run(
 const ast_

[PATCH] D36892: [clang-tidy] check_clang_tidy.py: support CHECK-NOTES prefix

2018-03-15 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment.

It looks like some existing tests could benefit from this:

  $ grep -R -l 'note: ' test/clang-tidy/
  test/clang-tidy/bugprone-forward-declaration-namespace.cpp
  test/clang-tidy/llvm-twine-local.cpp
  test/clang-tidy/overlapping.cpp
  test/clang-tidy/google-readability-nested-namespace-comments.cpp
  test/clang-tidy/bugprone-suspicious-enum-usage-strict.cpp
  test/clang-tidy/cert-static-object-exception.cpp
  test/clang-tidy/fix.cpp
  test/clang-tidy/google-readability-namespace-comments.cpp
  test/clang-tidy/cppcoreguidelines-avoid-goto.cpp
  test/clang-tidy/performance-move-constructor-init.cpp
  test/clang-tidy/fuchsia-default-arguments.cpp
  test/clang-tidy/readability-misleading-indentation.cpp
  test/clang-tidy/readability-inconsistent-declaration-parameter-name-macros.cpp
  test/clang-tidy/fix-errors.cpp
  test/clang-tidy/bugprone-use-after-move.cpp
  test/clang-tidy/cppcoreguidelines-owning-memory-containers.cpp
  test/clang-tidy/bugprone-argument-comment.cpp
  test/clang-tidy/cppcoreguidelines-owning-memory.cpp
  test/clang-tidy/readability-function-size.cpp
  test/clang-tidy/hicpp-exception-baseclass.cpp
  test/clang-tidy/readability-inconsistent-declaration-parameter-name.cpp
  test/clang-tidy/macros.cpp
  test/clang-tidy/readability-container-size-empty.cpp
  test/clang-tidy/Inputs/overlapping/o.h
  test/clang-tidy/bugprone-forwarding-reference-overload.cpp

If you're interested in changing some of these tests to use CHECK-NOTES, 
there's no need to wait for a resolution on https://reviews.llvm.org/D36836 
(which, unfortunately, may take really long). However, please update the 
relevant section of docs/clang-tidy/index.rst.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D36892



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


[PATCH] D44512: [AAch64] Tests for ACLE FP16 macros

2018-03-15 Thread Sjoerd Meijer via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL327623: [AAch64] Tests for ACLE FP16 macros (authored by 
SjoerdMeijer, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D44512?vs=138521&id=138541#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D44512

Files:
  cfe/trunk/test/Preprocessor/aarch64-target-features.c


Index: cfe/trunk/test/Preprocessor/aarch64-target-features.c
===
--- cfe/trunk/test/Preprocessor/aarch64-target-features.c
+++ cfe/trunk/test/Preprocessor/aarch64-target-features.c
@@ -89,6 +89,18 @@
 // RUN: %clang -target aarch64-none-linux-gnu -march=armv8-a+sve -x c -E -dM 
%s -o - | FileCheck --check-prefix=CHECK-SVE %s
 // CHECK-SVE: __ARM_FEATURE_SVE 1
 
+// RUN: %clang -target aarch64-none-linux-gnueabi -march=armv8.2a+fp16 -x c -E 
-dM %s -o - | FileCheck -match-full-lines 
--check-prefix=CHECK-FULLFP16-VECTOR-SCALAR %s
+// CHECK-FULLFP16-VECTOR-SCALAR: #define __ARM_FEATURE_FP16_SCALAR_ARITHMETIC 1
+// CHECK-FULLFP16-VECTOR-SCALAR: #define __ARM_FEATURE_FP16_VECTOR_ARITHMETIC 1
+// CHECK-FULLFP16-VECTOR-SCALAR: #define __ARM_FP 0xE
+// CHECK-FULLFP16-VECTOR-SCALAR: #define __ARM_FP16_FORMAT_IEEE 1
+
+// RUN: %clang -target aarch64-none-linux-gnueabi -march=armv8.2a+fp16+nosimd 
-x c -E -dM %s -o - | FileCheck -match-full-lines 
--check-prefix=CHECK-FULLFP16-SCALAR %s
+// CHECK-FULLFP16-SCALAR:   #define __ARM_FEATURE_FP16_SCALAR_ARITHMETIC 1
+// CHECK-FULLFP16-SCALAR-NOT:   #define __ARM_FEATURE_FP16_VECTOR_ARITHMETIC 1
+// CHECK-FULLFP16-SCALAR:   #define __ARM_FP 0xE
+// CHECK-FULLFP16-SCALAR:   #define __ARM_FP16_FORMAT_IEEE 1
+
 // == Check whether -mtune accepts mixed-case features.
 // RUN: %clang -target aarch64 -mtune=CYCLONE -### -c %s 2>&1 | FileCheck 
-check-prefix=CHECK-MTUNE-CYCLONE %s
 // CHECK-MTUNE-CYCLONE: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" 
"-target-feature" "+neon" "-target-feature" "+zcm" "-target-feature" "+zcz"


Index: cfe/trunk/test/Preprocessor/aarch64-target-features.c
===
--- cfe/trunk/test/Preprocessor/aarch64-target-features.c
+++ cfe/trunk/test/Preprocessor/aarch64-target-features.c
@@ -89,6 +89,18 @@
 // RUN: %clang -target aarch64-none-linux-gnu -march=armv8-a+sve -x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-SVE %s
 // CHECK-SVE: __ARM_FEATURE_SVE 1
 
+// RUN: %clang -target aarch64-none-linux-gnueabi -march=armv8.2a+fp16 -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-VECTOR-SCALAR %s
+// CHECK-FULLFP16-VECTOR-SCALAR: #define __ARM_FEATURE_FP16_SCALAR_ARITHMETIC 1
+// CHECK-FULLFP16-VECTOR-SCALAR: #define __ARM_FEATURE_FP16_VECTOR_ARITHMETIC 1
+// CHECK-FULLFP16-VECTOR-SCALAR: #define __ARM_FP 0xE
+// CHECK-FULLFP16-VECTOR-SCALAR: #define __ARM_FP16_FORMAT_IEEE 1
+
+// RUN: %clang -target aarch64-none-linux-gnueabi -march=armv8.2a+fp16+nosimd -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-SCALAR %s
+// CHECK-FULLFP16-SCALAR:   #define __ARM_FEATURE_FP16_SCALAR_ARITHMETIC 1
+// CHECK-FULLFP16-SCALAR-NOT:   #define __ARM_FEATURE_FP16_VECTOR_ARITHMETIC 1
+// CHECK-FULLFP16-SCALAR:   #define __ARM_FP 0xE
+// CHECK-FULLFP16-SCALAR:   #define __ARM_FP16_FORMAT_IEEE 1
+
 // == Check whether -mtune accepts mixed-case features.
 // RUN: %clang -target aarch64 -mtune=CYCLONE -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-MTUNE-CYCLONE %s
 // CHECK-MTUNE-CYCLONE: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" "+neon" "-target-feature" "+zcm" "-target-feature" "+zcz"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r327623 - [AAch64] Tests for ACLE FP16 macros

2018-03-15 Thread Sjoerd Meijer via cfe-commits
Author: sjoerdmeijer
Date: Thu Mar 15 06:36:30 2018
New Revision: 327623

URL: http://llvm.org/viewvc/llvm-project?rev=327623&view=rev
Log:
[AAch64] Tests for ACLE FP16 macros

This adds some missing tests for the AArch64 FP16 macros.

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

Modified:
cfe/trunk/test/Preprocessor/aarch64-target-features.c

Modified: cfe/trunk/test/Preprocessor/aarch64-target-features.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/aarch64-target-features.c?rev=327623&r1=327622&r2=327623&view=diff
==
--- cfe/trunk/test/Preprocessor/aarch64-target-features.c (original)
+++ cfe/trunk/test/Preprocessor/aarch64-target-features.c Thu Mar 15 06:36:30 
2018
@@ -89,6 +89,18 @@
 // RUN: %clang -target aarch64-none-linux-gnu -march=armv8-a+sve -x c -E -dM 
%s -o - | FileCheck --check-prefix=CHECK-SVE %s
 // CHECK-SVE: __ARM_FEATURE_SVE 1
 
+// RUN: %clang -target aarch64-none-linux-gnueabi -march=armv8.2a+fp16 -x c -E 
-dM %s -o - | FileCheck -match-full-lines 
--check-prefix=CHECK-FULLFP16-VECTOR-SCALAR %s
+// CHECK-FULLFP16-VECTOR-SCALAR: #define __ARM_FEATURE_FP16_SCALAR_ARITHMETIC 1
+// CHECK-FULLFP16-VECTOR-SCALAR: #define __ARM_FEATURE_FP16_VECTOR_ARITHMETIC 1
+// CHECK-FULLFP16-VECTOR-SCALAR: #define __ARM_FP 0xE
+// CHECK-FULLFP16-VECTOR-SCALAR: #define __ARM_FP16_FORMAT_IEEE 1
+
+// RUN: %clang -target aarch64-none-linux-gnueabi -march=armv8.2a+fp16+nosimd 
-x c -E -dM %s -o - | FileCheck -match-full-lines 
--check-prefix=CHECK-FULLFP16-SCALAR %s
+// CHECK-FULLFP16-SCALAR:   #define __ARM_FEATURE_FP16_SCALAR_ARITHMETIC 1
+// CHECK-FULLFP16-SCALAR-NOT:   #define __ARM_FEATURE_FP16_VECTOR_ARITHMETIC 1
+// CHECK-FULLFP16-SCALAR:   #define __ARM_FP 0xE
+// CHECK-FULLFP16-SCALAR:   #define __ARM_FP16_FORMAT_IEEE 1
+
 // == Check whether -mtune accepts mixed-case features.
 // RUN: %clang -target aarch64 -mtune=CYCLONE -### -c %s 2>&1 | FileCheck 
-check-prefix=CHECK-MTUNE-CYCLONE %s
 // CHECK-MTUNE-CYCLONE: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" 
"-target-feature" "+neon" "-target-feature" "+zcm" "-target-feature" "+zcz"


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


[PATCH] D44512: [AAch64] Tests for ACLE FP16 macros

2018-03-15 Thread Sjoerd Meijer via Phabricator via cfe-commits
SjoerdMeijer added a comment.

Thanks for reviewing!


https://reviews.llvm.org/D44512



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


[PATCH] D44352: [Concepts] Constrained template parameters

2018-03-15 Thread Saar Raz via Phabricator via cfe-commits
saar.raz updated this revision to Diff 138537.
saar.raz added a comment.

Fixed test.


Repository:
  rC Clang

https://reviews.llvm.org/D44352

Files:
  include/clang/AST/DeclTemplate.h
  include/clang/AST/RecursiveASTVisitor.h
  include/clang/AST/TemplateBase.h
  include/clang/Basic/DiagnosticParseKinds.td
  include/clang/Parse/Parser.h
  include/clang/Sema/Sema.h
  lib/AST/ASTContext.cpp
  lib/AST/ASTDumper.cpp
  lib/AST/ASTImporter.cpp
  lib/AST/DeclTemplate.cpp
  lib/AST/ODRHash.cpp
  lib/Parse/ParseExprCXX.cpp
  lib/Parse/ParseTemplate.cpp
  lib/Sema/SemaCXXScopeSpec.cpp
  lib/Sema/SemaConcept.cpp
  lib/Sema/SemaTemplate.cpp
  lib/Sema/SemaTemplateDeduction.cpp
  lib/Sema/SemaTemplateInstantiateDecl.cpp
  lib/Serialization/ASTReader.cpp
  lib/Serialization/ASTReaderDecl.cpp
  lib/Serialization/ASTWriter.cpp
  lib/Serialization/ASTWriterDecl.cpp
  test/CXX/concepts-ts/temp/temp.constr/temp.constr.decl/class-template-decl.cpp
  test/CXX/concepts-ts/temp/temp.param/p10.cpp
  test/Parser/cxx-constrained-template-param-with-partial-id.cpp
  test/Parser/cxx-constrained-template-param.cpp
  tools/libclang/CIndex.cpp

Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -750,6 +750,10 @@
 }
 
 bool CursorVisitor::VisitTemplateTypeParmDecl(TemplateTypeParmDecl *D) {
+  if (Expr *CE = D->getConstraintExpression())
+if (Visit(MakeCXCursor(CE, StmtParent, TU, RegionOfInterest)))
+  return true;
+
   // Visit the default argument.
   if (D->hasDefaultArgument() && !D->defaultArgumentWasInherited())
 if (TypeSourceInfo *DefArg = D->getDefaultArgumentInfo())
@@ -898,6 +902,10 @@
 bool CursorVisitor::VisitNonTypeTemplateParmDecl(NonTypeTemplateParmDecl *D) {
   if (VisitDeclaratorDecl(D))
 return true;
+
+  if (Expr *CE = D->getConstraintExpression())
+if (Visit(MakeCXCursor(CE, StmtParent, TU, RegionOfInterest)))
+  return true;
   
   if (D->hasDefaultArgument() && !D->defaultArgumentWasInherited())
 if (Expr *DefArg = D->getDefaultArgument())
@@ -929,7 +937,11 @@
 bool CursorVisitor::VisitTemplateTemplateParmDecl(TemplateTemplateParmDecl *D) {
   if (VisitTemplateParameters(D->getTemplateParameters()))
 return true;
-  
+
+  if (Expr *CE = D->getConstraintExpression())
+if (Visit(MakeCXCursor(CE, StmtParent, TU, RegionOfInterest)))
+  return true;
+
   if (D->hasDefaultArgument() && !D->defaultArgumentWasInherited() &&
   VisitTemplateArgumentLoc(D->getDefaultArgument()))
 return true;
Index: test/Parser/cxx-constrained-template-param.cpp
===
--- /dev/null
+++ test/Parser/cxx-constrained-template-param.cpp
@@ -0,0 +1,90 @@
+// RUN: %clang_cc1 -std=c++2a -fconcepts-ts -x c++ %s -verify
+// expected-no-diagnostics
+
+namespace type
+{
+  template
+  concept C1 = true;
+
+  template
+  using A = T[10];
+
+  using a = A;
+
+  namespace ns {
+template
+concept C2 = true;
+  }
+
+  template requires sizeof(T1) <= sizeof(T2)
+  struct B { };
+
+  using b = B;
+
+  template
+  struct C { };
+
+  using c1 = C;
+  using c2 = C;
+}
+
+namespace non_type
+{
+  template
+  concept C1 = true;
+
+  template
+  int A = v;
+
+  int& a = A<1>;
+
+  namespace ns {
+template
+concept C2 = true;
+  }
+
+  template requires sizeof(v1) <= sizeof(v2)
+  struct B { };
+
+  using b = B;
+
+  template
+  struct C { };
+
+  using c1 = C;
+  using c2 = C;
+}
+
+namespace temp
+{
+  template
+  struct test1 { };
+
+  template
+  struct test2 { };
+
+  template typename T>
+  concept C1 = true;
+
+  template
+  using A = TT;
+
+  using a = A;
+
+  namespace ns {
+template typename... TT>
+concept C2 = true;
+  }
+
+  template
+requires sizeof(TT1) <= sizeof(TT2)
+  struct B { };
+
+  using b = B;
+
+  template
+  struct C { };
+
+  using c1 = C;
+  using c2 = C;
+}
\ No newline at end of file
Index: test/Parser/cxx-constrained-template-param-with-partial-id.cpp
===
--- /dev/null
+++ test/Parser/cxx-constrained-template-param-with-partial-id.cpp
@@ -0,0 +1,36 @@
+// RUN: %clang_cc1 -std=c++2a -fconcepts-ts -x c++ %s -verify
+
+template
+concept C1 = true;
+
+template // expected-error {{concept 'C1' requires more than 1 template argument; provide the remaining arguments explicitly to use it here}}
+using badA = T[10];
+
+template T>
+using A = T[10];
+
+using a = A;
+
+namespace ns {
+  template
+  concept C2 = true;
+}
+
+template // expected-error 2{{concept 'C2' requires more than 1 template argument; provide the remaining arguments explicitly to use it here}}
+requires sizeof(T1) <= sizeof(T2)
+struct badB { };
+
+template T1, ::ns::C2 T2>
+  requires sizeof(T1) <= sizeof(T2)
+struct B { };
+
+using b = B;
+
+template // expected-error {{concept 'C2' requires more than 1 template argument; provide the remaining arguments explicitly

[PATCH] D44512: [AAch64] Tests for ACLE FP16 macros

2018-03-15 Thread Tim Northover via Phabricator via cfe-commits
t.p.northover accepted this revision.
t.p.northover added a comment.
This revision is now accepted and ready to land.

These look right to me.


https://reviews.llvm.org/D44512



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


[PATCH] D23130: [Clang-tidy] Add a check for definitions in the global namespace.

2018-03-15 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment.

In https://reviews.llvm.org/D23130#1037256, @bkramer wrote:

> I'd like to, but I don't know when I find time to rebase this thing after 
> more than a year of waiting for review.


Sorry, it was just lost ¯\_(ツ)_/¯. I may be relying on pings from the other 
side too much.


https://reviews.llvm.org/D23130



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


[PATCH] D43821: [SemaCXX] _Pragma("clang optimize off") not affecting lambda.

2018-03-15 Thread Carlos Alberto Enciso via Phabricator via cfe-commits
CarlosAlbertoEnciso added a comment.

Ping.

Thanks


Repository:
  rC Clang

https://reviews.llvm.org/D43821



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


[PATCH] D43764: [clang-apply-replacements] Convert tooling::Replacements to tooling::AtomicChange for conflict resolving of changes, code cleanup, and code formatting.

2018-03-15 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments.



Comment at: clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp:353
+const FileEntry *Entry = FileAndReplacements.first;
+ReplacementsToAtomicChanges DeduplicatedChanges(SM, Entry);
+for (const auto &R : FileAndReplacements.second)

jdemeule wrote:
> ioeric wrote:
> > jdemeule wrote:
> > > ioeric wrote:
> > > > Sorry that I didn't make myself clear... what you had in the previous 
> > > > revision was correct (for the current use case of 
> > > > apply-all-replacements) i.e. store all replacements in one 
> > > > `AtomicChange`, which allows you to detect conflicts on the fly. So the 
> > > > code here can be simplified as:
> > > > 
> > > > ```
> > > > ...
> > > > Entry = ...;
> > > > AtomicChange FileChange;
> > > > for (const auto &R : FileAndReplacements.second) {
> > > >   auto Err = FileChange.replace(  );
> > > >   if (Err)
> > > > reportConflict(Entry, std::move(Err));  // reportConflict as we go
> > > > }
> > > > FileChanges.emplace(Entry, {FileChange});
> > > > ...
> > > > ```
> > > > 
> > > > I think with this set up, you wouldn't need 
> > > > `ReplacementsToAtomicChanges`, `ConflictError` and `reportConflicts`.
> > > I think I have over-interpreting your previous comment :-)
> > > However, I am still confused about error reporting.
> > > Currently, clang-apply-replacements reports conflicting replacement per 
> > > file and per conflict.
> > > For example:
> > > ```
> > > There are conflicting changes to XXX:
> > > The following changes conflict:
> > >   Replace 8:8-8:33 with "AAA"
> > >   Replace 8:8-8:33 with "BBB"
> > >   Remove 8:10-8:15
> > >   Insert at 8:14 CCC
> > > ```
> > > 
> > > With this patch, conflict are reported by pair (first 
> > > replacement/conflicted one) and I am able to print the corresponding file 
> > > only once (thanks to the defered reporting).
> > > ```
> > > There are conflicting changes to XXX:
> > > The following changes conflict:
> > >   Replace 8:8-8:33 with "AAA"
> > >   Replace 8:8-8:33 with "BBB"
> > > The following changes conflict:
> > >   Replace 8:8-8:33 with "AAA"
> > >   Remove 8:10-8:15
> > > The following changes conflict:
> > >   Replace 8:8-8:33 with "AAA"
> > >   Insert at 8:14 CCC
> > > ```
> > > 
> > > I prefer the way you suggest to report conflict but we will loose or 
> > > print conflicting file at each conflict detected.
> > > I even more prefer to use `llvm::toString(std::move(Err))` but this will 
> > > fully change the reporting and will also be reported by pair.
> > > ```
> > > The new replacement overlaps with an existing replacement.
> > > New replacement: XXX: 106:+26:"AAA"
> > > Existing replacement: XXX: 106:+26:"BBB"
> > > The new replacement overlaps with an existing replacement.
> > > New replacement: XXX: 106:+26:"AAA"
> > > Existing replacement: XXX: 112:+0:"CCC"
> > > The new replacement overlaps with an existing replacement.
> > > New replacement: XXX: 106:+26:"AAA"
> > > Existing replacement: XXX: 108:+12:""
> > > ```
> > I am inclined to changing the current behavior and reporting by pairs 
> > because
> > 1) this would simplify the code a lot.because we could basically reuse the 
> > conflict detection from replacement library.
> > 2) IMO, pairs are easier to read than grouping multiple conflicts - users 
> > would still need to interpret the conflicts and figure out how all 
> > replacements conflict in a group, while reporting as pairs already gives 
> > you this information.
> > 3) in practice, it's uncommon to see more than two conflicting replacements 
> > at the same code location.
> > 
> > I would vote for the last approach (with `llvm::toString(std::move(Err))`) 
> > which is easy to implement and doesn't really regress the `UI` that much. 
> > WDYT?
> I think if we can use `llvm::toString(std::move(Err))` for this patch, it 
> will simplify the code and reuse already existing error message. Even if I 
> found file+offset conflict location less readable than file+line+column.
> 
> I have made some prototype to "enhance" error reporting but I think they 
> should be put in dedicated patches and need further discutions.
> During my "research" I found we can use:
> * Conflict marker format.
> ```
> /src/basic.h
> @@ 12:23-12:23 @@
> <<< Existing replacement
>   virtual void func() noexcept {}
> ===
>   virtual void func() override {}
> >>> New replacement
> ```
> * A unified diff like.
> ```
> /src/basic.h
> @@ 12:23-12:23 @@
> -   virtual void func() {}
> +   virtual void func() noexcept {}
> +   virtual void func() override {}
> ```
> * A clang like diagnostic message.
> ```
> /src/basic.h
> @@ 12:23-12:23 @@
>   virtual void func() {}
>   ^
>   noexcept 
>   virtual void func() {}
>   ^
>   override 
> ```
> 
The conflict marker format looks promising!

You are right regarding offset vs line+column location. I agree offset is less 
rea

[PATCH] D43764: [clang-apply-replacements] Convert tooling::Replacements to tooling::AtomicChange for conflict resolving of changes, code cleanup, and code formatting.

2018-03-15 Thread Jeremy Demeule via Phabricator via cfe-commits
jdemeule added inline comments.



Comment at: clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp:353
+const FileEntry *Entry = FileAndReplacements.first;
+ReplacementsToAtomicChanges DeduplicatedChanges(SM, Entry);
+for (const auto &R : FileAndReplacements.second)

ioeric wrote:
> jdemeule wrote:
> > ioeric wrote:
> > > Sorry that I didn't make myself clear... what you had in the previous 
> > > revision was correct (for the current use case of apply-all-replacements) 
> > > i.e. store all replacements in one `AtomicChange`, which allows you to 
> > > detect conflicts on the fly. So the code here can be simplified as:
> > > 
> > > ```
> > > ...
> > > Entry = ...;
> > > AtomicChange FileChange;
> > > for (const auto &R : FileAndReplacements.second) {
> > >   auto Err = FileChange.replace(  );
> > >   if (Err)
> > > reportConflict(Entry, std::move(Err));  // reportConflict as we go
> > > }
> > > FileChanges.emplace(Entry, {FileChange});
> > > ...
> > > ```
> > > 
> > > I think with this set up, you wouldn't need 
> > > `ReplacementsToAtomicChanges`, `ConflictError` and `reportConflicts`.
> > I think I have over-interpreting your previous comment :-)
> > However, I am still confused about error reporting.
> > Currently, clang-apply-replacements reports conflicting replacement per 
> > file and per conflict.
> > For example:
> > ```
> > There are conflicting changes to XXX:
> > The following changes conflict:
> >   Replace 8:8-8:33 with "AAA"
> >   Replace 8:8-8:33 with "BBB"
> >   Remove 8:10-8:15
> >   Insert at 8:14 CCC
> > ```
> > 
> > With this patch, conflict are reported by pair (first 
> > replacement/conflicted one) and I am able to print the corresponding file 
> > only once (thanks to the defered reporting).
> > ```
> > There are conflicting changes to XXX:
> > The following changes conflict:
> >   Replace 8:8-8:33 with "AAA"
> >   Replace 8:8-8:33 with "BBB"
> > The following changes conflict:
> >   Replace 8:8-8:33 with "AAA"
> >   Remove 8:10-8:15
> > The following changes conflict:
> >   Replace 8:8-8:33 with "AAA"
> >   Insert at 8:14 CCC
> > ```
> > 
> > I prefer the way you suggest to report conflict but we will loose or print 
> > conflicting file at each conflict detected.
> > I even more prefer to use `llvm::toString(std::move(Err))` but this will 
> > fully change the reporting and will also be reported by pair.
> > ```
> > The new replacement overlaps with an existing replacement.
> > New replacement: XXX: 106:+26:"AAA"
> > Existing replacement: XXX: 106:+26:"BBB"
> > The new replacement overlaps with an existing replacement.
> > New replacement: XXX: 106:+26:"AAA"
> > Existing replacement: XXX: 112:+0:"CCC"
> > The new replacement overlaps with an existing replacement.
> > New replacement: XXX: 106:+26:"AAA"
> > Existing replacement: XXX: 108:+12:""
> > ```
> I am inclined to changing the current behavior and reporting by pairs because
> 1) this would simplify the code a lot.because we could basically reuse the 
> conflict detection from replacement library.
> 2) IMO, pairs are easier to read than grouping multiple conflicts - users 
> would still need to interpret the conflicts and figure out how all 
> replacements conflict in a group, while reporting as pairs already gives you 
> this information.
> 3) in practice, it's uncommon to see more than two conflicting replacements 
> at the same code location.
> 
> I would vote for the last approach (with `llvm::toString(std::move(Err))`) 
> which is easy to implement and doesn't really regress the `UI` that much. 
> WDYT?
I think if we can use `llvm::toString(std::move(Err))` for this patch, it will 
simplify the code and reuse already existing error message. Even if I found 
file+offset conflict location less readable than file+line+column.

I have made some prototype to "enhance" error reporting but I think they should 
be put in dedicated patches and need further discutions.
During my "research" I found we can use:
* Conflict marker format.
```
/src/basic.h
@@ 12:23-12:23 @@
<<< Existing replacement
  virtual void func() noexcept {}
===
  virtual void func() override {}
>>> New replacement
```
* A unified diff like.
```
/src/basic.h
@@ 12:23-12:23 @@
-   virtual void func() {}
+   virtual void func() noexcept {}
+   virtual void func() override {}
```
* A clang like diagnostic message.
```
/src/basic.h
@@ 12:23-12:23 @@
  virtual void func() {}
  ^
  noexcept 
  virtual void func() {}
  ^
  override 
```



https://reviews.llvm.org/D43764



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


[PATCH] D44512: [AAch64] Tests for ACLE FP16 macros

2018-03-15 Thread Sjoerd Meijer via Phabricator via cfe-commits
SjoerdMeijer created this revision.
SjoerdMeijer added reviewers: samparker, olista01, evandro, az.
Herald added subscribers: kristof.beyls, javed.absar.

This adds some missing tests for the AArch64 FP16 macros.


https://reviews.llvm.org/D44512

Files:
  test/Preprocessor/aarch64-target-features.c


Index: test/Preprocessor/aarch64-target-features.c
===
--- test/Preprocessor/aarch64-target-features.c
+++ test/Preprocessor/aarch64-target-features.c
@@ -89,6 +89,18 @@
 // RUN: %clang -target aarch64-none-linux-gnu -march=armv8-a+sve -x c -E -dM 
%s -o - | FileCheck --check-prefix=CHECK-SVE %s
 // CHECK-SVE: __ARM_FEATURE_SVE 1
 
+// RUN: %clang -target aarch64-none-linux-gnueabi -march=armv8.2a+fp16 -x c -E 
-dM %s -o - | FileCheck -match-full-lines 
--check-prefix=CHECK-FULLFP16-VECTOR-SCALAR %s
+// CHECK-FULLFP16-VECTOR-SCALAR: #define __ARM_FEATURE_FP16_SCALAR_ARITHMETIC 1
+// CHECK-FULLFP16-VECTOR-SCALAR: #define __ARM_FEATURE_FP16_VECTOR_ARITHMETIC 1
+// CHECK-FULLFP16-VECTOR-SCALAR: #define __ARM_FP 0xE
+// CHECK-FULLFP16-VECTOR-SCALAR: #define __ARM_FP16_FORMAT_IEEE 1
+
+// RUN: %clang -target aarch64-none-linux-gnueabi -march=armv8.2a+fp16+nosimd 
-x c -E -dM %s -o - | FileCheck -match-full-lines 
--check-prefix=CHECK-FULLFP16-SCALAR %s
+// CHECK-FULLFP16-SCALAR:   #define __ARM_FEATURE_FP16_SCALAR_ARITHMETIC 1
+// CHECK-FULLFP16-SCALAR-NOT:   #define __ARM_FEATURE_FP16_VECTOR_ARITHMETIC 1
+// CHECK-FULLFP16-SCALAR:   #define __ARM_FP 0xE
+// CHECK-FULLFP16-SCALAR:   #define __ARM_FP16_FORMAT_IEEE 1
+
 // == Check whether -mtune accepts mixed-case features.
 // RUN: %clang -target aarch64 -mtune=CYCLONE -### -c %s 2>&1 | FileCheck 
-check-prefix=CHECK-MTUNE-CYCLONE %s
 // CHECK-MTUNE-CYCLONE: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" 
"-target-feature" "+neon" "-target-feature" "+zcm" "-target-feature" "+zcz"


Index: test/Preprocessor/aarch64-target-features.c
===
--- test/Preprocessor/aarch64-target-features.c
+++ test/Preprocessor/aarch64-target-features.c
@@ -89,6 +89,18 @@
 // RUN: %clang -target aarch64-none-linux-gnu -march=armv8-a+sve -x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-SVE %s
 // CHECK-SVE: __ARM_FEATURE_SVE 1
 
+// RUN: %clang -target aarch64-none-linux-gnueabi -march=armv8.2a+fp16 -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-VECTOR-SCALAR %s
+// CHECK-FULLFP16-VECTOR-SCALAR: #define __ARM_FEATURE_FP16_SCALAR_ARITHMETIC 1
+// CHECK-FULLFP16-VECTOR-SCALAR: #define __ARM_FEATURE_FP16_VECTOR_ARITHMETIC 1
+// CHECK-FULLFP16-VECTOR-SCALAR: #define __ARM_FP 0xE
+// CHECK-FULLFP16-VECTOR-SCALAR: #define __ARM_FP16_FORMAT_IEEE 1
+
+// RUN: %clang -target aarch64-none-linux-gnueabi -march=armv8.2a+fp16+nosimd -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-FULLFP16-SCALAR %s
+// CHECK-FULLFP16-SCALAR:   #define __ARM_FEATURE_FP16_SCALAR_ARITHMETIC 1
+// CHECK-FULLFP16-SCALAR-NOT:   #define __ARM_FEATURE_FP16_VECTOR_ARITHMETIC 1
+// CHECK-FULLFP16-SCALAR:   #define __ARM_FP 0xE
+// CHECK-FULLFP16-SCALAR:   #define __ARM_FP16_FORMAT_IEEE 1
+
 // == Check whether -mtune accepts mixed-case features.
 // RUN: %clang -target aarch64 -mtune=CYCLONE -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-MTUNE-CYCLONE %s
 // CHECK-MTUNE-CYCLONE: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" "+neon" "-target-feature" "+zcm" "-target-feature" "+zcz"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D44435: Add the module name to __cuda_module_ctor and __cuda_module_dtor for unique function names

2018-03-15 Thread Simeon Ehrig via Phabricator via cfe-commits
SimeonEhrig added inline comments.



Comment at: unittests/CodeGen/IncrementalProcessingTest.cpp:176-178
+
+// In CUDA incremental processing, a CUDA ctor or dtor will be generated for 
+// every statement if a fatbinary file exists.

tra wrote:
> SimeonEhrig wrote:
> > tra wrote:
> > > I don't understand the comment. What is 'CUDA incremental processing' and 
> > > what exactly is meant by 'statement' here? I'd appreciate if you could 
> > > give me more details. My understanding is that ctor/dtor are generated 
> > > once per TU. I suspect "incremental processing" may change that, but I 
> > > have no idea what exactly does it do.
> > A CUDA ctor/dtor will generates for every llvm::module. The TU can also 
> > composed of many modules. In our interpreter, we add new code to our AST 
> > with new modules at runtime. 
> > The ctor/dtor generation is depend on the fatbinary code. The CodeGen 
> > checks, if a path to a fatbinary file is set. If it is, it generates an 
> > ctor with at least a __cudaRegisterFatBinary() function call. So, the 
> > generation is independent of the source code in the module and we can use 
> > every statement. A statement can be an expression, a declaration, a 
> > definition and so one.   
> I still don't understand how it's going to work. Do you have some sort of 
> design document outlining how the interpreter is going to work with CUDA?
> 
> The purpose of the ctor/dtor is to stitch together host-side kernel launch 
> with the GPU-side kernel binary which resides in the GPU binary created by 
> device-side compilation. 
> 
> So, the question #1 -- if you pass GPU-side binary to the compiler, where did 
> you get it? Normally it's the result of device-side compilation of the same 
> TU. In your case it's not quite clear what exactly would that be, if you feed 
> the source to the compiler incrementally. I.e. do you somehow recompile 
> everything we've seen on device side so far for each new chunk of host-side 
> source you feed to the compiler? 
> 
> Next question is -- assuming that device side does have correct GPU-side 
> binary, when do you call those ctors/dtors? JIT model does not quite fit the 
> assumptions that drive regular CUDA compilation.
> 
> Let's consider this:
> ```
> __global__ void foo();
> __global__ void bar();
> 
> // If that's all we've  fed to compiler so far, we have no GPU code yet, so 
> there 
> // should be no fatbin file. If we do have it, what's in it?
> 
> void launch() {
>   foo<<<1,1>>>();
>   bar<<<1,1>>>();
> }
> // If you've generated ctors/dtors at this point they would be 
> // useless as no GPU code exists in the preceding code.
> 
> __global__ void foo() {}
> // Now we'd have some GPU code, but how can we need to retrofit it into 
> // all the ctors/dtors we've generated before. 
> __global__ void bar() {}
> // Does bar end up in its own fatbinary? Or is it combined into a new 
> // fatbin which contains both boo and bar?
> // If it's a new fatbin, you somehow need to update existing ctors/dtors, 
> // unless you want to leak CUDA resources fast.
> // If it's a separate fatbin, then you will need to at the very least change 
> the way 
> // ctors/dtors are generated by the 'launch' function, because now they need 
> to 
> // tie each kernel launch to a different fatbin.
> 
> ```
> 
> It looks to me that if you want to JIT CUDA code you will need to take over 
> GPU-side kernel management.
> ctors/dtors do that for full-TU compilation, but they rely on device-side 
> code being compiled and available during host-side compilation. For JIT, the 
> interpreter should be in charge of registering new kernels with the CUDA 
> runtime and unregistering/unloading them when a kernel goes away. This makes 
> ctors/dtors completely irrelevant.
At the moment, there is no documentation, because we still develop the feature. 
I try to describe how it works.

The device side compilation works with a second compiler (a normal clang), 
which we start via syscall. In the interpreter, we check if the input line is a 
kernel definition or a kernel launch. Then we write the source code to a file 
and compile it with the clang to a PCH-file.  Then the PCH-file will be 
compiled to PTX and then to a fatbin. If we add a new kernel, we will send the 
source code with the existing PCH-file to clang compiler. So we easy extend the 
AST and generate a PTX-file with all defined kernels. 

An implementation of this feature can you see at my prototype: 


Running the ctor/dtor isn't hard. I search after the JITSymbol and generate an 
function pointer. Than I can simply run it. This feature can you also see in my 
prototype. So, we can run the ctor, if new fatbin code is generated and the 
dtor before, if code was already registered. The CUDA runtime also provide the 
possibility to run the (un)register functions many times.

  __global__ void foo();
  __global__ void bar();

 

[PATCH] D43764: [clang-apply-replacements] Convert tooling::Replacements to tooling::AtomicChange for conflict resolving of changes, code cleanup, and code formatting.

2018-03-15 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments.



Comment at: clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp:353
+const FileEntry *Entry = FileAndReplacements.first;
+ReplacementsToAtomicChanges DeduplicatedChanges(SM, Entry);
+for (const auto &R : FileAndReplacements.second)

jdemeule wrote:
> ioeric wrote:
> > Sorry that I didn't make myself clear... what you had in the previous 
> > revision was correct (for the current use case of apply-all-replacements) 
> > i.e. store all replacements in one `AtomicChange`, which allows you to 
> > detect conflicts on the fly. So the code here can be simplified as:
> > 
> > ```
> > ...
> > Entry = ...;
> > AtomicChange FileChange;
> > for (const auto &R : FileAndReplacements.second) {
> >   auto Err = FileChange.replace(  );
> >   if (Err)
> > reportConflict(Entry, std::move(Err));  // reportConflict as we go
> > }
> > FileChanges.emplace(Entry, {FileChange});
> > ...
> > ```
> > 
> > I think with this set up, you wouldn't need `ReplacementsToAtomicChanges`, 
> > `ConflictError` and `reportConflicts`.
> I think I have over-interpreting your previous comment :-)
> However, I am still confused about error reporting.
> Currently, clang-apply-replacements reports conflicting replacement per file 
> and per conflict.
> For example:
> ```
> There are conflicting changes to XXX:
> The following changes conflict:
>   Replace 8:8-8:33 with "AAA"
>   Replace 8:8-8:33 with "BBB"
>   Remove 8:10-8:15
>   Insert at 8:14 CCC
> ```
> 
> With this patch, conflict are reported by pair (first replacement/conflicted 
> one) and I am able to print the corresponding file only once (thanks to the 
> defered reporting).
> ```
> There are conflicting changes to XXX:
> The following changes conflict:
>   Replace 8:8-8:33 with "AAA"
>   Replace 8:8-8:33 with "BBB"
> The following changes conflict:
>   Replace 8:8-8:33 with "AAA"
>   Remove 8:10-8:15
> The following changes conflict:
>   Replace 8:8-8:33 with "AAA"
>   Insert at 8:14 CCC
> ```
> 
> I prefer the way you suggest to report conflict but we will loose or print 
> conflicting file at each conflict detected.
> I even more prefer to use `llvm::toString(std::move(Err))` but this will 
> fully change the reporting and will also be reported by pair.
> ```
> The new replacement overlaps with an existing replacement.
> New replacement: XXX: 106:+26:"AAA"
> Existing replacement: XXX: 106:+26:"BBB"
> The new replacement overlaps with an existing replacement.
> New replacement: XXX: 106:+26:"AAA"
> Existing replacement: XXX: 112:+0:"CCC"
> The new replacement overlaps with an existing replacement.
> New replacement: XXX: 106:+26:"AAA"
> Existing replacement: XXX: 108:+12:""
> ```
I am inclined to changing the current behavior and reporting by pairs because
1) this would simplify the code a lot.because we could basically reuse the 
conflict detection from replacement library.
2) IMO, pairs are easier to read than grouping multiple conflicts - users would 
still need to interpret the conflicts and figure out how all replacements 
conflict in a group, while reporting as pairs already gives you this 
information.
3) in practice, it's uncommon to see more than two conflicting replacements at 
the same code location.

I would vote for the last approach (with `llvm::toString(std::move(Err))`) 
which is easy to implement and doesn't really regress the `UI` that much. WDYT?


https://reviews.llvm.org/D43764



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


r327618 - More warnings when double truncation to float: compound assignment is supported now.

2018-03-15 Thread Andrew V. Tischenko via cfe-commits
Author: avt77
Date: Thu Mar 15 03:03:35 2018
New Revision: 327618

URL: http://llvm.org/viewvc/llvm-project?rev=327618&view=rev
Log:
More warnings when double truncation to float: compound assignment is supported 
now.

Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaChecking.cpp
cfe/trunk/test/Sema/conversion.c

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=327618&r1=327617&r2=327618&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Mar 15 03:03:35 
2018
@@ -3093,6 +3093,9 @@ def err_impcast_complex_scalar : Error<
 def warn_impcast_float_precision : Warning<
   "implicit conversion loses floating-point precision: %0 to %1">,
   InGroup, DefaultIgnore;
+def warn_impcast_float_result_precision : Warning<
+  "implicit conversion when assigning computation result loses floating-point 
precision: %0 to %1">,
+  InGroup, DefaultIgnore;
 def warn_impcast_double_promotion : Warning<
   "implicit conversion increases floating-point precision: %0 to %1">,
   InGroup, DefaultIgnore;

Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=327618&r1=327617&r2=327618&view=diff
==
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Thu Mar 15 03:03:35 2018
@@ -9183,6 +9183,32 @@ static void DiagnoseImpCast(Sema &S, Exp
   DiagnoseImpCast(S, E, E->getType(), T, CContext, diag, pruneControlFlow);
 }
 
+/// Analyze the given compound assignment for the possible losing of
+/// floating-point precision.
+static void AnalyzeCompoundAssignment(Sema &S, BinaryOperator *E) {
+  assert(isa(E) &&
+ "Must be compound assignment operation");
+  // Recurse on the LHS and RHS in here
+  AnalyzeImplicitConversions(S, E->getLHS(), E->getOperatorLoc());
+  AnalyzeImplicitConversions(S, E->getRHS(), E->getOperatorLoc());
+
+  // Now check the outermost expression
+  const auto *ResultBT = E->getLHS()->getType()->getAs();
+  const auto *RBT = cast(E)
+->getComputationResultType()
+->getAs();
+
+  // If both source and target are floating points.
+  if (ResultBT && ResultBT->isFloatingPoint() && RBT && RBT->isFloatingPoint())
+// Builtin FP kinds are ordered by increasing FP rank.
+if (ResultBT->getKind() < RBT->getKind())
+  // We don't want to warn for system macro.
+  if (!S.SourceMgr.isInSystemMacro(E->getOperatorLoc()))
+// warn about dropping FP rank.
+DiagnoseImpCast(S, E->getRHS(), E->getLHS()->getType(),
+E->getOperatorLoc(),
+diag::warn_impcast_float_result_precision);
+}
 
 /// Diagnose an implicit cast from a floating point value to an integer value.
 static void DiagnoseFloatingImpCast(Sema &S, Expr *E, QualType T,
@@ -9550,7 +9576,7 @@ CheckImplicitConversion(Sema &S, Expr *E
 return;
   return DiagnoseImpCast(S, E, T, CC, diag::warn_impcast_vector_scalar);
 }
-
+
 // If the vector cast is cast between two vectors of the same size, it is
 // a bitcast, not a conversion.
 if (S.Context.getTypeSize(Source) == S.Context.getTypeSize(Target))
@@ -9827,7 +9853,7 @@ static void AnalyzeImplicitConversions(S
 
   if (E->isTypeDependent() || E->isValueDependent())
 return;
-  
+
   // For conditional operators, we analyze the arguments as if they
   // were being fed directly into the output.
   if (isa(E)) {
@@ -9871,6 +9897,9 @@ static void AnalyzeImplicitConversions(S
 // And with simple assignments.
 if (BO->getOpcode() == BO_Assign)
   return AnalyzeAssignment(S, BO);
+// And with compound assignments.
+if (BO->isAssignmentOp())
+  return AnalyzeCompoundAssignment(S, BO);
   }
 
   // These break the otherwise-useful invariant below.  Fortunately,

Modified: cfe/trunk/test/Sema/conversion.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/conversion.c?rev=327618&r1=327617&r2=327618&view=diff
==
--- cfe/trunk/test/Sema/conversion.c (original)
+++ cfe/trunk/test/Sema/conversion.c Thu Mar 15 03:03:35 2018
@@ -436,10 +436,15 @@ float double2float_test1(double a) {
 }
 
 void double2float_test2(double a, float *b) {
-*b += a;
+  *b += a; // expected-warning {{implicit conversion when assigning 
computation result loses floating-point precision: 'double' to 'float'}}
 }
 
 float sinf (float x);
 double double2float_test3(double a) {
 return sinf(a); // expected-warning {{implicit conversion loses 
floating-point precision: 'double' to 'f

[PATCH] D43322: Diagnose cases of "return x" that should be "return std::move(x)" for efficiency

2018-03-15 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment.

In https://reviews.llvm.org/D43322#1037889, @Quuxplusone wrote:

> The backward-compatibility-concerned diagnostic, 
> `-Wreturn-std-move-in-c++11`, is *not* appropriate for `-Wmove`;


Have you evaluated possibility of adding `-Wreturn-std-move-in-c++11` to one of 
`CXX..Compat` groups?


Repository:
  rC Clang

https://reviews.llvm.org/D43322



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


[PATCH] D41569: [Concepts] Constraint enforcement and diagnostics

2018-03-15 Thread Saar Raz via Phabricator via cfe-commits
saar.raz updated this revision to Diff 138509.
saar.raz added a comment.

Fixed another SpecializedConcept reference to NamedConcept.


Repository:
  rC Clang

https://reviews.llvm.org/D41569

Files:
  include/clang/AST/ExprCXX.h
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Sema.h
  include/clang/Sema/TemplateDeduction.h
  lib/AST/ExprCXX.cpp
  lib/Sema/SemaConcept.cpp
  lib/Sema/SemaOverload.cpp
  lib/Sema/SemaTemplate.cpp
  lib/Sema/SemaTemplateDeduction.cpp
  lib/Sema/SemaTemplateInstantiateDecl.cpp
  lib/Serialization/ASTReaderStmt.cpp
  lib/Serialization/ASTWriterStmt.cpp
  test/CXX/concepts-ts/expr/expr.prim/expr.prim.id/p3.cpp
  
test/CXX/concepts-ts/temp/temp.constr/temp.constr.constr/function-templates.cpp
  
test/CXX/concepts-ts/temp/temp.constr/temp.constr.constr/non-function-templates.cpp
  
test/CXX/concepts-ts/temp/temp.constr/temp.constr.constr/partial-specializations.cpp

Index: test/CXX/concepts-ts/temp/temp.constr/temp.constr.constr/partial-specializations.cpp
===
--- /dev/null
+++ test/CXX/concepts-ts/temp/temp.constr/temp.constr.constr/partial-specializations.cpp
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 -std=c++2a -fconcepts-ts -x c++ -verify %s
+
+namespace class_templates
+{
+  template requires sizeof(T) >= 4 // expected-note {{because 'sizeof(char) >= 4' (1 >= 4) evaluated to false}}
+  struct is_same { static constexpr bool value = false; };
+
+  template requires sizeof(T*) >= 4 && sizeof(T) >= 4
+  struct is_same { static constexpr bool value = true; };
+
+  static_assert(!is_same::value);
+  static_assert(!is_same::value);
+  static_assert(is_same::value);
+  static_assert(is_same::value); // expected-error {{constraints not satisfied for class template 'is_same' [with T = char, U = char]}}
+}
+
+namespace variable_templates
+{
+  template requires sizeof(T) >= 4
+  constexpr bool is_same_v = false;
+
+  template requires sizeof(T*) >= 4 && sizeof(T) >= 4
+  constexpr bool is_same_v = true;
+
+  static_assert(!is_same_v);
+  static_assert(!is_same_v);
+  static_assert(is_same_v);
+}
\ No newline at end of file
Index: test/CXX/concepts-ts/temp/temp.constr/temp.constr.constr/non-function-templates.cpp
===
--- /dev/null
+++ test/CXX/concepts-ts/temp/temp.constr/temp.constr.constr/non-function-templates.cpp
@@ -0,0 +1,79 @@
+// RUN: %clang_cc1 -std=c++2a -fconcepts-ts -x c++ -verify %s
+
+template requires sizeof(T) >= 2 // expected-note{{because 'sizeof(char) >= 2' (1 >= 2) evaluated to false}}
+struct A {
+  static constexpr int value = sizeof(T);
+};
+
+static_assert(A::value == 4);
+static_assert(A::value == 1); // expected-error{{constraints not satisfied for class template 'A' [with T = char]}}
+
+template
+  requires sizeof(T) != sizeof(U) // expected-note{{because 'sizeof(int) != sizeof(char [4])' (4 != 4) evaluated to false}}
+   && sizeof(T) >= 4 // expected-note{{because 'sizeof(char) >= 4' (1 >= 4) evaluated to false}}
+constexpr int SizeDiff = sizeof(T) > sizeof(U) ? sizeof(T) - sizeof(U) : sizeof(U) - sizeof(T);
+
+static_assert(SizeDiff == 3);
+static_assert(SizeDiff == 0); // expected-error{{constraints not satisfied for variable template 'SizeDiff' [with T = int, U = char [4]]}}
+static_assert(SizeDiff == 3); // expected-error{{constraints not satisfied for variable template 'SizeDiff' [with T = char, U = int]}}
+
+template
+  requires ((sizeof(Ts) == 4) || ...) // expected-note{{because 'sizeof(char) == 4' (1 == 4) evaluated to false}} expected-note{{'sizeof(long long) == 4' (8 == 4) evaluated to false}} expected-note{{'sizeof(int [20]) == 4' (80 == 4) evaluated to false}}
+constexpr auto SumSizes = (sizeof(Ts) + ...);
+
+static_assert(SumSizes == 13);
+static_assert(SumSizes == 89); // expected-error{{constraints not satisfied for variable template 'SumSizes' [with Ts = ]}}
+
+template
+concept IsBig = sizeof(T) > 100; // expected-note{{because 'sizeof(int) > 100' (4 > 100) evaluated to false}}
+
+template
+  requires IsBig // expected-note{{'int' does not satisfy 'IsBig'}}
+using BigPtr = T*;
+
+static_assert(sizeof(BigPtr)); // expected-error{{constraints not satisfied for alias template 'BigPtr' [with T = int]
+
+template requires T::value // expected-note{{because substituted constraint expression is ill-formed: type 'int' cannot be used prior to '::' because it has no members}}
+struct S { static constexpr bool value = true; };
+
+struct S2 { static constexpr bool value = true; };
+
+static_assert(S::value); // expected-error{{constraints not satisfied for class template 'S' [with T = int]}}
+static_assert(S::value);
+
+template
+struct AA
+{
+template requires sizeof(U) == sizeof(T) // expected-note{{because 'sizeof(int [2]) == sizeof(int)' (8 == 4) evaluated to false}}
+struct B
+{
+static constexpr int a = 0;
+};
+
+template requires sizeof(U) == sizeof(T) // expected-note{{

[PATCH] D41569: [Concepts] Constraint enforcement and diagnostics

2018-03-15 Thread Saar Raz via Phabricator via cfe-commits
saar.raz updated this revision to Diff 138505.
saar.raz added a comment.

Fixed SpecializedConcept reference to NamedConcept.


Repository:
  rC Clang

https://reviews.llvm.org/D41569

Files:
  include/clang/AST/ExprCXX.h
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Sema.h
  include/clang/Sema/TemplateDeduction.h
  lib/AST/ExprCXX.cpp
  lib/Sema/SemaConcept.cpp
  lib/Sema/SemaOverload.cpp
  lib/Sema/SemaTemplate.cpp
  lib/Sema/SemaTemplateDeduction.cpp
  lib/Sema/SemaTemplateInstantiateDecl.cpp
  lib/Serialization/ASTReaderStmt.cpp
  lib/Serialization/ASTWriterStmt.cpp
  test/CXX/concepts-ts/expr/expr.prim/expr.prim.id/p3.cpp
  
test/CXX/concepts-ts/temp/temp.constr/temp.constr.constr/function-templates.cpp
  
test/CXX/concepts-ts/temp/temp.constr/temp.constr.constr/non-function-templates.cpp
  
test/CXX/concepts-ts/temp/temp.constr/temp.constr.constr/partial-specializations.cpp

Index: test/CXX/concepts-ts/temp/temp.constr/temp.constr.constr/partial-specializations.cpp
===
--- /dev/null
+++ test/CXX/concepts-ts/temp/temp.constr/temp.constr.constr/partial-specializations.cpp
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 -std=c++2a -fconcepts-ts -x c++ -verify %s
+
+namespace class_templates
+{
+  template requires sizeof(T) >= 4 // expected-note {{because 'sizeof(char) >= 4' (1 >= 4) evaluated to false}}
+  struct is_same { static constexpr bool value = false; };
+
+  template requires sizeof(T*) >= 4 && sizeof(T) >= 4
+  struct is_same { static constexpr bool value = true; };
+
+  static_assert(!is_same::value);
+  static_assert(!is_same::value);
+  static_assert(is_same::value);
+  static_assert(is_same::value); // expected-error {{constraints not satisfied for class template 'is_same' [with T = char, U = char]}}
+}
+
+namespace variable_templates
+{
+  template requires sizeof(T) >= 4
+  constexpr bool is_same_v = false;
+
+  template requires sizeof(T*) >= 4 && sizeof(T) >= 4
+  constexpr bool is_same_v = true;
+
+  static_assert(!is_same_v);
+  static_assert(!is_same_v);
+  static_assert(is_same_v);
+}
\ No newline at end of file
Index: test/CXX/concepts-ts/temp/temp.constr/temp.constr.constr/non-function-templates.cpp
===
--- /dev/null
+++ test/CXX/concepts-ts/temp/temp.constr/temp.constr.constr/non-function-templates.cpp
@@ -0,0 +1,79 @@
+// RUN: %clang_cc1 -std=c++2a -fconcepts-ts -x c++ -verify %s
+
+template requires sizeof(T) >= 2 // expected-note{{because 'sizeof(char) >= 2' (1 >= 2) evaluated to false}}
+struct A {
+  static constexpr int value = sizeof(T);
+};
+
+static_assert(A::value == 4);
+static_assert(A::value == 1); // expected-error{{constraints not satisfied for class template 'A' [with T = char]}}
+
+template
+  requires sizeof(T) != sizeof(U) // expected-note{{because 'sizeof(int) != sizeof(char [4])' (4 != 4) evaluated to false}}
+   && sizeof(T) >= 4 // expected-note{{because 'sizeof(char) >= 4' (1 >= 4) evaluated to false}}
+constexpr int SizeDiff = sizeof(T) > sizeof(U) ? sizeof(T) - sizeof(U) : sizeof(U) - sizeof(T);
+
+static_assert(SizeDiff == 3);
+static_assert(SizeDiff == 0); // expected-error{{constraints not satisfied for variable template 'SizeDiff' [with T = int, U = char [4]]}}
+static_assert(SizeDiff == 3); // expected-error{{constraints not satisfied for variable template 'SizeDiff' [with T = char, U = int]}}
+
+template
+  requires ((sizeof(Ts) == 4) || ...) // expected-note{{because 'sizeof(char) == 4' (1 == 4) evaluated to false}} expected-note{{'sizeof(long long) == 4' (8 == 4) evaluated to false}} expected-note{{'sizeof(int [20]) == 4' (80 == 4) evaluated to false}}
+constexpr auto SumSizes = (sizeof(Ts) + ...);
+
+static_assert(SumSizes == 13);
+static_assert(SumSizes == 89); // expected-error{{constraints not satisfied for variable template 'SumSizes' [with Ts = ]}}
+
+template
+concept IsBig = sizeof(T) > 100; // expected-note{{because 'sizeof(int) > 100' (4 > 100) evaluated to false}}
+
+template
+  requires IsBig // expected-note{{'int' does not satisfy 'IsBig'}}
+using BigPtr = T*;
+
+static_assert(sizeof(BigPtr)); // expected-error{{constraints not satisfied for alias template 'BigPtr' [with T = int]
+
+template requires T::value // expected-note{{because substituted constraint expression is ill-formed: type 'int' cannot be used prior to '::' because it has no members}}
+struct S { static constexpr bool value = true; };
+
+struct S2 { static constexpr bool value = true; };
+
+static_assert(S::value); // expected-error{{constraints not satisfied for class template 'S' [with T = int]}}
+static_assert(S::value);
+
+template
+struct AA
+{
+template requires sizeof(U) == sizeof(T) // expected-note{{because 'sizeof(int [2]) == sizeof(int)' (8 == 4) evaluated to false}}
+struct B
+{
+static constexpr int a = 0;
+};
+
+template requires sizeof(U) == sizeof(T) // expected-note{{because 

[PATCH] D43766: [clang-tidy][modernize-make-unique] Checks c++14 flag before using std::make_unique

2018-03-15 Thread Haojian Wu via Phabricator via cfe-commits
hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.

LGTM.

> I think all requested modifications were done.

Next time, please consider marking all the review comments done in phabricator, 
so that reviewers can see it obviously.


https://reviews.llvm.org/D43766



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


[PATCH] D43764: [clang-apply-replacements] Convert tooling::Replacements to tooling::AtomicChange for conflict resolving of changes, code cleanup, and code formatting.

2018-03-15 Thread Jeremy Demeule via Phabricator via cfe-commits
jdemeule updated this revision to Diff 138504.
jdemeule added a comment.

Update after review.
Add tests to check identical insertions and order dependent insertions.


https://reviews.llvm.org/D43764

Files:
  
clang-apply-replacements/include/clang-apply-replacements/Tooling/ApplyReplacements.h
  clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
  clang-apply-replacements/tool/CMakeLists.txt
  clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp
  test/clang-apply-replacements/Inputs/basic/file2.yaml
  test/clang-apply-replacements/Inputs/conflict/expected.txt
  test/clang-apply-replacements/Inputs/identical/file1.yaml
  test/clang-apply-replacements/Inputs/identical/identical.cpp
  test/clang-apply-replacements/Inputs/order-dependent/expected.txt
  test/clang-apply-replacements/Inputs/order-dependent/file1.yaml
  test/clang-apply-replacements/Inputs/order-dependent/file2.yaml
  test/clang-apply-replacements/Inputs/order-dependent/order-dependent.cpp
  test/clang-apply-replacements/identical.cpp
  test/clang-apply-replacements/order-dependent.cpp
  unittests/clang-apply-replacements/ApplyReplacementsTest.cpp
  unittests/clang-apply-replacements/CMakeLists.txt
  unittests/clang-apply-replacements/ReformattingTest.cpp

Index: unittests/clang-apply-replacements/ReformattingTest.cpp
===
--- unittests/clang-apply-replacements/ReformattingTest.cpp
+++ /dev/null
@@ -1,67 +0,0 @@
-//===- clang-apply-replacements/ReformattingTest.cpp --===//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===--===//
-
-#include "clang-apply-replacements/Tooling/ApplyReplacements.h"
-#include "common/VirtualFileHelper.h"
-#include "clang/Format/Format.h"
-#include "clang/Tooling/Refactoring.h"
-#include "gtest/gtest.h"
-
-using namespace clang;
-using namespace clang::tooling;
-using namespace clang::replace;
-
-typedef std::vector ReplacementsVec;
-
-static Replacement makeReplacement(unsigned Offset, unsigned Length,
-   unsigned ReplacementLength,
-   llvm::StringRef FilePath = "") {
-  return Replacement(FilePath, Offset, Length,
- std::string(ReplacementLength, '~'));
-}
-
-// generate a set of replacements containing one element
-static ReplacementsVec makeReplacements(unsigned Offset, unsigned Length,
-unsigned ReplacementLength,
-llvm::StringRef FilePath = "~") {
-  ReplacementsVec Replaces;
-  Replaces.push_back(
-  makeReplacement(Offset, Length, ReplacementLength, FilePath));
-  return Replaces;
-}
-
-// Put these functions in the clang::tooling namespace so arg-dependent name
-// lookup finds these functions for the EXPECT_EQ macros below.
-namespace clang {
-namespace tooling {
-
-std::ostream &operator<<(std::ostream &os, const Range &R) {
-  return os << "Range(" << R.getOffset() << ", " << R.getLength() << ")";
-}
-
-} // namespace tooling
-} // namespace clang
-
-// Ensure zero-length ranges are produced. Even lines where things are deleted
-// need reformatting.
-TEST(CalculateChangedRangesTest, producesZeroLengthRange) {
-  RangeVector Changes = calculateChangedRanges(makeReplacements(0, 4, 0));
-  EXPECT_EQ(Range(0, 0), Changes.front());
-}
-
-// Basic test to ensure replacements turn into ranges properly.
-TEST(CalculateChangedRangesTest, calculatesRanges) {
-  ReplacementsVec R;
-  R.push_back(makeReplacement(2, 0, 3));
-  R.push_back(makeReplacement(5, 2, 4));
-  RangeVector Changes = calculateChangedRanges(R);
-
-  Range ExpectedRanges[] = { Range(2, 3), Range(8, 4) };
-  EXPECT_TRUE(std::equal(Changes.begin(), Changes.end(), ExpectedRanges));
-}
Index: unittests/clang-apply-replacements/CMakeLists.txt
===
--- unittests/clang-apply-replacements/CMakeLists.txt
+++ unittests/clang-apply-replacements/CMakeLists.txt
@@ -9,12 +9,12 @@
 
 add_extra_unittest(ClangApplyReplacementsTests
   ApplyReplacementsTest.cpp
-  ReformattingTest.cpp
   )
 
 target_link_libraries(ClangApplyReplacementsTests
   PRIVATE
   clangApplyReplacements
   clangBasic
   clangToolingCore
+  clangToolingRefactor
   )
Index: unittests/clang-apply-replacements/ApplyReplacementsTest.cpp
===
--- unittests/clang-apply-replacements/ApplyReplacementsTest.cpp
+++ unittests/clang-apply-replacements/ApplyReplacementsTest.cpp
@@ -9,6 +9,7 @@
 //===--===//
 
 #include "clang-apply-replacements/Tooling/ApplyReplacements.h"
+#include "clang/Format/Format.h"
 #include "gtest/gtest.h"
 
 using 

[PATCH] D41655: [clang-tidy] New check bugprone-unused-return-value

2018-03-15 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment.

In https://reviews.llvm.org/D41655#1037552, @khuttun wrote:

> In https://reviews.llvm.org/D41655#1037234, @alexfh wrote:
>
> > Do you need help committing the patch?
>
>
> Yes please, I don't have commit access to the repo.


The patch doesn't apply cleanly. Please rebase against current HEAD.

> I think the next step for improving this checker could be to make it work 
> with class template member functions. That would allow to add checking also 
> to those container functions that https://reviews.llvm.org/D17043 handles.

Yep, sounds reasonable. Thank you for working on this!


https://reviews.llvm.org/D41655



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


[clang-tools-extra] r327610 - [clang-tidy] rename_check.py misc-unused-raii bugprone-unused-raii --check_class_name=UnusedRAIICheck

2018-03-15 Thread Alexander Kornienko via cfe-commits
Author: alexfh
Date: Thu Mar 15 01:27:42 2018
New Revision: 327610

URL: http://llvm.org/viewvc/llvm-project?rev=327610&view=rev
Log:
[clang-tidy] rename_check.py misc-unused-raii bugprone-unused-raii 
--check_class_name=UnusedRAIICheck

Added:
clang-tools-extra/trunk/clang-tidy/bugprone/UnusedRaiiCheck.cpp
  - copied, changed from r327609, 
clang-tools-extra/trunk/clang-tidy/misc/UnusedRAIICheck.cpp
clang-tools-extra/trunk/clang-tidy/bugprone/UnusedRaiiCheck.h
  - copied, changed from r327609, 
clang-tools-extra/trunk/clang-tidy/misc/UnusedRAIICheck.h
clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-unused-raii.rst
  - copied, changed from r327609, 
clang-tools-extra/trunk/docs/clang-tidy/checks/misc-unused-raii.rst
clang-tools-extra/trunk/test/clang-tidy/bugprone-unused-raii.cpp
  - copied, changed from r327609, 
clang-tools-extra/trunk/test/clang-tidy/misc-unused-raii.cpp
Removed:
clang-tools-extra/trunk/clang-tidy/misc/UnusedRAIICheck.cpp
clang-tools-extra/trunk/clang-tidy/misc/UnusedRAIICheck.h
clang-tools-extra/trunk/docs/clang-tidy/checks/misc-unused-raii.rst
clang-tools-extra/trunk/test/clang-tidy/misc-unused-raii.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp
clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst

Modified: clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp?rev=327610&r1=327609&r2=327610&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp Thu Mar 
15 01:27:42 2018
@@ -42,6 +42,7 @@
 #include "ThrowKeywordMissingCheck.h"
 #include "UndefinedMemoryManipulationCheck.h"
 #include "UndelegatedConstructorCheck.h"
+#include "UnusedRaiiCheck.h"
 #include "UseAfterMoveCheck.h"
 #include "VirtualNearMissCheck.h"
 
@@ -116,6 +117,8 @@ public:
 "bugprone-undefined-memory-manipulation");
 CheckFactories.registerCheck(
 "bugprone-undelegated-constructor");
+CheckFactories.registerCheck(
+"bugprone-unused-raii");
 CheckFactories.registerCheck(
 "bugprone-use-after-move");
 CheckFactories.registerCheck(

Modified: clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt?rev=327610&r1=327609&r2=327610&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt Thu Mar 15 
01:27:42 2018
@@ -34,6 +34,7 @@ add_clang_library(clangTidyBugproneModul
   ThrowKeywordMissingCheck.cpp
   UndefinedMemoryManipulationCheck.cpp
   UndelegatedConstructorCheck.cpp
+  UnusedRaiiCheck.cpp
   UseAfterMoveCheck.cpp
   VirtualNearMissCheck.cpp
 

Copied: clang-tools-extra/trunk/clang-tidy/bugprone/UnusedRaiiCheck.cpp (from 
r327609, clang-tools-extra/trunk/clang-tidy/misc/UnusedRAIICheck.cpp)
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/UnusedRaiiCheck.cpp?p2=clang-tools-extra/trunk/clang-tidy/bugprone/UnusedRaiiCheck.cpp&p1=clang-tools-extra/trunk/clang-tidy/misc/UnusedRAIICheck.cpp&r1=327609&r2=327610&rev=327610&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/misc/UnusedRAIICheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/bugprone/UnusedRaiiCheck.cpp Thu Mar 15 
01:27:42 2018
@@ -1,4 +1,4 @@
-//===--- UnusedRAIICheck.cpp - clang-tidy 
-===//
+//===--- UnusedRaiiCheck.cpp - clang-tidy 
-===//
 //
 // The LLVM Compiler Infrastructure
 //
@@ -7,7 +7,7 @@
 //
 
//===--===//
 
-#include "UnusedRAIICheck.h"
+#include "UnusedRaiiCheck.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/Lex/Lexer.h"
 
@@ -15,7 +15,7 @@ using namespace clang::ast_matchers;
 
 namespace clang {
 namespace tidy {
-namespace misc {
+namespace bugprone {
 
 namespace {
 AST_MATCHER(CXXRecordDecl, hasNonTrivialDestructor) {
@@ -24,7 +24,7 @@ AST_MATCHER(CXXRecordDecl, hasNonTrivial
 }
 } // namespace
 
-void UnusedRAIICheck::registerMatchers(MatchFinder *Finder) {
+void UnusedRaiiCheck::registerMatchers(MatchFinder *Finder) {
   // Only register the matchers for C++; the functionality currently does not
   // provide 

[clang-tools-extra] r327607 - [clang-tidy] rename_check.py misc-sizeof-expression bugprone-sizeof-expression

2018-03-15 Thread Alexander Kornienko via cfe-commits
Author: alexfh
Date: Thu Mar 15 01:26:19 2018
New Revision: 327607

URL: http://llvm.org/viewvc/llvm-project?rev=327607&view=rev
Log:
[clang-tidy] rename_check.py misc-sizeof-expression bugprone-sizeof-expression

Added:
clang-tools-extra/trunk/clang-tidy/bugprone/SizeofExpressionCheck.cpp
  - copied, changed from r327606, 
clang-tools-extra/trunk/clang-tidy/misc/SizeofExpressionCheck.cpp
clang-tools-extra/trunk/clang-tidy/bugprone/SizeofExpressionCheck.h
  - copied, changed from r327606, 
clang-tools-extra/trunk/clang-tidy/misc/SizeofExpressionCheck.h

clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-sizeof-expression.rst
  - copied, changed from r327606, 
clang-tools-extra/trunk/docs/clang-tidy/checks/misc-sizeof-expression.rst
clang-tools-extra/trunk/test/clang-tidy/bugprone-sizeof-expression.cpp
  - copied, changed from r327606, 
clang-tools-extra/trunk/test/clang-tidy/misc-sizeof-expression.cpp
Removed:
clang-tools-extra/trunk/clang-tidy/misc/SizeofExpressionCheck.cpp
clang-tools-extra/trunk/clang-tidy/misc/SizeofExpressionCheck.h
clang-tools-extra/trunk/docs/clang-tidy/checks/misc-sizeof-expression.rst
clang-tools-extra/trunk/test/clang-tidy/misc-sizeof-expression.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp
clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst

Modified: clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp?rev=327607&r1=327606&r2=327607&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp Thu Mar 
15 01:26:19 2018
@@ -28,6 +28,7 @@
 #include "MisplacedWideningCastCheck.h"
 #include "MoveForwardingReferenceCheck.h"
 #include "MultipleStatementMacroCheck.h"
+#include "SizeofExpressionCheck.h"
 #include "StringConstructorCheck.h"
 #include "StringIntegerAssignmentCheck.h"
 #include "StringLiteralWithEmbeddedNulCheck.h"
@@ -86,6 +87,8 @@ public:
 "bugprone-move-forwarding-reference");
 CheckFactories.registerCheck(
 "bugprone-multiple-statement-macro");
+CheckFactories.registerCheck(
+"bugprone-sizeof-expression");
 CheckFactories.registerCheck(
 "bugprone-string-constructor");
 CheckFactories.registerCheck(

Modified: clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt?rev=327607&r1=327606&r2=327607&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt Thu Mar 15 
01:26:19 2018
@@ -20,6 +20,7 @@ add_clang_library(clangTidyBugproneModul
   MisplacedWideningCastCheck.cpp
   MoveForwardingReferenceCheck.cpp
   MultipleStatementMacroCheck.cpp
+  SizeofExpressionCheck.cpp
   StringConstructorCheck.cpp
   StringIntegerAssignmentCheck.cpp
   StringLiteralWithEmbeddedNulCheck.cpp

Copied: clang-tools-extra/trunk/clang-tidy/bugprone/SizeofExpressionCheck.cpp 
(from r327606, 
clang-tools-extra/trunk/clang-tidy/misc/SizeofExpressionCheck.cpp)
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/SizeofExpressionCheck.cpp?p2=clang-tools-extra/trunk/clang-tidy/bugprone/SizeofExpressionCheck.cpp&p1=clang-tools-extra/trunk/clang-tidy/misc/SizeofExpressionCheck.cpp&r1=327606&r2=327607&rev=327607&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/misc/SizeofExpressionCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/bugprone/SizeofExpressionCheck.cpp Thu 
Mar 15 01:26:19 2018
@@ -16,7 +16,7 @@ using namespace clang::ast_matchers;
 
 namespace clang {
 namespace tidy {
-namespace misc {
+namespace bugprone {
 
 namespace {
 
@@ -260,6 +260,6 @@ void SizeofExpressionCheck::check(const
   }
 }
 
-} // namespace misc
+} // namespace bugprone
 } // namespace tidy
 } // namespace clang

Copied: clang-tools-extra/trunk/clang-tidy/bugprone/SizeofExpressionCheck.h 
(from r327606, clang-tools-extra/trunk/clang-tidy/misc/SizeofExpressionCheck.h)
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/SizeofExpressionCheck.h?p2=clang-tools-extra/trunk/clang-tidy/bugprone/SizeofExpressionCheck.h&p1=clang-tools-extra/trunk/clang-tidy/misc/SizeofExpressionCheck.h&r1=327606&r2=327607&rev=327607&view=diff
=

[clang-tools-extra] r327609 - Fixed filename in a comment. NFC

2018-03-15 Thread Alexander Kornienko via cfe-commits
Author: alexfh
Date: Thu Mar 15 01:26:58 2018
New Revision: 327609

URL: http://llvm.org/viewvc/llvm-project?rev=327609&view=rev
Log:
Fixed filename in a comment. NFC

Modified:
clang-tools-extra/trunk/clang-tidy/bugprone/IncorrectRoundingsCheck.h

Modified: clang-tools-extra/trunk/clang-tidy/bugprone/IncorrectRoundingsCheck.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/IncorrectRoundingsCheck.h?rev=327609&r1=327608&r2=327609&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/bugprone/IncorrectRoundingsCheck.h 
(original)
+++ clang-tools-extra/trunk/clang-tidy/bugprone/IncorrectRoundingsCheck.h Thu 
Mar 15 01:26:58 2018
@@ -1,4 +1,4 @@
-//===--- IncorrectRoundingsCheckCheck.h - clang-tidy -*- C++ 
-*-===//
+//===--- IncorrectRoundingsCheck.h - clang-tidy -*- C++ 
-*-===//
 //
 // The LLVM Compiler Infrastructure
 //


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


[clang-tools-extra] r327608 - [clang-tidy] rename_check.py misc-sizeof-container bugprone-sizeof-container

2018-03-15 Thread Alexander Kornienko via cfe-commits
Author: alexfh
Date: Thu Mar 15 01:26:47 2018
New Revision: 327608

URL: http://llvm.org/viewvc/llvm-project?rev=327608&view=rev
Log:
[clang-tidy] rename_check.py misc-sizeof-container bugprone-sizeof-container

Added:
clang-tools-extra/trunk/clang-tidy/bugprone/SizeofContainerCheck.cpp
  - copied, changed from r327607, 
clang-tools-extra/trunk/clang-tidy/misc/SizeofContainerCheck.cpp
clang-tools-extra/trunk/clang-tidy/bugprone/SizeofContainerCheck.h
  - copied, changed from r327607, 
clang-tools-extra/trunk/clang-tidy/misc/SizeofContainerCheck.h
clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-sizeof-container.rst
  - copied, changed from r327607, 
clang-tools-extra/trunk/docs/clang-tidy/checks/misc-sizeof-container.rst
clang-tools-extra/trunk/test/clang-tidy/bugprone-sizeof-container.cpp
  - copied, changed from r327607, 
clang-tools-extra/trunk/test/clang-tidy/misc-sizeof-container.cpp
Removed:
clang-tools-extra/trunk/clang-tidy/misc/SizeofContainerCheck.cpp
clang-tools-extra/trunk/clang-tidy/misc/SizeofContainerCheck.h
clang-tools-extra/trunk/docs/clang-tidy/checks/misc-sizeof-container.rst
clang-tools-extra/trunk/test/clang-tidy/misc-sizeof-container.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp
clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst

Modified: clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp?rev=327608&r1=327607&r2=327608&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp Thu Mar 
15 01:26:47 2018
@@ -28,6 +28,7 @@
 #include "MisplacedWideningCastCheck.h"
 #include "MoveForwardingReferenceCheck.h"
 #include "MultipleStatementMacroCheck.h"
+#include "SizeofContainerCheck.h"
 #include "SizeofExpressionCheck.h"
 #include "StringConstructorCheck.h"
 #include "StringIntegerAssignmentCheck.h"
@@ -87,6 +88,8 @@ public:
 "bugprone-move-forwarding-reference");
 CheckFactories.registerCheck(
 "bugprone-multiple-statement-macro");
+CheckFactories.registerCheck(
+"bugprone-sizeof-container");
 CheckFactories.registerCheck(
 "bugprone-sizeof-expression");
 CheckFactories.registerCheck(

Modified: clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt?rev=327608&r1=327607&r2=327608&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt Thu Mar 15 
01:26:47 2018
@@ -20,6 +20,7 @@ add_clang_library(clangTidyBugproneModul
   MisplacedWideningCastCheck.cpp
   MoveForwardingReferenceCheck.cpp
   MultipleStatementMacroCheck.cpp
+  SizeofContainerCheck.cpp
   SizeofExpressionCheck.cpp
   StringConstructorCheck.cpp
   StringIntegerAssignmentCheck.cpp

Copied: clang-tools-extra/trunk/clang-tidy/bugprone/SizeofContainerCheck.cpp 
(from r327607, clang-tools-extra/trunk/clang-tidy/misc/SizeofContainerCheck.cpp)
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/SizeofContainerCheck.cpp?p2=clang-tools-extra/trunk/clang-tidy/bugprone/SizeofContainerCheck.cpp&p1=clang-tools-extra/trunk/clang-tidy/misc/SizeofContainerCheck.cpp&r1=327607&r2=327608&rev=327608&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/misc/SizeofContainerCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/bugprone/SizeofContainerCheck.cpp Thu 
Mar 15 01:26:47 2018
@@ -15,7 +15,7 @@ using namespace clang::ast_matchers;
 
 namespace clang {
 namespace tidy {
-namespace misc {
+namespace bugprone {
 
 void SizeofContainerCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(
@@ -44,6 +44,6 @@ void SizeofContainerCheck::check(const M
   "container; did you mean .size()?");
 }
 
-} // namespace misc
+} // namespace bugprone
 } // namespace tidy
 } // namespace clang

Copied: clang-tools-extra/trunk/clang-tidy/bugprone/SizeofContainerCheck.h 
(from r327607, clang-tools-extra/trunk/clang-tidy/misc/SizeofContainerCheck.h)
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/SizeofContainerCheck.h?p2=clang-tools-extra/trunk/clang-tidy/bugprone/SizeofContainerCheck.h&p1=clang-tools-extra/trunk/cla

[clang-tools-extra] r327606 - [clang-tidy] rename_check.py {misc, bugprone}-macro-parentheses

2018-03-15 Thread Alexander Kornienko via cfe-commits
Author: alexfh
Date: Thu Mar 15 01:25:39 2018
New Revision: 327606

URL: http://llvm.org/viewvc/llvm-project?rev=327606&view=rev
Log:
[clang-tidy] rename_check.py {misc,bugprone}-macro-parentheses

Added:
clang-tools-extra/trunk/clang-tidy/bugprone/MacroParenthesesCheck.cpp
  - copied, changed from r327590, 
clang-tools-extra/trunk/clang-tidy/misc/MacroParenthesesCheck.cpp
clang-tools-extra/trunk/clang-tidy/bugprone/MacroParenthesesCheck.h
  - copied, changed from r327590, 
clang-tools-extra/trunk/clang-tidy/misc/MacroParenthesesCheck.h

clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-macro-parentheses.rst
  - copied, changed from r327590, 
clang-tools-extra/trunk/docs/clang-tidy/checks/misc-macro-parentheses.rst

clang-tools-extra/trunk/test/clang-tidy/bugprone-macro-parentheses-cmdline.cpp
  - copied, changed from r327590, 
clang-tools-extra/trunk/test/clang-tidy/misc-macro-parentheses-cmdline.cpp
clang-tools-extra/trunk/test/clang-tidy/bugprone-macro-parentheses.cpp
  - copied, changed from r327590, 
clang-tools-extra/trunk/test/clang-tidy/misc-macro-parentheses.cpp
Removed:
clang-tools-extra/trunk/clang-tidy/misc/MacroParenthesesCheck.cpp
clang-tools-extra/trunk/clang-tidy/misc/MacroParenthesesCheck.h
clang-tools-extra/trunk/docs/clang-tidy/checks/misc-macro-parentheses.rst
clang-tools-extra/trunk/test/clang-tidy/misc-macro-parentheses-cmdline.cpp
clang-tools-extra/trunk/test/clang-tidy/misc-macro-parentheses.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp
clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst

Modified: clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp?rev=327606&r1=327605&r2=327606&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/bugprone/BugproneTidyModule.cpp Thu Mar 
15 01:25:39 2018
@@ -22,6 +22,7 @@
 #include "IncorrectRoundingsCheck.h"
 #include "IntegerDivisionCheck.h"
 #include "LambdaFunctionNameCheck.h"
+#include "MacroParenthesesCheck.h"
 #include "MacroRepeatedSideEffectsCheck.h"
 #include "MisplacedOperatorInStrlenInAllocCheck.h"
 #include "MisplacedWideningCastCheck.h"
@@ -73,6 +74,8 @@ public:
 "bugprone-integer-division");
 CheckFactories.registerCheck(
 "bugprone-lambda-function-name");
+CheckFactories.registerCheck(
+"bugprone-macro-parentheses");
 CheckFactories.registerCheck(
 "bugprone-macro-repeated-side-effects");
 CheckFactories.registerCheck(

Modified: clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt?rev=327606&r1=327605&r2=327606&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/bugprone/CMakeLists.txt Thu Mar 15 
01:25:39 2018
@@ -14,6 +14,7 @@ add_clang_library(clangTidyBugproneModul
   IncorrectRoundingsCheck.cpp
   IntegerDivisionCheck.cpp
   LambdaFunctionNameCheck.cpp
+  MacroParenthesesCheck.cpp
   MacroRepeatedSideEffectsCheck.cpp
   MisplacedOperatorInStrlenInAllocCheck.cpp
   MisplacedWideningCastCheck.cpp

Copied: clang-tools-extra/trunk/clang-tidy/bugprone/MacroParenthesesCheck.cpp 
(from r327590, 
clang-tools-extra/trunk/clang-tidy/misc/MacroParenthesesCheck.cpp)
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/MacroParenthesesCheck.cpp?p2=clang-tools-extra/trunk/clang-tidy/bugprone/MacroParenthesesCheck.cpp&p1=clang-tools-extra/trunk/clang-tidy/misc/MacroParenthesesCheck.cpp&r1=327590&r2=327606&rev=327606&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/misc/MacroParenthesesCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/bugprone/MacroParenthesesCheck.cpp Thu 
Mar 15 01:25:39 2018
@@ -14,7 +14,7 @@
 
 namespace clang {
 namespace tidy {
-namespace misc {
+namespace bugprone {
 
 namespace {
 class MacroParenthesesPPCallbacks : public PPCallbacks {
@@ -255,6 +255,6 @@ void MacroParenthesesCheck::registerPPCa
   &Compiler.getPreprocessor(), this));
 }
 
-} // namespace misc
+} // namespace bugprone
 } // namespace tidy
 } // namespace clang

Copied: clang-tools-extra/trunk/clang-tidy/bugprone/MacroParenthesesCheck.h 
(from r327590, clang-tools-extra/trunk/clang-tidy/misc/MacroParenthese

[PATCH] D44508: Remove unnecessary include from Driver.cpp

2018-03-15 Thread Yuka Takahashi via Phabricator via cfe-commits
yamaguchi created this revision.
yamaguchi added a reviewer: dlj.
Herald added a subscriber: llvm-commits.

Clang compiles and test passes without these includes.


Repository:
  rL LLVM

https://reviews.llvm.org/D44508

Files:
  clang/lib/Driver/Driver.cpp


Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -73,8 +73,6 @@
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Support/StringSaver.h"
 #include 
-#include 
-#include 
 #if LLVM_ON_UNIX
 #include  // getpid
 #endif


Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -73,8 +73,6 @@
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Support/StringSaver.h"
 #include 
-#include 
-#include 
 #if LLVM_ON_UNIX
 #include  // getpid
 #endif
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D41569: [Concepts] Constraint enforcement and diagnostics

2018-03-15 Thread Saar Raz via Phabricator via cfe-commits
saar.raz updated this revision to Diff 138501.
saar.raz added a comment.

Adjusted to changes in https://reviews.llvm.org/D41217


Repository:
  rC Clang

https://reviews.llvm.org/D41569

Files:
  include/clang/AST/ExprCXX.h
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Sema.h
  include/clang/Sema/TemplateDeduction.h
  lib/AST/ExprCXX.cpp
  lib/Sema/SemaConcept.cpp
  lib/Sema/SemaOverload.cpp
  lib/Sema/SemaTemplate.cpp
  lib/Sema/SemaTemplateDeduction.cpp
  lib/Sema/SemaTemplateInstantiateDecl.cpp
  lib/Serialization/ASTReaderStmt.cpp
  lib/Serialization/ASTWriterStmt.cpp
  test/CXX/concepts-ts/expr/expr.prim/expr.prim.id/p3.cpp
  
test/CXX/concepts-ts/temp/temp.constr/temp.constr.constr/function-templates.cpp
  
test/CXX/concepts-ts/temp/temp.constr/temp.constr.constr/non-function-templates.cpp
  
test/CXX/concepts-ts/temp/temp.constr/temp.constr.constr/partial-specializations.cpp

Index: test/CXX/concepts-ts/temp/temp.constr/temp.constr.constr/partial-specializations.cpp
===
--- /dev/null
+++ test/CXX/concepts-ts/temp/temp.constr/temp.constr.constr/partial-specializations.cpp
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 -std=c++2a -fconcepts-ts -x c++ -verify %s
+
+namespace class_templates
+{
+  template requires sizeof(T) >= 4 // expected-note {{because 'sizeof(char) >= 4' (1 >= 4) evaluated to false}}
+  struct is_same { static constexpr bool value = false; };
+
+  template requires sizeof(T*) >= 4 && sizeof(T) >= 4
+  struct is_same { static constexpr bool value = true; };
+
+  static_assert(!is_same::value);
+  static_assert(!is_same::value);
+  static_assert(is_same::value);
+  static_assert(is_same::value); // expected-error {{constraints not satisfied for class template 'is_same' [with T = char, U = char]}}
+}
+
+namespace variable_templates
+{
+  template requires sizeof(T) >= 4
+  constexpr bool is_same_v = false;
+
+  template requires sizeof(T*) >= 4 && sizeof(T) >= 4
+  constexpr bool is_same_v = true;
+
+  static_assert(!is_same_v);
+  static_assert(!is_same_v);
+  static_assert(is_same_v);
+}
\ No newline at end of file
Index: test/CXX/concepts-ts/temp/temp.constr/temp.constr.constr/non-function-templates.cpp
===
--- /dev/null
+++ test/CXX/concepts-ts/temp/temp.constr/temp.constr.constr/non-function-templates.cpp
@@ -0,0 +1,79 @@
+// RUN: %clang_cc1 -std=c++2a -fconcepts-ts -x c++ -verify %s
+
+template requires sizeof(T) >= 2 // expected-note{{because 'sizeof(char) >= 2' (1 >= 2) evaluated to false}}
+struct A {
+  static constexpr int value = sizeof(T);
+};
+
+static_assert(A::value == 4);
+static_assert(A::value == 1); // expected-error{{constraints not satisfied for class template 'A' [with T = char]}}
+
+template
+  requires sizeof(T) != sizeof(U) // expected-note{{because 'sizeof(int) != sizeof(char [4])' (4 != 4) evaluated to false}}
+   && sizeof(T) >= 4 // expected-note{{because 'sizeof(char) >= 4' (1 >= 4) evaluated to false}}
+constexpr int SizeDiff = sizeof(T) > sizeof(U) ? sizeof(T) - sizeof(U) : sizeof(U) - sizeof(T);
+
+static_assert(SizeDiff == 3);
+static_assert(SizeDiff == 0); // expected-error{{constraints not satisfied for variable template 'SizeDiff' [with T = int, U = char [4]]}}
+static_assert(SizeDiff == 3); // expected-error{{constraints not satisfied for variable template 'SizeDiff' [with T = char, U = int]}}
+
+template
+  requires ((sizeof(Ts) == 4) || ...) // expected-note{{because 'sizeof(char) == 4' (1 == 4) evaluated to false}} expected-note{{'sizeof(long long) == 4' (8 == 4) evaluated to false}} expected-note{{'sizeof(int [20]) == 4' (80 == 4) evaluated to false}}
+constexpr auto SumSizes = (sizeof(Ts) + ...);
+
+static_assert(SumSizes == 13);
+static_assert(SumSizes == 89); // expected-error{{constraints not satisfied for variable template 'SumSizes' [with Ts = ]}}
+
+template
+concept IsBig = sizeof(T) > 100; // expected-note{{because 'sizeof(int) > 100' (4 > 100) evaluated to false}}
+
+template
+  requires IsBig // expected-note{{'int' does not satisfy 'IsBig'}}
+using BigPtr = T*;
+
+static_assert(sizeof(BigPtr)); // expected-error{{constraints not satisfied for alias template 'BigPtr' [with T = int]
+
+template requires T::value // expected-note{{because substituted constraint expression is ill-formed: type 'int' cannot be used prior to '::' because it has no members}}
+struct S { static constexpr bool value = true; };
+
+struct S2 { static constexpr bool value = true; };
+
+static_assert(S::value); // expected-error{{constraints not satisfied for class template 'S' [with T = int]}}
+static_assert(S::value);
+
+template
+struct AA
+{
+template requires sizeof(U) == sizeof(T) // expected-note{{because 'sizeof(int [2]) == sizeof(int)' (8 == 4) evaluated to false}}
+struct B
+{
+static constexpr int a = 0;
+};
+
+template requires sizeof(U) == sizeof(T) // expected-note{{becau

[PATCH] D41284: [Concepts] Associated constraints infrastructure.

2018-03-15 Thread Saar Raz via Phabricator via cfe-commits
saar.raz updated this revision to Diff 138500.
saar.raz added a comment.

Fixed reference to TemplateParams member in assertion.


Repository:
  rC Clang

https://reviews.llvm.org/D41284

Files:
  include/clang/AST/DeclTemplate.h
  include/clang/AST/RecursiveASTVisitor.h
  include/clang/Sema/Sema.h
  lib/AST/DeclTemplate.cpp
  lib/Sema/SemaConcept.cpp
  lib/Sema/SemaTemplate.cpp
  lib/Sema/SemaTemplateInstantiateDecl.cpp
  lib/Serialization/ASTReader.cpp
  lib/Serialization/ASTReaderDecl.cpp
  lib/Serialization/ASTWriter.cpp
  lib/Serialization/ASTWriterDecl.cpp
  test/CXX/concepts-ts/temp/temp.constr/temp.constr.decl/class-template-decl.cpp
  test/CXX/concepts-ts/temp/temp.constr/temp.constr.decl/func-template-decl.cpp
  test/CXX/concepts-ts/temp/temp.constr/temp.constr.decl/var-template-decl.cpp

Index: test/CXX/concepts-ts/temp/temp.constr/temp.constr.decl/var-template-decl.cpp
===
--- /dev/null
+++ test/CXX/concepts-ts/temp/temp.constr/temp.constr.decl/var-template-decl.cpp
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -std=c++2a -fconcepts-ts -x c++ -verify %s
+
+namespace nodiag {
+
+struct B {
+template  requires bool(T())
+static int A;
+};
+
+template  requires bool(U())
+int B::A = int(U());
+
+} // end namespace nodiag
+
+namespace diag {
+
+struct B {
+template  requires bool(T()) // expected-note{{previous template declaration is here}}
+static int A;
+};
+
+template  requires !bool(U())  // expected-error{{associated constraints differ in template redeclaration}}
+int B::A = int(U());
+
+} // end namespace diag
\ No newline at end of file
Index: test/CXX/concepts-ts/temp/temp.constr/temp.constr.decl/func-template-decl.cpp
===
--- test/CXX/concepts-ts/temp/temp.constr/temp.constr.decl/func-template-decl.cpp
+++ test/CXX/concepts-ts/temp/temp.constr/temp.constr.decl/func-template-decl.cpp
@@ -1,65 +1,52 @@
-// RUN: %clang_cc1 -std=c++14 -fconcepts-ts -x c++ -verify %s
+// RUN: %clang_cc1 -std=c++2a -fconcepts-ts -x c++ -verify %s
 
 namespace nodiag {
 
 template  requires bool(T())
-struct A;
+int A();
 template  requires bool(U())
-struct A;
+int A();
 
 } // end namespace nodiag
 
 namespace diag {
 
 template  requires true // expected-note{{previous template declaration is here}}
-struct A;
-template  struct A; // expected-error{{associated constraints differ in template redeclaration}}
+int A();
+template  int A(); // expected-error{{associated constraints differ in template redeclaration}}
 
-template  struct B; // expected-note{{previous template declaration is here}}
+template  int B(); // expected-note{{previous template declaration is here}}
 template  requires true // expected-error{{associated constraints differ in template redeclaration}}
-struct B;
+int B();
 
 template  requires true // expected-note{{previous template declaration is here}}
-struct C;
+int C();
 template  requires !0 // expected-error{{associated constraints differ in template redeclaration}}
-struct C;
+int C();
 
 } // end namespace diag
 
 namespace nodiag {
 
 struct AA {
   template  requires someFunc(T())
-  struct A;
+  int A();
 };
 
 template  requires someFunc(T())
-struct AA::A { };
-
-struct AAF {
-  template  requires someFunc(T())
-  friend struct AA::A;
-};
+int AA::A() { return sizeof(T); }
 
 } // end namespace nodiag
 
 namespace diag {
 
 template 
 struct TA {
-  template  class TT> requires TT::happy // expected-note 2{{previous template declaration is here}}
-  struct A;
-
-  struct AF;
+  template  class TT> requires TT::happy // expected-note{{previous template declaration is here}}
+  int A();
 };
 
 template 
-template  class TT> struct TA::A { }; // expected-error{{associated constraints differ in template redeclaration}}
-
-template 
-struct TA::AF {
-  template  class TT> requires TT::happy // expected-error{{associated constraints differ in template redeclaration}}
-  friend struct TA::A;
-};
+template  class TT> int TA::A() { return sizeof(TT); } // expected-error{{associated constraints differ in template redeclaration}}
 
 } // end namespace diag
Index: test/CXX/concepts-ts/temp/temp.constr/temp.constr.decl/class-template-decl.cpp
===
--- test/CXX/concepts-ts/temp/temp.constr/temp.constr.decl/class-template-decl.cpp
+++ test/CXX/concepts-ts/temp/temp.constr/temp.constr.decl/class-template-decl.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -std=c++14 -fconcepts-ts -x c++ -verify %s
+// RUN: %clang_cc1 -std=c++2a -fconcepts-ts -x c++ -verify %s
 
 namespace nodiag {
 
@@ -33,7 +33,7 @@
   struct A;
 };
 
-template  requires someFunc(T())
+template  requires someFunc(U())
 struct AA::A { };
 
 struct AAF {
@@ -47,18 +47,26 @@
 
 template 
 struct TA {
-  template  class TT> requires TT::happy // expected-note 2{{previous template declaration is here}}
+  template  class TT> requires TT::happy