[PATCH] D50144: Add Windows support for the GNUstep Objective-C ABI V2.

2018-08-07 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments.



Comment at: lib/AST/MicrosoftMangle.cpp:448
 mangleVariableEncoding(VD);
-  else
+  else if (!isa(D))
 llvm_unreachable("Tried to mangle unexpected NamedDecl!");

I don't think we want `ObjCInterfaceDecl`s to enter this function normally; I'd 
prefer to leave that assertion intact.



Comment at: lib/AST/MicrosoftMangle.cpp:2574
   mangleTagTypeKind(TTK_Struct);
-  mangleName(T->getDecl());
+  mangle(T->getDecl(), ".objc_cls_");
 }

You're not really reusing any interesting logic from `mangle` here; please just 
stream the literal directly into `Out`.

Also, this will add the actual identifier to the substitution set, whereas your 
implementation below in the case for ObjCObjectType adds the prefixed 
identifier to the substitution set.



Comment at: lib/CodeGen/CGBlocks.cpp:1262
+  if (IsWindows) {
+auto *Init = llvm::Function::Create(llvm::FunctionType::get(CGM.VoidTy,
+  {}), llvm::GlobalValue::InternalLinkage, ".block_isa_init",

theraven wrote:
> DHowett-MSFT wrote:
> > Is there value in emitting a list of blocks that need to be initialized, 
> > then initializing them in one go in a COMDAT-foldable function?
> I think that the best solution is to move this into the back end, so that 
> this code goes away in the front end, but anything that's referring to a 
> dllimport global in a global initialiser is transformed in the back end to a 
> list of initialisations and a comdat function that walks the list and sets 
> them up.  That said, this seems sufficiently generally useful that it would 
> be nice for the function to be in the CRT bits.  
> 
> 
> I should be in Redmond some time in October, so maybe we can discuss it with 
> some of the VS team then?
Can the blocks part of this patch be split out?  It's basically a totally 
different bugfix.



Comment at: lib/CodeGen/CGBlocks.cpp:1216
   bool IsOpenCL = CGM.getLangOpts().OpenCL;
+  bool IsWindows = CGM.getTarget().getTriple().isOSWindows();
   if (!IsOpenCL) {

Should this be a PE/COFF-specific restriction?  Or otherwise somehow restricted 
to the "native" environment?  I don't know enough about all the UNIX-on-Windows 
approaches to know whether the same restriction would apply to all of them.  I 
did think they generally used the Itanium C++ ABI, and the Itanium ABI relies 
on linking v-tables from the C++ standard library.  Maybe those environments 
just all use static linking to get around that.



Comment at: lib/CodeGen/CGBlocks.cpp:1276
+InitVar->setSection(".CRT$XCLa");
+CGM.addUsedGlobal(InitVar);
+  }

Is the priority system not good enough?



Comment at: lib/CodeGen/CGObjCGNU.cpp:915
+return name;
+  }
   /// The GCC ABI superclass message lookup function.  Takes a pointer to a

Can this section-names cleanup also just be in a separate patch?



Comment at: lib/CodeGen/CGObjCGNU.cpp:2262
 llvm::Constant *CGObjCGNUstep::GetEHType(QualType T) {
+  if (CGM.getContext().getTargetInfo().getTriple().isWindowsMSVCEnvironment())
+return CGM.getCXXABI().getAddrOfRTTIDescriptor(T);

I think this is the third different way that you test for Windows in this 
patch. :) Is there a reason why this is just testing for MSVC and the others 
are testing for PE/COFF or for Windows generally?



Comment at: lib/CodeGen/CGObjCGNU.cpp:3817
+  if (isRethrow && CGF.CGM.getTarget().getTriple().isWindowsMSVCEnvironment()) 
{
+CGF.EmitRuntimeCallOrInvoke(ExceptionReThrowFn).setDoesNotReturn();
+  }

You're sure here that the static information aligns with the dynamic?



Comment at: lib/CodeGen/CGObjCRuntime.cpp:125
 llvm::Constant *TypeInfo;
+unsigned Flags;
   };

Please add some sort of comment about the meaning and source of these flags.



Comment at: lib/CodeGen/EHScopeStack.h:424
+   void Emit(CodeGenFunction , Flags F) override;
+};
+

Please add a function on SGF to push this cleanup or something instead of 
globally declaring it.


Repository:
  rC Clang

https://reviews.llvm.org/D50144



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


[PATCH] D50099: [DebugInfo][OpenCL] Address post-commit review of D49930

2018-08-07 Thread Eric Christopher via Phabricator via cfe-commits
echristo accepted this revision.
echristo added a comment.
This revision is now accepted and ready to land.

LGTM. Thanks.


https://reviews.llvm.org/D50099



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


[PATCH] D50152: [CodeGen] Merge equivalent block copy/helper functions

2018-08-07 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

In https://reviews.llvm.org/D50152#1191777, @ahatanak wrote:

> Since BlockVarCopyInits is a map with key `VarDecl *`, I think we want to add 
> a flag to VarDecl (NonParmVarDeclBits) that indicates whether the copy 
> expression can throw or not. Or is there a reason to store the bit in 
> `BlockDecl::Capture` instead?


I was thinking about the non-`__block` capture case, sorry.  For `__block` 
variables, I think the values of that map should just be... well, I'd make a 
struct wrapping it, but basically a `PointerIntPair` of an `Expr*` and the 
`canThrow` flag.


Repository:
  rC Clang

https://reviews.llvm.org/D50152



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


[libcxx] r339218 - [libcxx] [test] Avoid -Wunused-local-typedef in node_handle.pass.cpp.

2018-08-07 Thread Billy Robert O'Neal III via cfe-commits
Author: bion
Date: Tue Aug  7 21:24:47 2018
New Revision: 339218

URL: http://llvm.org/viewvc/llvm-project?rev=339218=rev
Log:
[libcxx] [test] Avoid -Wunused-local-typedef in node_handle.pass.cpp.

Modified:
libcxx/trunk/test/std/containers/container.node/node_handle.pass.cpp

Modified: libcxx/trunk/test/std/containers/container.node/node_handle.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/containers/container.node/node_handle.pass.cpp?rev=339218=339217=339218=diff
==
--- libcxx/trunk/test/std/containers/container.node/node_handle.pass.cpp 
(original)
+++ libcxx/trunk/test/std/containers/container.node/node_handle.pass.cpp Tue 
Aug  7 21:24:47 2018
@@ -121,10 +121,12 @@ void test_node_handle_operations_multi()
 assert(nt2.empty());
 }
 
+template  void test_typedef() {}
+
 template 
 void test_insert_return_type()
 {
-using irt_type = typename Container::insert_return_type;
+test_typedef();
 }
 
 int main()


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


[PATCH] D50412: [libunwind] Fix pointer-to-integer cast warnings on LLP64.

2018-08-07 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL339217: [libunwind] Fix pointer-to-integer cast warnings on 
LLP64. (authored by cdavis, committed by ).

Repository:
  rL LLVM

https://reviews.llvm.org/D50412

Files:
  libunwind/trunk/src/UnwindLevel1-gcc-ext.c
  libunwind/trunk/src/UnwindLevel1.c


Index: libunwind/trunk/src/UnwindLevel1-gcc-ext.c
===
--- libunwind/trunk/src/UnwindLevel1-gcc-ext.c
+++ libunwind/trunk/src/UnwindLevel1-gcc-ext.c
@@ -33,9 +33,9 @@
(void *)exception_object,
(long)exception_object->unwinder_cache.reserved1);
 #else
-  _LIBUNWIND_TRACE_API("_Unwind_Resume_or_Rethrow(ex_obj=%p), private_1=%ld",
+  _LIBUNWIND_TRACE_API("_Unwind_Resume_or_Rethrow(ex_obj=%p), private_1=%" 
PRIdPTR,
(void *)exception_object,
-   (long)exception_object->private_1);
+   (intptr_t)exception_object->private_1);
 #endif
 
 #if defined(_LIBUNWIND_ARM_EHABI)
@@ -92,9 +92,9 @@
   unw_proc_info_t info;
   unw_getcontext();
   unw_init_local(, );
-  unw_set_reg(, UNW_REG_IP, (unw_word_t)(long) pc);
+  unw_set_reg(, UNW_REG_IP, (unw_word_t)(intptr_t) pc);
   if (unw_get_proc_info(, ) == UNW_ESUCCESS)
-return (void *)(long) info.start_ip;
+return (void *)(intptr_t) info.start_ip;
   else
 return NULL;
 }
@@ -190,14 +190,14 @@
   unw_proc_info_t info;
   unw_getcontext();
   unw_init_local(, );
-  unw_set_reg(, UNW_REG_IP, (unw_word_t)(long) pc);
+  unw_set_reg(, UNW_REG_IP, (unw_word_t)(intptr_t) pc);
   unw_get_proc_info(, );
   bases->tbase = (uintptr_t)info.extra;
   bases->dbase = 0; // dbase not used on Mac OS X
   bases->func = (uintptr_t)info.start_ip;
   _LIBUNWIND_TRACE_API("_Unwind_Find_FDE(pc=%p) => %p", pc,
-  (void *)(long) info.unwind_info);
-  return (void *)(long) info.unwind_info;
+  (void *)(intptr_t) info.unwind_info);
+  return (void *)(intptr_t) info.unwind_info;
 }
 
 /// Returns the CFA (call frame area, or stack pointer at start of function)
Index: libunwind/trunk/src/UnwindLevel1.c
===
--- libunwind/trunk/src/UnwindLevel1.c
+++ libunwind/trunk/src/UnwindLevel1.c
@@ -287,7 +287,7 @@
 // If there is a personality routine, tell it we are unwinding.
 if (frameInfo.handler != 0) {
   __personality_routine p =
-  (__personality_routine)(long)(frameInfo.handler);
+  (__personality_routine)(intptr_t)(frameInfo.handler);
   _LIBUNWIND_TRACE_UNWINDING(
   "unwind_phase2_forced(ex_ojb=%p): calling personality function %p",
   (void *)exception_object, (void *)(uintptr_t)p);


Index: libunwind/trunk/src/UnwindLevel1-gcc-ext.c
===
--- libunwind/trunk/src/UnwindLevel1-gcc-ext.c
+++ libunwind/trunk/src/UnwindLevel1-gcc-ext.c
@@ -33,9 +33,9 @@
(void *)exception_object,
(long)exception_object->unwinder_cache.reserved1);
 #else
-  _LIBUNWIND_TRACE_API("_Unwind_Resume_or_Rethrow(ex_obj=%p), private_1=%ld",
+  _LIBUNWIND_TRACE_API("_Unwind_Resume_or_Rethrow(ex_obj=%p), private_1=%" PRIdPTR,
(void *)exception_object,
-   (long)exception_object->private_1);
+   (intptr_t)exception_object->private_1);
 #endif
 
 #if defined(_LIBUNWIND_ARM_EHABI)
@@ -92,9 +92,9 @@
   unw_proc_info_t info;
   unw_getcontext();
   unw_init_local(, );
-  unw_set_reg(, UNW_REG_IP, (unw_word_t)(long) pc);
+  unw_set_reg(, UNW_REG_IP, (unw_word_t)(intptr_t) pc);
   if (unw_get_proc_info(, ) == UNW_ESUCCESS)
-return (void *)(long) info.start_ip;
+return (void *)(intptr_t) info.start_ip;
   else
 return NULL;
 }
@@ -190,14 +190,14 @@
   unw_proc_info_t info;
   unw_getcontext();
   unw_init_local(, );
-  unw_set_reg(, UNW_REG_IP, (unw_word_t)(long) pc);
+  unw_set_reg(, UNW_REG_IP, (unw_word_t)(intptr_t) pc);
   unw_get_proc_info(, );
   bases->tbase = (uintptr_t)info.extra;
   bases->dbase = 0; // dbase not used on Mac OS X
   bases->func = (uintptr_t)info.start_ip;
   _LIBUNWIND_TRACE_API("_Unwind_Find_FDE(pc=%p) => %p", pc,
-  (void *)(long) info.unwind_info);
-  return (void *)(long) info.unwind_info;
+  (void *)(intptr_t) info.unwind_info);
+  return (void *)(intptr_t) info.unwind_info;
 }
 
 /// Returns the CFA (call frame area, or stack pointer at start of function)
Index: libunwind/trunk/src/UnwindLevel1.c
===
--- libunwind/trunk/src/UnwindLevel1.c
+++ libunwind/trunk/src/UnwindLevel1.c
@@ -287,7 +287,7 @@
 // If there is a personality routine, tell it we are unwinding.
 if (frameInfo.handler != 0) {
   __personality_routine p =
-  

[libunwind] r339217 - [libunwind] Fix pointer-to-integer cast warnings on LLP64.

2018-08-07 Thread Charles Davis via cfe-commits
Author: cdavis
Date: Tue Aug  7 21:21:24 2018
New Revision: 339217

URL: http://llvm.org/viewvc/llvm-project?rev=339217=rev
Log:
[libunwind] Fix pointer-to-integer cast warnings on LLP64.

Summary:
`long` is too short on LLP64. We have to use `intptr_t` to
avoid truncating pointers.

Reviewers: mstorsjo, rnk, compnerd, smeenai

Subscribers: christof, cfe-commits, llvm-commits

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

Modified:
libunwind/trunk/src/UnwindLevel1-gcc-ext.c
libunwind/trunk/src/UnwindLevel1.c

Modified: libunwind/trunk/src/UnwindLevel1-gcc-ext.c
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/UnwindLevel1-gcc-ext.c?rev=339217=339216=339217=diff
==
--- libunwind/trunk/src/UnwindLevel1-gcc-ext.c (original)
+++ libunwind/trunk/src/UnwindLevel1-gcc-ext.c Tue Aug  7 21:21:24 2018
@@ -33,9 +33,9 @@ _Unwind_Resume_or_Rethrow(_Unwind_Except
(void *)exception_object,
(long)exception_object->unwinder_cache.reserved1);
 #else
-  _LIBUNWIND_TRACE_API("_Unwind_Resume_or_Rethrow(ex_obj=%p), private_1=%ld",
+  _LIBUNWIND_TRACE_API("_Unwind_Resume_or_Rethrow(ex_obj=%p), private_1=%" 
PRIdPTR,
(void *)exception_object,
-   (long)exception_object->private_1);
+   (intptr_t)exception_object->private_1);
 #endif
 
 #if defined(_LIBUNWIND_ARM_EHABI)
@@ -92,9 +92,9 @@ _LIBUNWIND_EXPORT void *_Unwind_FindEncl
   unw_proc_info_t info;
   unw_getcontext();
   unw_init_local(, );
-  unw_set_reg(, UNW_REG_IP, (unw_word_t)(long) pc);
+  unw_set_reg(, UNW_REG_IP, (unw_word_t)(intptr_t) pc);
   if (unw_get_proc_info(, ) == UNW_ESUCCESS)
-return (void *)(long) info.start_ip;
+return (void *)(intptr_t) info.start_ip;
   else
 return NULL;
 }
@@ -190,14 +190,14 @@ _LIBUNWIND_EXPORT const void *_Unwind_Fi
   unw_proc_info_t info;
   unw_getcontext();
   unw_init_local(, );
-  unw_set_reg(, UNW_REG_IP, (unw_word_t)(long) pc);
+  unw_set_reg(, UNW_REG_IP, (unw_word_t)(intptr_t) pc);
   unw_get_proc_info(, );
   bases->tbase = (uintptr_t)info.extra;
   bases->dbase = 0; // dbase not used on Mac OS X
   bases->func = (uintptr_t)info.start_ip;
   _LIBUNWIND_TRACE_API("_Unwind_Find_FDE(pc=%p) => %p", pc,
-  (void *)(long) info.unwind_info);
-  return (void *)(long) info.unwind_info;
+  (void *)(intptr_t) info.unwind_info);
+  return (void *)(intptr_t) info.unwind_info;
 }
 
 /// Returns the CFA (call frame area, or stack pointer at start of function)

Modified: libunwind/trunk/src/UnwindLevel1.c
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/UnwindLevel1.c?rev=339217=339216=339217=diff
==
--- libunwind/trunk/src/UnwindLevel1.c (original)
+++ libunwind/trunk/src/UnwindLevel1.c Tue Aug  7 21:21:24 2018
@@ -287,7 +287,7 @@ unwind_phase2_forced(unw_context_t *uc,
 // If there is a personality routine, tell it we are unwinding.
 if (frameInfo.handler != 0) {
   __personality_routine p =
-  (__personality_routine)(long)(frameInfo.handler);
+  (__personality_routine)(intptr_t)(frameInfo.handler);
   _LIBUNWIND_TRACE_UNWINDING(
   "unwind_phase2_forced(ex_ojb=%p): calling personality function %p",
   (void *)exception_object, (void *)(uintptr_t)p);


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


[PATCH] D50412: [libunwind] Fix pointer-to-integer cast warnings on LLP64.

2018-08-07 Thread Martin Storsjö via Phabricator via cfe-commits
mstorsjo accepted this revision.
mstorsjo added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rUNW libunwind

https://reviews.llvm.org/D50412



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


r339216 - Fixed a breaking test case

2018-08-07 Thread Balaji V. Iyer via cfe-commits
Author: bviyer
Date: Tue Aug  7 19:47:28 2018
New Revision: 339216

URL: http://llvm.org/viewvc/llvm-project?rev=339216=rev
Log:
Fixed a breaking test case

Modified:
cfe/trunk/test/CodeGenCXX/empty-struct-init-list.cpp

Modified: cfe/trunk/test/CodeGenCXX/empty-struct-init-list.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/empty-struct-init-list.cpp?rev=339216=339215=339216=diff
==
--- cfe/trunk/test/CodeGenCXX/empty-struct-init-list.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/empty-struct-init-list.cpp Tue Aug  7 19:47:28 
2018
@@ -8,5 +8,5 @@ typedef struct {
   a b[];
 } c;
 
-// CHECK: global %struct.c zeroinitializer, align 1
+// CHECK: {{(dso_local )?}}global %struct.c{{.*}}zeroinitializer
 c d{ };


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


[PATCH] D50152: [CodeGen] Merge equivalent block copy/helper functions

2018-08-07 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment.

Since BlockVarCopyInits is a map with key `VarDecl *`, I think we want to add a 
flag to VarDecl (NonParmVarDeclBits) that indicates whether the copy expression 
can throw or not. Or is there a reason to store the bit in `BlockDecl::Capture` 
instead?


Repository:
  rC Clang

https://reviews.llvm.org/D50152



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


[PATCH] D50372: Introduce the VTable interleaving scheme to the CFI design documentation

2018-08-07 Thread Zhaomo Yang via Phabricator via cfe-commits
zhaomo updated this revision to Diff 159636.
zhaomo added a comment.

Fix mistakes and provide more information about the interleaving algorithm


https://reviews.llvm.org/D50372

Files:
  clang/docs/ControlFlowIntegrityDesign.rst

Index: clang/docs/ControlFlowIntegrityDesign.rst
===
--- clang/docs/ControlFlowIntegrityDesign.rst
+++ clang/docs/ControlFlowIntegrityDesign.rst
@@ -274,6 +274,154 @@
 need to check that the address is in range and well aligned. This is more
 likely to occur if the virtual tables are padded.
 
+Forward-Edge CFI for Virtual Calls by Interleaving Virtual Tables
+-
+
+Dimitar et. al. proposed a novel approach that interleaves virtual tables in [1]_.  
+This approach is more efficient in terms of space because padding and bit vectors are no longer needed. 
+At the same time, it is also more efficient in terms of performance because in the interleaved layout 
+address points of the virtual tables are consecutive, thus the validity check of a virtual 
+vtable pointer is always a range check. 
+
+At a high level, the interleaving scheme consists of three steps: 1) split virtual table groups into 
+separate virtual tables, 2) order virtual tables by a pre-order traversal of the class hierarchy 
+and 3) interleave virtual tables.
+
+The interleaving scheme implemented in LLVM is inspired by [1]_ but has its own
+enhancements (more in `Interleave virtual tables`_).
+
+.. [1] `Protecting C++ Dynamic Dispatch Through VTable Interleaving `_. Dimitar Bounov, Rami Gökhan Kıcı, Sorin Lerner.
+
+Split virtual table groups into separate virtual tables
+~~~
+
+The Itanium C++ ABI glues multiple individual virtual tables for a class into a combined virtual table (virtual table group). 
+The interleaving scheme, however, can only work with individual virtual tables so it must split the combined virtual tables first.
+In comparison, the old scheme does not require the splitting but it is more efficient when the combined virtual tables have been split.
+The `GlobalSplit`_ pass is responsible for splitting combined virtual tables into individual ones. 
+
+.. _GlobalSplit: https://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/GlobalSplit.cpp?view=markup
+
+Order virtual tables by a pre-order traversal of the class hierarchy 
+
+
+This step is common to both the old scheme described above and the interleaving scheme. 
+For the interleaving scheme, since the combined virtual tables have been split in the previous step, 
+this step ensures that for any class all the compatible virtual tables will appear consecutively. 
+For the old scheme, the same property may not hold since it may work on combined virtual tables. 
+
+For example, consider the following four C++ classes:
+
+.. code-block:: c++
+
+  struct A {
+virtual void f1();
+  };
+
+  struct B : A {
+virtual void f1();
+virtual void f2();
+  };
+
+  struct C : A {
+virtual void f1();
+virtual void f3();
+  };
+
+  struct D : B {
+virtual void f1();
+virtual void f2();
+  };
+
+This step will arrange the virtual tables for A, B, C, and D in the order of *vtable-of-A, vtable-of-B, vtable-of-D, vtable-of-C*.
+
+Interleave virtual tables
+~
+
+This step is where the interleaving scheme deviates from the old scheme. Instead of laying out 
+whole virtual tables in the previously computed order, the interleaving scheme lays out table 
+entries of the virtual tables strategically to ensure the following properties:  
+
+(1) offset-to-top and RTTI fields layout property
+
+The Itanium C++ ABI specifies that offset-to-top and RTTI fields appear at the offsets behind the 
+address point. Note that libraries like libcxxabi do assume this property. 
+
+(2) virtual function entry layout property
+
+For each virtual function the distance between an virtual table entry for this function and the corresponding 
+address point is always the same. Note that dynamic dispatch relies on this property.
+
+Note that the interleaving scheme in the CFI implementation guarantees both properties above whereas the original scheme proposed   
+in [1]_ only guarantees the second property. 
+
+To illustrate how the interleaving algorithm works, let us continue with the running example.
+The algorithm first separates all the virtual table entries into two work lists. To do so, 
+it starts by allocating two work lists, one initialized with all the offset-to-top entries of virtual tables in the order 
+computed in the last step, one initlized with all the RTTI entries in the same order. 
+
+.. csv-table:: Work list 1 Layout 
+  :header: 0, 1, 2, 3
+  
+  A::offset-to-top, B::offset-to-top, D::offset-to-top, 

Re: [PATCH] D50154: [clangd] capitalize diagnostic messages

2018-08-07 Thread Alex L via cfe-commits
On 7 August 2018 at 17:40, Fangrui Song  wrote:

>
> On 2018-08-07, David Blaikie wrote:
>
>>
>> On Tue, Aug 7, 2018 at 4:02 PM Alex L  wrote:
>>
>>On Tue, 7 Aug 2018 at 11:38, David Blaikie  wrote:
>>
>>On Tue, Aug 7, 2018 at 11:22 AM Alex L  wrote:
>>
>>On Tue, 7 Aug 2018 at 10:52, David Blaikie via cfe-commits <
>>cfe-commits@lists.llvm.org> wrote:
>>
>>On Tue, Aug 7, 2018 at 10:33 AM Alex Lorenz via
>> Phabricator <
>>revi...@reviews.llvm.org> wrote:
>>
>>arphaman added a comment.
>>
>>In https://reviews.llvm.org/D50154#1191002, @dblaikie
>>wrote:
>>
>>> What's the motivation for clangd to differ from clang
>>here? (& if the first
>>>  letter is going to be capitalized, should there be a
>>period at the end? But
>>>  also the phrasing of most/all diagnostic text isn't
>> in
>>the form of complete
>>>  sentences, so this might not make sense)
>>
>>It's mostly for the presentation purposes, to match the
>>needs of our client. I first implemented it as an
>> opt-in
>>feature, but the consensus was to capitalize the
>> messages
>>all the time.
>>
>>Doesn't seem like it'd be any more expensive (amount of
>> code or
>>performance) to do that up in your client code, then,
>> would it?
>>I guess if most users of this API in time ended up
>> preferring
>>capitalized values, it'd make sense to share that
>>implementation - but to me it seems like a strange
>>transformation to happen at this level. (since it depends
>> on
>>what kind of client/how they want to render things - so it
>>seems an odd choice to bake in to the API (or even provide
>> an
>>option for, unless there are lots of users/the code was
>>especially complicated))
>>
>>My 2c - I've no vested interest or authority here.
>>
>>I think it's more in spirit with Clangd to provide output
>> that's as
>>close to the one presented by the client as possible.
>>
>>That assumes there's one client though, right? Different clients
>> might
>>reasonably have different needs for how they'd want the text
>> rendered,
>>I'd imagine.
>>
>>True. This transformation is lossless though, so the clients will
>> still be
>>able to get back the original text if needed.
>>
>> I'm not sure that c == lower(upper(c)) - well, if the character is ever
>> uppercase to begin with, it's clearly not. But even in the case of a
>> lowercase
>> character to start with I didn't think that was always true - I guess for
>> ASCII
>> /English it is, and that's all we're dealing with here.
>>
>
> Not bijection :) classical German example 'ß'.upper().lower() => 'ss'
> Though for the context (diagnostic messages where i18n is not
> concerned), this works well enough.
>
>
>>
>>And if the consensus about this particular text transformation changes
>> I'm
>>willing to revert this change for sure.
>>
>> *nod* I mean if everyone who's invested in/working on clangd agrees with
>> your
>> direction, that's cool/good. My 2c is that this sort of thing seems like
>> the
>> responsibility of the client, not the API - but that's by no means
>> authoritative.
>>
>
> Alex, would you mind sharing the discussion thread of the editor you use?


I'm not working with any particular editor right now, just working on
supporting Clangd for an internal client that used libclang before.


>
>
>
>>I would argue there's already a precedence for this kind of
>>transformations, for example, Clangd merges the diagnostic
>> messages
>>of notes and the main diagnostics into one, to make it a better
>>presentation experience in the client:
>>
>>https://github.com/llvm-mirror/clang-tools-extra/blob/
>>55bfabcc1bd75447d6338ffe6ff27c1624a8c15a/clangd/Diagnostics.
>> cpp#
>>L161
>>
>>I'm assuming that's because the API/protocol only supports a single
>>message, so the multi-message/location nuance of LLVM's
>> diagnostics are
>>necessarily lost when converting to that format?
>>
>>If that's not the case, and the API/protocol does support a
>>multiple-message diagnostic & ClangD is collapsing them early -
>> that
>>also seems like an unfortunate loss in fidelity.
>>
>>Clangd sends out both the main diagnostic, and the attached notes (that
>>don't have fixits) in a list (i.e. the hierarchy isn't directly
>> preserved,
>>although it can be recreated by the client).
>>So it looks like they're collapsed early, 

[PATCH] D50415: [clangd] extend the publishDiagnostics response to send back fixits to the client directly as well (if requested)

2018-08-07 Thread Sam McCall via Phabricator via cfe-commits
sammccall added subscribers: ilya-biryukov, jkorous.
sammccall added a comment.

Couple of thoughts. (Technically I'm out on leave so will let Jan/Ilya
review implementation and happy with whatever you decide)

Enabling

- negotiating LSP extensions is probably better done in the "capabilities"

message exchange than as a command-line flag. Generally, we want this
extension on if the *client* is aware of it. Roughly, the client
capabilities are owned by the client, and the flags are owned by the *user*.

- for simplicity, we could always enable this, unless we really think the

message size is a problem, or are worried about conflicts with future LSP
versions

Naming

- elsewhere in clangd we settled on calling a "Fix" what clang calls a

"FixItHint". The latter is long/awkward/jargon, and often gets shortened to
"FixIt" which isn't obviously a noun. The former mostly has its plain
English meaning. I'd prefer "fix" in the protocol/flags, for the same
reasons.

- obviously feel free to give these any name you prefer in your UI!


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50415



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


Re: [PATCH] D50415: [clangd] extend the publishDiagnostics response to send back fixits to the client directly as well (if requested)

2018-08-07 Thread Sam McCall via cfe-commits
Couple of thoughts. (Technically I'm out on leave so will let Jan/Ilya
review implementation and happy with whatever you decide)

Enabling
- negotiating LSP extensions is probably better done in the "capabilities"
message exchange than as a command-line flag. Generally, we want this
extension on if the *client* is aware of it. Roughly, the client
capabilities are owned by the client, and the flags are owned by the *user*.
- for simplicity, we could always enable this, unless we really think the
message size is a problem, or are worried about conflicts with future LSP
versions

Naming
- elsewhere in clangd we settled on calling a "Fix" what clang calls a
"FixItHint". The latter is long/awkward/jargon, and often gets shortened to
"FixIt" which isn't obviously a noun. The former mostly has its plain
English meaning. I'd prefer "fix" in the protocol/flags, for the same
reasons.
- obviously feel free to give these any name you prefer in your UI!

On Wed, Aug 8, 2018, 00:53 Alex Lorenz via Phabricator <
revi...@reviews.llvm.org> wrote:

> arphaman created this revision.
> arphaman added reviewers: jkorous, sammccall, ilya-biryukov.
> Herald added subscribers: dexonsmith, MaskRay, ioeric.
>
> This change extends the 'textDocument/publishDiagnostics' notification
> sent from Clangd to the client. The extension can be enabled using the
> '-fixit-usage=embed-in-diagnostic' argument. When it's enabled, Clangd
> sends out the fixits associated with the appropriate diagnostic in the body
> of the 'publicDiagnostics' notification.
>
>
> Repository:
>   rCTE Clang Tools Extra
>
> https://reviews.llvm.org/D50415
>
> Files:
>   clangd/ClangdLSPServer.cpp
>   clangd/ClangdLSPServer.h
>   clangd/Diagnostics.h
>   clangd/fuzzer/ClangdFuzzer.cpp
>   clangd/tool/ClangdMain.cpp
>   test/clangd/fixits-embed-in-diagnostic.test
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r339215 - [NFC][VFS] Fix typos in comments.

2018-08-07 Thread Volodymyr Sapsai via cfe-commits
Author: vsapsai
Date: Tue Aug  7 18:28:37 2018
New Revision: 339215

URL: http://llvm.org/viewvc/llvm-project?rev=339215=rev
Log:
[NFC][VFS] Fix typos in comments.

Modified:
cfe/trunk/lib/Basic/VirtualFileSystem.cpp

Modified: cfe/trunk/lib/Basic/VirtualFileSystem.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/VirtualFileSystem.cpp?rev=339215=339214=339215=diff
==
--- cfe/trunk/lib/Basic/VirtualFileSystem.cpp (original)
+++ cfe/trunk/lib/Basic/VirtualFileSystem.cpp Tue Aug  7 18:28:37 2018
@@ -990,7 +990,7 @@ public:
 ///   'type': 'file',
 ///   'name': ,
 ///   'use-external-name':  # Optional
-///   'external-contents': )
+///   'external-contents': 
 /// }
 /// \endverbatim
 ///
@@ -1021,7 +1021,7 @@ class RedirectingFileSystem : public vfs
   /// Currently, case-insensitive matching only works correctly with ASCII.
   bool CaseSensitive = true;
 
-  /// IsRelativeOverlay marks whether a IsExternalContentsPrefixDir path must
+  /// IsRelativeOverlay marks whether a ExternalContentsPrefixDir path must
   /// be prefixed in every 'external-contents' when reading from YAML files.
   bool IsRelativeOverlay = false;
 


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


[PATCH] D47400: [libcxx] [test] Allow a standard library that implements LWG 1203 in istream.rvalue/rvalue.pass.cpp

2018-08-07 Thread Billy Robert O'Neal III via Phabricator via cfe-commits
BillyONeal added a reviewer: ldionne.
BillyONeal added a comment.

Adding ldionne as suggested by Eric.


https://reviews.llvm.org/D47400



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


[PATCH] D47395: [libcxx] [test] Remove nonportable locale assumption in basic.ios.members/narrow.pass.cpp

2018-08-07 Thread Billy Robert O'Neal III via Phabricator via cfe-commits
BillyONeal added a reviewer: ldionne.
BillyONeal added a comment.

Adding ldionne as suggested by Eric.


https://reviews.llvm.org/D47395



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


[PATCH] D47400: [libcxx] [test] Allow a standard library that implements LWG 1203 in istream.rvalue/rvalue.pass.cpp

2018-08-07 Thread Billy Robert O'Neal III via Phabricator via cfe-commits
BillyONeal added a comment.

Committed r339214

(If there are changes requested I'll do them in a follow up commit)


https://reviews.llvm.org/D47400



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


[libcxx] r339214 - [libcxx] [test] Allow a standard library that implements LWG 1203 in istream.rvalue/rvalue.pass.cpp

2018-08-07 Thread Billy Robert O'Neal III via cfe-commits
Author: bion
Date: Tue Aug  7 17:49:02 2018
New Revision: 339214

URL: http://llvm.org/viewvc/llvm-project?rev=339214=rev
Log:
[libcxx] [test] Allow a standard library that implements LWG 1203 in 
istream.rvalue/rvalue.pass.cpp

(Still pending review at https://reviews.llvm.org/D47400 which has been open 
since may; will ask for forgiveness rather than permission :) )

Modified:

libcxx/trunk/test/std/input.output/iostream.format/input.streams/istream.rvalue/rvalue.pass.cpp

Modified: 
libcxx/trunk/test/std/input.output/iostream.format/input.streams/istream.rvalue/rvalue.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/input.output/iostream.format/input.streams/istream.rvalue/rvalue.pass.cpp?rev=339214=339213=339214=diff
==
--- 
libcxx/trunk/test/std/input.output/iostream.format/input.streams/istream.rvalue/rvalue.pass.cpp
 (original)
+++ 
libcxx/trunk/test/std/input.output/iostream.format/input.streams/istream.rvalue/rvalue.pass.cpp
 Tue Aug  7 17:49:02 2018
@@ -65,7 +65,7 @@ int main()
 { // test perfect forwarding
 assert(called == false);
 std::istringstream ss;
-auto& out = (std::move(ss) >> A{});
+auto&& out = (std::move(ss) >> A{});
 assert( == );
 assert(called);
 }


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


[PATCH] D47395: [libcxx] [test] Remove nonportable locale assumption in basic.ios.members/narrow.pass.cpp

2018-08-07 Thread Billy Robert O'Neal III via Phabricator via cfe-commits
BillyONeal added a comment.

Committed r339213

(If there are changes requested I'll do them in a follow up commit)


https://reviews.llvm.org/D47395



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


[libcxx] r339213 - [libcxx] [test] Remove nonportable locale assumption in basic.ios.members/narrow.pass.cpp

2018-08-07 Thread Billy Robert O'Neal III via cfe-commits
Author: bion
Date: Tue Aug  7 17:47:29 2018
New Revision: 339213

URL: http://llvm.org/viewvc/llvm-project?rev=339213=rev
Log:
[libcxx] [test] Remove nonportable locale assumption in 
basic.ios.members/narrow.pass.cpp

I'm not sure if libcxx is asserting UTF-8 here; but on Windows the full char 
value is always passed through in its entirety, since the default codepage is 
something like Windows-1252. The replacement character is only used for 
non-chars there; and that should be a more portable test everywhere.

(Still pending review at https://reviews.llvm.org/D47395 which has been open 
since may; will ask for forgiveness rather than permission :) )

Modified:

libcxx/trunk/test/std/input.output/iostreams.base/ios/basic.ios.members/narrow.pass.cpp

Modified: 
libcxx/trunk/test/std/input.output/iostreams.base/ios/basic.ios.members/narrow.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/input.output/iostreams.base/ios/basic.ios.members/narrow.pass.cpp?rev=339213=339212=339213=diff
==
--- 
libcxx/trunk/test/std/input.output/iostreams.base/ios/basic.ios.members/narrow.pass.cpp
 (original)
+++ 
libcxx/trunk/test/std/input.output/iostreams.base/ios/basic.ios.members/narrow.pass.cpp
 Tue Aug  7 17:47:29 2018
@@ -18,7 +18,7 @@
 
 int main()
 {
-const std::ios ios(0);
-assert(ios.narrow('c', '*') == 'c');
-assert(ios.narrow('\xFE', '*') == '*');
+const std::wios ios(0);
+assert(ios.narrow(L'c', '*') == 'c');
+assert(ios.narrow(L'\u203C', '*') == '*');
 }


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


Re: r339210 - PR38286: Don't crash when attempting to define a constructor for an

2018-08-07 Thread Richard Smith via cfe-commits
Would be good to get this into the Clang 7 release.

On Tue, 7 Aug 2018 at 17:43, Richard Smith via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: rsmith
> Date: Tue Aug  7 17:42:42 2018
> New Revision: 339210
>
> URL: http://llvm.org/viewvc/llvm-project?rev=339210=rev
> Log:
> PR38286: Don't crash when attempting to define a constructor for an
> incomplete class template.
>
> Modified:
> cfe/trunk/lib/Sema/SemaExprCXX.cpp
> cfe/trunk/test/SemaCXX/constructor.cpp
>
> Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=339210=339209=339210=diff
>
> ==
> --- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Tue Aug  7 17:42:42 2018
> @@ -113,9 +113,15 @@ ParsedType Sema::getConstructorName(Iden
>break;
>  }
>}
> -  if (!InjectedClassName && CurClass->isInvalidDecl())
> +  if (!InjectedClassName) {
> +if (!CurClass->isInvalidDecl()) {
> +  // FIXME: RequireCompleteDeclContext doesn't check dependent
> contexts
> +  // properly. Work around it here for now.
> +  Diag(SS.getLastQualifierNameLoc(),
> +   diag::err_incomplete_nested_name_spec) << CurClass <<
> SS.getRange();
> +}
>  return ParsedType();
> -  assert(InjectedClassName && "couldn't find injected class name");
> +  }
>
>QualType T = Context.getTypeDeclType(InjectedClassName);
>DiagnoseUseOfDecl(InjectedClassName, NameLoc);
>
> Modified: cfe/trunk/test/SemaCXX/constructor.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/constructor.cpp?rev=339210=339209=339210=diff
>
> ==
> --- cfe/trunk/test/SemaCXX/constructor.cpp (original)
> +++ cfe/trunk/test/SemaCXX/constructor.cpp Tue Aug  7 17:42:42 2018
> @@ -86,3 +86,14 @@ A::S::operator int() { return 1; }
>
>  A::S::~S() {}
>
> +namespace PR38286 {
> +  // FIXME: It'd be nice to give more consistent diagnostics for these
> cases
> +  // (but they're all failing for somewhat different reasons...).
> +  template struct A;
> +  template A::A() {} // expected-error {{incomplete type
> 'A' named in nested name specifier}}
> +  /*FIXME: needed to recover properly from previous error*/;
> +  template struct B;
> +  template void B::f() {} // expected-error {{out-of-line
> definition of 'f' from class 'B'}}
> +  template struct C;
> +  template C::~C() {} // expected-error {{no type named
> 'C' in 'C'}}
> +}
>
>
> ___
> 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] D50421: [libcxx] [test] Remove assertion that includes and .

2018-08-07 Thread Billy Robert O'Neal III via Phabricator via cfe-commits
BillyONeal closed this revision.
BillyONeal marked 2 inline comments as done.
BillyONeal added a comment.

Committed r339212


https://reviews.llvm.org/D50421



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


[PATCH] D50421: [libcxx] [test] Remove assertion that includes and .

2018-08-07 Thread Billy Robert O'Neal III via Phabricator via cfe-commits
BillyONeal updated this revision to Diff 159632.
BillyONeal added a comment.

Remove comma.


https://reviews.llvm.org/D50421

Files:
  test/std/utilities/template.bitset/includes.pass.cpp


Index: test/std/utilities/template.bitset/includes.pass.cpp
===
--- test/std/utilities/template.bitset/includes.pass.cpp
+++ test/std/utilities/template.bitset/includes.pass.cpp
@@ -7,26 +7,17 @@
 //
 
//===--===//
 
-// test that  includes , ,  and 
+// test that  includes  and 
 
 #include 
 
 template  void test_typedef() {}
 
 int main()
 {
-  { // test for 
-std::ptrdiff_t p; ((void)p);
-std::size_t s; ((void)s);
-std::nullptr_t np; ((void)np);
-  }
   { // test for 
 std::string s; ((void)s);
   }
-  { // test for 
-std::logic_error le("blah"); ((void)le);
-std::runtime_error re("blah"); ((void)re);
-  }
   { // test for 
 test_typedef();
 test_typedef();


Index: test/std/utilities/template.bitset/includes.pass.cpp
===
--- test/std/utilities/template.bitset/includes.pass.cpp
+++ test/std/utilities/template.bitset/includes.pass.cpp
@@ -7,26 +7,17 @@
 //
 //===--===//
 
-// test that  includes , ,  and 
+// test that  includes  and 
 
 #include 
 
 template  void test_typedef() {}
 
 int main()
 {
-  { // test for 
-std::ptrdiff_t p; ((void)p);
-std::size_t s; ((void)s);
-std::nullptr_t np; ((void)np);
-  }
   { // test for 
 std::string s; ((void)s);
   }
-  { // test for 
-std::logic_error le("blah"); ((void)le);
-std::runtime_error re("blah"); ((void)re);
-  }
   { // test for 
 test_typedef();
 test_typedef();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[libcxx] r339212 - [libcxx] [test] Remove asserts that and are included by

2018-08-07 Thread Billy Robert O'Neal III via cfe-commits
Author: bion
Date: Tue Aug  7 17:43:38 2018
New Revision: 339212

URL: http://llvm.org/viewvc/llvm-project?rev=339212=rev
Log:
[libcxx] [test] Remove asserts that  and  are included by 


Reviewed as https://reviews.llvm.org/D50421

Modified:
libcxx/trunk/test/std/utilities/template.bitset/includes.pass.cpp

Modified: libcxx/trunk/test/std/utilities/template.bitset/includes.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/utilities/template.bitset/includes.pass.cpp?rev=339212=339211=339212=diff
==
--- libcxx/trunk/test/std/utilities/template.bitset/includes.pass.cpp (original)
+++ libcxx/trunk/test/std/utilities/template.bitset/includes.pass.cpp Tue Aug  
7 17:43:38 2018
@@ -7,7 +7,7 @@
 //
 
//===--===//
 
-// test that  includes , ,  and 
+// test that  includes  and 
 
 #include 
 
@@ -15,18 +15,9 @@ template  void test_typedef() {}
 
 int main()
 {
-  { // test for 
-std::ptrdiff_t p; ((void)p);
-std::size_t s; ((void)s);
-std::nullptr_t np; ((void)np);
-  }
   { // test for 
 std::string s; ((void)s);
   }
-  { // test for 
-std::logic_error le("blah"); ((void)le);
-std::runtime_error re("blah"); ((void)re);
-  }
   { // test for 
 test_typedef();
 test_typedef();


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


r339210 - PR38286: Don't crash when attempting to define a constructor for an

2018-08-07 Thread Richard Smith via cfe-commits
Author: rsmith
Date: Tue Aug  7 17:42:42 2018
New Revision: 339210

URL: http://llvm.org/viewvc/llvm-project?rev=339210=rev
Log:
PR38286: Don't crash when attempting to define a constructor for an
incomplete class template.

Modified:
cfe/trunk/lib/Sema/SemaExprCXX.cpp
cfe/trunk/test/SemaCXX/constructor.cpp

Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=339210=339209=339210=diff
==
--- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Tue Aug  7 17:42:42 2018
@@ -113,9 +113,15 @@ ParsedType Sema::getConstructorName(Iden
   break;
 }
   }
-  if (!InjectedClassName && CurClass->isInvalidDecl())
+  if (!InjectedClassName) {
+if (!CurClass->isInvalidDecl()) {
+  // FIXME: RequireCompleteDeclContext doesn't check dependent contexts
+  // properly. Work around it here for now.
+  Diag(SS.getLastQualifierNameLoc(),
+   diag::err_incomplete_nested_name_spec) << CurClass << SS.getRange();
+}
 return ParsedType();
-  assert(InjectedClassName && "couldn't find injected class name");
+  }
 
   QualType T = Context.getTypeDeclType(InjectedClassName);
   DiagnoseUseOfDecl(InjectedClassName, NameLoc);

Modified: cfe/trunk/test/SemaCXX/constructor.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/constructor.cpp?rev=339210=339209=339210=diff
==
--- cfe/trunk/test/SemaCXX/constructor.cpp (original)
+++ cfe/trunk/test/SemaCXX/constructor.cpp Tue Aug  7 17:42:42 2018
@@ -86,3 +86,14 @@ A::S::operator int() { return 1; }
 
 A::S::~S() {}
 
+namespace PR38286 {
+  // FIXME: It'd be nice to give more consistent diagnostics for these cases
+  // (but they're all failing for somewhat different reasons...).
+  template struct A;
+  template A::A() {} // expected-error {{incomplete type 'A' 
named in nested name specifier}}
+  /*FIXME: needed to recover properly from previous error*/;
+  template struct B;
+  template void B::f() {} // expected-error {{out-of-line 
definition of 'f' from class 'B'}}
+  template struct C;
+  template C::~C() {} // expected-error {{no type named 'C' in 
'C'}}
+}


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


[PATCH] D50420: [libcxx] [test] Add missing to several tests.

2018-08-07 Thread Billy Robert O'Neal III via Phabricator via cfe-commits
BillyONeal closed this revision.
BillyONeal added a comment.

Committed r339209


https://reviews.llvm.org/D50420



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


Re: [PATCH] D50154: [clangd] capitalize diagnostic messages

2018-08-07 Thread Fangrui Song via cfe-commits


On 2018-08-07, David Blaikie wrote:


On Tue, Aug 7, 2018 at 4:02 PM Alex L  wrote:

   On Tue, 7 Aug 2018 at 11:38, David Blaikie  wrote:

   On Tue, Aug 7, 2018 at 11:22 AM Alex L  wrote:

   On Tue, 7 Aug 2018 at 10:52, David Blaikie via cfe-commits <
   cfe-commits@lists.llvm.org> wrote:

   On Tue, Aug 7, 2018 at 10:33 AM Alex Lorenz via Phabricator <
   revi...@reviews.llvm.org> wrote:

   arphaman added a comment.

   In https://reviews.llvm.org/D50154#1191002, @dblaikie
   wrote:

   > What's the motivation for clangd to differ from clang
   here? (& if the first
   >  letter is going to be capitalized, should there be a
   period at the end? But
   >  also the phrasing of most/all diagnostic text isn't in
   the form of complete
   >  sentences, so this might not make sense)

   It's mostly for the presentation purposes, to match the
   needs of our client. I first implemented it as an opt-in
   feature, but the consensus was to capitalize the messages
   all the time.

   Doesn't seem like it'd be any more expensive (amount of code or
   performance) to do that up in your client code, then, would it?
   I guess if most users of this API in time ended up preferring
   capitalized values, it'd make sense to share that
   implementation - but to me it seems like a strange
   transformation to happen at this level. (since it depends on
   what kind of client/how they want to render things - so it
   seems an odd choice to bake in to the API (or even provide an
   option for, unless there are lots of users/the code was
   especially complicated))

   My 2c - I've no vested interest or authority here.

   I think it's more in spirit with Clangd to provide output that's as
   close to the one presented by the client as possible. 

   That assumes there's one client though, right? Different clients might
   reasonably have different needs for how they'd want the text rendered,
   I'd imagine.

   True. This transformation is lossless though, so the clients will still be
   able to get back the original text if needed.

I'm not sure that c == lower(upper(c)) - well, if the character is ever
uppercase to begin with, it's clearly not. But even in the case of a lowercase
character to start with I didn't think that was always true - I guess for ASCII
/English it is, and that's all we're dealing with here.


Not bijection :) classical German example 'ß'.upper().lower() => 'ss'
Though for the context (diagnostic messages where i18n is not
concerned), this works well enough.


 

   And if the consensus about this particular text transformation changes I'm
   willing to revert this change for sure.

*nod* I mean if everyone who's invested in/working on clangd agrees with your
direction, that's cool/good. My 2c is that this sort of thing seems like the
responsibility of the client, not the API - but that's by no means
authoritative.


Alex, would you mind sharing the discussion thread of the editor you use?



   I would argue there's already a precedence for this kind of
   transformations, for example, Clangd merges the diagnostic messages
   of notes and the main diagnostics into one, to make it a better
   presentation experience in the client:

   https://github.com/llvm-mirror/clang-tools-extra/blob/
   55bfabcc1bd75447d6338ffe6ff27c1624a8c15a/clangd/Diagnostics.cpp#
   L161

   I'm assuming that's because the API/protocol only supports a single
   message, so the multi-message/location nuance of LLVM's diagnostics are
   necessarily lost when converting to that format?

   If that's not the case, and the API/protocol does support a
   multiple-message diagnostic & ClangD is collapsing them early - that
   also seems like an unfortunate loss in fidelity.

   Clangd sends out both the main diagnostic, and the attached notes (that
   don't have fixits) in a list (i.e. the hierarchy isn't directly preserved,
   although it can be recreated by the client).
   So it looks like they're collapsed early, but at the same time the client
   has enough information to do this transformation itself if desired.
   I'm planning to work on an option to remove this behavior if desired by the
   client.

*nod* I'd (completely in the abstract - not having worked on clangd or its
clients, so take with grain of salt, etc) probably still leave this up to the
client (which would be easier if the client were an in-process API kind of
thing rather than a strict protocol - because it'd be easy to include a
function clients could 

[libcxx] r339209 - [libcxx] [test] Add missing in several tests.

2018-08-07 Thread Billy Robert O'Neal III via cfe-commits
Author: bion
Date: Tue Aug  7 17:40:32 2018
New Revision: 339209

URL: http://llvm.org/viewvc/llvm-project?rev=339209=rev
Log:
[libcxx] [test] Add missing  in several tests.

Reviewed as https://reviews.llvm.org/D50420

Modified:

libcxx/trunk/test/std/strings/basic.string/string.capacity/over_max_size.pass.cpp
libcxx/trunk/test/std/strings/string.conversions/stod.pass.cpp
libcxx/trunk/test/std/strings/string.conversions/stof.pass.cpp
libcxx/trunk/test/std/strings/string.conversions/stoi.pass.cpp
libcxx/trunk/test/std/strings/string.conversions/stol.pass.cpp
libcxx/trunk/test/std/strings/string.conversions/stoll.pass.cpp
libcxx/trunk/test/std/strings/string.conversions/stoul.pass.cpp
libcxx/trunk/test/std/strings/string.conversions/stoull.pass.cpp

libcxx/trunk/test/std/strings/string.view/string.view.ops/compare.pointer_size.pass.cpp

libcxx/trunk/test/std/strings/string.view/string.view.ops/compare.size_size_sv.pass.cpp

libcxx/trunk/test/std/strings/string.view/string.view.ops/compare.size_size_sv_pointer_size.pass.cpp

libcxx/trunk/test/std/strings/string.view/string.view.ops/compare.size_size_sv_size_size.pass.cpp
libcxx/trunk/test/std/strings/string.view/string.view.ops/copy.pass.cpp

libcxx/trunk/test/std/utilities/template.bitset/bitset.members/flip_one.pass.cpp

libcxx/trunk/test/std/utilities/template.bitset/bitset.members/reset_one.pass.cpp

libcxx/trunk/test/std/utilities/template.bitset/bitset.members/set_one.pass.cpp
libcxx/trunk/test/std/utilities/template.bitset/bitset.members/test.pass.cpp

Modified: 
libcxx/trunk/test/std/strings/basic.string/string.capacity/over_max_size.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/strings/basic.string/string.capacity/over_max_size.pass.cpp?rev=339209=339208=339209=diff
==
--- 
libcxx/trunk/test/std/strings/basic.string/string.capacity/over_max_size.pass.cpp
 (original)
+++ 
libcxx/trunk/test/std/strings/basic.string/string.capacity/over_max_size.pass.cpp
 Tue Aug  7 17:40:32 2018
@@ -20,6 +20,7 @@
 
 #include 
 #include 
+#include 
 
 #include "min_allocator.h"
 

Modified: libcxx/trunk/test/std/strings/string.conversions/stod.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/strings/string.conversions/stod.pass.cpp?rev=339209=339208=339209=diff
==
--- libcxx/trunk/test/std/strings/string.conversions/stod.pass.cpp (original)
+++ libcxx/trunk/test/std/strings/string.conversions/stod.pass.cpp Tue Aug  7 
17:40:32 2018
@@ -15,6 +15,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "test_macros.h"
 

Modified: libcxx/trunk/test/std/strings/string.conversions/stof.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/strings/string.conversions/stof.pass.cpp?rev=339209=339208=339209=diff
==
--- libcxx/trunk/test/std/strings/string.conversions/stof.pass.cpp (original)
+++ libcxx/trunk/test/std/strings/string.conversions/stof.pass.cpp Tue Aug  7 
17:40:32 2018
@@ -19,6 +19,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "test_macros.h"
 

Modified: libcxx/trunk/test/std/strings/string.conversions/stoi.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/strings/string.conversions/stoi.pass.cpp?rev=339209=339208=339209=diff
==
--- libcxx/trunk/test/std/strings/string.conversions/stoi.pass.cpp (original)
+++ libcxx/trunk/test/std/strings/string.conversions/stoi.pass.cpp Tue Aug  7 
17:40:32 2018
@@ -14,6 +14,7 @@
 
 #include 
 #include 
+#include 
 
 #include "test_macros.h"
 

Modified: libcxx/trunk/test/std/strings/string.conversions/stol.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/strings/string.conversions/stol.pass.cpp?rev=339209=339208=339209=diff
==
--- libcxx/trunk/test/std/strings/string.conversions/stol.pass.cpp (original)
+++ libcxx/trunk/test/std/strings/string.conversions/stol.pass.cpp Tue Aug  7 
17:40:32 2018
@@ -18,6 +18,7 @@
 
 #include 
 #include 
+#include 
 
 #include "test_macros.h"
 

Modified: libcxx/trunk/test/std/strings/string.conversions/stoll.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/strings/string.conversions/stoll.pass.cpp?rev=339209=339208=339209=diff
==
--- libcxx/trunk/test/std/strings/string.conversions/stoll.pass.cpp (original)
+++ libcxx/trunk/test/std/strings/string.conversions/stoll.pass.cpp Tue Aug  7 
17:40:32 2018
@@ -18,6 +18,7 @@
 
 #include 
 #include 
+#include 
 
 #include "test_macros.h"
 

Modified: 

[PATCH] D50418: [Sema] Support for P0961R1: Relaxing the structured bindings customization point finding rules

2018-08-07 Thread Erik Pilkington via Phabricator via cfe-commits
erik.pilkington added inline comments.



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:1118-1130
+//   ... and if that finds at least one declaration that is a function
+//   template whose first template parameter is a non-type parameter ...
+LookupResult::Filter Filter = MemberGet.makeFilter();
+while (Filter.hasNext()) {
+  NamedDecl *ND = Filter.next();
+  if (auto *FTD = dyn_cast(ND)) {
+TemplateParameterList *TPL = FTD->getTemplateParameters();

rsmith wrote:
> This should be done by walking the lookup results, not by filtering them. 
> Testcase:
> 
> ```
> struct A {
>   int get();
> };
> struct B {
>   template int get();
> };
> struct C : A, B {};
> // plus specializations of tuple_size and tuple_element
> auto [x] = C();
> ```
> 
> This should be ill-formed due to ambiguity when looking up `C::get`. We 
> should not resolve the ambiguity by selecting `B::get`.
Ah, I see. In the new patch we finish the class member access lookup, then we 
check this condition separately. I added this test to the patch. Thanks!


https://reviews.llvm.org/D50418



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


[PATCH] D50418: [Sema] Support for P0961R1: Relaxing the structured bindings customization point finding rules

2018-08-07 Thread Erik Pilkington via Phabricator via cfe-commits
erik.pilkington updated this revision to Diff 159629.
erik.pilkington added a comment.

Address review comments.


https://reviews.llvm.org/D50418

Files:
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CXX/dcl.decl/dcl.decomp/p3.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -755,7 +755,7 @@
   
 
 http://wg21.link/p0961r1;>P0961R1 (DR)
-No
+SVN
   
   
 
Index: clang/test/CXX/dcl.decl/dcl.decomp/p3.cpp
===
--- clang/test/CXX/dcl.decl/dcl.decomp/p3.cpp
+++ clang/test/CXX/dcl.decl/dcl.decomp/p3.cpp
@@ -36,7 +36,7 @@
   auto [a0, a1, a2] = A(); // expected-error {{undeclared identifier 'get'}} expected-note {{in implicit initialization of binding declaration 'a0'}}
 }
 
-template float (A);
+template float (A); // expected-note 2 {{no known conversion}}
 
 void no_tuple_element_1() {
   auto [a0, a1, a2] = A(); // expected-error-re {{'std::tuple_element<0U{{L*}}, A>::type' does not name a type}} expected-note {{in implicit}}
@@ -57,7 +57,7 @@
 template<> struct std::tuple_element<1, A> { typedef float  };
 template<> struct std::tuple_element<2, A> { typedef const float  };
 
-template auto get(B) -> int (&)[N + 1];
+template auto get(B) -> int (&)[N + 1]; // expected-note 2 {{no known conversion}}
 template struct std::tuple_element { typedef int type[N +1 ]; };
 
 template struct std::tuple_size : std::tuple_size {};
@@ -138,19 +138,25 @@
   return c;
 }
 
-struct D { template struct get {}; }; // expected-note {{declared here}}
+struct D {
+  // FIXME: Emit a note here explaining why this was ignored.
+  template struct get {};
+};
 template<> struct std::tuple_size { static const int value = 1; };
 template<> struct std::tuple_element<0, D> { typedef D::get<0> type; };
 void member_get_class_template() {
-  auto [d] = D(); // expected-error {{cannot refer to member 'get' in 'D' with '.'}} expected-note {{in implicit init}}
+  auto [d] = D(); // expected-error {{no matching function for call to 'get'}} expected-note {{in implicit init}}
 }
 
-struct E { int get(); };
+struct E {
+  // FIXME: Emit a note here explaining why this was ignored.
+  int get();
+};
 template<> struct std::tuple_size { static const int value = 1; };
 template<> struct std::tuple_element<0, E> { typedef int type; };
 void member_get_non_template() {
   // FIXME: This diagnostic is not very good.
-  auto [e] = E(); // expected-error {{no member named 'get'}} expected-note {{in implicit init}}
+  auto [e] = E(); // expected-error {{no matching function for call to 'get'}} expected-note {{in implicit init}}
 }
 
 namespace ADL {
@@ -230,3 +236,48 @@
   }
   static_assert(g() == 4); // expected-error {{constant}} expected-note {{in call to 'g()'}}
 }
+
+// P0961R1
+struct InvalidMemberGet {
+  int get();
+  template  int get();
+  struct get {};
+};
+template <> struct std::tuple_size { static constexpr size_t value = 1; };
+template <> struct std::tuple_element<0, InvalidMemberGet> { typedef float type; };
+template  float get(InvalidMemberGet) { return 0; }
+int f() {
+  InvalidMemberGet img;
+  auto [x] = img;
+  typedef decltype(x) same_as_float;
+  typedef float same_as_float;
+}
+
+struct ValidMemberGet {
+  int get();
+  template  int get() { return 0; }
+  template  float get() { return 0; }
+};
+template <> struct std::tuple_size { static constexpr size_t value = 1; };
+template <> struct std::tuple_element<0, ValidMemberGet> { typedef float type; };
+// Don't use this one; we should use the member get.
+template  int get(ValidMemberGet) { static_assert(N && false, ""); }
+int f2() {
+  ValidMemberGet img;
+  auto [x] = img;
+  typedef decltype(x) same_as_float;
+  typedef float same_as_float;
+}
+
+struct Base1 {
+  int get(); // expected-note{{member found by ambiguous name lookup}}
+};
+struct Base2 {
+  template int get(); // expected-note{{member found by ambiguous name lookup}}
+};
+struct Derived : Base1, Base2 {};
+
+template <> struct std::tuple_size { static constexpr size_t value = 1; };
+template <> struct std::tuple_element<0, Derived> { typedef int type; };
+
+auto [x] = Derived(); // expected-error{{member 'get' found in multiple base classes of different types}}
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -1108,15 +1108,26 @@
 
   // [dcl.decomp]p3:
   //   The unqualified-id get is looked up in the scope of E by class member
-  //   access lookup
+  //   access lookup ...
   LookupResult MemberGet(S, GetDN, Src->getLocation(), Sema::LookupMemberName);
   bool UseMemberGet = false;
   if (S.isCompleteType(Src->getLocation(), DecompType)) {
 if (auto *RD = DecompType->getAsCXXRecordDecl())
   

[PATCH] D50421: [libcxx] [test] Remove assertion that includes and .

2018-08-07 Thread Billy Robert O'Neal III via Phabricator via cfe-commits
BillyONeal added inline comments.



Comment at: test/std/utilities/template.bitset/includes.pass.cpp:11
+// test that  includes , and 

 #include 
 template  void test_typedef() {}

Quuxplusone wrote:
> grammar nit: comma
NP: Oxford Comma by Vampire Weekend


https://reviews.llvm.org/D50421



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


[PATCH] D50421: [libcxx] [test] Remove assertion that includes and .

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



Comment at: test/std/utilities/template.bitset/includes.pass.cpp:11
+// test that  includes , and 

 #include 
 template  void test_typedef() {}

grammar nit: comma


https://reviews.llvm.org/D50421



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


[PATCH] D50413: [libunwind][include] Add some missing definitions to .

2018-08-07 Thread Kamil Rytarowski via Phabricator via cfe-commits
krytarowski added a comment.

NetBSD uses `typedef void *_Unwind_Ptr;` unconditionally in its ``... 
if that has to be matched.

Additionally: `typedef long _Unwind_Word;`.


Repository:
  rUNW libunwind

https://reviews.llvm.org/D50413



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


[PATCH] D50421: [libcxx] [test] Remove assertion that includes and .

2018-08-07 Thread Eric Fiselier via Phabricator via cfe-commits
EricWF accepted this revision.
EricWF added a comment.
This revision is now accepted and ready to land.

This looks fine to me.


https://reviews.llvm.org/D50421



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


r339207 - [CodeGen] IncompleteArray Support

2018-08-07 Thread Balaji V. Iyer via cfe-commits
Author: bviyer
Date: Tue Aug  7 17:01:21 2018
New Revision: 339207

URL: http://llvm.org/viewvc/llvm-project?rev=339207=rev
Log:
[CodeGen] IncompleteArray Support 

Added code to support ArrayType that is not ConstantArray.

https://reviews.llvm.org/D49952
rdar://42476155



Added:
cfe/trunk/test/CodeGenCXX/empty-struct-init-list.cpp
Modified:
cfe/trunk/lib/CodeGen/CGExprConstant.cpp

Modified: cfe/trunk/lib/CodeGen/CGExprConstant.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprConstant.cpp?rev=339207=339206=339207=diff
==
--- cfe/trunk/lib/CodeGen/CGExprConstant.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExprConstant.cpp Tue Aug  7 17:01:21 2018
@@ -1968,6 +1968,16 @@ llvm::Constant *ConstantEmitter::tryEmit
   Elts.push_back(C);
 }
 
+// This means that the array type is probably "IncompleteType" or some
+// type that is not ConstantArray.
+if (CAT == nullptr && CommonElementType == nullptr && !NumInitElts) {
+  const ArrayType *AT = CGM.getContext().getAsArrayType(DestType);
+  CommonElementType = CGM.getTypes().ConvertType(AT->getElementType());
+  llvm::ArrayType *AType = llvm::ArrayType::get(CommonElementType,
+NumElements);
+  return llvm::ConstantAggregateZero::get(AType);
+}
+
 return EmitArrayConstant(CGM, CAT, CommonElementType, NumElements, Elts,
  Filler);
   }

Added: cfe/trunk/test/CodeGenCXX/empty-struct-init-list.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/empty-struct-init-list.cpp?rev=339207=auto
==
--- cfe/trunk/test/CodeGenCXX/empty-struct-init-list.cpp (added)
+++ cfe/trunk/test/CodeGenCXX/empty-struct-init-list.cpp Tue Aug  7 17:01:21 
2018
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -std=c++11 -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -std=c++14 -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -std=c++17 -emit-llvm -o - %s | FileCheck %s
+
+// CHECK: struct.a
+typedef struct { } a;
+typedef struct {
+  a b[];
+} c;
+
+// CHECK: global %struct.c zeroinitializer, align 1
+c d{ };


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


[PATCH] D50168: [Builtins] Implement __builtin_clrsb to be compatible with gcc

2018-08-07 Thread Craig Topper via Phabricator via cfe-commits
craig.topper updated this revision to Diff 159627.
craig.topper added a comment.

Add the test case that I failed to pick up in the original diff.


https://reviews.llvm.org/D50168

Files:
  include/clang/Basic/Builtins.def
  lib/CodeGen/CGBuiltin.cpp
  test/CodeGen/builtin_clrsb.c


Index: test/CodeGen/builtin_clrsb.c
===
--- /dev/null
+++ test/CodeGen/builtin_clrsb.c
@@ -0,0 +1,24 @@
+// RUN: %clang_cc1 %s -emit-llvm -o - | FileCheck %s
+
+int test__builtin_clrsb(int x) {
+// CHECK-LABEL: test__builtin_clrsb
+// CHECK: [[C:%.*]] = icmp slt i32 [[X:%.*]], 0
+// CHECK: [[INV:%.*]] = xor i32 [[X]], -1
+// CHECK: [[SEL:%.*]] = select i1 [[C]], i32 [[INV]], i32 [[X]]
+// CHECK: [[SHL:%.*]] = shl i32 [[SEL]], 1
+// CHECK: [[OR:%.*]] = or i32 [[SHL]], 1
+// CHECK: call i32 @llvm.ctlz.i32(i32 [[OR]], i1 true)
+  return __builtin_clrsb(x);
+}
+
+int test__builtin_clrsbll(long long x) {
+// CHECK-LABEL: test__builtin_clrsbll
+// CHECK: [[C:%.*]] = icmp slt i64 [[X:%.*]], 0
+// CHECK-NEXT: [[INV:%.*]] = xor i64 [[X]], -1
+// CHECK-NEXT: [[SEL:%.*]] = select i1 [[C]], i64 [[INV]], i64 [[X]]
+// CHECK-NEXT: [[SHL:%.*]] = shl i64 [[SEL]], 1
+// CHECK-NEXT: [[OR:%.*]] = or i64 [[SHL]], 1
+// CHECK-NEXT: [[CTLZ:%.*]] = call i64 @llvm.ctlz.i64(i64 [[OR]], i1 true)
+// CHECK-NEXT: trunc i64 [[CTLZ]] to i32
+  return __builtin_clrsbll(x);
+}
Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -1537,6 +1537,33 @@
 return RValue::get(ComplexVal.second);
   }
 
+  case Builtin::BI__builtin_clrsb:
+  case Builtin::BI__builtin_clrsbl:
+  case Builtin::BI__builtin_clrsbll: {
+// clrsb(x) -> clz(x < 0 ? ~x : x) - 1 or
+//  -> clz(((x < 0 ? ~x : x) << 1) | 1)
+Value *ArgValue = EmitScalarExpr(E->getArg(0));
+
+llvm::Type *ArgType = ArgValue->getType();
+Value *F = CGM.getIntrinsic(Intrinsic::ctlz, ArgType);
+
+llvm::Type *ResultType = ConvertType(E->getType());
+Value *Zero = llvm::Constant::getNullValue(ArgType);
+Value *IsNeg = Builder.CreateICmpSLT(ArgValue, Zero, "isneg");
+Value *Inverse = Builder.CreateNot(ArgValue, "not");
+Value *Tmp = Builder.CreateSelect(IsNeg, Inverse, ArgValue);
+// Now we need to calculate ctlz(Tmp)-1, but Tmp might be zero. We know
+// the sign bit is zero, so we can shift it out. Then put a 1 in the LSB.
+// This removes one leading zero like the subtract does, and replaces it
+// with a guaranteed one to prevent the value being 0.
+Value *One = llvm::ConstantInt::get(ArgType, 1);
+Tmp = Builder.CreateShl(Tmp, One);
+Tmp = Builder.CreateOr(Tmp, One);
+Value *Result = Builder.CreateCall(F, {Tmp, Builder.getTrue()});
+Result = Builder.CreateIntCast(Result, ResultType, /*isSigned*/true,
+   "cast");
+return RValue::get(Result);
+  }
   case Builtin::BI__builtin_ctzs:
   case Builtin::BI__builtin_ctz:
   case Builtin::BI__builtin_ctzl:
Index: include/clang/Basic/Builtins.def
===
--- include/clang/Basic/Builtins.def
+++ include/clang/Basic/Builtins.def
@@ -413,6 +413,9 @@
 BUILTIN(__builtin_popcount  , "iUi"  , "nc")
 BUILTIN(__builtin_popcountl , "iULi" , "nc")
 BUILTIN(__builtin_popcountll, "iULLi", "nc")
+BUILTIN(__builtin_clrsb  , "ii"  , "nc")
+BUILTIN(__builtin_clrsbl , "iLi" , "nc")
+BUILTIN(__builtin_clrsbll, "iLLi", "nc")
 
 // FIXME: These type signatures are not correct for targets with int != 32-bits
 // or with ULL != 64-bits.


Index: test/CodeGen/builtin_clrsb.c
===
--- /dev/null
+++ test/CodeGen/builtin_clrsb.c
@@ -0,0 +1,24 @@
+// RUN: %clang_cc1 %s -emit-llvm -o - | FileCheck %s
+
+int test__builtin_clrsb(int x) {
+// CHECK-LABEL: test__builtin_clrsb
+// CHECK: [[C:%.*]] = icmp slt i32 [[X:%.*]], 0
+// CHECK: [[INV:%.*]] = xor i32 [[X]], -1
+// CHECK: [[SEL:%.*]] = select i1 [[C]], i32 [[INV]], i32 [[X]]
+// CHECK: [[SHL:%.*]] = shl i32 [[SEL]], 1
+// CHECK: [[OR:%.*]] = or i32 [[SHL]], 1
+// CHECK: call i32 @llvm.ctlz.i32(i32 [[OR]], i1 true)
+  return __builtin_clrsb(x);
+}
+
+int test__builtin_clrsbll(long long x) {
+// CHECK-LABEL: test__builtin_clrsbll
+// CHECK: [[C:%.*]] = icmp slt i64 [[X:%.*]], 0
+// CHECK-NEXT: [[INV:%.*]] = xor i64 [[X]], -1
+// CHECK-NEXT: [[SEL:%.*]] = select i1 [[C]], i64 [[INV]], i64 [[X]]
+// CHECK-NEXT: [[SHL:%.*]] = shl i64 [[SEL]], 1
+// CHECK-NEXT: [[OR:%.*]] = or i64 [[SHL]], 1
+// CHECK-NEXT: [[CTLZ:%.*]] = call i64 @llvm.ctlz.i64(i64 [[OR]], i1 true)
+// CHECK-NEXT: trunc i64 [[CTLZ]] to i32
+  return __builtin_clrsbll(x);
+}
Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -1537,6 +1537,33 @@
 return 

[PATCH] D50418: [Sema] Support for P0961R1: Relaxing the structured bindings customization point finding rules

2018-08-07 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments.



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:1118-1130
+//   ... and if that finds at least one declaration that is a function
+//   template whose first template parameter is a non-type parameter ...
+LookupResult::Filter Filter = MemberGet.makeFilter();
+while (Filter.hasNext()) {
+  NamedDecl *ND = Filter.next();
+  if (auto *FTD = dyn_cast(ND)) {
+TemplateParameterList *TPL = FTD->getTemplateParameters();

This should be done by walking the lookup results, not by filtering them. 
Testcase:

```
struct A {
  int get();
};
struct B {
  template int get();
};
struct C : A, B {};
// plus specializations of tuple_size and tuple_element
auto [x] = C();
```

This should be ill-formed due to ambiguity when looking up `C::get`. We should 
not resolve the ambiguity by selecting `B::get`.


Repository:
  rC Clang

https://reviews.llvm.org/D50418



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


[PATCH] D50421: [libcxx] [test] Remove assertion that includes and .

2018-08-07 Thread Billy Robert O'Neal III via Phabricator via cfe-commits
BillyONeal created this revision.
BillyONeal added reviewers: mclow.lists, EricWF.

[template.bitset] says only that  and  are included by 
, but libcxx tests are asserting that  and  are 
also included.

Note that libcxx's nonstandard assertion of this is already handled by 
test/libcxx/utilities/template.bitset/includes.pass.cpp


https://reviews.llvm.org/D50421

Files:
  test/std/utilities/template.bitset/includes.pass.cpp


Index: test/std/utilities/template.bitset/includes.pass.cpp
===
--- test/std/utilities/template.bitset/includes.pass.cpp
+++ test/std/utilities/template.bitset/includes.pass.cpp
@@ -1,32 +1,24 @@
+
 
//===--===//
 //
 // The LLVM Compiler Infrastructure
 //
 // This file is dual licensed under the MIT and the University of Illinois Open
 // Source Licenses. See LICENSE.TXT for details.
 //
 
//===--===//

-// test that  includes , ,  and 
+// test that  includes , and 

 #include 

 template  void test_typedef() {}

 int main()
 {
-  { // test for 
-std::ptrdiff_t p; ((void)p);
-std::size_t s; ((void)s);
-std::nullptr_t np; ((void)np);
-  }
   { // test for 
 std::string s; ((void)s);
   }
-  { // test for 
-std::logic_error le("blah"); ((void)le);
-std::runtime_error re("blah"); ((void)re);
-  }
   { // test for 
 test_typedef();
 test_typedef();


Index: test/std/utilities/template.bitset/includes.pass.cpp
===
--- test/std/utilities/template.bitset/includes.pass.cpp
+++ test/std/utilities/template.bitset/includes.pass.cpp
@@ -1,32 +1,24 @@
+
 //===--===//
 //
 // The LLVM Compiler Infrastructure
 //
 // This file is dual licensed under the MIT and the University of Illinois Open
 // Source Licenses. See LICENSE.TXT for details.
 //
 //===--===//

-// test that  includes , ,  and 
+// test that  includes , and 

 #include 

 template  void test_typedef() {}

 int main()
 {
-  { // test for 
-std::ptrdiff_t p; ((void)p);
-std::size_t s; ((void)s);
-std::nullptr_t np; ((void)np);
-  }
   { // test for 
 std::string s; ((void)s);
   }
-  { // test for 
-std::logic_error le("blah"); ((void)le);
-std::runtime_error re("blah"); ((void)re);
-  }
   { // test for 
 test_typedef();
 test_typedef();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D50420: [libcxx] [test] Add missing to several tests.

2018-08-07 Thread Billy Robert O'Neal III via Phabricator via cfe-commits
BillyONeal created this revision.
BillyONeal added reviewers: EricWF, mclow.lists.

https://reviews.llvm.org/D50420

Files:
  test/std/strings/basic.string/string.capacity/over_max_size.pass.cpp
  test/std/strings/string.conversions/stod.pass.cpp
  test/std/strings/string.conversions/stof.pass.cpp
  test/std/strings/string.conversions/stoi.pass.cpp
  test/std/strings/string.conversions/stol.pass.cpp
  test/std/strings/string.conversions/stoll.pass.cpp
  test/std/strings/string.conversions/stoul.pass.cpp
  test/std/strings/string.conversions/stoull.pass.cpp
  test/std/strings/string.view/string.view.ops/compare.pointer_size.pass.cpp
  test/std/strings/string.view/string.view.ops/compare.size_size_sv.pass.cpp
  
test/std/strings/string.view/string.view.ops/compare.size_size_sv_pointer_size.pass.cpp
  
test/std/strings/string.view/string.view.ops/compare.size_size_sv_size_size.pass.cpp
  test/std/strings/string.view/string.view.ops/copy.pass.cpp
  test/std/utilities/template.bitset/bitset.members/flip_one.pass.cpp
  test/std/utilities/template.bitset/bitset.members/reset_one.pass.cpp
  test/std/utilities/template.bitset/bitset.members/set_one.pass.cpp
  test/std/utilities/template.bitset/bitset.members/test.pass.cpp

Index: test/std/utilities/template.bitset/bitset.members/test.pass.cpp
===
--- test/std/utilities/template.bitset/bitset.members/test.pass.cpp
+++ test/std/utilities/template.bitset/bitset.members/test.pass.cpp
@@ -12,6 +12,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "test_macros.h"
 
Index: test/std/utilities/template.bitset/bitset.members/set_one.pass.cpp
===
--- test/std/utilities/template.bitset/bitset.members/set_one.pass.cpp
+++ test/std/utilities/template.bitset/bitset.members/set_one.pass.cpp
@@ -11,6 +11,7 @@
 
 #include 
 #include 
+#include 
 
 #include "test_macros.h"
 
Index: test/std/utilities/template.bitset/bitset.members/reset_one.pass.cpp
===
--- test/std/utilities/template.bitset/bitset.members/reset_one.pass.cpp
+++ test/std/utilities/template.bitset/bitset.members/reset_one.pass.cpp
@@ -11,6 +11,7 @@
 
 #include 
 #include 
+#include 
 
 #include "test_macros.h"
 
Index: test/std/utilities/template.bitset/bitset.members/flip_one.pass.cpp
===
--- test/std/utilities/template.bitset/bitset.members/flip_one.pass.cpp
+++ test/std/utilities/template.bitset/bitset.members/flip_one.pass.cpp
@@ -12,6 +12,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "test_macros.h"
 
Index: test/std/strings/string.view/string.view.ops/copy.pass.cpp
===
--- test/std/strings/string.view/string.view.ops/copy.pass.cpp
+++ test/std/strings/string.view/string.view.ops/copy.pass.cpp
@@ -21,6 +21,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "test_macros.h"
 
Index: test/std/strings/string.view/string.view.ops/compare.size_size_sv_size_size.pass.cpp
===
--- test/std/strings/string.view/string.view.ops/compare.size_size_sv_size_size.pass.cpp
+++ test/std/strings/string.view/string.view.ops/compare.size_size_sv_size_size.pass.cpp
@@ -14,6 +14,7 @@
 
 #include 
 #include 
+#include 
 
 #include "test_macros.h"
 #include "constexpr_char_traits.hpp"
Index: test/std/strings/string.view/string.view.ops/compare.size_size_sv_pointer_size.pass.cpp
===
--- test/std/strings/string.view/string.view.ops/compare.size_size_sv_pointer_size.pass.cpp
+++ test/std/strings/string.view/string.view.ops/compare.size_size_sv_pointer_size.pass.cpp
@@ -14,6 +14,7 @@
 
 #include 
 #include 
+#include 
 
 #include "test_macros.h"
 #include "constexpr_char_traits.hpp"
Index: test/std/strings/string.view/string.view.ops/compare.size_size_sv.pass.cpp
===
--- test/std/strings/string.view/string.view.ops/compare.size_size_sv.pass.cpp
+++ test/std/strings/string.view/string.view.ops/compare.size_size_sv.pass.cpp
@@ -13,6 +13,7 @@
 
 #include 
 #include 
+#include 
 
 #include "test_macros.h"
 #include "constexpr_char_traits.hpp"
Index: test/std/strings/string.view/string.view.ops/compare.pointer_size.pass.cpp
===
--- test/std/strings/string.view/string.view.ops/compare.pointer_size.pass.cpp
+++ test/std/strings/string.view/string.view.ops/compare.pointer_size.pass.cpp
@@ -13,6 +13,7 @@
 
 #include 
 #include 
+#include 
 
 #include "test_macros.h"
 #include "constexpr_char_traits.hpp"
Index: test/std/strings/string.conversions/stoull.pass.cpp
===
--- 

[PATCH] D45444: [clang-tidy] implement new check for const-correctness

2018-08-07 Thread Shuai Wang via Phabricator via cfe-commits
shuaiwang added a comment.



> - there seems to be a false positive with array-to-pointer decay. 
> ExprMutAnalyzer does think of it, but maybe there is a bug in it.

Could you give a concrete example of this?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45444



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


[PATCH] D50418: [Sema] Support for P0961R1: Relaxing the structured bindings customization point finding rules

2018-08-07 Thread Erik Pilkington via Phabricator via cfe-commits
erik.pilkington created this revision.
erik.pilkington added a reviewer: rsmith.
Herald added a subscriber: dexonsmith.

http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p0961r1.html

I don't believe an actual defect report was filed for this, but the paper (in 
the "wording" section) claims that we should back-port this to C++17 as if it 
were a defect report. Besides that, this is pretty straightforward.

Thanks for taking a look!
Erik


Repository:
  rC Clang

https://reviews.llvm.org/D50418

Files:
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CXX/dcl.decl/dcl.decomp/p3.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -755,7 +755,7 @@
   
 
 http://wg21.link/p0961r1;>P0961R1 (DR)
-No
+SVN
   
   
 
Index: clang/test/CXX/dcl.decl/dcl.decomp/p3.cpp
===
--- clang/test/CXX/dcl.decl/dcl.decomp/p3.cpp
+++ clang/test/CXX/dcl.decl/dcl.decomp/p3.cpp
@@ -36,7 +36,7 @@
   auto [a0, a1, a2] = A(); // expected-error {{undeclared identifier 'get'}} expected-note {{in implicit initialization of binding declaration 'a0'}}
 }
 
-template float (A);
+template float (A); // expected-note 2 {{no known conversion}}
 
 void no_tuple_element_1() {
   auto [a0, a1, a2] = A(); // expected-error-re {{'std::tuple_element<0U{{L*}}, A>::type' does not name a type}} expected-note {{in implicit}}
@@ -57,7 +57,7 @@
 template<> struct std::tuple_element<1, A> { typedef float  };
 template<> struct std::tuple_element<2, A> { typedef const float  };
 
-template auto get(B) -> int (&)[N + 1];
+template auto get(B) -> int (&)[N + 1]; // expected-note 2 {{no known conversion}}
 template struct std::tuple_element { typedef int type[N +1 ]; };
 
 template struct std::tuple_size : std::tuple_size {};
@@ -138,19 +138,25 @@
   return c;
 }
 
-struct D { template struct get {}; }; // expected-note {{declared here}}
+struct D {
+  // FIXME: Emit a note here explaining why this was ignored.
+  template struct get {};
+};
 template<> struct std::tuple_size { static const int value = 1; };
 template<> struct std::tuple_element<0, D> { typedef D::get<0> type; };
 void member_get_class_template() {
-  auto [d] = D(); // expected-error {{cannot refer to member 'get' in 'D' with '.'}} expected-note {{in implicit init}}
+  auto [d] = D(); // expected-error {{no matching function for call to 'get'}} expected-note {{in implicit init}}
 }
 
-struct E { int get(); };
+struct E {
+  // FIXME: Emit a note here explaining why this was ignored.
+  int get();
+};
 template<> struct std::tuple_size { static const int value = 1; };
 template<> struct std::tuple_element<0, E> { typedef int type; };
 void member_get_non_template() {
   // FIXME: This diagnostic is not very good.
-  auto [e] = E(); // expected-error {{no member named 'get'}} expected-note {{in implicit init}}
+  auto [e] = E(); // expected-error {{no matching function for call to 'get'}} expected-note {{in implicit init}}
 }
 
 namespace ADL {
@@ -230,3 +236,35 @@
   }
   static_assert(g() == 4); // expected-error {{constant}} expected-note {{in call to 'g()'}}
 }
+
+// P0961R1
+struct InvalidMemberGet {
+  int get();
+  template  int get();
+  struct get {};
+};
+template <> struct std::tuple_size { static constexpr size_t value = 1; };
+template <> struct std::tuple_element<0, InvalidMemberGet> { typedef float type; };
+template  float get(InvalidMemberGet) { return 0; }
+int f() {
+  InvalidMemberGet img;
+  auto [x] = img;
+  typedef decltype(x) same_as_float;
+  typedef float same_as_float;
+}
+
+struct ValidMemberGet {
+  int get();
+  template  int get() { return 0; }
+  template  float get() { return 0; }
+};
+template <> struct std::tuple_size { static constexpr size_t value = 1; };
+template <> struct std::tuple_element<0, ValidMemberGet> { typedef float type; };
+// Don't use this one; we should use the member get.
+template  int get(ValidMemberGet) { static_assert(N && false, ""); }
+int f2() {
+  ValidMemberGet img;
+  auto [x] = img;
+  typedef decltype(x) same_as_float;
+  typedef float same_as_float;
+}
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -1108,14 +1108,29 @@
 
   // [dcl.decomp]p3:
   //   The unqualified-id get is looked up in the scope of E by class member
-  //   access lookup
+  //   access lookup ...
   LookupResult MemberGet(S, GetDN, Src->getLocation(), Sema::LookupMemberName);
   bool UseMemberGet = false;
   if (S.isCompleteType(Src->getLocation(), DecompType)) {
 if (auto *RD = DecompType->getAsCXXRecordDecl())
   S.LookupQualifiedName(MemberGet, RD);
+
+//   ... and if that finds at least one declaration that is a function
+//   template whose first 

Re: [PATCH] D50154: [clangd] capitalize diagnostic messages

2018-08-07 Thread David Blaikie via cfe-commits
On Tue, Aug 7, 2018 at 4:02 PM Alex L  wrote:

> On Tue, 7 Aug 2018 at 11:38, David Blaikie  wrote:
>
>>
>>
>> On Tue, Aug 7, 2018 at 11:22 AM Alex L  wrote:
>>
>>> On Tue, 7 Aug 2018 at 10:52, David Blaikie via cfe-commits <
>>> cfe-commits@lists.llvm.org> wrote:
>>>


 On Tue, Aug 7, 2018 at 10:33 AM Alex Lorenz via Phabricator <
 revi...@reviews.llvm.org> wrote:

> arphaman added a comment.
>
> In https://reviews.llvm.org/D50154#1191002, @dblaikie wrote:
>
> > What's the motivation for clangd to differ from clang here? (& if
> the first
> >  letter is going to be capitalized, should there be a period at the
> end? But
> >  also the phrasing of most/all diagnostic text isn't in the form of
> complete
> >  sentences, so this might not make sense)
>
>
> It's mostly for the presentation purposes, to match the needs of our
> client. I first implemented it as an opt-in feature, but the consensus was
> to capitalize the messages all the time.
>

 Doesn't seem like it'd be any more expensive (amount of code or
 performance) to do that up in your client code, then, would it? I guess if
 most users of this API in time ended up preferring capitalized values, it'd
 make sense to share that implementation - but to me it seems like a strange
 transformation to happen at this level. (since it depends on what kind of
 client/how they want to render things - so it seems an odd choice to bake
 in to the API (or even provide an option for, unless there are lots of
 users/the code was especially complicated))

 My 2c - I've no vested interest or authority here.

>>>
>>> I think it's more in spirit with Clangd to provide output that's as
>>> close to the one presented by the client as possible.
>>>
>>
>> That assumes there's one client though, right? Different clients might
>> reasonably have different needs for how they'd want the text rendered, I'd
>> imagine.
>>
>
> True. This transformation is lossless though, so the clients will still be
> able to get back the original text if needed.
>

I'm not sure that c == lower(upper(c)) - well, if the character is ever
uppercase to begin with, it's clearly not. But even in the case of a
lowercase character to start with I didn't think that was always true - I
guess for ASCII/English it is, and that's all we're dealing with here.


> And if the consensus about this particular text transformation changes I'm
> willing to revert this change for sure.
>

*nod* I mean if everyone who's invested in/working on clangd agrees with
your direction, that's cool/good. My 2c is that this sort of thing seems
like the responsibility of the client, not the API - but that's by no means
authoritative.


> I would argue there's already a precedence for this kind of
>>> transformations, for example, Clangd merges the diagnostic messages of
>>> notes and the main diagnostics into one, to make it a better presentation
>>> experience in the client:
>>>
>>>
>>> https://github.com/llvm-mirror/clang-tools-extra/blob/55bfabcc1bd75447d6338ffe6ff27c1624a8c15a/clangd/Diagnostics.cpp#L161
>>>
>>
>> I'm assuming that's because the API/protocol only supports a single
>> message, so the multi-message/location nuance of LLVM's diagnostics are
>> necessarily lost when converting to that format?
>>
>> If that's not the case, and the API/protocol does support a
>> multiple-message diagnostic & ClangD is collapsing them early - that also
>> seems like an unfortunate loss in fidelity.
>>
>
> Clangd sends out both the main diagnostic, and the attached notes (that
> don't have fixits) in a list (i.e. the hierarchy isn't directly preserved,
> although it can be recreated by the client).
> So it looks like they're collapsed early, but at the same time the client
> has enough information to do this transformation itself if desired.
> I'm planning to work on an option to remove this behavior if desired by
> the client.
>

*nod* I'd (completely in the abstract - not having worked on clangd or its
clients, so take with grain of salt, etc) probably still leave this up to
the client (which would be easier if the client were an in-process API kind
of thing rather than a strict protocol - because it'd be easy to include a
function clients could call to collapse warnings+notes if they wanted to
without needing to change the base behavior). Though this case is
admittedly a fraction more nuanced rather than uppercasing one letter - bit
more code to concatenate things together, etc. (though perhaps then all the
more reason that it's likely that different clients might have different
concatenation needs/formatting preferences/etc)


>
>
>>
>>
>>>
>>>
>>>
>>>
>>>


> I don't think it would make sense to insert the period at the end,
> because, as you said, not all diagnostics are complete sentences
>
>
> Repository:
>   rCTE Clang Tools Extra
>
> 

[PATCH] D50408: [analyzer] Avoid querying this-pointers for static-methods.

2018-08-07 Thread Matt Davis via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC339201: [analyzer] Avoid querying this-pointers for 
static-methods. (authored by mattd, committed by ).

Repository:
  rC Clang

https://reviews.llvm.org/D50408

Files:
  lib/StaticAnalyzer/Core/LoopWidening.cpp
  test/Analysis/loop-widening-ignore-static-methods.cpp


Index: test/Analysis/loop-widening-ignore-static-methods.cpp
===
--- test/Analysis/loop-widening-ignore-static-methods.cpp
+++ test/Analysis/loop-widening-ignore-static-methods.cpp
@@ -0,0 +1,12 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-config 
widen-loops=true -analyzer-max-loop 2 %s
+// REQUIRES: asserts
+// expected-no-diagnostics
+//
+// This test checks that the loop-widening code ignores static methods.  If 
that is not the
+// case, then an assertion will trigger.
+
+class Test {
+  static void foo() {
+for (;;) {}
+  }
+};
Index: lib/StaticAnalyzer/Core/LoopWidening.cpp
===
--- lib/StaticAnalyzer/Core/LoopWidening.cpp
+++ lib/StaticAnalyzer/Core/LoopWidening.cpp
@@ -81,8 +81,10 @@
 
   // 'this' pointer is not an lvalue, we should not invalidate it. If the loop
   // is located in a method, constructor or destructor, the value of 'this'
-  // pointer shoule remain unchanged.
-  if (const CXXMethodDecl *CXXMD = dyn_cast(STC->getDecl())) {
+  // pointer should remain unchanged.  Ignore static methods, since they do not
+  // have 'this' pointers.
+  const CXXMethodDecl *CXXMD = dyn_cast(STC->getDecl());
+  if (CXXMD && !CXXMD->isStatic()) {
 const CXXThisRegion *ThisR = MRMgr.getCXXThisRegion(
 CXXMD->getThisType(STC->getAnalysisDeclContext()->getASTContext()),
 STC);


Index: test/Analysis/loop-widening-ignore-static-methods.cpp
===
--- test/Analysis/loop-widening-ignore-static-methods.cpp
+++ test/Analysis/loop-widening-ignore-static-methods.cpp
@@ -0,0 +1,12 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-config widen-loops=true -analyzer-max-loop 2 %s
+// REQUIRES: asserts
+// expected-no-diagnostics
+//
+// This test checks that the loop-widening code ignores static methods.  If that is not the
+// case, then an assertion will trigger.
+
+class Test {
+  static void foo() {
+for (;;) {}
+  }
+};
Index: lib/StaticAnalyzer/Core/LoopWidening.cpp
===
--- lib/StaticAnalyzer/Core/LoopWidening.cpp
+++ lib/StaticAnalyzer/Core/LoopWidening.cpp
@@ -81,8 +81,10 @@
 
   // 'this' pointer is not an lvalue, we should not invalidate it. If the loop
   // is located in a method, constructor or destructor, the value of 'this'
-  // pointer shoule remain unchanged.
-  if (const CXXMethodDecl *CXXMD = dyn_cast(STC->getDecl())) {
+  // pointer should remain unchanged.  Ignore static methods, since they do not
+  // have 'this' pointers.
+  const CXXMethodDecl *CXXMD = dyn_cast(STC->getDecl());
+  if (CXXMD && !CXXMD->isStatic()) {
 const CXXThisRegion *ThisR = MRMgr.getCXXThisRegion(
 CXXMD->getThisType(STC->getAnalysisDeclContext()->getASTContext()),
 STC);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r339201 - [analyzer] Avoid querying this-pointers for static-methods.

2018-08-07 Thread Matt Davis via cfe-commits
Author: mattd
Date: Tue Aug  7 16:13:28 2018
New Revision: 339201

URL: http://llvm.org/viewvc/llvm-project?rev=339201=rev
Log:
[analyzer] Avoid querying this-pointers for static-methods.

Summary:
The loop-widening code processes c++ methods looking for `this` pointers.  In
the case of static methods (which do not have `this` pointers), an assertion
was triggering.   This patch avoids trying to process `this` pointers for
static methods, and thus avoids triggering the assertion .


Reviewers: dcoughlin, george.karpenkov, NoQ

Reviewed By: NoQ

Subscribers: NoQ, xazax.hun, szepet, a.sidorin, mikhail.ramalho, cfe-commits

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

Added:
cfe/trunk/test/Analysis/loop-widening-ignore-static-methods.cpp
Modified:
cfe/trunk/lib/StaticAnalyzer/Core/LoopWidening.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Core/LoopWidening.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/LoopWidening.cpp?rev=339201=339200=339201=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/LoopWidening.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/LoopWidening.cpp Tue Aug  7 16:13:28 2018
@@ -81,8 +81,10 @@ ProgramStateRef getWidenedLoopState(Prog
 
   // 'this' pointer is not an lvalue, we should not invalidate it. If the loop
   // is located in a method, constructor or destructor, the value of 'this'
-  // pointer shoule remain unchanged.
-  if (const CXXMethodDecl *CXXMD = dyn_cast(STC->getDecl())) {
+  // pointer should remain unchanged.  Ignore static methods, since they do not
+  // have 'this' pointers.
+  const CXXMethodDecl *CXXMD = dyn_cast(STC->getDecl());
+  if (CXXMD && !CXXMD->isStatic()) {
 const CXXThisRegion *ThisR = MRMgr.getCXXThisRegion(
 CXXMD->getThisType(STC->getAnalysisDeclContext()->getASTContext()),
 STC);

Added: cfe/trunk/test/Analysis/loop-widening-ignore-static-methods.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/loop-widening-ignore-static-methods.cpp?rev=339201=auto
==
--- cfe/trunk/test/Analysis/loop-widening-ignore-static-methods.cpp (added)
+++ cfe/trunk/test/Analysis/loop-widening-ignore-static-methods.cpp Tue Aug  7 
16:13:28 2018
@@ -0,0 +1,12 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-config 
widen-loops=true -analyzer-max-loop 2 %s
+// REQUIRES: asserts
+// expected-no-diagnostics
+//
+// This test checks that the loop-widening code ignores static methods.  If 
that is not the
+// case, then an assertion will trigger.
+
+class Test {
+  static void foo() {
+for (;;) {}
+  }
+};


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


Re: [PATCH] D50154: [clangd] capitalize diagnostic messages

2018-08-07 Thread Alex L via cfe-commits
On Tue, 7 Aug 2018 at 11:38, David Blaikie  wrote:

>
>
> On Tue, Aug 7, 2018 at 11:22 AM Alex L  wrote:
>
>> On Tue, 7 Aug 2018 at 10:52, David Blaikie via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>>>
>>>
>>> On Tue, Aug 7, 2018 at 10:33 AM Alex Lorenz via Phabricator <
>>> revi...@reviews.llvm.org> wrote:
>>>
 arphaman added a comment.

 In https://reviews.llvm.org/D50154#1191002, @dblaikie wrote:

 > What's the motivation for clangd to differ from clang here? (& if the
 first
 >  letter is going to be capitalized, should there be a period at the
 end? But
 >  also the phrasing of most/all diagnostic text isn't in the form of
 complete
 >  sentences, so this might not make sense)


 It's mostly for the presentation purposes, to match the needs of our
 client. I first implemented it as an opt-in feature, but the consensus was
 to capitalize the messages all the time.

>>>
>>> Doesn't seem like it'd be any more expensive (amount of code or
>>> performance) to do that up in your client code, then, would it? I guess if
>>> most users of this API in time ended up preferring capitalized values, it'd
>>> make sense to share that implementation - but to me it seems like a strange
>>> transformation to happen at this level. (since it depends on what kind of
>>> client/how they want to render things - so it seems an odd choice to bake
>>> in to the API (or even provide an option for, unless there are lots of
>>> users/the code was especially complicated))
>>>
>>> My 2c - I've no vested interest or authority here.
>>>
>>
>> I think it's more in spirit with Clangd to provide output that's as close
>> to the one presented by the client as possible.
>>
>
> That assumes there's one client though, right? Different clients might
> reasonably have different needs for how they'd want the text rendered, I'd
> imagine.
>

True. This transformation is lossless though, so the clients will still be
able to get back the original text if needed. And if the consensus about
this particular text transformation changes I'm willing to revert this
change for sure.


>
>
>> I would argue there's already a precedence for this kind of
>> transformations, for example, Clangd merges the diagnostic messages of
>> notes and the main diagnostics into one, to make it a better presentation
>> experience in the client:
>>
>>
>> https://github.com/llvm-mirror/clang-tools-extra/blob/55bfabcc1bd75447d6338ffe6ff27c1624a8c15a/clangd/Diagnostics.cpp#L161
>>
>
> I'm assuming that's because the API/protocol only supports a single
> message, so the multi-message/location nuance of LLVM's diagnostics are
> necessarily lost when converting to that format?
>
> If that's not the case, and the API/protocol does support a
> multiple-message diagnostic & ClangD is collapsing them early - that also
> seems like an unfortunate loss in fidelity.
>

Clangd sends out both the main diagnostic, and the attached notes (that
don't have fixits) in a list (i.e. the hierarchy isn't directly preserved,
although it can be recreated by the client).
So it looks like they're collapsed early, but at the same time the client
has enough information to do this transformation itself if desired.
I'm planning to work on an option to remove this behavior if desired by the
client.


>
>
>>
>>
>>
>>
>>
>>>
>>>
 I don't think it would make sense to insert the period at the end,
 because, as you said, not all diagnostics are complete sentences


 Repository:
   rCTE Clang Tools Extra

 https://reviews.llvm.org/D50154



 ___
>>> 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] D50118: [VFS] Unify iteration code for VFSFromYamlDirIterImpl, NFC intended.

2018-08-07 Thread Volodymyr Sapsai via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC339199: [VFS] Unify iteration code for 
VFSFromYamlDirIterImpl, NFC intended. (authored by vsapsai, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D50118?vs=158420=159614#toc

Repository:
  rC Clang

https://reviews.llvm.org/D50118

Files:
  lib/Basic/VirtualFileSystem.cpp


Index: lib/Basic/VirtualFileSystem.cpp
===
--- lib/Basic/VirtualFileSystem.cpp
+++ lib/Basic/VirtualFileSystem.cpp
@@ -933,6 +933,8 @@
   RedirectingFileSystem 
   RedirectingDirectoryEntry::iterator Current, End;
 
+  std::error_code incrementImpl();
+
 public:
   VFSFromYamlDirIterImpl(const Twine , RedirectingFileSystem ,
  RedirectingDirectoryEntry::iterator Begin,
@@ -1997,36 +1999,25 @@
 RedirectingDirectoryEntry::iterator Begin,
 RedirectingDirectoryEntry::iterator End, std::error_code )
 : Dir(_Path.str()), FS(FS), Current(Begin), End(End) {
-  while (Current != End) {
-SmallString<128> PathStr(Dir);
-llvm::sys::path::append(PathStr, (*Current)->getName());
-llvm::ErrorOr S = FS.status(PathStr);
-if (S) {
-  CurrentEntry = *S;
-  return;
-}
-// Skip entries which do not map to a reliable external content.
-if (FS.ignoreNonExistentContents() &&
-S.getError() == llvm::errc::no_such_file_or_directory) {
-  ++Current;
-  continue;
-} else {
-  EC = S.getError();
-  break;
-}
-  }
+  EC = incrementImpl();
 }
 
 std::error_code VFSFromYamlDirIterImpl::increment() {
   assert(Current != End && "cannot iterate past end");
-  while (++Current != End) {
+  ++Current;
+  return incrementImpl();
+}
+
+std::error_code VFSFromYamlDirIterImpl::incrementImpl() {
+  while (Current != End) {
 SmallString<128> PathStr(Dir);
 llvm::sys::path::append(PathStr, (*Current)->getName());
 llvm::ErrorOr S = FS.status(PathStr);
 if (!S) {
   // Skip entries which do not map to a reliable external content.
   if (FS.ignoreNonExistentContents() &&
   S.getError() == llvm::errc::no_such_file_or_directory) {
+++Current;
 continue;
   } else {
 return S.getError();


Index: lib/Basic/VirtualFileSystem.cpp
===
--- lib/Basic/VirtualFileSystem.cpp
+++ lib/Basic/VirtualFileSystem.cpp
@@ -933,6 +933,8 @@
   RedirectingFileSystem 
   RedirectingDirectoryEntry::iterator Current, End;
 
+  std::error_code incrementImpl();
+
 public:
   VFSFromYamlDirIterImpl(const Twine , RedirectingFileSystem ,
  RedirectingDirectoryEntry::iterator Begin,
@@ -1997,36 +1999,25 @@
 RedirectingDirectoryEntry::iterator Begin,
 RedirectingDirectoryEntry::iterator End, std::error_code )
 : Dir(_Path.str()), FS(FS), Current(Begin), End(End) {
-  while (Current != End) {
-SmallString<128> PathStr(Dir);
-llvm::sys::path::append(PathStr, (*Current)->getName());
-llvm::ErrorOr S = FS.status(PathStr);
-if (S) {
-  CurrentEntry = *S;
-  return;
-}
-// Skip entries which do not map to a reliable external content.
-if (FS.ignoreNonExistentContents() &&
-S.getError() == llvm::errc::no_such_file_or_directory) {
-  ++Current;
-  continue;
-} else {
-  EC = S.getError();
-  break;
-}
-  }
+  EC = incrementImpl();
 }
 
 std::error_code VFSFromYamlDirIterImpl::increment() {
   assert(Current != End && "cannot iterate past end");
-  while (++Current != End) {
+  ++Current;
+  return incrementImpl();
+}
+
+std::error_code VFSFromYamlDirIterImpl::incrementImpl() {
+  while (Current != End) {
 SmallString<128> PathStr(Dir);
 llvm::sys::path::append(PathStr, (*Current)->getName());
 llvm::ErrorOr S = FS.status(PathStr);
 if (!S) {
   // Skip entries which do not map to a reliable external content.
   if (FS.ignoreNonExistentContents() &&
   S.getError() == llvm::errc::no_such_file_or_directory) {
+++Current;
 continue;
   } else {
 return S.getError();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r339199 - [VFS] Unify iteration code for VFSFromYamlDirIterImpl, NFC intended.

2018-08-07 Thread Volodymyr Sapsai via cfe-commits
Author: vsapsai
Date: Tue Aug  7 16:00:40 2018
New Revision: 339199

URL: http://llvm.org/viewvc/llvm-project?rev=339199=rev
Log:
[VFS] Unify iteration code for VFSFromYamlDirIterImpl, NFC intended.

First and subsequent iteration steps are similar, capture this similarity.

Reviewers: bruno, benlangmuir

Reviewed By: bruno

Subscribers: dexonsmith, cfe-commits

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


Modified:
cfe/trunk/lib/Basic/VirtualFileSystem.cpp

Modified: cfe/trunk/lib/Basic/VirtualFileSystem.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/VirtualFileSystem.cpp?rev=339199=339198=339199=diff
==
--- cfe/trunk/lib/Basic/VirtualFileSystem.cpp (original)
+++ cfe/trunk/lib/Basic/VirtualFileSystem.cpp Tue Aug  7 16:00:40 2018
@@ -933,6 +933,8 @@ class VFSFromYamlDirIterImpl : public cl
   RedirectingFileSystem 
   RedirectingDirectoryEntry::iterator Current, End;
 
+  std::error_code incrementImpl();
+
 public:
   VFSFromYamlDirIterImpl(const Twine , RedirectingFileSystem ,
  RedirectingDirectoryEntry::iterator Begin,
@@ -1997,29 +1999,17 @@ VFSFromYamlDirIterImpl::VFSFromYamlDirIt
 RedirectingDirectoryEntry::iterator Begin,
 RedirectingDirectoryEntry::iterator End, std::error_code )
 : Dir(_Path.str()), FS(FS), Current(Begin), End(End) {
-  while (Current != End) {
-SmallString<128> PathStr(Dir);
-llvm::sys::path::append(PathStr, (*Current)->getName());
-llvm::ErrorOr S = FS.status(PathStr);
-if (S) {
-  CurrentEntry = *S;
-  return;
-}
-// Skip entries which do not map to a reliable external content.
-if (FS.ignoreNonExistentContents() &&
-S.getError() == llvm::errc::no_such_file_or_directory) {
-  ++Current;
-  continue;
-} else {
-  EC = S.getError();
-  break;
-}
-  }
+  EC = incrementImpl();
 }
 
 std::error_code VFSFromYamlDirIterImpl::increment() {
   assert(Current != End && "cannot iterate past end");
-  while (++Current != End) {
+  ++Current;
+  return incrementImpl();
+}
+
+std::error_code VFSFromYamlDirIterImpl::incrementImpl() {
+  while (Current != End) {
 SmallString<128> PathStr(Dir);
 llvm::sys::path::append(PathStr, (*Current)->getName());
 llvm::ErrorOr S = FS.status(PathStr);
@@ -2027,6 +2017,7 @@ std::error_code VFSFromYamlDirIterImpl::
   // Skip entries which do not map to a reliable external content.
   if (FS.ignoreNonExistentContents() &&
   S.getError() == llvm::errc::no_such_file_or_directory) {
+++Current;
 continue;
   } else {
 return S.getError();


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


[PATCH] D50088: [Sema] Fix an error with C++17 auto non-type template parameters

2018-08-07 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL339198: [Sema] Ensure an auto non-type template parameter is 
dependent (authored by epilk, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D50088?vs=159449=159613#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D50088

Files:
  cfe/trunk/lib/Sema/SemaTemplate.cpp
  cfe/trunk/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp


Index: cfe/trunk/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
===
--- cfe/trunk/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
+++ cfe/trunk/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
@@ -335,3 +335,46 @@
   void g(int, int);
   using Int = A::B<>::param2;
 }
+
+namespace rdar41852459 {
+template  struct G {};
+
+template  struct S {
+  template  void f() {
+G x;
+  }
+  template  void f2() {
+G x;
+  }
+  template  void f3() {
+G x;
+  }
+};
+
+template  struct I {};
+
+template  struct K {
+  template  void f() {
+I x;
+  }
+  template  void f2() {
+I x;
+  }
+  template  void f3() {
+I x;
+  }
+};
+
+template  struct L {};
+template  struct M {
+  template  void f() {
+L x;
+  }
+  template  void f() {
+L x;
+  }
+  template  void f() {
+L x;
+  }
+};
+}
Index: cfe/trunk/lib/Sema/SemaTemplate.cpp
===
--- cfe/trunk/lib/Sema/SemaTemplate.cpp
+++ cfe/trunk/lib/Sema/SemaTemplate.cpp
@@ -974,7 +974,7 @@
 QualType Sema::CheckNonTypeTemplateParameterType(TypeSourceInfo *,
  SourceLocation Loc) {
   if (TSI->getType()->isUndeducedType()) {
-// C++1z [temp.dep.expr]p3:
+// C++17 [temp.dep.expr]p3:
 //   An id-expression is type-dependent if it contains
 //- an identifier associated by name lookup with a non-type
 //  template-parameter declared with a type that contains a
@@ -9866,6 +9866,15 @@
 if (!NewTSI)
   return true;
 
+if (NewTSI->getType()->isUndeducedType()) {
+  // C++17 [temp.dep.expr]p3:
+  //   An id-expression is type-dependent if it contains
+  //- an identifier associated by name lookup with a non-type
+  //  template-parameter declared with a type that contains a
+  //  placeholder type (7.1.7.4),
+  NewTSI = SubstAutoTypeSourceInfo(NewTSI, Context.DependentTy);
+}
+
 if (NewTSI != NTTP->getTypeSourceInfo()) {
   NTTP->setTypeSourceInfo(NewTSI);
   NTTP->setType(NewTSI->getType());


Index: cfe/trunk/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
===
--- cfe/trunk/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
+++ cfe/trunk/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
@@ -335,3 +335,46 @@
   void g(int, int);
   using Int = A::B<>::param2;
 }
+
+namespace rdar41852459 {
+template  struct G {};
+
+template  struct S {
+  template  void f() {
+G x;
+  }
+  template  void f2() {
+G x;
+  }
+  template  void f3() {
+G x;
+  }
+};
+
+template  struct I {};
+
+template  struct K {
+  template  void f() {
+I x;
+  }
+  template  void f2() {
+I x;
+  }
+  template  void f3() {
+I x;
+  }
+};
+
+template  struct L {};
+template  struct M {
+  template  void f() {
+L x;
+  }
+  template  void f() {
+L x;
+  }
+  template  void f() {
+L x;
+  }
+};
+}
Index: cfe/trunk/lib/Sema/SemaTemplate.cpp
===
--- cfe/trunk/lib/Sema/SemaTemplate.cpp
+++ cfe/trunk/lib/Sema/SemaTemplate.cpp
@@ -974,7 +974,7 @@
 QualType Sema::CheckNonTypeTemplateParameterType(TypeSourceInfo *,
  SourceLocation Loc) {
   if (TSI->getType()->isUndeducedType()) {
-// C++1z [temp.dep.expr]p3:
+// C++17 [temp.dep.expr]p3:
 //   An id-expression is type-dependent if it contains
 //- an identifier associated by name lookup with a non-type
 //  template-parameter declared with a type that contains a
@@ -9866,6 +9866,15 @@
 if (!NewTSI)
   return true;
 
+if (NewTSI->getType()->isUndeducedType()) {
+  // C++17 [temp.dep.expr]p3:
+  //   An id-expression is type-dependent if it contains
+  //- an identifier associated by name lookup with a non-type
+  //  template-parameter declared with a type that contains a
+  //  placeholder type (7.1.7.4),
+  NewTSI = SubstAutoTypeSourceInfo(NewTSI, Context.DependentTy);
+}
+
 if (NewTSI != NTTP->getTypeSourceInfo()) {
   NTTP->setTypeSourceInfo(NewTSI);
   NTTP->setType(NewTSI->getType());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D50088: [Sema] Fix an error with C++17 auto non-type template parameters

2018-08-07 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC339198: [Sema] Ensure an auto non-type template parameter is 
dependent (authored by epilk, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D50088?vs=159449=159612#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D50088

Files:
  lib/Sema/SemaTemplate.cpp
  test/SemaTemplate/temp_arg_nontype_cxx1z.cpp


Index: test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
===
--- test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
+++ test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
@@ -335,3 +335,46 @@
   void g(int, int);
   using Int = A::B<>::param2;
 }
+
+namespace rdar41852459 {
+template  struct G {};
+
+template  struct S {
+  template  void f() {
+G x;
+  }
+  template  void f2() {
+G x;
+  }
+  template  void f3() {
+G x;
+  }
+};
+
+template  struct I {};
+
+template  struct K {
+  template  void f() {
+I x;
+  }
+  template  void f2() {
+I x;
+  }
+  template  void f3() {
+I x;
+  }
+};
+
+template  struct L {};
+template  struct M {
+  template  void f() {
+L x;
+  }
+  template  void f() {
+L x;
+  }
+  template  void f() {
+L x;
+  }
+};
+}
Index: lib/Sema/SemaTemplate.cpp
===
--- lib/Sema/SemaTemplate.cpp
+++ lib/Sema/SemaTemplate.cpp
@@ -974,7 +974,7 @@
 QualType Sema::CheckNonTypeTemplateParameterType(TypeSourceInfo *,
  SourceLocation Loc) {
   if (TSI->getType()->isUndeducedType()) {
-// C++1z [temp.dep.expr]p3:
+// C++17 [temp.dep.expr]p3:
 //   An id-expression is type-dependent if it contains
 //- an identifier associated by name lookup with a non-type
 //  template-parameter declared with a type that contains a
@@ -9866,6 +9866,15 @@
 if (!NewTSI)
   return true;
 
+if (NewTSI->getType()->isUndeducedType()) {
+  // C++17 [temp.dep.expr]p3:
+  //   An id-expression is type-dependent if it contains
+  //- an identifier associated by name lookup with a non-type
+  //  template-parameter declared with a type that contains a
+  //  placeholder type (7.1.7.4),
+  NewTSI = SubstAutoTypeSourceInfo(NewTSI, Context.DependentTy);
+}
+
 if (NewTSI != NTTP->getTypeSourceInfo()) {
   NTTP->setTypeSourceInfo(NewTSI);
   NTTP->setType(NewTSI->getType());


Index: test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
===
--- test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
+++ test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
@@ -335,3 +335,46 @@
   void g(int, int);
   using Int = A::B<>::param2;
 }
+
+namespace rdar41852459 {
+template  struct G {};
+
+template  struct S {
+  template  void f() {
+G x;
+  }
+  template  void f2() {
+G x;
+  }
+  template  void f3() {
+G x;
+  }
+};
+
+template  struct I {};
+
+template  struct K {
+  template  void f() {
+I x;
+  }
+  template  void f2() {
+I x;
+  }
+  template  void f3() {
+I x;
+  }
+};
+
+template  struct L {};
+template  struct M {
+  template  void f() {
+L x;
+  }
+  template  void f() {
+L x;
+  }
+  template  void f() {
+L x;
+  }
+};
+}
Index: lib/Sema/SemaTemplate.cpp
===
--- lib/Sema/SemaTemplate.cpp
+++ lib/Sema/SemaTemplate.cpp
@@ -974,7 +974,7 @@
 QualType Sema::CheckNonTypeTemplateParameterType(TypeSourceInfo *,
  SourceLocation Loc) {
   if (TSI->getType()->isUndeducedType()) {
-// C++1z [temp.dep.expr]p3:
+// C++17 [temp.dep.expr]p3:
 //   An id-expression is type-dependent if it contains
 //- an identifier associated by name lookup with a non-type
 //  template-parameter declared with a type that contains a
@@ -9866,6 +9866,15 @@
 if (!NewTSI)
   return true;
 
+if (NewTSI->getType()->isUndeducedType()) {
+  // C++17 [temp.dep.expr]p3:
+  //   An id-expression is type-dependent if it contains
+  //- an identifier associated by name lookup with a non-type
+  //  template-parameter declared with a type that contains a
+  //  placeholder type (7.1.7.4),
+  NewTSI = SubstAutoTypeSourceInfo(NewTSI, Context.DependentTy);
+}
+
 if (NewTSI != NTTP->getTypeSourceInfo()) {
   NTTP->setTypeSourceInfo(NewTSI);
   NTTP->setType(NewTSI->getType());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r339198 - [Sema] Ensure an auto non-type template parameter is dependent

2018-08-07 Thread Erik Pilkington via cfe-commits
Author: epilk
Date: Tue Aug  7 15:59:02 2018
New Revision: 339198

URL: http://llvm.org/viewvc/llvm-project?rev=339198=rev
Log:
[Sema] Ensure an auto non-type template parameter is dependent

The dependent auto was getting stripped away while rebuilding the template
parameter type, so substitute it in.

rdar://41852459

Differential revision: https://reviews.llvm.org/D50088

Modified:
cfe/trunk/lib/Sema/SemaTemplate.cpp
cfe/trunk/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp

Modified: cfe/trunk/lib/Sema/SemaTemplate.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplate.cpp?rev=339198=339197=339198=diff
==
--- cfe/trunk/lib/Sema/SemaTemplate.cpp (original)
+++ cfe/trunk/lib/Sema/SemaTemplate.cpp Tue Aug  7 15:59:02 2018
@@ -974,7 +974,7 @@ NamedDecl *Sema::ActOnTypeParameter(Scop
 QualType Sema::CheckNonTypeTemplateParameterType(TypeSourceInfo *,
  SourceLocation Loc) {
   if (TSI->getType()->isUndeducedType()) {
-// C++1z [temp.dep.expr]p3:
+// C++17 [temp.dep.expr]p3:
 //   An id-expression is type-dependent if it contains
 //- an identifier associated by name lookup with a non-type
 //  template-parameter declared with a type that contains a
@@ -9866,6 +9866,15 @@ bool Sema::RebuildTemplateParamsInCurren
 if (!NewTSI)
   return true;
 
+if (NewTSI->getType()->isUndeducedType()) {
+  // C++17 [temp.dep.expr]p3:
+  //   An id-expression is type-dependent if it contains
+  //- an identifier associated by name lookup with a non-type
+  //  template-parameter declared with a type that contains a
+  //  placeholder type (7.1.7.4),
+  NewTSI = SubstAutoTypeSourceInfo(NewTSI, Context.DependentTy);
+}
+
 if (NewTSI != NTTP->getTypeSourceInfo()) {
   NTTP->setTypeSourceInfo(NewTSI);
   NTTP->setType(NewTSI->getType());

Modified: cfe/trunk/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp?rev=339198=339197=339198=diff
==
--- cfe/trunk/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp (original)
+++ cfe/trunk/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp Tue Aug  7 15:59:02 
2018
@@ -335,3 +335,46 @@ namespace Nested {
   void g(int, int);
   using Int = A::B<>::param2;
 }
+
+namespace rdar41852459 {
+template  struct G {};
+
+template  struct S {
+  template  void f() {
+G x;
+  }
+  template  void f2() {
+G x;
+  }
+  template  void f3() {
+G x;
+  }
+};
+
+template  struct I {};
+
+template  struct K {
+  template  void f() {
+I x;
+  }
+  template  void f2() {
+I x;
+  }
+  template  void f3() {
+I x;
+  }
+};
+
+template  struct L {};
+template  struct M {
+  template  void f() {
+L x;
+  }
+  template  void f() {
+L x;
+  }
+  template  void f() {
+L x;
+  }
+};
+}


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


[PATCH] D50415: [clangd] extend the publishDiagnostics response to send back fixits to the client directly as well (if requested)

2018-08-07 Thread Alex Lorenz via Phabricator via cfe-commits
arphaman created this revision.
arphaman added reviewers: jkorous, sammccall, ilya-biryukov.
Herald added subscribers: dexonsmith, MaskRay, ioeric.

This change extends the 'textDocument/publishDiagnostics' notification sent 
from Clangd to the client. The extension can be enabled using the 
'-fixit-usage=embed-in-diagnostic' argument. When it's enabled, Clangd sends 
out the fixits associated with the appropriate diagnostic in the body of the 
'publicDiagnostics' notification.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50415

Files:
  clangd/ClangdLSPServer.cpp
  clangd/ClangdLSPServer.h
  clangd/Diagnostics.h
  clangd/fuzzer/ClangdFuzzer.cpp
  clangd/tool/ClangdMain.cpp
  test/clangd/fixits-embed-in-diagnostic.test

Index: test/clangd/fixits-embed-in-diagnostic.test
===
--- /dev/null
+++ test/clangd/fixits-embed-in-diagnostic.test
@@ -0,0 +1,66 @@
+# RUN: clangd -fixit-usage=embed-in-diagnostic -lit-test < %s | FileCheck -strict-whitespace %s
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
+---
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///foo.c","languageId":"c","version":1,"text":"struct Point {}; union Point p;"}}}
+#  CHECK:"method": "textDocument/publishDiagnostics",
+# CHECK-NEXT:"params": {
+# CHECK-NEXT: "diagnostics": [
+# CHECK-NEXT:  {
+# CHECK-NEXT:"clangd.fixes": [
+# CHECK-NEXT:  {
+# CHECK-NEXT:"edit": {
+# CHECK-NEXT:  "changes": {
+# CHECK-NEXT:"file://{{.*}}/foo.c": [
+# CHECK-NEXT:  {
+# CHECK-NEXT:"newText": "struct",
+# CHECK-NEXT:"range": {
+# CHECK-NEXT:  "end": {
+# CHECK-NEXT:"character": 22,
+# CHECK-NEXT:"line": 0
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "start": {
+# CHECK-NEXT:"character": 17,
+# CHECK-NEXT:"line": 0
+# CHECK-NEXT:  }
+# CHECK-NEXT:}
+# CHECK-NEXT:  }
+# CHECK-NEXT:]
+# CHECK-NEXT:  }
+# CHECK-NEXT:},
+# CHECK-NEXT:"title": "change 'union' to 'struct'"
+# CHECK-NEXT:  }
+# CHECK-NEXT:],
+# CHECK-NEXT:"message": "Use of 'Point' with tag type that does not match previous declaration\n\nfoo.c:1:8: note: previous use is here",
+# CHECK-NEXT:"range": {
+# CHECK-NEXT:  "end": {
+# CHECK-NEXT:"character": 22,
+# CHECK-NEXT:"line": 0
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "start": {
+# CHECK-NEXT:"character": 17,
+# CHECK-NEXT:"line": 0
+# CHECK-NEXT:  }
+# CHECK-NEXT:},
+# CHECK-NEXT:"severity": 1
+# CHECK-NEXT:  },
+# CHECK-NEXT:  {
+# CHECK-NEXT:"message": "Previous use is here\n\nfoo.c:1:18: error: use of 'Point' with tag type that does not match previous declaration",
+# CHECK-NEXT:"range": {
+# CHECK-NEXT:  "end": {
+# CHECK-NEXT:"character": 12,
+# CHECK-NEXT:"line": 0
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "start": {
+# CHECK-NEXT:"character": 7,
+# CHECK-NEXT:"line": 0
+# CHECK-NEXT:  }
+# CHECK-NEXT:},
+# CHECK-NEXT:"severity": 3
+# CHECK-NEXT:  }
+# CHECK-NEXT:],
+# CHECK-NEXT:"uri": "file://{{.*}}/foo.c"
+# CHECK-NEXT:  }
+---
+{"jsonrpc":"2.0","id":4,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}
Index: clangd/tool/ClangdMain.cpp
===
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -185,6 +185,21 @@
 "'compile_commands.json' files")),
 llvm::cl::init(FilesystemCompileArgs), llvm::cl::Hidden);
 
+static llvm::cl::opt FixitUsage(
+"fixit-usage",
+llvm::cl::desc("Controls how the diagnostic fixits are used by the client"),
+llvm::cl::values(
+clEnumValN(ClangdDiagnosticOptions::FixitUsageOption::CodeAction,
+   "code-action",
+   "The fixits can be discovered by the client using the "
+   "'textDocument/codeAction' request"),
+clEnumValN(ClangdDiagnosticOptions::FixitUsageOption::EmbedInDiagnostic,
+   "embed-in-diagnostic",
+   "The fixits are sent to the client together with the "
+   "diagnostics using an LSP extension")),
+llvm::cl::init(ClangdDiagnosticOptions::FixitUsageOption::CodeAction),
+llvm::cl::Hidden);
+
 int main(int argc, char *argv[]) {
   llvm::sys::PrintStackTraceOnErrorSignal(argv[0]);
   llvm::cl::SetVersionPrinter([](llvm::raw_ostream ) {
@@ 

[PATCH] D50414: [libunwind][include] Add SEH declarations to .

2018-08-07 Thread Charles Davis via Phabricator via cfe-commits
cdavis5x created this revision.
cdavis5x added reviewers: mstorsjo, rnk, compnerd, smeenai.
Herald added subscribers: cfe-commits, chrib, christof.

Make the `_Unwind_Exception` struct correct under SEH. Add a
declaration of `_GCC_specific_handler()`, which is used by SEH versions
of Itanium personality handlers to do common setup. Roughly corresponds
to Clang's https://reviews.llvm.org/D50380.


Repository:
  rUNW libunwind

https://reviews.llvm.org/D50414

Files:
  include/unwind.h


Index: include/unwind.h
===
--- include/unwind.h
+++ include/unwind.h
@@ -19,6 +19,10 @@
 #include 
 #include 
 
+#if defined(__SEH__) && !defined(__USING_SJLJ_EXCEPTIONS__)
+#include 
+#endif
+
 #if defined(__APPLE__)
 #define LIBUNWIND_UNAVAIL __attribute__ (( unavailable ))
 #else
@@ -120,13 +124,17 @@
   uint64_t exception_class;
   void (*exception_cleanup)(_Unwind_Reason_Code reason,
 _Unwind_Exception *exc);
+#if defined(__SEH__) && !defined(__USING_SJLJ_EXCEPTIONS__)
+  uintptr_t private_[6];
+#else
   uintptr_t private_1; // non-zero means forced unwind
   uintptr_t private_2; // holds sp that phase1 found for phase2 to use
+#endif
 #if __SIZEOF_POINTER__ == 4
   // The implementation of _Unwind_Exception uses an attribute mode on the
   // above fields which has the side effect of causing this whole struct to
-  // round up to 32 bytes in size. To be more explicit, we add pad fields
-  // added for binary compatibility.
+  // round up to 32 bytes in size (48 with SEH). To be more explicit, we add
+  // pad fields added for binary compatibility.
   uint32_t reserved[3];
 #endif
   // The Itanium ABI requires that _Unwind_Exception objects are "double-word
@@ -369,6 +377,24 @@
 extern void *__deregister_frame_info_bases(const void *fde)
 LIBUNWIND_UNAVAIL;
 
+#if defined(__SEH__) && !defined(__USING_SJLJ_EXCEPTIONS__)
+// This is the common wrapper for GCC-style personality functions with SEH.
+#ifdef __x86_64__
+// The DISPATCHER_CONTEXT struct is only defined on x64.
+extern EXCEPTION_DISPOSITION _GCC_specific_handler(PEXCEPTION_RECORD exc,
+   PVOID frame,
+   PCONTEXT ctx,
+   PDISPATCHER_CONTEXT disp,
+   _Unwind_Personality_Fn 
pers);
+#else
+extern EXCEPTION_DISPOSITION _GCC_specific_handler(PEXCEPTION_RECORD exc,
+   PVOID frame,
+   PCONTEXT ctx,
+   PVOID disp,
+   _Unwind_Personality_Fn 
pers);
+#endif
+#endif
+
 #ifdef __cplusplus
 }
 #endif


Index: include/unwind.h
===
--- include/unwind.h
+++ include/unwind.h
@@ -19,6 +19,10 @@
 #include 
 #include 
 
+#if defined(__SEH__) && !defined(__USING_SJLJ_EXCEPTIONS__)
+#include 
+#endif
+
 #if defined(__APPLE__)
 #define LIBUNWIND_UNAVAIL __attribute__ (( unavailable ))
 #else
@@ -120,13 +124,17 @@
   uint64_t exception_class;
   void (*exception_cleanup)(_Unwind_Reason_Code reason,
 _Unwind_Exception *exc);
+#if defined(__SEH__) && !defined(__USING_SJLJ_EXCEPTIONS__)
+  uintptr_t private_[6];
+#else
   uintptr_t private_1; // non-zero means forced unwind
   uintptr_t private_2; // holds sp that phase1 found for phase2 to use
+#endif
 #if __SIZEOF_POINTER__ == 4
   // The implementation of _Unwind_Exception uses an attribute mode on the
   // above fields which has the side effect of causing this whole struct to
-  // round up to 32 bytes in size. To be more explicit, we add pad fields
-  // added for binary compatibility.
+  // round up to 32 bytes in size (48 with SEH). To be more explicit, we add
+  // pad fields added for binary compatibility.
   uint32_t reserved[3];
 #endif
   // The Itanium ABI requires that _Unwind_Exception objects are "double-word
@@ -369,6 +377,24 @@
 extern void *__deregister_frame_info_bases(const void *fde)
 LIBUNWIND_UNAVAIL;
 
+#if defined(__SEH__) && !defined(__USING_SJLJ_EXCEPTIONS__)
+// This is the common wrapper for GCC-style personality functions with SEH.
+#ifdef __x86_64__
+// The DISPATCHER_CONTEXT struct is only defined on x64.
+extern EXCEPTION_DISPOSITION _GCC_specific_handler(PEXCEPTION_RECORD exc,
+   PVOID frame,
+   PCONTEXT ctx,
+   PDISPATCHER_CONTEXT disp,
+   _Unwind_Personality_Fn pers);
+#else
+extern EXCEPTION_DISPOSITION _GCC_specific_handler(PEXCEPTION_RECORD exc,
+   PVOID frame,
+

[PATCH] D50413: [libunwind][include] Add some missing definitions to .

2018-08-07 Thread Charles Davis via Phabricator via cfe-commits
cdavis5x created this revision.
cdavis5x added reviewers: mstorsjo, rnk, compnerd, smeenai.
Herald added subscribers: cfe-commits, chrib, christof, krytarowski.

Add these declarations which should be present in ``,
but aren't. Not that it matters, since most of the time we'll be using
Clang's `` anyway.


Repository:
  rUNW libunwind

https://reviews.llvm.org/D50413

Files:
  include/unwind.h


Index: include/unwind.h
===
--- include/unwind.h
+++ include/unwind.h
@@ -25,6 +25,26 @@
 #define LIBUNWIND_UNAVAIL
 #endif
 
+typedef uintptr_t _Unwind_Word;
+typedef intptr_t _Unwind_Sword;
+typedef uintptr_t _Unwind_Internal_Ptr;
+typedef uint64_t _Unwind_Exception_Class;
+
+typedef intptr_t _sleb128_t;
+typedef uintptr_t _uleb128_t;
+
+#if _LIBUNWIND_ARM_EHABI
+#if defined(__FreeBSD__)
+typedef void *_Unwind_Ptr;
+#elif defined(__linux__)
+typedef unsigned long *_Unwind_Ptr;
+#else
+typedef uintptr_t _Unwind_Ptr;
+#endif
+#else
+typedef uintptr_t _Unwind_Ptr;
+#endif
+
 typedef enum {
   _URC_NO_REASON = 0,
   _URC_OK = 0,
@@ -107,10 +127,11 @@
_Unwind_Exception* exceptionObject,
struct _Unwind_Context* context);
 
-typedef _Unwind_Reason_Code (*__personality_routine)
+typedef _Unwind_Reason_Code (*_Unwind_Personality_Fn)
   (_Unwind_State state,
_Unwind_Exception* exceptionObject,
struct _Unwind_Context* context);
+typedef _Unwind_Personality_Fn __personality_routine;
 #else
 struct _Unwind_Context;   // opaque
 struct _Unwind_Exception; // forward declaration
@@ -142,12 +163,13 @@
  struct _Unwind_Context* context,
  void* stop_parameter );
 
-typedef _Unwind_Reason_Code (*__personality_routine)
+typedef _Unwind_Reason_Code (*_Unwind_Personality_Fn)
   (int version,
_Unwind_Action actions,
uint64_t exceptionClass,
_Unwind_Exception* exceptionObject,
struct _Unwind_Context* context);
+typedef _Unwind_Personality_Fn __personality_routine;
 #endif
 
 #ifdef __cplusplus


Index: include/unwind.h
===
--- include/unwind.h
+++ include/unwind.h
@@ -25,6 +25,26 @@
 #define LIBUNWIND_UNAVAIL
 #endif
 
+typedef uintptr_t _Unwind_Word;
+typedef intptr_t _Unwind_Sword;
+typedef uintptr_t _Unwind_Internal_Ptr;
+typedef uint64_t _Unwind_Exception_Class;
+
+typedef intptr_t _sleb128_t;
+typedef uintptr_t _uleb128_t;
+
+#if _LIBUNWIND_ARM_EHABI
+#if defined(__FreeBSD__)
+typedef void *_Unwind_Ptr;
+#elif defined(__linux__)
+typedef unsigned long *_Unwind_Ptr;
+#else
+typedef uintptr_t _Unwind_Ptr;
+#endif
+#else
+typedef uintptr_t _Unwind_Ptr;
+#endif
+
 typedef enum {
   _URC_NO_REASON = 0,
   _URC_OK = 0,
@@ -107,10 +127,11 @@
_Unwind_Exception* exceptionObject,
struct _Unwind_Context* context);
 
-typedef _Unwind_Reason_Code (*__personality_routine)
+typedef _Unwind_Reason_Code (*_Unwind_Personality_Fn)
   (_Unwind_State state,
_Unwind_Exception* exceptionObject,
struct _Unwind_Context* context);
+typedef _Unwind_Personality_Fn __personality_routine;
 #else
 struct _Unwind_Context;   // opaque
 struct _Unwind_Exception; // forward declaration
@@ -142,12 +163,13 @@
  struct _Unwind_Context* context,
  void* stop_parameter );
 
-typedef _Unwind_Reason_Code (*__personality_routine)
+typedef _Unwind_Reason_Code (*_Unwind_Personality_Fn)
   (int version,
_Unwind_Action actions,
uint64_t exceptionClass,
_Unwind_Exception* exceptionObject,
struct _Unwind_Context* context);
+typedef _Unwind_Personality_Fn __personality_routine;
 #endif
 
 #ifdef __cplusplus
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D50412: Fix pointer-to-integer cast warnings on LLP64.

2018-08-07 Thread Charles Davis via Phabricator via cfe-commits
cdavis5x updated this revision to Diff 159607.
cdavis5x added a comment.

Rebasing against HEAD.


Repository:
  rUNW libunwind

https://reviews.llvm.org/D50412

Files:
  src/UnwindLevel1-gcc-ext.c
  src/UnwindLevel1.c


Index: src/UnwindLevel1.c
===
--- src/UnwindLevel1.c
+++ src/UnwindLevel1.c
@@ -287,7 +287,7 @@
 // If there is a personality routine, tell it we are unwinding.
 if (frameInfo.handler != 0) {
   __personality_routine p =
-  (__personality_routine)(long)(frameInfo.handler);
+  (__personality_routine)(intptr_t)(frameInfo.handler);
   _LIBUNWIND_TRACE_UNWINDING(
   "unwind_phase2_forced(ex_ojb=%p): calling personality function %p",
   (void *)exception_object, (void *)(uintptr_t)p);
Index: src/UnwindLevel1-gcc-ext.c
===
--- src/UnwindLevel1-gcc-ext.c
+++ src/UnwindLevel1-gcc-ext.c
@@ -33,9 +33,9 @@
(void *)exception_object,
(long)exception_object->unwinder_cache.reserved1);
 #else
-  _LIBUNWIND_TRACE_API("_Unwind_Resume_or_Rethrow(ex_obj=%p), private_1=%ld",
+  _LIBUNWIND_TRACE_API("_Unwind_Resume_or_Rethrow(ex_obj=%p), private_1=%" 
PRIdPTR,
(void *)exception_object,
-   (long)exception_object->private_1);
+   (intptr_t)exception_object->private_1);
 #endif
 
 #if defined(_LIBUNWIND_ARM_EHABI)
@@ -92,9 +92,9 @@
   unw_proc_info_t info;
   unw_getcontext();
   unw_init_local(, );
-  unw_set_reg(, UNW_REG_IP, (unw_word_t)(long) pc);
+  unw_set_reg(, UNW_REG_IP, (unw_word_t)(intptr_t) pc);
   if (unw_get_proc_info(, ) == UNW_ESUCCESS)
-return (void *)(long) info.start_ip;
+return (void *)(intptr_t) info.start_ip;
   else
 return NULL;
 }
@@ -190,14 +190,14 @@
   unw_proc_info_t info;
   unw_getcontext();
   unw_init_local(, );
-  unw_set_reg(, UNW_REG_IP, (unw_word_t)(long) pc);
+  unw_set_reg(, UNW_REG_IP, (unw_word_t)(intptr_t) pc);
   unw_get_proc_info(, );
   bases->tbase = (uintptr_t)info.extra;
   bases->dbase = 0; // dbase not used on Mac OS X
   bases->func = (uintptr_t)info.start_ip;
   _LIBUNWIND_TRACE_API("_Unwind_Find_FDE(pc=%p) => %p", pc,
-  (void *)(long) info.unwind_info);
-  return (void *)(long) info.unwind_info;
+  (void *)(intptr_t) info.unwind_info);
+  return (void *)(intptr_t) info.unwind_info;
 }
 
 /// Returns the CFA (call frame area, or stack pointer at start of function)


Index: src/UnwindLevel1.c
===
--- src/UnwindLevel1.c
+++ src/UnwindLevel1.c
@@ -287,7 +287,7 @@
 // If there is a personality routine, tell it we are unwinding.
 if (frameInfo.handler != 0) {
   __personality_routine p =
-  (__personality_routine)(long)(frameInfo.handler);
+  (__personality_routine)(intptr_t)(frameInfo.handler);
   _LIBUNWIND_TRACE_UNWINDING(
   "unwind_phase2_forced(ex_ojb=%p): calling personality function %p",
   (void *)exception_object, (void *)(uintptr_t)p);
Index: src/UnwindLevel1-gcc-ext.c
===
--- src/UnwindLevel1-gcc-ext.c
+++ src/UnwindLevel1-gcc-ext.c
@@ -33,9 +33,9 @@
(void *)exception_object,
(long)exception_object->unwinder_cache.reserved1);
 #else
-  _LIBUNWIND_TRACE_API("_Unwind_Resume_or_Rethrow(ex_obj=%p), private_1=%ld",
+  _LIBUNWIND_TRACE_API("_Unwind_Resume_or_Rethrow(ex_obj=%p), private_1=%" PRIdPTR,
(void *)exception_object,
-   (long)exception_object->private_1);
+   (intptr_t)exception_object->private_1);
 #endif
 
 #if defined(_LIBUNWIND_ARM_EHABI)
@@ -92,9 +92,9 @@
   unw_proc_info_t info;
   unw_getcontext();
   unw_init_local(, );
-  unw_set_reg(, UNW_REG_IP, (unw_word_t)(long) pc);
+  unw_set_reg(, UNW_REG_IP, (unw_word_t)(intptr_t) pc);
   if (unw_get_proc_info(, ) == UNW_ESUCCESS)
-return (void *)(long) info.start_ip;
+return (void *)(intptr_t) info.start_ip;
   else
 return NULL;
 }
@@ -190,14 +190,14 @@
   unw_proc_info_t info;
   unw_getcontext();
   unw_init_local(, );
-  unw_set_reg(, UNW_REG_IP, (unw_word_t)(long) pc);
+  unw_set_reg(, UNW_REG_IP, (unw_word_t)(intptr_t) pc);
   unw_get_proc_info(, );
   bases->tbase = (uintptr_t)info.extra;
   bases->dbase = 0; // dbase not used on Mac OS X
   bases->func = (uintptr_t)info.start_ip;
   _LIBUNWIND_TRACE_API("_Unwind_Find_FDE(pc=%p) => %p", pc,
-  (void *)(long) info.unwind_info);
-  return (void *)(long) info.unwind_info;
+  (void *)(intptr_t) info.unwind_info);
+  return (void *)(intptr_t) info.unwind_info;
 }
 
 /// Returns the CFA (call frame area, or stack pointer at start of function)
___

r339196 - [NFC] Improve auto-var-init alignment check

2018-08-07 Thread JF Bastien via cfe-commits
Author: jfb
Date: Tue Aug  7 15:43:44 2018
New Revision: 339196

URL: http://llvm.org/viewvc/llvm-project?rev=339196=rev
Log:
[NFC] Improve auto-var-init alignment check

We're not actually testing for alignment, we just want to know that whatever 
incoming alignment got propagated. Do that by capturing the alignment and 
checking that it's actually what's passed later, instead of hard-coding an 
alignment value.

Modified:
cfe/trunk/test/CodeGenCXX/auto-var-init.cpp

Modified: cfe/trunk/test/CodeGenCXX/auto-var-init.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/auto-var-init.cpp?rev=339196=339195=339196=diff
==
--- cfe/trunk/test/CodeGenCXX/auto-var-init.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/auto-var-init.cpp Tue Aug  7 15:43:44 2018
@@ -54,459 +54,459 @@ extern "C" {
 
 TEST_UNINIT(char, char);
 // CHECK-LABEL: @test_char_uninit()
-// CHECK:   %uninit = alloca i8, align 1
+// CHECK:   %uninit = alloca i8, align
 // CHECK-NEXT:  call void @{{.*}}used{{.*}}%uninit)
 
 TEST_BRACES(char, char);
 // CHECK-LABEL: @test_char_braces()
-// CHECK:   %braces = alloca i8, align 1
-// CHECK-NEXT:  store i8 0, i8* %braces, align 1
+// CHECK:   %braces = alloca i8, align [[ALIGN:[0-9]*]]
+// CHECK-NEXT:  store i8 0, i8* %braces, align [[ALIGN]]
 // CHECK-NEXT:  call void @{{.*}}used{{.*}}%braces)
 
 TEST_UNINIT(uchar, unsigned char);
 // CHECK-LABEL: @test_uchar_uninit()
-// CHECK:   %uninit = alloca i8, align 1
+// CHECK:   %uninit = alloca i8, align
 // CHECK-NEXT:  call void @{{.*}}used{{.*}}%uninit)
 
 TEST_BRACES(uchar, unsigned char);
 // CHECK-LABEL: @test_uchar_braces()
-// CHECK:   %braces = alloca i8, align 1
-// CHECK-NEXT:  store i8 0, i8* %braces, align 1
+// CHECK:   %braces = alloca i8, align [[ALIGN:[0-9]*]]
+// CHECK-NEXT:  store i8 0, i8* %braces, align [[ALIGN]]
 // CHECK-NEXT:  call void @{{.*}}used{{.*}}%braces)
 
 TEST_UNINIT(schar, signed char);
 // CHECK-LABEL: @test_schar_uninit()
-// CHECK:   %uninit = alloca i8, align 1
+// CHECK:   %uninit = alloca i8, align
 // CHECK-NEXT:  call void @{{.*}}used{{.*}}%uninit)
 
 TEST_BRACES(schar, signed char);
 // CHECK-LABEL: @test_schar_braces()
-// CHECK:   %braces = alloca i8, align 1
-// CHECK-NEXT:  store i8 0, i8* %braces, align 1
+// CHECK:   %braces = alloca i8, align [[ALIGN:[0-9]*]]
+// CHECK-NEXT:  store i8 0, i8* %braces, align [[ALIGN]]
 // CHECK-NEXT:  call void @{{.*}}used{{.*}}%braces)
 
 TEST_UNINIT(wchar_t, wchar_t);
 // CHECK-LABEL: @test_wchar_t_uninit()
-// CHECK:   %uninit = alloca i32, align 4
+// CHECK:   %uninit = alloca i32, align
 // CHECK-NEXT:  call void @{{.*}}used{{.*}}%uninit)
 
 TEST_BRACES(wchar_t, wchar_t);
 // CHECK-LABEL: @test_wchar_t_braces()
-// CHECK:   %braces = alloca i32, align 4
-// CHECK-NEXT:  store i32 0, i32* %braces, align 4
+// CHECK:   %braces = alloca i32, align [[ALIGN:[0-9]*]]
+// CHECK-NEXT:  store i32 0, i32* %braces, align [[ALIGN]]
 // CHECK-NEXT:  call void @{{.*}}used{{.*}}%braces)
 
 TEST_UNINIT(short, short);
 // CHECK-LABEL: @test_short_uninit()
-// CHECK:   %uninit = alloca i16, align 2
+// CHECK:   %uninit = alloca i16, align
 // CHECK-NEXT:  call void @{{.*}}used{{.*}}%uninit)
 
 TEST_BRACES(short, short);
 // CHECK-LABEL: @test_short_braces()
-// CHECK:   %braces = alloca i16, align 2
-// CHECK-NEXT:  store i16 0, i16* %braces, align 2
+// CHECK:   %braces = alloca i16, align [[ALIGN:[0-9]*]]
+// CHECK-NEXT:  store i16 0, i16* %braces, align [[ALIGN]]
 // CHECK-NEXT:  call void @{{.*}}used{{.*}}%braces)
 
 TEST_UNINIT(ushort, unsigned short);
 // CHECK-LABEL: @test_ushort_uninit()
-// CHECK:   %uninit = alloca i16, align 2
+// CHECK:   %uninit = alloca i16, align
 // CHECK-NEXT:  call void @{{.*}}used{{.*}}%uninit)
 
 TEST_BRACES(ushort, unsigned short);
 // CHECK-LABEL: @test_ushort_braces()
-// CHECK:   %braces = alloca i16, align 2
-// CHECK-NEXT:  store i16 0, i16* %braces, align 2
+// CHECK:   %braces = alloca i16, align [[ALIGN:[0-9]*]]
+// CHECK-NEXT:  store i16 0, i16* %braces, align [[ALIGN]]
 // CHECK-NEXT:  call void @{{.*}}used{{.*}}%braces)
 
 TEST_UNINIT(int, int);
 // CHECK-LABEL: @test_int_uninit()
-// CHECK:   %uninit = alloca i32, align 4
+// CHECK:   %uninit = alloca i32, align
 // CHECK-NEXT:  call void @{{.*}}used{{.*}}%uninit)
 
 TEST_BRACES(int, int);
 // CHECK-LABEL: @test_int_braces()
-// CHECK:   %braces = alloca i32, align 4
-// CHECK-NEXT:  store i32 0, i32* %braces, align 4
+// CHECK:   %braces = alloca i32, align [[ALIGN:[0-9]*]]
+// CHECK-NEXT:  store i32 0, i32* %braces, align [[ALIGN]]
 // CHECK-NEXT:  call void @{{.*}}used{{.*}}%braces)
 
 TEST_UNINIT(unsigned, unsigned);
 // CHECK-LABEL: @test_unsigned_uninit()
-// CHECK:   %uninit = alloca i32, align 4
+// CHECK:   %uninit = alloca i32, align
 // CHECK-NEXT:  call void 

[PATCH] D50372: Introduce the VTable interleaving scheme to the CFI design documentation

2018-08-07 Thread Zhaomo Yang via Phabricator via cfe-commits
zhaomo updated this revision to Diff 159606.
zhaomo added a comment.

Updated version of the patch


https://reviews.llvm.org/D50372

Files:
  clang/docs/ControlFlowIntegrityDesign.rst


Index: clang/docs/ControlFlowIntegrityDesign.rst
===
--- clang/docs/ControlFlowIntegrityDesign.rst
+++ clang/docs/ControlFlowIntegrityDesign.rst
@@ -274,6 +274,90 @@
 need to check that the address is in range and well aligned. This is more
 likely to occur if the virtual tables are padded.
 
+Forward-Edge CFI for Virtual Calls by Interleaving Virtual Tables
+-
+
+Dimitar et. al. first proposed a novel approach that interleaves virtual 
tables in [1]_. 
+This approach is more efficient in terms of space because padding and bit 
vectors are no longer needed. 
+At the same time, it is also more efficient in terms of performance because in 
the interleaved layout 
+address points of the virtual tables are consecutive, thus the validity check 
of a virtual 
+vtable pointer is simplified  to a range check. 
+
+At a high level, the interleaving scheme consists of three steps: 1) split 
virtual table groups into 
+separate virtual tables, 2) order virtual tables by a pre-order traversal of 
the class hierarchy 
+and 3) interleave virtual tables.
+
+.. [1] `Protecting C++ Dynamic Dispatch Through VTable Interleaving 
`_. Dimitar Bounov, 
Rami Gökhan Kıcı, Sorin Lerner.
+
+Split virtual table groups into separate virtual tables
+~~~
+
+The Itanium C++ ABI glues multiple individual virtual tables for a class into 
a combined virtual table (virtual table group). 
+The interleaving scheme, however, can only work with individual virtual tables 
so it must split the combined virtual tables first.
+In comparison, the old scheme does not require the splitting but it is more 
efficient when the combined virtual tables have been split.
+The `GlobalSplit`_ pass is responsible for splitting combined virtual tables 
into individual ones. 
+
+.. _GlobalSplit: 
https://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/GlobalSplit.cpp?view=markup
+
+Order virtual tables by a pre-order traversal of the class hierarchy 
+
+
+This step is common to both the old scheme described above and the 
interleaving scheme. 
+For the interleaving scheme, since the combined virtual tables have been split 
in the previous step, 
+this step ensures that for any class all the compatible virtual tables will 
appear consecutively. 
+For the old scheme, the same property may not hold since it may work on 
combined virtual tables. 
+
+For example, consider the following four C++ classes:
+
+.. code-block:: c++
+
+  struct A {
+virtual void f1();
+  };
+
+  struct B : A {
+virtual void f1();
+virtual void f2();
+  };
+
+  struct C : A {
+virtual void f1();
+virtual void f3();
+  };
+
+  struct D : B {
+virtual void f1();
+virtual void f2();
+virtual void f4();
+  };
+
+This step will arrange the virtual tables for A, B, C, and D in the order of 
*vtable-of-A, vtable-of-B, vtable-of-D, vtable-of-C*.
+
+Interleave virtual tables
+~
+
+This step is where the interleaving scheme deviates from the old scheme. 
Instead of laying out 
+whole virtual tables in the previously computed order, the interleaving scheme 
lays out table 
+entries one by one from the virtual tables in that order. Note that the 
Itanium C++ ABI specifies 
+that offset-to-top and RTTI fields appear at the offsets behind the address 
point, and libraries like 
+libcxxabi do assume this. To ensure the interleaved layout is compatible with 
the Itanium C++ ABI, 
+the interleaving scheme always lays out these two fields consecutively, and 
the address of the entry after the RTTI field 
+is considered the new address point for the virtual table in the interleave 
layout. Dynamic dispatch still 
+works under this scheme because the interleaved layout has the property that 
for 
+each virtual function the distance between an virtual table entry for this 
function and the corresponding 
+address point is always the same. 
+
+To follow the example used in the previous step, the interleaved layout will 
look like this:
+
+.. csv-table:: Interleaved Virtual Table Layout for A, B, C, D
+  :header: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15
+  
+  A::offset-to-top, ::rtti, B::offset-to-top, ::rtti, D::offset-to-top, 
::rtti, C::offset-to-top, ::rtti, ::f1, ::f1, ::f1, ::f1, ::f2, 
::f2, ::f3, ::f4
+
+Let us take f2 as an example to see the aforementioned property. In the 
interleaved layout, 
+there are two entries for f2: B::f2 and D::f2. The distance between ::f2 
+and its address point D::offset-to-top (the entry immediately after ::rtti) 

Re: r338301 - Avoid returning an invalid end source loc

2018-08-07 Thread David Blaikie via cfe-commits
*nod* Maybe consistency's enough for now. But yeah, if you can test whether
the assertion fires (though for invalid locs, that usually means invalid
code - and we don't have nice big repositories of weird invalid code - just
the clang regression tests).



On Tue, Aug 7, 2018 at 3:27 PM Stephen Kelly  wrote:

>
> Ok, I can look into adding the assertion and run the tests with it to see
> if anything comes up.
>
> I made a tool which dumps SourceLocations reachable from an AST node (I
> intend to integrate it into clang-query), and I noticed the large amount of
> mixing of get{Start,End}Loc and getLoc{Start,End} (see other pending
> reviews). I reviewed all of them and found that this method is the only
> case where a pair of methods of that name pattern differ in what they
> return (by eye, at least), so I thought it should be fixed.
>
> Thanks,
>
> Stephen.
>
>
> On 07/08/18 23:18, David Blaikie wrote:
>
> If it never comes up, maybe an assertion would suffice? (& if the
> assertion ever does fire - hey, we've found a test case to use)
>
> How'd you find this/what motivated you to make the change?
>
> On Tue, Aug 7, 2018 at 3:11 PM Stephen Kelly  wrote:
>
>>
>> Hi David,
>>
>> I'm happy to add a test case, but I don't know how to catch this case.
>> It's not obvious to me if any code path intentionally creates a
>> DeclarationNameInfo with a valid start loc and an invalid end loc. Can you
>> suggest a test case?
>>
>> Thanks,
>>
>> Stephen.
>>
>>
>> On 07/08/18 03:23, David Blaikie wrote:
>>
>> test case?
>>
>> On Mon, Jul 30, 2018 at 1:39 PM Stephen Kelly via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>>> Author: steveire
>>> Date: Mon Jul 30 13:39:14 2018
>>> New Revision: 338301
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=338301=rev
>>> Log:
>>> Avoid returning an invalid end source loc
>>>
>>> Modified:
>>> cfe/trunk/include/clang/AST/DeclarationName.h
>>> cfe/trunk/lib/AST/DeclarationName.cpp
>>>
>>> Modified: cfe/trunk/include/clang/AST/DeclarationName.h
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclarationName.h?rev=338301=338300=338301=diff
>>>
>>> ==
>>> --- cfe/trunk/include/clang/AST/DeclarationName.h (original)
>>> +++ cfe/trunk/include/clang/AST/DeclarationName.h Mon Jul 30 13:39:14
>>> 2018
>>> @@ -558,7 +558,7 @@ public:
>>>SourceLocation getBeginLoc() const { return NameLoc; }
>>>
>>>/// getEndLoc - Retrieve the location of the last token.
>>> -  SourceLocation getEndLoc() const;
>>> +  SourceLocation getEndLoc() const { return getLocEnd(); }
>>>
>>>/// getSourceRange - The range of the declaration name.
>>>SourceRange getSourceRange() const LLVM_READONLY {
>>> @@ -570,9 +570,11 @@ public:
>>>}
>>>
>>>SourceLocation getLocEnd() const LLVM_READONLY {
>>> -SourceLocation EndLoc = getEndLoc();
>>> +SourceLocation EndLoc = getEndLocPrivate();
>>>  return EndLoc.isValid() ? EndLoc : getLocStart();
>>>}
>>> +private:
>>> +  SourceLocation getEndLocPrivate() const;
>>>  };
>>>
>>>  /// Insertion operator for diagnostics.  This allows sending
>>> DeclarationName's
>>>
>>> Modified: cfe/trunk/lib/AST/DeclarationName.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclarationName.cpp?rev=338301=338300=338301=diff
>>>
>>> ==
>>> --- cfe/trunk/lib/AST/DeclarationName.cpp (original)
>>> +++ cfe/trunk/lib/AST/DeclarationName.cpp Mon Jul 30 13:39:14 2018
>>> @@ -689,7 +689,7 @@ void DeclarationNameInfo::printName(raw_
>>>llvm_unreachable("Unexpected declaration name kind");
>>>  }
>>>
>>> -SourceLocation DeclarationNameInfo::getEndLoc() const {
>>> +SourceLocation DeclarationNameInfo::getEndLocPrivate() const {
>>>switch (Name.getNameKind()) {
>>>case DeclarationName::Identifier:
>>>case DeclarationName::CXXDeductionGuideName:
>>>
>>>
>>> ___
>>> 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] D50341: [libcxx] Mark aligned allocation tests as XFAIL on old OSX versions

2018-08-07 Thread Volodymyr Sapsai via Phabricator via cfe-commits
vsapsai added inline comments.



Comment at: 
libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/new_size_align.fail.cpp:15
 
 // UNSUPPORTED: c++98, c++03, c++11, c++14, c++17
 // UNSUPPORTED: clang-3.3, clang-3.4, clang-3.5, clang-3.6, clang-3.7, 
clang-3.8

In what cases are we supposed to run these tests? Such extensive collection of 
unsupported C++ standards looks suspicious and I guess that is the reason why I 
haven't seen test failures with older libc++ dylibs.


Repository:
  rCXX libc++

https://reviews.llvm.org/D50341



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


[PATCH] D50412: Fix pointer-to-integer cast warnings on LLP64.

2018-08-07 Thread Charles Davis via Phabricator via cfe-commits
cdavis5x created this revision.
cdavis5x added reviewers: mstorsjo, rnk, compnerd, smeenai.
Herald added subscribers: cfe-commits, christof.

`long` is too short on LLP64. We have to use `intptr_t` to
avoid truncating pointers.


Repository:
  rUNW libunwind

https://reviews.llvm.org/D50412

Files:
  src/UnwindLevel1-gcc-ext.c
  src/UnwindLevel1.c


Index: src/UnwindLevel1.c
===
--- src/UnwindLevel1.c
+++ src/UnwindLevel1.c
@@ -287,7 +287,7 @@
 // If there is a personality routine, tell it we are unwinding.
 if (frameInfo.handler != 0) {
   __personality_routine p =
-  (__personality_routine)(long)(frameInfo.handler);
+  (__personality_routine)(intptr_t)(frameInfo.handler);
   _LIBUNWIND_TRACE_UNWINDING(
   "unwind_phase2_forced(ex_ojb=%p): calling personality function %p",
   (void *)exception_object, (void *)(uintptr_t)p);
Index: src/UnwindLevel1-gcc-ext.c
===
--- src/UnwindLevel1-gcc-ext.c
+++ src/UnwindLevel1-gcc-ext.c
@@ -33,9 +33,9 @@
(void *)exception_object,
(long)exception_object->unwinder_cache.reserved1);
 #else
-  _LIBUNWIND_TRACE_API("_Unwind_Resume_or_Rethrow(ex_obj=%p), private_1=%ld",
+  _LIBUNWIND_TRACE_API("_Unwind_Resume_or_Rethrow(ex_obj=%p), private_1=%" 
PRIdPTR,
(void *)exception_object,
-   (long)exception_object->private_1);
+   (intptr_t)exception_object->private_1);
 #endif
 
 #if defined(_LIBUNWIND_ARM_EHABI)
@@ -92,9 +92,9 @@
   unw_proc_info_t info;
   unw_getcontext();
   unw_init_local(, );
-  unw_set_reg(, UNW_REG_IP, (unw_word_t)(long) pc);
+  unw_set_reg(, UNW_REG_IP, (unw_word_t)(intptr_t) pc);
   if (unw_get_proc_info(, ) == UNW_ESUCCESS)
-return (void *)(long) info.start_ip;
+return (void *)(intptr_t) info.start_ip;
   else
 return NULL;
 }
@@ -190,14 +190,14 @@
   unw_proc_info_t info;
   unw_getcontext();
   unw_init_local(, );
-  unw_set_reg(, UNW_REG_IP, (unw_word_t)(long) pc);
+  unw_set_reg(, UNW_REG_IP, (unw_word_t)(intptr_t) pc);
   unw_get_proc_info(, );
   bases->tbase = (uintptr_t)info.extra;
   bases->dbase = 0; // dbase not used on Mac OS X
   bases->func = (uintptr_t)info.start_ip;
   _LIBUNWIND_TRACE_API("_Unwind_Find_FDE(pc=%p) => %p", pc,
-  (void *)(long) info.unwind_info);
-  return (void *)(long) info.unwind_info;
+  (void *)(intptr_t) info.unwind_info);
+  return (void *)(intptr_t) info.unwind_info;
 }
 
 /// Returns the CFA (call frame area, or stack pointer at start of function)


Index: src/UnwindLevel1.c
===
--- src/UnwindLevel1.c
+++ src/UnwindLevel1.c
@@ -287,7 +287,7 @@
 // If there is a personality routine, tell it we are unwinding.
 if (frameInfo.handler != 0) {
   __personality_routine p =
-  (__personality_routine)(long)(frameInfo.handler);
+  (__personality_routine)(intptr_t)(frameInfo.handler);
   _LIBUNWIND_TRACE_UNWINDING(
   "unwind_phase2_forced(ex_ojb=%p): calling personality function %p",
   (void *)exception_object, (void *)(uintptr_t)p);
Index: src/UnwindLevel1-gcc-ext.c
===
--- src/UnwindLevel1-gcc-ext.c
+++ src/UnwindLevel1-gcc-ext.c
@@ -33,9 +33,9 @@
(void *)exception_object,
(long)exception_object->unwinder_cache.reserved1);
 #else
-  _LIBUNWIND_TRACE_API("_Unwind_Resume_or_Rethrow(ex_obj=%p), private_1=%ld",
+  _LIBUNWIND_TRACE_API("_Unwind_Resume_or_Rethrow(ex_obj=%p), private_1=%" PRIdPTR,
(void *)exception_object,
-   (long)exception_object->private_1);
+   (intptr_t)exception_object->private_1);
 #endif
 
 #if defined(_LIBUNWIND_ARM_EHABI)
@@ -92,9 +92,9 @@
   unw_proc_info_t info;
   unw_getcontext();
   unw_init_local(, );
-  unw_set_reg(, UNW_REG_IP, (unw_word_t)(long) pc);
+  unw_set_reg(, UNW_REG_IP, (unw_word_t)(intptr_t) pc);
   if (unw_get_proc_info(, ) == UNW_ESUCCESS)
-return (void *)(long) info.start_ip;
+return (void *)(intptr_t) info.start_ip;
   else
 return NULL;
 }
@@ -190,14 +190,14 @@
   unw_proc_info_t info;
   unw_getcontext();
   unw_init_local(, );
-  unw_set_reg(, UNW_REG_IP, (unw_word_t)(long) pc);
+  unw_set_reg(, UNW_REG_IP, (unw_word_t)(intptr_t) pc);
   unw_get_proc_info(, );
   bases->tbase = (uintptr_t)info.extra;
   bases->dbase = 0; // dbase not used on Mac OS X
   bases->func = (uintptr_t)info.start_ip;
   _LIBUNWIND_TRACE_API("_Unwind_Find_FDE(pc=%p) => %p", pc,
-  (void *)(long) info.unwind_info);
-  return (void *)(long) info.unwind_info;
+  (void *)(intptr_t) info.unwind_info);
+  return (void *)(intptr_t) info.unwind_info;
 }
 
 

[PATCH] D50344: [libc++] Enable aligned allocation based on feature test macro, irrespective of standard

2018-08-07 Thread Volodymyr Sapsai via Phabricator via cfe-commits
vsapsai added inline comments.



Comment at: libcxx/include/__config:993
+!defined(_LIBCPP_BUILDING_LIBRARY) && \
+!defined(__cpp_aligned_new)
+#  define _LIBCPP_HAS_NO_ALIGNED_ALLOCATION

I think I'd rather keep `__cpp_aligned_new >= 201606` check. I don't know about 
cases where the smaller value is possible, Clang always had 201606 as far as I 
(and SVN) know. But it seems to be safer and more correct.

Don't have objective arguments for keeping the check, so if you think it's not 
worth it, I trust your judgement.



Comment at: libcxx/include/new:111-116
 #if !__has_builtin(__builtin_operator_new) || \
__has_builtin(__builtin_operator_new) < 201802L || \
defined(_LIBCPP_HAS_NO_ALIGNED_ALLOCATION) || \
!defined(__cpp_aligned_new) || __cpp_aligned_new < 201606
 #define _LIBCPP_HAS_NO_BUILTIN_ALIGNED_OPERATOR_NEW_DELETE
 #endif

Maybe move this to `__config` too? This way we'll have 
`__cpp_aligned_new`-related macros together.



Comment at: libcxx/test/libcxx/memory/aligned_allocation_macro.pass.cpp:11
+// UNSUPPORTED: c++98, c++03, c++11, c++14
+// XFAIL: with_system_cxx_lib=macosx10.12
+// XFAIL: with_system_cxx_lib=macosx10.11

Initially `with_system_cxx_lib` made me suspicious because macro 
`_LIBCPP_HAS_NO_ALIGNED_ALLOCATION` doesn't need specific dylib. And `XFAIL: 
availability=macosx10.12` seems to be working.

[Documentation](https://github.com/llvm-mirror/libcxx/blob/master/docs/DesignDocs/AvailabilityMarkup.rst#testing)
 describes which feature should be used in different cases but in this case I 
cannot definitely say if test uses unavailable feature. I think it is 
acceptable to stick with `with_system_cxx_lib` but I decided to brought to your 
attention the alternative.


Repository:
  rCXX libc++

https://reviews.llvm.org/D50344



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


Re: r338301 - Avoid returning an invalid end source loc

2018-08-07 Thread Stephen Kelly via cfe-commits


Ok, I can look into adding the assertion and run the tests with it to 
see if anything comes up.


I made a tool which dumps SourceLocations reachable from an AST node (I 
intend to integrate it into clang-query), and I noticed the large amount 
of mixing of get{Start,End}Loc and getLoc{Start,End} (see other pending 
reviews). I reviewed all of them and found that this method is the only 
case where a pair of methods of that name pattern differ in what they 
return (by eye, at least), so I thought it should be fixed.


Thanks,

Stephen.

On 07/08/18 23:18, David Blaikie wrote:
If it never comes up, maybe an assertion would suffice? (& if the 
assertion ever does fire - hey, we've found a test case to use)


How'd you find this/what motivated you to make the change?

On Tue, Aug 7, 2018 at 3:11 PM Stephen Kelly > wrote:



Hi David,

I'm happy to add a test case, but I don't know how to catch this
case. It's not obvious to me if any code path intentionally
creates a DeclarationNameInfo with a valid start loc and an
invalid end loc. Can you suggest a test case?

Thanks,

Stephen.


On 07/08/18 03:23, David Blaikie wrote:

test case?

On Mon, Jul 30, 2018 at 1:39 PM Stephen Kelly via cfe-commits
mailto:cfe-commits@lists.llvm.org>>
wrote:

Author: steveire
Date: Mon Jul 30 13:39:14 2018
New Revision: 338301

URL: http://llvm.org/viewvc/llvm-project?rev=338301=rev
Log:
Avoid returning an invalid end source loc

Modified:
    cfe/trunk/include/clang/AST/DeclarationName.h
    cfe/trunk/lib/AST/DeclarationName.cpp

Modified: cfe/trunk/include/clang/AST/DeclarationName.h
URL:

http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclarationName.h?rev=338301=338300=338301=diff

==
--- cfe/trunk/include/clang/AST/DeclarationName.h (original)
+++ cfe/trunk/include/clang/AST/DeclarationName.h Mon Jul 30
13:39:14 2018
@@ -558,7 +558,7 @@ public:
   SourceLocation getBeginLoc() const { return NameLoc; }

   /// getEndLoc - Retrieve the location of the last token.
-  SourceLocation getEndLoc() const;
+  SourceLocation getEndLoc() const { return getLocEnd(); }

   /// getSourceRange - The range of the declaration name.
   SourceRange getSourceRange() const LLVM_READONLY {
@@ -570,9 +570,11 @@ public:
   }

   SourceLocation getLocEnd() const LLVM_READONLY {
-    SourceLocation EndLoc = getEndLoc();
+    SourceLocation EndLoc = getEndLocPrivate();
     return EndLoc.isValid() ? EndLoc : getLocStart();
   }
+private:
+  SourceLocation getEndLocPrivate() const;
 };

 /// Insertion operator for diagnostics.  This allows sending
DeclarationName's

Modified: cfe/trunk/lib/AST/DeclarationName.cpp
URL:

http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclarationName.cpp?rev=338301=338300=338301=diff

==
--- cfe/trunk/lib/AST/DeclarationName.cpp (original)
+++ cfe/trunk/lib/AST/DeclarationName.cpp Mon Jul 30 13:39:14
2018
@@ -689,7 +689,7 @@ void DeclarationNameInfo::printName(raw_
   llvm_unreachable("Unexpected declaration name kind");
 }

-SourceLocation DeclarationNameInfo::getEndLoc() const {
+SourceLocation DeclarationNameInfo::getEndLocPrivate() const {
   switch (Name.getNameKind()) {
   case DeclarationName::Identifier:
   case DeclarationName::CXXDeductionGuideName:


___
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] D50088: [Sema] Fix an error with C++17 auto non-type template parameters

2018-08-07 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment.

Can you also add a test where the `auto` is not top-level (eg, `template`) and a test using `decltype(auto)`?


https://reviews.llvm.org/D50088



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


Re: r338301 - Avoid returning an invalid end source loc

2018-08-07 Thread David Blaikie via cfe-commits
If it never comes up, maybe an assertion would suffice? (& if the assertion
ever does fire - hey, we've found a test case to use)

How'd you find this/what motivated you to make the change?

On Tue, Aug 7, 2018 at 3:11 PM Stephen Kelly  wrote:

>
> Hi David,
>
> I'm happy to add a test case, but I don't know how to catch this case.
> It's not obvious to me if any code path intentionally creates a
> DeclarationNameInfo with a valid start loc and an invalid end loc. Can you
> suggest a test case?
>
> Thanks,
>
> Stephen.
>
>
> On 07/08/18 03:23, David Blaikie wrote:
>
> test case?
>
> On Mon, Jul 30, 2018 at 1:39 PM Stephen Kelly via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: steveire
>> Date: Mon Jul 30 13:39:14 2018
>> New Revision: 338301
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=338301=rev
>> Log:
>> Avoid returning an invalid end source loc
>>
>> Modified:
>> cfe/trunk/include/clang/AST/DeclarationName.h
>> cfe/trunk/lib/AST/DeclarationName.cpp
>>
>> Modified: cfe/trunk/include/clang/AST/DeclarationName.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclarationName.h?rev=338301=338300=338301=diff
>>
>> ==
>> --- cfe/trunk/include/clang/AST/DeclarationName.h (original)
>> +++ cfe/trunk/include/clang/AST/DeclarationName.h Mon Jul 30 13:39:14 2018
>> @@ -558,7 +558,7 @@ public:
>>SourceLocation getBeginLoc() const { return NameLoc; }
>>
>>/// getEndLoc - Retrieve the location of the last token.
>> -  SourceLocation getEndLoc() const;
>> +  SourceLocation getEndLoc() const { return getLocEnd(); }
>>
>>/// getSourceRange - The range of the declaration name.
>>SourceRange getSourceRange() const LLVM_READONLY {
>> @@ -570,9 +570,11 @@ public:
>>}
>>
>>SourceLocation getLocEnd() const LLVM_READONLY {
>> -SourceLocation EndLoc = getEndLoc();
>> +SourceLocation EndLoc = getEndLocPrivate();
>>  return EndLoc.isValid() ? EndLoc : getLocStart();
>>}
>> +private:
>> +  SourceLocation getEndLocPrivate() const;
>>  };
>>
>>  /// Insertion operator for diagnostics.  This allows sending
>> DeclarationName's
>>
>> Modified: cfe/trunk/lib/AST/DeclarationName.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclarationName.cpp?rev=338301=338300=338301=diff
>>
>> ==
>> --- cfe/trunk/lib/AST/DeclarationName.cpp (original)
>> +++ cfe/trunk/lib/AST/DeclarationName.cpp Mon Jul 30 13:39:14 2018
>> @@ -689,7 +689,7 @@ void DeclarationNameInfo::printName(raw_
>>llvm_unreachable("Unexpected declaration name kind");
>>  }
>>
>> -SourceLocation DeclarationNameInfo::getEndLoc() const {
>> +SourceLocation DeclarationNameInfo::getEndLocPrivate() const {
>>switch (Name.getNameKind()) {
>>case DeclarationName::Identifier:
>>case DeclarationName::CXXDeductionGuideName:
>>
>>
>> ___
>> 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] D45444: [clang-tidy] implement new check for const-correctness

2018-08-07 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment.

For reference, here is the output for llvm/lib.
F6896569: const_correctness_2_llvm_lib_references 


F6896567: const_correctness_2_llvm_lib_values_pointers 


Things i noticed:

- lambdas are warned as potential const, i think they should be excluded for 
the values
- for-loops that initialize two values (usually the start and end-iterator) are 
correctly diagnosed, but the diagnosis might be misleading. Especially because 
there is no way to have a const-iterator initialized together with the changing 
iterator.

`for (auto begin = vec.begin(), end = vec.end(); ...)`

- there seems to be a false positive with array-to-pointer decay. 
ExprMutAnalyzer does think of it, but maybe there is a bug in it.
- pointer diagnostics don't seem to worker (pointer-as-value). The test-case 
does work, This must be analyzed further
- there was a bug in the `check` method, where `AnalyzeValues == 0` did imply 
no analysis is done.

Given the volume of diagnostics for the value-case i did not investigate many 
cases manually. I think it makes sense to strive for code-transformation and 
check if the code still compiles.

References on the other hand seem to function correctly, and most references 
are single-var-decls, too. That means code transformation is very simple and 
therefor an easy target to check functionality.
References and values are in general treated the same, so it might give a hint 
about the performance for values.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45444



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


[PATCH] D49885: Thread safety analysis: Allow relockable scopes

2018-08-07 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert marked an inline comment as done.
aaronpuchert added a comment.

@delesley Did you have a chance to look at this yet?


Repository:
  rC Clang

https://reviews.llvm.org/D49885



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


[PATCH] D50408: [analyzer] Avoid querying this-pointers for static-methods.

2018-08-07 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.

Yup, fair enough.


https://reviews.llvm.org/D50408



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


Re: r338301 - Avoid returning an invalid end source loc

2018-08-07 Thread Stephen Kelly via cfe-commits


Hi David,

I'm happy to add a test case, but I don't know how to catch this case. 
It's not obvious to me if any code path intentionally creates a 
DeclarationNameInfo with a valid start loc and an invalid end loc. Can 
you suggest a test case?


Thanks,

Stephen.

On 07/08/18 03:23, David Blaikie wrote:

test case?

On Mon, Jul 30, 2018 at 1:39 PM Stephen Kelly via cfe-commits 
mailto:cfe-commits@lists.llvm.org>> wrote:


Author: steveire
Date: Mon Jul 30 13:39:14 2018
New Revision: 338301

URL: http://llvm.org/viewvc/llvm-project?rev=338301=rev
Log:
Avoid returning an invalid end source loc

Modified:
    cfe/trunk/include/clang/AST/DeclarationName.h
    cfe/trunk/lib/AST/DeclarationName.cpp

Modified: cfe/trunk/include/clang/AST/DeclarationName.h
URL:

http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclarationName.h?rev=338301=338300=338301=diff

==
--- cfe/trunk/include/clang/AST/DeclarationName.h (original)
+++ cfe/trunk/include/clang/AST/DeclarationName.h Mon Jul 30
13:39:14 2018
@@ -558,7 +558,7 @@ public:
   SourceLocation getBeginLoc() const { return NameLoc; }

   /// getEndLoc - Retrieve the location of the last token.
-  SourceLocation getEndLoc() const;
+  SourceLocation getEndLoc() const { return getLocEnd(); }

   /// getSourceRange - The range of the declaration name.
   SourceRange getSourceRange() const LLVM_READONLY {
@@ -570,9 +570,11 @@ public:
   }

   SourceLocation getLocEnd() const LLVM_READONLY {
-    SourceLocation EndLoc = getEndLoc();
+    SourceLocation EndLoc = getEndLocPrivate();
     return EndLoc.isValid() ? EndLoc : getLocStart();
   }
+private:
+  SourceLocation getEndLocPrivate() const;
 };

 /// Insertion operator for diagnostics.  This allows sending
DeclarationName's

Modified: cfe/trunk/lib/AST/DeclarationName.cpp
URL:

http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclarationName.cpp?rev=338301=338300=338301=diff

==
--- cfe/trunk/lib/AST/DeclarationName.cpp (original)
+++ cfe/trunk/lib/AST/DeclarationName.cpp Mon Jul 30 13:39:14 2018
@@ -689,7 +689,7 @@ void DeclarationNameInfo::printName(raw_
   llvm_unreachable("Unexpected declaration name kind");
 }

-SourceLocation DeclarationNameInfo::getEndLoc() const {
+SourceLocation DeclarationNameInfo::getEndLocPrivate() const {
   switch (Name.getNameKind()) {
   case DeclarationName::Identifier:
   case DeclarationName::CXXDeductionGuideName:


___
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] D50408: [analyzer] Avoid querying this-pointers for static-methods.

2018-08-07 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a reviewer: NoQ.
george.karpenkov added a subscriber: NoQ.
george.karpenkov added a comment.

Looks reasonable. @NoQ any further comments?


https://reviews.llvm.org/D50408



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


[PATCH] D50410: Removing -debug-info-macros from option suggestions test

2018-08-07 Thread Arnaud Coomans via Phabricator via cfe-commits
acoomans added a subscriber: probinson.
acoomans added a comment.

@probinson does this sound a plausible explanation? Do you have access to a PS4 
SDK to confirm the `-debug-info-macro` isn't available at all?


Repository:
  rC Clang

https://reviews.llvm.org/D50410



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


[PATCH] D50410: Removing -debug-info-macros from option suggestions test

2018-08-07 Thread Arnaud Coomans via Phabricator via cfe-commits
acoomans created this revision.
acoomans added reviewers: cfe-commits, modocache.
Herald added subscribers: dexonsmith, JDevlieghere, mehdi_amini.

https://reviews.llvm.org/D46776 added support for "did you mean ...?" command 
line option suggestions. One of the tests was checking against the 
`-debug-info-macro` option, which was failing on the PS4 build bot. The same 
test would succeed again the `--help` and `--version` options.

>From https://llvm.org/devmtg/2013-11/slides/Robinson-PS4Toolchain.pdf, it 
>looks like the PS4 SDK forces optimizations and could be disabling the 
>`-debug-info-macro` altogether.

This diff removes `-debug-info-macro` altogether.

Note: untested since we do not have access to a PS4 with the SDK.


Repository:
  rC Clang

https://reviews.llvm.org/D50410

Files:
  test/Driver/unknown-arg.c


Index: test/Driver/unknown-arg.c
===
--- test/Driver/unknown-arg.c
+++ test/Driver/unknown-arg.c
@@ -14,7 +14,7 @@
 // RUN: FileCheck %s --check-prefix=CL-ERROR-DID-YOU-MEAN
 // RUN: %clang_cl -cake-is-lie -%0 -%d - -munknown-to-clang-option 
-print-stats -funknown-to-clang-option -c -Wno-unknown-argument -### -- %s 2>&1 
| \
 // RUN: FileCheck %s --check-prefix=SILENT
-// RUN: not %clang -cc1as -hell --version -debug-info-macros 2>&1 | \
+// RUN: not %clang -cc1as -hell --version 2>&1 | \
 // RUN: FileCheck %s --check-prefix=CC1AS-DID-YOU-MEAN
 // RUN: not %clang -cc1asphalt -help 2>&1 | \
 // RUN: FileCheck %s --check-prefix=UNKNOWN-INTEGRATED


Index: test/Driver/unknown-arg.c
===
--- test/Driver/unknown-arg.c
+++ test/Driver/unknown-arg.c
@@ -14,7 +14,7 @@
 // RUN: FileCheck %s --check-prefix=CL-ERROR-DID-YOU-MEAN
 // RUN: %clang_cl -cake-is-lie -%0 -%d - -munknown-to-clang-option -print-stats -funknown-to-clang-option -c -Wno-unknown-argument -### -- %s 2>&1 | \
 // RUN: FileCheck %s --check-prefix=SILENT
-// RUN: not %clang -cc1as -hell --version -debug-info-macros 2>&1 | \
+// RUN: not %clang -cc1as -hell --version 2>&1 | \
 // RUN: FileCheck %s --check-prefix=CC1AS-DID-YOU-MEAN
 // RUN: not %clang -cc1asphalt -help 2>&1 | \
 // RUN: FileCheck %s --check-prefix=UNKNOWN-INTEGRATED
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D49355: Thread safety analysis: Allow lock upgrading and downgrading

2018-08-07 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment.

It seems @phosek was able to fix the issue in 
https://github.com/flutter/engine/pull/5944. By the way, a nice way to think 
about the attributes is that they encode state transitions as shown in the 
following table. This change fills the remaining two gaps.

|  | unlocked | locked exclusive | locked  shared|
| unlocked/unknown | `EXCLUDES`   | `ACQUIRE`| `ACQUIRE_SHARED`  |
| locked exclusive | `RELEASE`| `REQUIRES`   |   |
| locked shared| `RELEASE_SHARED` |  | `REQUIRES_SHARED` |
|

If more people stumble into the issue, another approach would be possible. My 
understanding is that the order of attributes is preserved. So we could treat 
`ACQUIRE(m) RELEASE(m)` = `EXCLUDES(m)` differently than `RELEASE(m) 
ACQUIRE(m)` = `REQUIRES(m)`. But I'm not sure if that's desirable, since 
normally the order of attributes does not matter.


Repository:
  rC Clang

https://reviews.llvm.org/D49355



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


[PATCH] D50409: Remove obsolete set call

2018-08-07 Thread Stephen Kelly via Phabricator via cfe-commits
steveire created this revision.
steveire added reviewers: rsmith, dblaikie.
Herald added a subscriber: cfe-commits.

Case case of the switch statement here makes the same call, but it is
already done at the start of the function.


Repository:
  rC Clang

https://reviews.llvm.org/D50409

Files:
  lib/Sema/SemaDecl.cpp


Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -4954,7 +4954,6 @@
   case UnqualifiedIdKind::IK_ImplicitSelfParam:
   case UnqualifiedIdKind::IK_Identifier:
 NameInfo.setName(Name.Identifier);
-NameInfo.setLoc(Name.StartLocation);
 return NameInfo;
 
   case UnqualifiedIdKind::IK_DeductionGuideName: {
@@ -4981,14 +4980,12 @@
 
 NameInfo.setName(
 Context.DeclarationNames.getCXXDeductionGuideName(Template));
-NameInfo.setLoc(Name.StartLocation);
 return NameInfo;
   }
 
   case UnqualifiedIdKind::IK_OperatorFunctionId:
 NameInfo.setName(Context.DeclarationNames.getCXXOperatorName(
Name.OperatorFunctionId.Operator));
-NameInfo.setLoc(Name.StartLocation);
 NameInfo.getInfo().CXXOperatorName.BeginOpNameLoc
   = Name.OperatorFunctionId.SymbolLocations[0];
 NameInfo.getInfo().CXXOperatorName.EndOpNameLoc
@@ -4998,7 +4995,6 @@
   case UnqualifiedIdKind::IK_LiteralOperatorId:
 NameInfo.setName(Context.DeclarationNames.getCXXLiteralOperatorName(
Name.Identifier));
-NameInfo.setLoc(Name.StartLocation);
 NameInfo.setCXXLiteralOperatorNameLoc(Name.EndLocation);
 return NameInfo;
 
@@ -5009,7 +5005,6 @@
   return DeclarationNameInfo();
 NameInfo.setName(Context.DeclarationNames.getCXXConversionFunctionName(
Context.getCanonicalType(Ty)));
-NameInfo.setLoc(Name.StartLocation);
 NameInfo.setNamedTypeInfo(TInfo);
 return NameInfo;
   }
@@ -5021,7 +5016,6 @@
   return DeclarationNameInfo();
 NameInfo.setName(Context.DeclarationNames.getCXXConstructorName(
   Context.getCanonicalType(Ty)));
-NameInfo.setLoc(Name.StartLocation);
 NameInfo.setNamedTypeInfo(TInfo);
 return NameInfo;
   }
@@ -5043,7 +5037,6 @@
 
 NameInfo.setName(Context.DeclarationNames.getCXXConstructorName(
 Context.getCanonicalType(CurClassType)));
-NameInfo.setLoc(Name.StartLocation);
 // FIXME: should we retrieve TypeSourceInfo?
 NameInfo.setNamedTypeInfo(nullptr);
 return NameInfo;
@@ -5056,7 +5049,6 @@
   return DeclarationNameInfo();
 NameInfo.setName(Context.DeclarationNames.getCXXDestructorName(
   Context.getCanonicalType(Ty)));
-NameInfo.setLoc(Name.StartLocation);
 NameInfo.setNamedTypeInfo(TInfo);
 return NameInfo;
   }


Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -4954,7 +4954,6 @@
   case UnqualifiedIdKind::IK_ImplicitSelfParam:
   case UnqualifiedIdKind::IK_Identifier:
 NameInfo.setName(Name.Identifier);
-NameInfo.setLoc(Name.StartLocation);
 return NameInfo;
 
   case UnqualifiedIdKind::IK_DeductionGuideName: {
@@ -4981,14 +4980,12 @@
 
 NameInfo.setName(
 Context.DeclarationNames.getCXXDeductionGuideName(Template));
-NameInfo.setLoc(Name.StartLocation);
 return NameInfo;
   }
 
   case UnqualifiedIdKind::IK_OperatorFunctionId:
 NameInfo.setName(Context.DeclarationNames.getCXXOperatorName(
Name.OperatorFunctionId.Operator));
-NameInfo.setLoc(Name.StartLocation);
 NameInfo.getInfo().CXXOperatorName.BeginOpNameLoc
   = Name.OperatorFunctionId.SymbolLocations[0];
 NameInfo.getInfo().CXXOperatorName.EndOpNameLoc
@@ -4998,7 +4995,6 @@
   case UnqualifiedIdKind::IK_LiteralOperatorId:
 NameInfo.setName(Context.DeclarationNames.getCXXLiteralOperatorName(
Name.Identifier));
-NameInfo.setLoc(Name.StartLocation);
 NameInfo.setCXXLiteralOperatorNameLoc(Name.EndLocation);
 return NameInfo;
 
@@ -5009,7 +5005,6 @@
   return DeclarationNameInfo();
 NameInfo.setName(Context.DeclarationNames.getCXXConversionFunctionName(
Context.getCanonicalType(Ty)));
-NameInfo.setLoc(Name.StartLocation);
 NameInfo.setNamedTypeInfo(TInfo);
 return NameInfo;
   }
@@ -5021,7 +5016,6 @@
   return DeclarationNameInfo();
 NameInfo.setName(Context.DeclarationNames.getCXXConstructorName(
   Context.getCanonicalType(Ty)));
-NameInfo.setLoc(Name.StartLocation);
 NameInfo.setNamedTypeInfo(TInfo);
 return NameInfo;
   }
@@ -5043,7 +5037,6 @@
 
  

[PATCH] D50408: [analyzer] Avoid querying this-pointers for static-methods.

2018-08-07 Thread Matt Davis via Phabricator via cfe-commits
mattd created this revision.
mattd added a reviewer: dcoughlin.
Herald added subscribers: mikhail.ramalho, a.sidorin, szepet, xazax.hun.
Herald added a reviewer: george.karpenkov.

The loop-widening code processes c++ methods looking for `this` pointers.  In
the case of static methods (which do not have `this` pointers), an assertion
was triggering.   This patch avoids trying to process `this` pointers for
static methods, and thus avoids triggering the assertion .


https://reviews.llvm.org/D50408

Files:
  lib/StaticAnalyzer/Core/LoopWidening.cpp
  test/Analysis/loop-widening-ignore-static-methods.cpp


Index: test/Analysis/loop-widening-ignore-static-methods.cpp
===
--- test/Analysis/loop-widening-ignore-static-methods.cpp
+++ test/Analysis/loop-widening-ignore-static-methods.cpp
@@ -0,0 +1,12 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-config 
widen-loops=true -analyzer-max-loop 2 %s
+// REQUIRES: asserts
+// expected-no-diagnostics
+//
+// This test checks that the loop-widening code ignores static methods.  If 
that is not the
+// case, then an assertion will trigger.
+
+class Test {
+  static void foo() {
+for (;;) {}
+  }
+};
Index: lib/StaticAnalyzer/Core/LoopWidening.cpp
===
--- lib/StaticAnalyzer/Core/LoopWidening.cpp
+++ lib/StaticAnalyzer/Core/LoopWidening.cpp
@@ -81,8 +81,10 @@
 
   // 'this' pointer is not an lvalue, we should not invalidate it. If the loop
   // is located in a method, constructor or destructor, the value of 'this'
-  // pointer shoule remain unchanged.
-  if (const CXXMethodDecl *CXXMD = dyn_cast(STC->getDecl())) {
+  // pointer should remain unchanged.  Ignore static methods, since they do not
+  // have 'this' pointers.
+  const CXXMethodDecl *CXXMD = dyn_cast(STC->getDecl());
+  if (CXXMD && !CXXMD->isStatic()) {
 const CXXThisRegion *ThisR = MRMgr.getCXXThisRegion(
 CXXMD->getThisType(STC->getAnalysisDeclContext()->getASTContext()),
 STC);


Index: test/Analysis/loop-widening-ignore-static-methods.cpp
===
--- test/Analysis/loop-widening-ignore-static-methods.cpp
+++ test/Analysis/loop-widening-ignore-static-methods.cpp
@@ -0,0 +1,12 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-config widen-loops=true -analyzer-max-loop 2 %s
+// REQUIRES: asserts
+// expected-no-diagnostics
+//
+// This test checks that the loop-widening code ignores static methods.  If that is not the
+// case, then an assertion will trigger.
+
+class Test {
+  static void foo() {
+for (;;) {}
+  }
+};
Index: lib/StaticAnalyzer/Core/LoopWidening.cpp
===
--- lib/StaticAnalyzer/Core/LoopWidening.cpp
+++ lib/StaticAnalyzer/Core/LoopWidening.cpp
@@ -81,8 +81,10 @@
 
   // 'this' pointer is not an lvalue, we should not invalidate it. If the loop
   // is located in a method, constructor or destructor, the value of 'this'
-  // pointer shoule remain unchanged.
-  if (const CXXMethodDecl *CXXMD = dyn_cast(STC->getDecl())) {
+  // pointer should remain unchanged.  Ignore static methods, since they do not
+  // have 'this' pointers.
+  const CXXMethodDecl *CXXMD = dyn_cast(STC->getDecl());
+  if (CXXMD && !CXXMD->isStatic()) {
 const CXXThisRegion *ThisR = MRMgr.getCXXThisRegion(
 CXXMD->getThisType(STC->getAnalysisDeclContext()->getASTContext()),
 STC);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r339191 - [NFC] CGDecl factor out constant emission

2018-08-07 Thread JF Bastien via cfe-commits
Author: jfb
Date: Tue Aug  7 14:55:13 2018
New Revision: 339191

URL: http://llvm.org/viewvc/llvm-project?rev=339191=rev
Log:
[NFC] CGDecl factor out constant emission

The code is cleaner this way, and with some changes I'm playing with it makes 
sense to split it out so we can reuse it.

Modified:
cfe/trunk/lib/CodeGen/CGDecl.cpp

Modified: cfe/trunk/lib/CodeGen/CGDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDecl.cpp?rev=339191=339190=339191=diff
==
--- cfe/trunk/lib/CodeGen/CGDecl.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDecl.cpp Tue Aug  7 14:55:13 2018
@@ -1055,6 +1055,61 @@ static BytePattern shouldUseMemSetToInit
   return constantIsRepeatedBytePattern(Init);
 }
 
+static void emitStoresForConstant(CodeGenModule , const VarDecl ,
+  Address Loc, bool isVolatile,
+  CGBuilderTy ,
+  llvm::Constant *constant) {
+  auto *Int8Ty = llvm::IntegerType::getInt8Ty(CGM.getLLVMContext());
+  auto *IntPtrTy = CGM.getDataLayout().getIntPtrType(CGM.getLLVMContext());
+
+  // If the initializer is all or mostly the same, codegen with bzero / memset
+  // then do a few stores afterward.
+  uint64_t ConstantSize =
+  CGM.getDataLayout().getTypeAllocSize(constant->getType());
+  auto *SizeVal = llvm::ConstantInt::get(IntPtrTy, ConstantSize);
+  if (shouldUseBZeroPlusStoresToInitialize(constant, ConstantSize)) {
+Builder.CreateMemSet(Loc, llvm::ConstantInt::get(Int8Ty, 0), SizeVal,
+ isVolatile);
+
+bool valueAlreadyCorrect =
+constant->isNullValue() || isa(constant);
+if (!valueAlreadyCorrect) {
+  Loc = Builder.CreateBitCast(
+  Loc, constant->getType()->getPointerTo(Loc.getAddressSpace()));
+  emitStoresForInitAfterBZero(CGM, constant, Loc, isVolatile, Builder);
+}
+return;
+  }
+
+  BytePattern Pattern = shouldUseMemSetToInitialize(constant, ConstantSize);
+  if (!Pattern.isNone()) {
+uint8_t Value = Pattern.isAny() ? 0x00 : Pattern.getValue();
+Builder.CreateMemSet(Loc, llvm::ConstantInt::get(Int8Ty, Value), SizeVal,
+ isVolatile);
+return;
+  }
+
+  // Otherwise, create a temporary global with the initializer then memcpy from
+  // the global to the alloca.
+  std::string Name = getStaticDeclName(CGM, D);
+  unsigned AS = CGM.getContext().getTargetAddressSpace(
+  CGM.getStringLiteralAddressSpace());
+  llvm::Type *BP = llvm::PointerType::getInt8PtrTy(CGM.getLLVMContext(), AS);
+
+  llvm::GlobalVariable *GV = new llvm::GlobalVariable(
+  CGM.getModule(), constant->getType(), true,
+  llvm::GlobalValue::PrivateLinkage, constant, Name, nullptr,
+  llvm::GlobalValue::NotThreadLocal, AS);
+  GV->setAlignment(Loc.getAlignment().getQuantity());
+  GV->setUnnamedAddr(llvm::GlobalValue::UnnamedAddr::Global);
+
+  Address SrcPtr = Address(GV, Loc.getAlignment());
+  if (SrcPtr.getType() != BP)
+SrcPtr = Builder.CreateBitCast(SrcPtr, BP);
+
+  Builder.CreateMemCpy(Loc, SrcPtr, SizeVal, isVolatile);
+}
+
 /// EmitAutoVarDecl - Emit code and set up an entry in LocalDeclMap for a
 /// variable declaration with auto, register, or no storage class specifier.
 /// These turn into simple stack objects, or GlobalValues depending on target.
@@ -1500,57 +1555,11 @@ void CodeGenFunction::EmitAutoVarInit(co
   // in various ways.
   bool isVolatile = type.isVolatileQualified();
 
-  llvm::Value *SizeVal =
-llvm::ConstantInt::get(IntPtrTy,
-   
getContext().getTypeSizeInChars(type).getQuantity());
-
   llvm::Type *BP = CGM.Int8Ty->getPointerTo(Loc.getAddressSpace());
   if (Loc.getType() != BP)
 Loc = Builder.CreateBitCast(Loc, BP);
 
-  // If the initializer is all or mostly the same, codegen with bzero / memset
-  // then do a few stores afterward.
-  uint64_t ConstantSize =
-  CGM.getDataLayout().getTypeAllocSize(constant->getType());
-  if (shouldUseBZeroPlusStoresToInitialize(constant, ConstantSize)) {
-Builder.CreateMemSet(Loc, llvm::ConstantInt::get(Int8Ty, 0), SizeVal,
- isVolatile);
-// Zero and undef don't require a stores.
-if (!constant->isNullValue() && !isa(constant)) {
-  Loc = Builder.CreateBitCast(Loc,
-constant->getType()->getPointerTo(Loc.getAddressSpace()));
-  emitStoresForInitAfterBZero(CGM, constant, Loc, isVolatile, Builder);
-}
-return;
-  }
-
-  BytePattern Pattern = shouldUseMemSetToInitialize(constant, ConstantSize);
-  if (!Pattern.isNone()) {
-uint8_t Value = Pattern.isAny() ? 0x00 : Pattern.getValue();
-Builder.CreateMemSet(Loc, llvm::ConstantInt::get(Int8Ty, Value), SizeVal,
- isVolatile);
-return;
-  }
-
-  // Otherwise, create a temporary global with the initializer then
-  // memcpy from the global to the alloca.
-  std::string Name = 

[PATCH] D50389: [clang-tidy] new check for Abseil

2018-08-07 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments.



Comment at: clang-tidy/abseil/DurationDivisionCheck.cpp:37
+void DurationDivisionCheck::check(const MatchFinder::MatchResult& result) {
+  const auto* op = result.Nodes.getNodeAs("op");
+  diag(op->getOperatorLoc(),

JonasToth wrote:
> Please follow the naming convention here as well -> `Op` or better `OpCall` 
> or similar to have more telling names.
`op` still is lowercase.



Comment at: clang-tidy/abseil/DurationDivisionCheck.cpp:30
+  hasOverloadedOperatorName("/"), argumentCountIs(2),
+  hasArgument(0, expr(is_duration)),
+  hasArgument(1, expr(is_duration)), expr().bind("OpCall",

s/is_duration/IsDuration/ twice



Comment at: docs/ReleaseNotes.rst:59
 --
+- New :doc:`abseil-duration-division
+  ` check.

Please add one empty line above and please remove the `The improvements are...` 
line.

This might clash with other checks that are in review right now, but yours 
might be the first one that lands.


https://reviews.llvm.org/D50389



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


r339188 - Fix one hard coded value I missed in r339185.

2018-08-07 Thread Douglas Yung via cfe-commits
Author: dyung
Date: Tue Aug  7 14:37:14 2018
New Revision: 339188

URL: http://llvm.org/viewvc/llvm-project?rev=339188=rev
Log:
Fix one hard coded value I missed in r339185.

Modified:
cfe/trunk/test/CodeGenOpenCL/enqueue-kernel-non-entry-block.cl

Modified: cfe/trunk/test/CodeGenOpenCL/enqueue-kernel-non-entry-block.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenOpenCL/enqueue-kernel-non-entry-block.cl?rev=339188=339187=339188=diff
==
--- cfe/trunk/test/CodeGenOpenCL/enqueue-kernel-non-entry-block.cl (original)
+++ cfe/trunk/test/CodeGenOpenCL/enqueue-kernel-non-entry-block.cl Tue Aug  7 
14:37:14 2018
@@ -29,5 +29,5 @@ kernel void test(int i) {
 
 // CHECK-DEBUG: ![[TESTFILE:[0-9]+]] = !DIFile(filename: ""
 // CHECK-DEBUG: ![[TESTSCOPE:[0-9]+]] = distinct !DISubprogram(name: "test", 
{{.*}} file: ![[TESTFILE]]
-// CHECK-DEBUG: ![[IFSCOPE:[0-9]+]] = distinct !DILexicalBlock(scope: 
![[TESTSCOPE]], file: !1, line: 24)
+// CHECK-DEBUG: ![[IFSCOPE:[0-9]+]] = distinct !DILexicalBlock(scope: 
![[TESTSCOPE]], file: ![[TESTFILE]], line: 24)
 // CHECK-DEBUG: ![[TEMPLOCATION]] = !DILocation(line: 25, scope: ![[IFSCOPE]])


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


r339187 - Clean up and simplify RequireCompleteType.

2018-08-07 Thread Richard Smith via cfe-commits
Author: rsmith
Date: Tue Aug  7 14:35:41 2018
New Revision: 339187

URL: http://llvm.org/viewvc/llvm-project?rev=339187=rev
Log:
Clean up and simplify RequireCompleteType.

No functional change intended, except that we will now produce more
"declared here" notes.

Modified:
cfe/trunk/lib/Sema/SemaCXXScopeSpec.cpp
cfe/trunk/lib/Sema/SemaType.cpp
cfe/trunk/test/CXX/stmt.stmt/stmt.iter/stmt.ranged/p1.cpp
cfe/trunk/test/SemaObjC/arc.m

Modified: cfe/trunk/lib/Sema/SemaCXXScopeSpec.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCXXScopeSpec.cpp?rev=339187=339186=339187=diff
==
--- cfe/trunk/lib/Sema/SemaCXXScopeSpec.cpp (original)
+++ cfe/trunk/lib/Sema/SemaCXXScopeSpec.cpp Tue Aug  7 14:35:41 2018
@@ -209,11 +209,13 @@ bool Sema::RequireCompleteDeclContext(CX
   if (!tag || tag->isDependentContext())
 return false;
 
+  // Grab the tag definition, if there is one.
+  QualType type = Context.getTypeDeclType(tag);
+  tag = type->getAsTagDecl();
+
   // If we're currently defining this type, then lookup into the
   // type is okay: don't complain that it isn't complete yet.
-  QualType type = Context.getTypeDeclType(tag);
-  const TagType *tagType = type->getAs();
-  if (tagType && tagType->isBeingDefined())
+  if (tag->isBeingDefined())
 return false;
 
   SourceLocation loc = SS.getLastQualifierNameLoc();
@@ -229,13 +231,13 @@ bool Sema::RequireCompleteDeclContext(CX
   // Fixed enum types are complete, but they aren't valid as scopes
   // until we see a definition, so awkwardly pull out this special
   // case.
-  const EnumType *enumType = dyn_cast_or_null(tagType);
-  if (!enumType)
+  auto *EnumD = dyn_cast(tag);
+  if (!EnumD)
 return false;
-  if (enumType->getDecl()->isCompleteDefinition()) {
+  if (EnumD->isCompleteDefinition()) {
 // If we know about the definition but it is not visible, complain.
 NamedDecl *SuggestedDef = nullptr;
-if (!hasVisibleDefinition(enumType->getDecl(), ,
+if (!hasVisibleDefinition(EnumD, ,
   /*OnlyNeedComplete*/false)) {
   // If the user is going to see an error here, recover by making the
   // definition visible.
@@ -249,11 +251,11 @@ bool Sema::RequireCompleteDeclContext(CX
 
   // Try to instantiate the definition, if this is a specialization of an
   // enumeration temploid.
-  EnumDecl *ED = enumType->getDecl();
-  if (EnumDecl *Pattern = ED->getInstantiatedFromMemberEnum()) {
-MemberSpecializationInfo *MSI = ED->getMemberSpecializationInfo();
+  if (EnumDecl *Pattern = EnumD->getInstantiatedFromMemberEnum()) {
+MemberSpecializationInfo *MSI = EnumD->getMemberSpecializationInfo();
 if (MSI->getTemplateSpecializationKind() != TSK_ExplicitSpecialization) {
-  if (InstantiateEnum(loc, ED, Pattern, getTemplateInstantiationArgs(ED),
+  if (InstantiateEnum(loc, EnumD, Pattern,
+  getTemplateInstantiationArgs(EnumD),
   TSK_ImplicitInstantiation)) {
 SS.SetInvalid(SS.getRange());
 return true;

Modified: cfe/trunk/lib/Sema/SemaType.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaType.cpp?rev=339187=339186=339187=diff
==
--- cfe/trunk/lib/Sema/SemaType.cpp (original)
+++ cfe/trunk/lib/Sema/SemaType.cpp Tue Aug  7 14:35:41 2018
@@ -7684,39 +7684,24 @@ bool Sema::RequireCompleteTypeImpl(Sourc
 return false;
   }
 
-  const TagType *Tag = T->getAs();
-  const ObjCInterfaceType *IFace = T->getAs();
+  TagDecl *Tag = dyn_cast_or_null(Def);
+  ObjCInterfaceDecl *IFace = dyn_cast_or_null(Def);
 
-  // If there's an unimported definition of this type in a module (for
-  // instance, because we forward declared it, then imported the definition),
-  // import that definition now.
-  //
-  // FIXME: What about other cases where an import extends a redeclaration
-  // chain for a declaration that can be accessed through a mechanism other
-  // than name lookup (eg, referenced in a template, or a variable whose type
-  // could be completed by the module)?
-  //
-  // FIXME: Should we map through to the base array element type before
-  // checking for a tag type?
+  // Give the external source a chance to provide a definition of the type.
+  // This is kept separate from completing the redeclaration chain so that
+  // external sources such as LLDB can avoid synthesizing a type definition
+  // unless it's actually needed.
   if (Tag || IFace) {
-NamedDecl *D =
-Tag ? static_cast(Tag->getDecl()) : IFace->getDecl();
-
 // Avoid diagnosing invalid decls as incomplete.
-if (D->isInvalidDecl())
+if (Def->isInvalidDecl())
   return true;
 
 // Give the external AST source a chance to complete the type.
 if (auto *Source = Context.getExternalSource()) {
-  if (Tag) {
-TagDecl *TagD = 

Re: r338467 - Avoid exposing name for range-based for '__range' variables in lifetime warnings.

2018-08-07 Thread Richard Smith via cfe-commits
There's definitely scope for improving this diagnostic text further. Right
now I don't think there's an easy way to figure out that the variable is
the range variable in a range-based for loop, but I think that case is
common enough that that's the level of special-case we should be looking at
here. If we track that state, something like "error: range refers to a
temporary object that will be destroyed before the first iteration of the
loop" would seem much preferable.

On Tue, 7 Aug 2018 at 09:28, David Blaikie via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Reckon there's a chance of improved diagnostic text in cases like this?
> Will users understand what the problem is/how to fix it when they read 
> "temporary
> implicitly bound to local reference will be destroyed at the end of the
> full-expression" - feels very standard-ese-y to me? & I appreciate teh
> desire/need for precision, I wonder if there's better ways to communicate
> it to the user... :/
>
> On Tue, Jul 31, 2018 at 6:03 PM Richard Smith via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: rsmith
>> Date: Tue Jul 31 18:03:33 2018
>> New Revision: 338467
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=338467=rev
>> Log:
>> Avoid exposing name for range-based for '__range' variables in lifetime
>> warnings.
>>
>> Modified:
>> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>> cfe/trunk/lib/Sema/SemaInit.cpp
>> cfe/trunk/test/SemaCXX/attr-lifetimebound.cpp
>>
>> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=338467=338466=338467=diff
>>
>> ==
>> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
>> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Jul 31
>> 18:03:33 2018
>> @@ -7875,7 +7875,8 @@ def warn_ret_addr_label : Warning<
>>  def err_ret_local_block : Error<
>>"returning block that lives on the local stack">;
>>  def note_local_var_initializer : Note<
>> -  "%select{via initialization of|binding reference}0 variable %1 here">;
>> +  "%select{via initialization of|binding reference}0 variable "
>> +  "%select{%2 |}1here">;
>>  def note_init_with_default_member_initalizer : Note<
>>"initializing field %0 with default member initializer">;
>>
>> @@ -7907,13 +7908,14 @@ def note_lifetime_extending_member_decla
>>"member with %select{reference|'std::initializer_list'}0 subobject}1 "
>>"declared here">;
>>  def warn_dangling_variable : Warning<
>> -  "%select{temporary %select{whose address is used as value of|bound
>> to}3 "
>> -  "%select{%select{|reference }3member of local variable|"
>> -  "local %select{variable|reference}3}1|"
>> +  "%select{temporary %select{whose address is used as value of|"
>> +  "%select{|implicitly }2bound to}4 "
>> +  "%select{%select{|reference }4member of local variable|"
>> +  "local %select{variable|reference}4}1|"
>>"array backing "
>>"%select{initializer list subobject of local variable|"
>>"local initializer list}1}0 "
>> -  "%2 will be destroyed at the end of the full-expression">,
>> +  "%select{%3 |}2will be destroyed at the end of the full-expression">,
>>InGroup;
>>  def warn_new_dangling_reference : Warning<
>>"temporary bound to reference member of allocated object "
>>
>> Modified: cfe/trunk/lib/Sema/SemaInit.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.cpp?rev=338467=338466=338467=diff
>>
>> ==
>> --- cfe/trunk/lib/Sema/SemaInit.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaInit.cpp Tue Jul 31 18:03:33 2018
>> @@ -6847,8 +6847,9 @@ void Sema::checkInitializerLifetime(cons
>>return false;
>>
>>  Diag(DiagLoc, diag::warn_dangling_variable)
>> -<< RK << !Entity.getParent() << ExtendingEntity->getDecl()
>> -<< Init->isGLValue() << DiagRange;
>> +<< RK << !Entity.getParent()
>> +<< ExtendingEntity->getDecl()->isImplicit()
>> +<< ExtendingEntity->getDecl() << Init->isGLValue() <<
>> DiagRange;
>>}
>>break;
>>  }
>> @@ -6969,7 +6970,8 @@ void Sema::checkInitializerLifetime(cons
>>case IndirectLocalPathEntry::VarInit:
>>  const VarDecl *VD = cast(Elem.D);
>>  Diag(VD->getLocation(), diag::note_local_var_initializer)
>> -<< VD->getType()->isReferenceType() << VD->getDeclName()
>> +<< VD->getType()->isReferenceType()
>> +<< VD->isImplicit() << VD->getDeclName()
>>  << nextPathEntryRange(Path, I + 1, L);
>>  break;
>>}
>>
>> Modified: cfe/trunk/test/SemaCXX/attr-lifetimebound.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/attr-lifetimebound.cpp?rev=338467=338466=338467=diff
>>
>> 

[PATCH] D50372: Introduce the VTable interleaving scheme to the CFI design documentation

2018-08-07 Thread Vlad Tsyrklevich via Phabricator via cfe-commits
vlad.tsyrklevich added inline comments.



Comment at: clang/docs/ControlFlowIntegrityDesign.rst:283
+At the same time, it is also more efficient in terms of performance because in 
the interleaved virtual 
+table address points are consecutive, thus the validity check of a virtual 
vtable pointer is simplified 
+to a range check. 

simplified to a range check -> always a range check


Repository:
  rC Clang

https://reviews.llvm.org/D50372



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


RE: r339185 - Make test more robust by not checking hard coded debug info values, but instead check the relationships between them.

2018-08-07 Thread via cfe-commits


> -Original Message-
> From: cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] On Behalf Of
> Douglas Yung via cfe-commits
> Sent: Tuesday, August 07, 2018 5:23 PM
> To: cfe-commits@lists.llvm.org
> Subject: r339185 - Make test more robust by not checking hard coded debug
> info values, but instead check the relationships between them.
> 
> Author: dyung
> Date: Tue Aug  7 14:22:49 2018
> New Revision: 339185
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=339185=rev
> Log:
> Make test more robust by not checking hard coded debug info values, but
> instead check the relationships between them.
> 
> Modified:
> cfe/trunk/test/CodeGenOpenCL/enqueue-kernel-non-entry-block.cl
> 
> Modified: cfe/trunk/test/CodeGenOpenCL/enqueue-kernel-non-entry-block.cl
> URL: http://llvm.org/viewvc/llvm-
> project/cfe/trunk/test/CodeGenOpenCL/enqueue-kernel-non-entry-
> block.cl?rev=339185=339184=339185=diff
> ==
> 
> --- cfe/trunk/test/CodeGenOpenCL/enqueue-kernel-non-entry-block.cl
> (original)
> +++ cfe/trunk/test/CodeGenOpenCL/enqueue-kernel-non-entry-block.cl Tue Aug
> 7 14:22:49 2018
> @@ -16,7 +16,7 @@ kernel void test(int i) {
>  // SPIR64: %block_sizes = alloca [1 x i64]
>  // COMMON-LABEL: if.then:
>  // COMMON-NOT: alloca
> -// CHECK-DEBUG: getelementptr {{.*}} %block_sizes, {{.*}} !dbg !34
> +// CHECK-DEBUG: getelementptr {{.*}} %block_sizes, {{.*}} !dbg
> ![[TEMPLOCATION:[0-9]+]]
>  // COMMON-LABEL: if.end
>queue_t default_queue;
>unsigned flags = 0;
> @@ -27,5 +27,7 @@ kernel void test(int i) {
> 
>  // Check that the temporary is scoped to the `if`
> 
> -// CHECK-DEBUG: !32 = distinct !DILexicalBlock(scope: !7, file: !1, line:
> 24)
> -// CHECK-DEBUG: !34 = !DILocation(line: 25, scope: !32)
> +// CHECK-DEBUG: ![[TESTFILE:[0-9]+]] = !DIFile(filename: ""
> +// CHECK-DEBUG: ![[TESTSCOPE:[0-9]+]] = distinct !DISubprogram(name:
> "test", {{.*}} file: ![[TESTFILE]]
> +// CHECK-DEBUG: ![[IFSCOPE:[0-9]+]] = distinct !DILexicalBlock(scope:
> ![[TESTSCOPE]], file: !1, line: 24)

You missed a hard-coded !1 here, should be [[TESTFILE]].

> +// CHECK-DEBUG: ![[TEMPLOCATION]] = !DILocation(line: 25, scope:
> ![[IFSCOPE]])
> 
> 
> ___
> 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] D50349: Port getStartLoc -> getBeginLoc

2018-08-07 Thread Stephen Kelly via Phabricator via cfe-commits
steveire added a comment.

Thanks Hans. Yes, I made these patches with some simple sed oneliners.

Once these are approved in principle, I'll re-generate them before pushing.

I have also run clang-format on them, but didn't update Phab with the results 
to reduce noise.


Repository:
  rC Clang

https://reviews.llvm.org/D50349



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


r339185 - Make test more robust by not checking hard coded debug info values, but instead check the relationships between them.

2018-08-07 Thread Douglas Yung via cfe-commits
Author: dyung
Date: Tue Aug  7 14:22:49 2018
New Revision: 339185

URL: http://llvm.org/viewvc/llvm-project?rev=339185=rev
Log:
Make test more robust by not checking hard coded debug info values, but instead 
check the relationships between them.

Modified:
cfe/trunk/test/CodeGenOpenCL/enqueue-kernel-non-entry-block.cl

Modified: cfe/trunk/test/CodeGenOpenCL/enqueue-kernel-non-entry-block.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenOpenCL/enqueue-kernel-non-entry-block.cl?rev=339185=339184=339185=diff
==
--- cfe/trunk/test/CodeGenOpenCL/enqueue-kernel-non-entry-block.cl (original)
+++ cfe/trunk/test/CodeGenOpenCL/enqueue-kernel-non-entry-block.cl Tue Aug  7 
14:22:49 2018
@@ -16,7 +16,7 @@ kernel void test(int i) {
 // SPIR64: %block_sizes = alloca [1 x i64]
 // COMMON-LABEL: if.then:
 // COMMON-NOT: alloca
-// CHECK-DEBUG: getelementptr {{.*}} %block_sizes, {{.*}} !dbg !34
+// CHECK-DEBUG: getelementptr {{.*}} %block_sizes, {{.*}} !dbg 
![[TEMPLOCATION:[0-9]+]]
 // COMMON-LABEL: if.end
   queue_t default_queue;
   unsigned flags = 0;
@@ -27,5 +27,7 @@ kernel void test(int i) {
 
 // Check that the temporary is scoped to the `if`
 
-// CHECK-DEBUG: !32 = distinct !DILexicalBlock(scope: !7, file: !1, line: 24)
-// CHECK-DEBUG: !34 = !DILocation(line: 25, scope: !32)
+// CHECK-DEBUG: ![[TESTFILE:[0-9]+]] = !DIFile(filename: ""
+// CHECK-DEBUG: ![[TESTSCOPE:[0-9]+]] = distinct !DISubprogram(name: "test", 
{{.*}} file: ![[TESTFILE]]
+// CHECK-DEBUG: ![[IFSCOPE:[0-9]+]] = distinct !DILexicalBlock(scope: 
![[TESTSCOPE]], file: !1, line: 24)
+// CHECK-DEBUG: ![[TEMPLOCATION]] = !DILocation(line: 25, scope: ![[IFSCOPE]])


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


r339183 - [analyzer] [tests] Do not be verbose by default when updating reference results.

2018-08-07 Thread George Karpenkov via cfe-commits
Author: george.karpenkov
Date: Tue Aug  7 14:14:35 2018
New Revision: 339183

URL: http://llvm.org/viewvc/llvm-project?rev=339183=rev
Log:
[analyzer] [tests] Do not be verbose by default when updating reference results.

Modified:
cfe/trunk/utils/analyzer/SATestUpdateDiffs.py

Modified: cfe/trunk/utils/analyzer/SATestUpdateDiffs.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/analyzer/SATestUpdateDiffs.py?rev=339183=339182=339183=diff
==
--- cfe/trunk/utils/analyzer/SATestUpdateDiffs.py (original)
+++ cfe/trunk/utils/analyzer/SATestUpdateDiffs.py Tue Aug  7 14:14:35 2018
@@ -10,7 +10,7 @@ from subprocess import check_call
 import os
 import sys
 
-Verbose = 1
+Verbose = 0
 
 
 def runCmd(Command, **kwargs):


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


[PATCH] D43357: [Concepts] Function trailing requires clauses

2018-08-07 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments.



Comment at: lib/Sema/SemaDecl.cpp:8377-8381
+  } else if (D.hasTrailingRequiresClause()) {
+// C++2a [class.virtual]p6
+// A virtual method shall not have a requires-clause.
+Diag(NewFD->getTrailingRequiresClause()->getLocStart(),
+ diag::err_constrained_virtual_method);

saar.raz wrote:
> rsmith wrote:
> > This is the wrong place for this check. We don't yet know whether the 
> > function is virtual here in general. A function can become virtual due to 
> > template instantiation:
> > 
> > ```
> > template struct A : T { void f() requires true; };
> > struct B { virtual void f(); };
> > template struct A; // error, A::f is constrained and virtual
> > ```
> > 
> > This is perhaps a wording defect: it's not clear that `A::f()` really 
> > should override `B::f()`, but that is the consequence of the current rules. 
> > I've posted a question to the core reflector.
> I don't really see why A::f() should override B::f() indeed - since it is not 
> marked virtual nor override, shouldn't it just hide B::f()? or am I missing 
> something here?
Functions override base class virtual functions if they have matching name, 
parameter types, cv-qualifiers and ref-qualifiers, *regardless* of whether 
they're declared `virtual` etc. Eg:

```
struct A { virtual void f(); };
struct B : A { void f(); };
```

`B::f` overrides `A::f`, and as a consequence, `B::f` is implicitly a virtual 
function. See [class.virtual]p2 and its footnote.

Note that the determination of whether the derived class function overrides the 
base class function doesn't care about the associated constraints on the 
derived class function. (After checking with the core reflector, general 
consensus seems to be that this is the right rule, and my previous example 
should indeed be ill-formed because it declares a constrained virtual function.)


Repository:
  rC Clang

https://reviews.llvm.org/D43357



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


[PATCH] D50168: [Builtins] Implement __builtin_clrsb to be compatible with gcc

2018-08-07 Thread Benjamin Kramer via Phabricator via cfe-commits
bkramer added a comment.

Test case?




Comment at: lib/CodeGen/CGBuiltin.cpp:1563
+Value *Result = Builder.CreateCall(F, {Tmp, Builder.getTrue()});
+if (Result->getType() != ResultType)
+  Result = Builder.CreateIntCast(Result, ResultType, /*isSigned*/true,

CreateIntCast just does nothing if the types match, so this check isn't needed.


https://reviews.llvm.org/D50168



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


[PATCH] D50389: [clang-tidy] new check for Abseil

2018-08-07 Thread Deanna Garcia via Phabricator via cfe-commits
deannagarcia updated this revision to Diff 159590.

https://reviews.llvm.org/D50389

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/DurationDivisionCheck.cpp
  clang-tidy/abseil/DurationDivisionCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-duration-division.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-duration-division.cpp

Index: test/clang-tidy/abseil-duration-division.cpp
===
--- test/clang-tidy/abseil-duration-division.cpp
+++ test/clang-tidy/abseil-duration-division.cpp
@@ -0,0 +1,44 @@
+// RUN: %check_clang_tidy %s abseil-duration-division %t
+
+namespace absl {
+
+class Duration {};
+
+int operator/(Duration lhs, Duration rhs);
+
+double FDivDuration(Duration num, Duration den);
+
+}  // namespace absl
+
+void TakesInt(int);
+void TakesDouble(double);
+
+absl::Duration d;
+
+#define MACRO_EQ(x, y) (x == y)
+#define MACRO_DIVEQ(x,y,z) (x/y == z)
+#define CHECK(x) (x)
+
+void Positives() {
+  const double num = d/d;
+  // CHECK-MESSAGES: [[@LINE-1]]:23: warning: operator/ on Duration objects performs integer division; did you mean to use FDivDuration()? [abseil-duration-division]
+  // CHECK-FIXES: const double num = absl::FDivDuration(d, d);
+  if (MACRO_EQ(d/d, 0.0)) {}
+  // CHECK-MESSAGES: [[@LINE-1]]:17: warning: operator/ on Duration objects
+  // CHECK-FIXES: if (MACRO_EQ(absl::FDivDuration(d, d), 0.0)) {}
+  if (CHECK(MACRO_EQ(d/d, 0.0))) {}
+  // CHECK-MESSAGES: [[@LINE-1]]:23: warning: operator/ on Duration objects
+  // CHECK-FIXES: if (CHECK(MACRO_EQ(absl::FDivDuration(d, d), 0.0))) {}
+
+  // This one generates a message, but no fix.
+  if (MACRO_DIVEQ(d, d, 0.0)) {}
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: operator/ on Duration objects
+  // CHECK-FIXES: if (MACRO_DIVEQ(d, d, 0.0)) {}
+}
+
+void Negatives() {
+  const int num = d/d;
+
+  // Explicit cast should disable the warning.
+  const double num_d = static_cast(d/d);
+}
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -4,6 +4,7 @@
 =
 
 .. toctree::
+   abseil-duration-division
abseil-string-find-startswith
android-cloexec-accept
android-cloexec-accept4
Index: docs/clang-tidy/checks/abseil-duration-division.rst
===
--- docs/clang-tidy/checks/abseil-duration-division.rst
+++ docs/clang-tidy/checks/abseil-duration-division.rst
@@ -0,0 +1,36 @@
+.. title:: clang-tidy - abseil-duration-division
+  
+abseil-duration-division
+
+
+``absl::Duration`` arithmetic works like it does with integers. That means that
+division of two ``absl::Duration`` objects returns an ``int64`` with any fractional
+component truncated toward 0.
+
+For example:
+
+.. code-block:: c++
+
+ absl::Duration d = absl::Seconds(3.5);
+ int64 sec1 = d / absl::Seconds(1); // Truncates toward 0.
+ int64 sec2 = absl::ToInt64Seconds(d);  // Equivalent to division.
+ assert(sec1 == 3 && sec2 == 3);
+
+ double dsec = d / absl::Seconds(1);  // WRONG: Still truncates toward 0.
+ assert(dsec == 3.0);
+
+If you want floating-point division, you should use either the
+``absl::FDivDuration()`` function, or one of the unit conversion functions such
+as ``absl::ToDoubleSeconds()``. For example:
+
+.. code-block:: c++
+
+ absl::Duration d = absl::Seconds(3.5);
+ double dsec1 = absl::FDivDuration(d, absl::Seconds(1));  // GOOD: No truncation.
+ double dsec2 = absl::ToDoubleSeconds(d); // GOOD: No truncation.
+ assert(dsec1 == 3.5 && dsec2 == 3.5);
+
+
+This check looks for uses of ``absl::Duration`` division that is done in a
+floating-point context, and recommends the use of a function that returns a
+floating-point value.
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -56,6 +56,12 @@
 
 Improvements to clang-tidy
 --
+- New :doc:`abseil-duration-division
+  ` check.
+
+  Checks for uses of ``absl::Duration`` division that is done in a
+  floating-point context, and recommends the use of a function that
+  returns a floating-point value.
 
 The improvements are...
 
Index: clang-tidy/abseil/DurationDivisionCheck.h
===
--- clang-tidy/abseil/DurationDivisionCheck.h
+++ clang-tidy/abseil/DurationDivisionCheck.h
@@ -0,0 +1,31 @@
+//===--- DurationDivisionCheck.h - clang-tidy*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef 

[PATCH] D15225: [Driver] Sanitizer support based on runtime library presence

2018-08-07 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a subscriber: beanz.
rnk added a comment.

In https://reviews.llvm.org/D15225#1191304, @george.karpenkov wrote:

> @rnk As discussed, would it be acceptable for you to just have empty 
> sanitizer runtime files in the resource directory?


I was talking to @beanz, and he suggested adding a cmake flag, something like 
CLANG_UNSUPPORTED_SANITIZERS=asan;ubsan;tsan;msan etc to control this. 
Alternatively, it could be a positive list of supported sanitizers, whatever is 
preferable.

I think my main objection to this is that while it's convenient from a 
packaging perspective, it ascribes too much significance to the existence or 
non-existence of some library files that aren't needed during compilation in 
the first place.

Changing the wording from "not supported" to "not available" doesn't seem that 
helpful. It would still lead someone down the path of needing to read the clang 
source code to understand that some library files are missing, whereas a link 
error would be more obvious.

It's also easy to imagine scenarios where the user has a slightly non-standard 
link setup and the runtime library ultimately doesn't come from the resource 
directory. For example, users checking out compiler-rt and building these 
libraries themselves, perhaps with additional instrumentation.

Overall, I feel like this is too tight coupling.


Repository:
  rL LLVM

https://reviews.llvm.org/D15225



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


[PATCH] D48436: [analyzer][UninitializedObjectChecker] Fixed a false negative by no longer filtering out certain constructor calls

2018-08-07 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.

Ok, let's commit this and see how to fix it later. I still think it's more 
important to come up with clear rules of who is responsible for initializing 
fields than making sure our warnings are properly grouped together.

> ideally we wouldn't like to have a warning for an object (`t`) and a separate 
> warning for it's field (`t.bptr.x`)

I don't quite understand this. How would the first warning look like? What 
would it warn about?




Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:669-671
+  Optional CurrentObject = getObjectVal(Ctor, 
Context);
+  if (!CurrentObject)
+return false;

All uses of `getObjectVal` so far are followed by retrieving the parent region 
from the `LazyCompoundVal`. Why do you need to obtain the `LazyCompoundVal` in 
the first place, i.e. do the second `getSVal` "for '*this'" in `getObjectVal`? 
Why not just operate over the this-region on the current Store? I think there 
isn't even a guarantee that these two regions are the same. Like, in this case 
they probably will be the same, but we shouldn't rely on that.


https://reviews.llvm.org/D48436



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


[PATCH] D43357: [Concepts] Function trailing requires clauses

2018-08-07 Thread Saar Raz via Phabricator via cfe-commits
saar.raz added inline comments.



Comment at: lib/Sema/SemaDecl.cpp:8377-8381
+  } else if (D.hasTrailingRequiresClause()) {
+// C++2a [class.virtual]p6
+// A virtual method shall not have a requires-clause.
+Diag(NewFD->getTrailingRequiresClause()->getLocStart(),
+ diag::err_constrained_virtual_method);

rsmith wrote:
> This is the wrong place for this check. We don't yet know whether the 
> function is virtual here in general. A function can become virtual due to 
> template instantiation:
> 
> ```
> template struct A : T { void f() requires true; };
> struct B { virtual void f(); };
> template struct A; // error, A::f is constrained and virtual
> ```
> 
> This is perhaps a wording defect: it's not clear that `A::f()` really should 
> override `B::f()`, but that is the consequence of the current rules. I've 
> posted a question to the core reflector.
I don't really see why A::f() should override B::f() indeed - since it is not 
marked virtual nor override, shouldn't it just hide B::f()? or am I missing 
something here?


Repository:
  rC Clang

https://reviews.llvm.org/D43357



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


[PATCH] D49199: [analyzer][UninitializedObjectChecker] Pointer/reference objects are dereferenced according to dynamic type

2018-08-07 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ accepted this revision.
NoQ added a comment.

This looks roughly correct, but at the same time none of the tests actually 
exercise the dynamic type propagation. In these tests all the necessary 
information is obtained from the structure of the MemRegion (directly or via 
the initial `StripCasts`), not from the dynamic type map that is an additional 
layer of metadata over the program state. The actual test would assume, as an 
example, chasing undefined values through a symbolic pointer produced by 
`operator new()` - which is a symbolic void pointer, but it points to a 
well-defined type of object. Because we skip symbolic pointers for now, i guess 
you cannot really write such tests. But at the same time chasing through 
//heap// symbolic pointers (i.e., pointers in the heap //memory space//) should 
be safe (so safe that they shouldn't really have been implemented as symbolic 
pointers in the first place).


https://reviews.llvm.org/D49199



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


Re: [PATCH] D50154: [clangd] capitalize diagnostic messages

2018-08-07 Thread Fangrui Song via cfe-commits

It might be useful to know what other editors do here to know whether
this should be a client transformation or baked into the server.

Vim syntastic/ale, Emacs flycheck do not seem to do such transformation. What's
the editor you are using?

On 2018-08-07, Alex L wrote:


On Tue, 7 Aug 2018 at 10:52, David Blaikie via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

   On Tue, Aug 7, 2018 at 10:33 AM Alex Lorenz via Phabricator <
   revi...@reviews.llvm.org> wrote:

   arphaman added a comment.

   In https://reviews.llvm.org/D50154#1191002, @dblaikie wrote:

   > What's the motivation for clangd to differ from clang here? (& if the
   first
   >  letter is going to be capitalized, should there be a period at the
   end? But
   >  also the phrasing of most/all diagnostic text isn't in the form of
   complete
   >  sentences, so this might not make sense)

   It's mostly for the presentation purposes, to match the needs of our
   client. I first implemented it as an opt-in feature, but the consensus
   was to capitalize the messages all the time.

   Doesn't seem like it'd be any more expensive (amount of code or
   performance) to do that up in your client code, then, would it? I guess if
   most users of this API in time ended up preferring capitalized values, it'd
   make sense to share that implementation - but to me it seems like a strange
   transformation to happen at this level. (since it depends on what kind of
   client/how they want to render things - so it seems an odd choice to bake
   in to the API (or even provide an option for, unless there are lots of
   users/the code was especially complicated))

   My 2c - I've no vested interest or authority here.

I think it's more in spirit with Clangd to provide output that's as close to
the one presented by the client as possible. 
I would argue there's already a precedence for this kind of transformations,
for example, Clangd merges the diagnostic messages of notes and the main
diagnostics into one, to make it a better presentation experience in the
client:

https://github.com/llvm-mirror/clang-tools-extra/blob/
55bfabcc1bd75447d6338ffe6ff27c1624a8c15a/clangd/Diagnostics.cpp#L161

 

    

   I don't think it would make sense to insert the period at the end,
   because, as you said, not all diagnostics are complete sentences

   Repository:
     rCTE Clang Tools Extra

   https://reviews.llvm.org/D50154

   ___
   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


Re: r338732 - [analyzer] Make RegionVector use const reference

2018-08-07 Thread Fangrui Song via cfe-commits

On 2018-08-07, David Blaikie wrote:

Looks good! Though it may be useful in the future to describe, in the commit
message, the motivation for a change - how'd you find this? What motivated you
to make this particular fix just now, etc? ("identified using clang-tidy" or
"spotted during post-commit review of change r", etc...)


Thanks for the tip! Will try to do this next time.


On Thu, Aug 2, 2018 at 9:29 AM Fangrui Song via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

   Author: maskray
   Date: Thu Aug  2 09:29:36 2018
   New Revision: 338732

   URL: http://llvm.org/viewvc/llvm-project?rev=338732=rev
   Log:
   [analyzer] Make RegionVector use const reference

   Modified:
       cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp

   Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
   URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/
   BugReporterVisitors.cpp?rev=338732=338731=338732=diff
   ===
   ===
   --- cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp (original)
   +++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp Thu Aug  2
   09:29:36 2018
   @@ -395,7 +395,7 @@ private:
      const Optional
      findRegionOfInterestInRecord(const RecordDecl *RD, ProgramStateRef
   State,
                                   const MemRegion *R,
   -                               RegionVector Vec = {},
   +                               const RegionVector  = {},
                                   int depth = 0) {

        if (depth == DEREFERENCE_LIMIT) // Limit the recursion depth.
   @@ -548,14 +548,10 @@ private:

      /// \return Diagnostics piece for region not modified in the current
   function.
      std::shared_ptr
   -  notModifiedDiagnostics(const LocationContext *Ctx,
   -                         CallExitBegin ,
   -                         CallEventRef<> Call,
   -                         RegionVector FieldChain,
   -                         const MemRegion *MatchedRegion,
   -                         StringRef FirstElement,
   -                         bool FirstIsReferenceType,
   -                         unsigned IndirectionLevel) {
   +  notModifiedDiagnostics(const LocationContext *Ctx, CallExitBegin &
   CallExitLoc,
   +                         CallEventRef<> Call, const RegionVector &
   FieldChain,
   +                         const MemRegion *MatchedRegion, StringRef
   FirstElement,
   +                         bool FirstIsReferenceType, unsigned
   IndirectionLevel) {

        PathDiagnosticLocation L;
        if (const ReturnStmt *RS = CallExitLoc.getReturnStmt()) {
   @@ -579,7 +575,8 @@ private:
      /// Pretty-print region \p MatchedRegion to \p os.
      void prettyPrintRegionName(StringRef FirstElement, bool
   FirstIsReferenceType,
                                 const MemRegion *MatchedRegion,
   -                             RegionVector FieldChain, int
   IndirectionLevel,
   +                             const RegionVector ,
   +                             int IndirectionLevel,
                                 llvm::raw_svector_ostream ) {

        if (FirstIsReferenceType)

   ___
   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] D50380: [Headers] Expand _Unwind_Exception for SEH on MinGW/x86_64

2018-08-07 Thread Martin Storsjö via Phabricator via cfe-commits
mstorsjo added a comment.

@hans Can you merge this for 7.0? This is necessary for 
https://reviews.llvm.org/D49638 (merged well before the branch) to work 
properly without causing heap corruption.


Repository:
  rL LLVM

https://reviews.llvm.org/D50380



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


[PATCH] D50380: [Headers] Expand _Unwind_Exception for SEH on MinGW/x86_64

2018-08-07 Thread Martin Storsjö via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL339170: [Headers] Expand _Unwind_Exception for SEH on 
MinGW/x86_64 (authored by mstorsjo, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D50380?vs=159481=159580#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D50380

Files:
  cfe/trunk/lib/Headers/unwind.h


Index: cfe/trunk/lib/Headers/unwind.h
===
--- cfe/trunk/lib/Headers/unwind.h
+++ cfe/trunk/lib/Headers/unwind.h
@@ -154,8 +154,12 @@
 struct _Unwind_Exception {
   _Unwind_Exception_Class exception_class;
   _Unwind_Exception_Cleanup_Fn exception_cleanup;
+#if !defined (__USING_SJLJ_EXCEPTIONS__) && defined (__SEH__)
+  _Unwind_Word private_[6];
+#else
   _Unwind_Word private_1;
   _Unwind_Word private_2;
+#endif
   /* The Itanium ABI requires that _Unwind_Exception objects are "double-word
* aligned".  GCC has interpreted this to mean "use the maximum useful
* alignment for the target"; so do we. */


Index: cfe/trunk/lib/Headers/unwind.h
===
--- cfe/trunk/lib/Headers/unwind.h
+++ cfe/trunk/lib/Headers/unwind.h
@@ -154,8 +154,12 @@
 struct _Unwind_Exception {
   _Unwind_Exception_Class exception_class;
   _Unwind_Exception_Cleanup_Fn exception_cleanup;
+#if !defined (__USING_SJLJ_EXCEPTIONS__) && defined (__SEH__)
+  _Unwind_Word private_[6];
+#else
   _Unwind_Word private_1;
   _Unwind_Word private_2;
+#endif
   /* The Itanium ABI requires that _Unwind_Exception objects are "double-word
* aligned".  GCC has interpreted this to mean "use the maximum useful
* alignment for the target"; so do we. */
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r339170 - [Headers] Expand _Unwind_Exception for SEH on MinGW/x86_64

2018-08-07 Thread Martin Storsjo via cfe-commits
Author: mstorsjo
Date: Tue Aug  7 13:02:40 2018
New Revision: 339170

URL: http://llvm.org/viewvc/llvm-project?rev=339170=rev
Log:
[Headers] Expand _Unwind_Exception for SEH on MinGW/x86_64

This matches how GCC defines this struct.

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

Modified:
cfe/trunk/lib/Headers/unwind.h

Modified: cfe/trunk/lib/Headers/unwind.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/unwind.h?rev=339170=339169=339170=diff
==
--- cfe/trunk/lib/Headers/unwind.h (original)
+++ cfe/trunk/lib/Headers/unwind.h Tue Aug  7 13:02:40 2018
@@ -154,8 +154,12 @@ struct _Unwind_Control_Block {
 struct _Unwind_Exception {
   _Unwind_Exception_Class exception_class;
   _Unwind_Exception_Cleanup_Fn exception_cleanup;
+#if !defined (__USING_SJLJ_EXCEPTIONS__) && defined (__SEH__)
+  _Unwind_Word private_[6];
+#else
   _Unwind_Word private_1;
   _Unwind_Word private_2;
+#endif
   /* The Itanium ABI requires that _Unwind_Exception objects are "double-word
* aligned".  GCC has interpreted this to mean "use the maximum useful
* alignment for the target"; so do we. */


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


[PATCH] D50278: [Sema] Fix for crash on conditional operation with address_space pointer

2018-08-07 Thread Leonard Chan via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL339167: [Sema] Fix for crash on conditional operation with 
address_space pointer (authored by leonardchan, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D50278?vs=159559=159576#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D50278

Files:
  cfe/trunk/lib/Sema/SemaExpr.cpp
  cfe/trunk/test/Sema/address_spaces.c
  cfe/trunk/test/Sema/conditional-expr.c

Index: cfe/trunk/test/Sema/conditional-expr.c
===
--- cfe/trunk/test/Sema/conditional-expr.c
+++ cfe/trunk/test/Sema/conditional-expr.c
@@ -73,10 +73,10 @@
 
   int __attribute__((address_space(2))) *adr2;
   int __attribute__((address_space(3))) *adr3;
-  test0 ? adr2 : adr3; // expected-warning {{pointer type mismatch}} expected-warning {{expression result unused}}
+  test0 ? adr2 : adr3; // expected-error{{conditional operator with the second and third operands of type  ('__attribute__((address_space(2))) int *' and '__attribute__((address_space(3))) int *') which are pointers to non-overlapping address spaces}}
 
   // Make sure address-space mask ends up in the result type
-  (test0 ? (test0 ? adr2 : adr2) : nonconst_int); // expected-warning {{pointer type mismatch}} expected-warning {{expression result unused}}
+  (test0 ? (test0 ? adr2 : adr2) : nonconst_int); // expected-error{{conditional operator with the second and third operands of type  ('__attribute__((address_space(2))) int *' and 'int *') which are pointers to non-overlapping address spaces}}
 }
 
 int Postgresql() {
Index: cfe/trunk/test/Sema/address_spaces.c
===
--- cfe/trunk/test/Sema/address_spaces.c
+++ cfe/trunk/test/Sema/address_spaces.c
@@ -71,5 +71,5 @@
 
 // Clang extension doesn't forbid operations on pointers to different address spaces.
 char* cmp(_AS1 char *x,  _AS2 char *y) {
-  return x < y ? x : y; // expected-warning {{pointer type mismatch ('__attribute__((address_space(1))) char *' and '__attribute__((address_space(2))) char *')}}
+  return x < y ? x : y; // expected-error{{conditional operator with the second and third operands of type  ('__attribute__((address_space(1))) char *' and '__attribute__((address_space(2))) char *') which are pointers to non-overlapping address spaces}}
 }
Index: cfe/trunk/lib/Sema/SemaExpr.cpp
===
--- cfe/trunk/lib/Sema/SemaExpr.cpp
+++ cfe/trunk/lib/Sema/SemaExpr.cpp
@@ -6460,20 +6460,18 @@
   LangAS ResultAddrSpace = LangAS::Default;
   LangAS LAddrSpace = lhQual.getAddressSpace();
   LangAS RAddrSpace = rhQual.getAddressSpace();
-  if (S.getLangOpts().OpenCL) {
-// OpenCL v1.1 s6.5 - Conversion between pointers to distinct address
-// spaces is disallowed.
-if (lhQual.isAddressSpaceSupersetOf(rhQual))
-  ResultAddrSpace = LAddrSpace;
-else if (rhQual.isAddressSpaceSupersetOf(lhQual))
-  ResultAddrSpace = RAddrSpace;
-else {
-  S.Diag(Loc,
- diag::err_typecheck_op_on_nonoverlapping_address_space_pointers)
-  << LHSTy << RHSTy << 2 << LHS.get()->getSourceRange()
-  << RHS.get()->getSourceRange();
-  return QualType();
-}
+
+  // OpenCL v1.1 s6.5 - Conversion between pointers to distinct address
+  // spaces is disallowed.
+  if (lhQual.isAddressSpaceSupersetOf(rhQual))
+ResultAddrSpace = LAddrSpace;
+  else if (rhQual.isAddressSpaceSupersetOf(lhQual))
+ResultAddrSpace = RAddrSpace;
+  else {
+S.Diag(Loc, diag::err_typecheck_op_on_nonoverlapping_address_space_pointers)
+<< LHSTy << RHSTy << 2 << LHS.get()->getSourceRange()
+<< RHS.get()->getSourceRange();
+return QualType();
   }
 
   unsigned MergedCVRQual = lhQual.getCVRQualifiers() | rhQual.getCVRQualifiers();
@@ -6491,16 +6489,12 @@
   // Thus for conditional operator we merge CVR and address space unqualified
   // pointees and if there is a composite type we return a pointer to it with
   // merged qualifiers.
-  if (S.getLangOpts().OpenCL) {
-LHSCastKind = LAddrSpace == ResultAddrSpace
-  ? CK_BitCast
-  : CK_AddressSpaceConversion;
-RHSCastKind = RAddrSpace == ResultAddrSpace
-  ? CK_BitCast
-  : CK_AddressSpaceConversion;
-lhQual.removeAddressSpace();
-rhQual.removeAddressSpace();
-  }
+  LHSCastKind =
+  LAddrSpace == ResultAddrSpace ? CK_BitCast : CK_AddressSpaceConversion;
+  RHSCastKind =
+  RAddrSpace == ResultAddrSpace ? CK_BitCast : CK_AddressSpaceConversion;
+  lhQual.removeAddressSpace();
+  rhQual.removeAddressSpace();
 
   lhptee = S.Context.getQualifiedType(lhptee.getUnqualifiedType(), lhQual);
   rhptee = S.Context.getQualifiedType(rhptee.getUnqualifiedType(), rhQual);
@@ -6516,6 +6510,7 @@
 

r339167 - [Sema] Fix for crash on conditional operation with address_space pointer

2018-08-07 Thread Leonard Chan via cfe-commits
Author: leonardchan
Date: Tue Aug  7 12:43:53 2018
New Revision: 339167

URL: http://llvm.org/viewvc/llvm-project?rev=339167=rev
Log:
[Sema] Fix for crash on conditional operation with address_space pointer

Compiling the following causes clang to crash

```
char *cmp(__attribute__((address_space(1))) char *x, 
__attribute__((address_space(2))) char *y) {
  return x < y ? x : y;
}
```

with the message: "wrong cast for pointers in different address
spaces(must be an address space cast)!"

This is because during IR emission, the source and dest type for a
bitcast should not have differing address spaces.

This fix prints an error since the code shouldn't compile in the first place.

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

Modified:
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/test/Sema/address_spaces.c
cfe/trunk/test/Sema/conditional-expr.c

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=339167=339166=339167=diff
==
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Tue Aug  7 12:43:53 2018
@@ -6460,20 +6460,18 @@ static QualType checkConditionalPointerC
   LangAS ResultAddrSpace = LangAS::Default;
   LangAS LAddrSpace = lhQual.getAddressSpace();
   LangAS RAddrSpace = rhQual.getAddressSpace();
-  if (S.getLangOpts().OpenCL) {
-// OpenCL v1.1 s6.5 - Conversion between pointers to distinct address
-// spaces is disallowed.
-if (lhQual.isAddressSpaceSupersetOf(rhQual))
-  ResultAddrSpace = LAddrSpace;
-else if (rhQual.isAddressSpaceSupersetOf(lhQual))
-  ResultAddrSpace = RAddrSpace;
-else {
-  S.Diag(Loc,
- diag::err_typecheck_op_on_nonoverlapping_address_space_pointers)
-  << LHSTy << RHSTy << 2 << LHS.get()->getSourceRange()
-  << RHS.get()->getSourceRange();
-  return QualType();
-}
+
+  // OpenCL v1.1 s6.5 - Conversion between pointers to distinct address
+  // spaces is disallowed.
+  if (lhQual.isAddressSpaceSupersetOf(rhQual))
+ResultAddrSpace = LAddrSpace;
+  else if (rhQual.isAddressSpaceSupersetOf(lhQual))
+ResultAddrSpace = RAddrSpace;
+  else {
+S.Diag(Loc, 
diag::err_typecheck_op_on_nonoverlapping_address_space_pointers)
+<< LHSTy << RHSTy << 2 << LHS.get()->getSourceRange()
+<< RHS.get()->getSourceRange();
+return QualType();
   }
 
   unsigned MergedCVRQual = lhQual.getCVRQualifiers() | 
rhQual.getCVRQualifiers();
@@ -6491,16 +6489,12 @@ static QualType checkConditionalPointerC
   // Thus for conditional operator we merge CVR and address space unqualified
   // pointees and if there is a composite type we return a pointer to it with
   // merged qualifiers.
-  if (S.getLangOpts().OpenCL) {
-LHSCastKind = LAddrSpace == ResultAddrSpace
-  ? CK_BitCast
-  : CK_AddressSpaceConversion;
-RHSCastKind = RAddrSpace == ResultAddrSpace
-  ? CK_BitCast
-  : CK_AddressSpaceConversion;
-lhQual.removeAddressSpace();
-rhQual.removeAddressSpace();
-  }
+  LHSCastKind =
+  LAddrSpace == ResultAddrSpace ? CK_BitCast : CK_AddressSpaceConversion;
+  RHSCastKind =
+  RAddrSpace == ResultAddrSpace ? CK_BitCast : CK_AddressSpaceConversion;
+  lhQual.removeAddressSpace();
+  rhQual.removeAddressSpace();
 
   lhptee = S.Context.getQualifiedType(lhptee.getUnqualifiedType(), lhQual);
   rhptee = S.Context.getQualifiedType(rhptee.getUnqualifiedType(), rhQual);
@@ -6516,6 +6510,7 @@ static QualType checkConditionalPointerC
 S.Context.getAddrSpaceQualType(S.Context.VoidTy, ResultAddrSpace));
 LHS = S.ImpCastExprToType(LHS.get(), incompatTy, LHSCastKind);
 RHS = S.ImpCastExprToType(RHS.get(), incompatTy, RHSCastKind);
+
 // FIXME: For OpenCL the warning emission and cast to void* leaves a room
 // for casts between types with incompatible address space qualifiers.
 // For the following code the compiler produces casts between global and
@@ -6526,6 +6521,7 @@ static QualType checkConditionalPointerC
 S.Diag(Loc, diag::ext_typecheck_cond_incompatible_pointers)
 << LHSTy << RHSTy << LHS.get()->getSourceRange()
 << RHS.get()->getSourceRange();
+
 return incompatTy;
   }
 

Modified: cfe/trunk/test/Sema/address_spaces.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/address_spaces.c?rev=339167=339166=339167=diff
==
--- cfe/trunk/test/Sema/address_spaces.c (original)
+++ cfe/trunk/test/Sema/address_spaces.c Tue Aug  7 12:43:53 2018
@@ -71,5 +71,5 @@ __attribute__((address_space("12"))) int
 
 // Clang extension doesn't forbid operations on pointers to different address 
spaces.
 char* cmp(_AS1 char *x,  _AS2 char *y) {
-  return x < y ? x : y; // expected-warning {{pointer type mismatch 

[PATCH] D50278: [Sema] Fix for crash on conditional operation with address_space pointer

2018-08-07 Thread Leonard Chan via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC339167: [Sema] Fix for crash on conditional operation with 
address_space pointer (authored by leonardchan, committed by ).

Repository:
  rL LLVM

https://reviews.llvm.org/D50278

Files:
  lib/Sema/SemaExpr.cpp
  test/Sema/address_spaces.c
  test/Sema/conditional-expr.c

Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -6460,20 +6460,18 @@
   LangAS ResultAddrSpace = LangAS::Default;
   LangAS LAddrSpace = lhQual.getAddressSpace();
   LangAS RAddrSpace = rhQual.getAddressSpace();
-  if (S.getLangOpts().OpenCL) {
-// OpenCL v1.1 s6.5 - Conversion between pointers to distinct address
-// spaces is disallowed.
-if (lhQual.isAddressSpaceSupersetOf(rhQual))
-  ResultAddrSpace = LAddrSpace;
-else if (rhQual.isAddressSpaceSupersetOf(lhQual))
-  ResultAddrSpace = RAddrSpace;
-else {
-  S.Diag(Loc,
- diag::err_typecheck_op_on_nonoverlapping_address_space_pointers)
-  << LHSTy << RHSTy << 2 << LHS.get()->getSourceRange()
-  << RHS.get()->getSourceRange();
-  return QualType();
-}
+
+  // OpenCL v1.1 s6.5 - Conversion between pointers to distinct address
+  // spaces is disallowed.
+  if (lhQual.isAddressSpaceSupersetOf(rhQual))
+ResultAddrSpace = LAddrSpace;
+  else if (rhQual.isAddressSpaceSupersetOf(lhQual))
+ResultAddrSpace = RAddrSpace;
+  else {
+S.Diag(Loc, diag::err_typecheck_op_on_nonoverlapping_address_space_pointers)
+<< LHSTy << RHSTy << 2 << LHS.get()->getSourceRange()
+<< RHS.get()->getSourceRange();
+return QualType();
   }
 
   unsigned MergedCVRQual = lhQual.getCVRQualifiers() | rhQual.getCVRQualifiers();
@@ -6491,16 +6489,12 @@
   // Thus for conditional operator we merge CVR and address space unqualified
   // pointees and if there is a composite type we return a pointer to it with
   // merged qualifiers.
-  if (S.getLangOpts().OpenCL) {
-LHSCastKind = LAddrSpace == ResultAddrSpace
-  ? CK_BitCast
-  : CK_AddressSpaceConversion;
-RHSCastKind = RAddrSpace == ResultAddrSpace
-  ? CK_BitCast
-  : CK_AddressSpaceConversion;
-lhQual.removeAddressSpace();
-rhQual.removeAddressSpace();
-  }
+  LHSCastKind =
+  LAddrSpace == ResultAddrSpace ? CK_BitCast : CK_AddressSpaceConversion;
+  RHSCastKind =
+  RAddrSpace == ResultAddrSpace ? CK_BitCast : CK_AddressSpaceConversion;
+  lhQual.removeAddressSpace();
+  rhQual.removeAddressSpace();
 
   lhptee = S.Context.getQualifiedType(lhptee.getUnqualifiedType(), lhQual);
   rhptee = S.Context.getQualifiedType(rhptee.getUnqualifiedType(), rhQual);
@@ -6516,6 +6510,7 @@
 S.Context.getAddrSpaceQualType(S.Context.VoidTy, ResultAddrSpace));
 LHS = S.ImpCastExprToType(LHS.get(), incompatTy, LHSCastKind);
 RHS = S.ImpCastExprToType(RHS.get(), incompatTy, RHSCastKind);
+
 // FIXME: For OpenCL the warning emission and cast to void* leaves a room
 // for casts between types with incompatible address space qualifiers.
 // For the following code the compiler produces casts between global and
@@ -6526,6 +6521,7 @@
 S.Diag(Loc, diag::ext_typecheck_cond_incompatible_pointers)
 << LHSTy << RHSTy << LHS.get()->getSourceRange()
 << RHS.get()->getSourceRange();
+
 return incompatTy;
   }
 
Index: test/Sema/address_spaces.c
===
--- test/Sema/address_spaces.c
+++ test/Sema/address_spaces.c
@@ -71,5 +71,5 @@
 
 // Clang extension doesn't forbid operations on pointers to different address spaces.
 char* cmp(_AS1 char *x,  _AS2 char *y) {
-  return x < y ? x : y; // expected-warning {{pointer type mismatch ('__attribute__((address_space(1))) char *' and '__attribute__((address_space(2))) char *')}}
+  return x < y ? x : y; // expected-error{{conditional operator with the second and third operands of type  ('__attribute__((address_space(1))) char *' and '__attribute__((address_space(2))) char *') which are pointers to non-overlapping address spaces}}
 }
Index: test/Sema/conditional-expr.c
===
--- test/Sema/conditional-expr.c
+++ test/Sema/conditional-expr.c
@@ -73,10 +73,10 @@
 
   int __attribute__((address_space(2))) *adr2;
   int __attribute__((address_space(3))) *adr3;
-  test0 ? adr2 : adr3; // expected-warning {{pointer type mismatch}} expected-warning {{expression result unused}}
+  test0 ? adr2 : adr3; // expected-error{{conditional operator with the second and third operands of type  ('__attribute__((address_space(2))) int *' and '__attribute__((address_space(3))) int *') which are pointers to non-overlapping address spaces}}
 
   // Make sure address-space mask ends up in the result type
-  (test0 ? (test0 ? 

[PATCH] D47849: [OpenMP][Clang][NVPTX] Enable math functions called in an OpenMP NVPTX target device region to be resolved as device-native function calls

2018-08-07 Thread Gheorghe-Teodor Bercea via Phabricator via cfe-commits
gtbercea updated this revision to Diff 159574.
gtbercea added a comment.

Prevent math builtins from being used for nvptx toolchain.


Repository:
  rC Clang

https://reviews.llvm.org/D47849

Files:
  include/clang/Driver/ToolChain.h
  lib/Driver/ToolChains/Clang.cpp
  lib/Driver/ToolChains/Cuda.cpp
  lib/Driver/ToolChains/Cuda.h
  lib/Headers/CMakeLists.txt
  lib/Headers/__clang_cuda_device_functions.h
  lib/Headers/__clang_cuda_libdevice_declares.h
  test/CodeGen/nvptx_device_math_functions.c
  test/Driver/openmp-offload-gpu.c

Index: test/Driver/openmp-offload-gpu.c
===
--- test/Driver/openmp-offload-gpu.c
+++ test/Driver/openmp-offload-gpu.c
@@ -76,9 +76,9 @@
 // RUN:  -no-canonical-prefixes -save-temps %t.o -fopenmp-use-target-bundling 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHK-CUBIN-UNBUNDLING-NVLINK %s
 
-/// Use DAG to ensure that cubin file has been unbundled.
+/// Use DAG to ensure that object file has not been unbundled.
 // CHK-CUBIN-UNBUNDLING-NVLINK-DAG: nvlink{{.*}}" {{.*}}"[[CUBIN:.*\.cubin]]"
-// CHK-CUBIN-UNBUNDLING-NVLINK-DAG: clang-offload-bundler{{.*}}" "-type=o" {{.*}}"-outputs={{.*}}[[CUBIN]]
+// CHK-CUBIN-UNBUNDLING-NVLINK-DAG: clang-offload-bundler{{.*}}" "-type=o" {{.*}}[[CUBIN]]
 // CHK-CUBIN-UNBUNDLING-NVLINK-DAG-SAME: "-unbundle"
 
 /// ###
Index: test/CodeGen/nvptx_device_math_functions.c
===
--- /dev/null
+++ test/CodeGen/nvptx_device_math_functions.c
@@ -0,0 +1,20 @@
+// Test calling of device math functions.
+///==///
+
+// RUN: %clang -fmath-errno -S -emit-llvm -o - %s -fopenmp -fopenmp-targets=nvptx64-nvidia-cuda | FileCheck -check-prefix CHECK-YES %s
+
+void test_sqrt(double a1) {
+  #pragma omp target
+  {
+// CHECK-YES: call double @llvm.nvvm.sqrt.rn.d(double
+double l1 = sqrt(a1);
+  }
+}
+
+void test_pow(float a0, double a1, long double a2) {
+  #pragma omp target
+  {
+// CHECK-YES: call double @__internal_accurate_pow(double
+double l1 = pow(a1, a1);
+  }
+}
Index: lib/Headers/__clang_cuda_libdevice_declares.h
===
--- lib/Headers/__clang_cuda_libdevice_declares.h
+++ lib/Headers/__clang_cuda_libdevice_declares.h
@@ -24,443 +24,455 @@
 #ifndef __CLANG_CUDA_LIBDEVICE_DECLARES_H__
 #define __CLANG_CUDA_LIBDEVICE_DECLARES_H__
 
+#if defined(_OPENMP)
+#define __DEVICE__
+#elif defined(__CUDA__)
+#define __DEVICE__ __device__
+#endif
+
+#if defined(__cplusplus)
 extern "C" {
+#endif
 
-__device__ int __nv_abs(int __a);
-__device__ double __nv_acos(double __a);
-__device__ float __nv_acosf(float __a);
-__device__ double __nv_acosh(double __a);
-__device__ float __nv_acoshf(float __a);
-__device__ double __nv_asin(double __a);
-__device__ float __nv_asinf(float __a);
-__device__ double __nv_asinh(double __a);
-__device__ float __nv_asinhf(float __a);
-__device__ double __nv_atan2(double __a, double __b);
-__device__ float __nv_atan2f(float __a, float __b);
-__device__ double __nv_atan(double __a);
-__device__ float __nv_atanf(float __a);
-__device__ double __nv_atanh(double __a);
-__device__ float __nv_atanhf(float __a);
-__device__ int __nv_brev(int __a);
-__device__ long long __nv_brevll(long long __a);
-__device__ int __nv_byte_perm(int __a, int __b, int __c);
-__device__ double __nv_cbrt(double __a);
-__device__ float __nv_cbrtf(float __a);
-__device__ double __nv_ceil(double __a);
-__device__ float __nv_ceilf(float __a);
-__device__ int __nv_clz(int __a);
-__device__ int __nv_clzll(long long __a);
-__device__ double __nv_copysign(double __a, double __b);
-__device__ float __nv_copysignf(float __a, float __b);
-__device__ double __nv_cos(double __a);
-__device__ float __nv_cosf(float __a);
-__device__ double __nv_cosh(double __a);
-__device__ float __nv_coshf(float __a);
-__device__ double __nv_cospi(double __a);
-__device__ float __nv_cospif(float __a);
-__device__ double __nv_cyl_bessel_i0(double __a);
-__device__ float __nv_cyl_bessel_i0f(float __a);
-__device__ double __nv_cyl_bessel_i1(double __a);
-__device__ float __nv_cyl_bessel_i1f(float __a);
-__device__ double __nv_dadd_rd(double __a, double __b);
-__device__ double __nv_dadd_rn(double __a, double __b);
-__device__ double __nv_dadd_ru(double __a, double __b);
-__device__ double __nv_dadd_rz(double __a, double __b);
-__device__ double __nv_ddiv_rd(double __a, double __b);
-__device__ double __nv_ddiv_rn(double __a, double __b);
-__device__ double __nv_ddiv_ru(double __a, double __b);
-__device__ double __nv_ddiv_rz(double __a, double __b);
-__device__ double __nv_dmul_rd(double __a, double __b);
-__device__ double __nv_dmul_rn(double __a, double __b);
-__device__ double __nv_dmul_ru(double __a, double __b);
-__device__ double __nv_dmul_rz(double __a, 

[PATCH] D41217: [Concepts] Concept Specialization Expressions

2018-08-07 Thread Saar Raz via Phabricator via cfe-commits
saar.raz added inline comments.



Comment at: lib/Sema/SemaConcept.cpp:34
+  Diag(ConstraintExpression->getExprLoc(),
+   diag::err_non_bool_atomic_constraint)
+  << ConstraintExpression << ConstraintExpression->getType();

saar.raz wrote:
> rsmith wrote:
> > What justifies rejecting this prior to any use of the concept that would 
> > result in a satisfaction check?
> > 
> > (I think checking this is a good thing; what I'm wondering is whether we 
> > need to file a core issue to get the wording updated to allow us to reject 
> > such bogus concepts even if they're never used.)
> I guess this is already justified, if awkwardly (NDR) by [temp.res]p8.2
Correction - it says that IFNDR occurs when no substitution would result in a 
valid expression, so maybe this is well formed after all.
In this case it is a valid expression but not a valid constraint expression, 
maybe that's the missing word here?


Repository:
  rC Clang

https://reviews.llvm.org/D41217



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


  1   2   3   >