[PATCH] D47111: : Implement monotonic_buffer_resource.

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone updated this revision to Diff 148845.
Quuxplusone added a comment.

Refactor to remove unused fields from the header structs.

Before this change, `sizeof(monotonic_buffer_resource) == 56` and the overhead 
per allocated chunk was `40` bytes.
After this change, `sizeof(monotonic_buffer_resource) == 48` and the overhead 
per allocated chunk is `24` bytes.

Eric suggests replacing `size_t __align_` with `uint8_t __log2_align_`. I'm 
amenable to this idea, but I'd want to know what's the best way to compute the 
log2 of a user-supplied number.


Repository:
  rCXX libc++

https://reviews.llvm.org/D47111

Files:
  include/experimental/memory_resource
  src/experimental/memory_resource.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.ctor/copy_move.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.ctor/with_default_resource.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.ctor/without_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_deallocate.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_exception_safety.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_from_initial_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_from_underaligned_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_from_zero_sized_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_in_geometric_progression.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_overaligned_request.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/equality.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/nothing_to_do.pass.cpp

Index: test/std/experimental/memory/memory.resource.monotonic.buffer/nothing_to_do.pass.cpp
===
--- /dev/null
+++ test/std/experimental/memory/memory.resource.monotonic.buffer/nothing_to_do.pass.cpp
@@ -0,0 +1,13 @@
+// -*- C++ -*-
+//===--===//
+//
+// 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.
+//
+//===--===//
+
+int main()
+{
+}
Index: test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/equality.pass.cpp
===
--- /dev/null
+++ test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/equality.pass.cpp
@@ -0,0 +1,61 @@
+//===--===//
+//
+// 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.
+//
+//===--===//
+
+// UNSUPPORTED: c++98, c++03
+
+// 
+
+// class monotonic_buffer_resource
+
+#include 
+#include 
+#include 
+#include 
+
+struct assert_on_compare : public std::experimental::pmr::memory_resource
+{
+protected:
+virtual void * do_allocate(size_t, size_t)
+{ assert(false); }
+
+virtual void do_deallocate(void *, size_t, size_t)
+{ assert(false); }
+
+virtual bool do_is_equal(std::experimental::pmr::memory_resource const &) const noexcept
+{ assert(false); }
+};
+
+int main()
+{
+// Same object
+{
+std::experimental::pmr::monotonic_buffer_resource r1;
+std::experimental::pmr::monotonic_buffer_resource r2;
+assert(r1 == r1);
+assert(r1 != r2);
+
+std::experimental::pmr::memory_resource & p1 = r1;
+std::experimental::pmr::memory_resource & p2 = r2;
+assert(p1 == p1);
+assert(p1 != p2);
+assert(p1 == r1);
+assert(r1 == p1);
+assert(p1 != r2);
+assert(r2 != p1);
+}
+// Different types
+{
+std::experimental::pmr::monotonic_buffer_resource mono1;
+std::experimental::pmr::memory_resource & r1 = mono1;
+assert_on_compare c;
+std::experimental::pmr::memory_resource & r2 = c;
+assert(r1 != r2);
+assert(!(r1 == r2));
+}
+}
Index: test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_overaligned_request.pass.cpp
===
--- /dev/null
+++ 

[PATCH] D45045: [DebugInfo] Generate debug information for labels.

[Hsiangkai Wang via Phabricator via cfe-commits]

HsiangKai added a comment.

In https://reviews.llvm.org/D45045#1092697, @hans wrote:

> This broke the Chromium build. I've uploaded a reproducer at 
> https://bugs.chromium.org/p/chromium/issues/detail?id=841170#c1
>
> I'm guessing maybe a Clang bootstrap with debug info might also reproduce the 
> problem, but I haven't tried that.
>
> Reverted in r331861.


https://reviews.llvm.org/D46738 should fix the bug.


Repository:
  rL LLVM

https://reviews.llvm.org/D45045



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

[PATCH] D47111: : Implement monotonic_buffer_resource.

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone updated this revision to Diff 148843.
Quuxplusone added a comment.

Add `override`; disintegrate the unit test; adopt some tests from Eric's 
https://reviews.llvm.org/D27402.
Also fix one QOI issue discovered by Eric's tests: if the user passes an 
`initial_size` to the constructor, then they are //probably// intending that 
our first upstream-allocation be big enough to serve at least `initial_size` 
1-byte allocations (or one `initial_size`-byte allocation with a suitably small 
alignment). This is explicitly //not// mandated by the Standard (it merely 
requires that our next upstream-allocation be of size at least `initial_size`), 
but it's probably a healthy choice.


Repository:
  rCXX libc++

https://reviews.llvm.org/D47111

Files:
  include/experimental/memory_resource
  src/experimental/memory_resource.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.ctor/copy_move.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.ctor/with_default_resource.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.ctor/without_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_deallocate.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_exception_safety.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_from_initial_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_from_underaligned_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_from_zero_sized_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_in_geometric_progression.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_overaligned_request.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/equality.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/nothing_to_do.pass.cpp

Index: test/std/experimental/memory/memory.resource.monotonic.buffer/nothing_to_do.pass.cpp
===
--- /dev/null
+++ test/std/experimental/memory/memory.resource.monotonic.buffer/nothing_to_do.pass.cpp
@@ -0,0 +1,13 @@
+// -*- C++ -*-
+//===--===//
+//
+// 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.
+//
+//===--===//
+
+int main()
+{
+}
Index: test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/equality.pass.cpp
===
--- /dev/null
+++ test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/equality.pass.cpp
@@ -0,0 +1,61 @@
+//===--===//
+//
+// 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.
+//
+//===--===//
+
+// UNSUPPORTED: c++98, c++03
+
+// 
+
+// class monotonic_buffer_resource
+
+#include 
+#include 
+#include 
+#include 
+
+struct assert_on_compare : public std::experimental::pmr::memory_resource
+{
+protected:
+virtual void * do_allocate(size_t, size_t)
+{ assert(false); }
+
+virtual void do_deallocate(void *, size_t, size_t)
+{ assert(false); }
+
+virtual bool do_is_equal(std::experimental::pmr::memory_resource const &) const noexcept
+{ assert(false); }
+};
+
+int main()
+{
+// Same object
+{
+std::experimental::pmr::monotonic_buffer_resource r1;
+std::experimental::pmr::monotonic_buffer_resource r2;
+assert(r1 == r1);
+assert(r1 != r2);
+
+std::experimental::pmr::memory_resource & p1 = r1;
+std::experimental::pmr::memory_resource & p2 = r2;
+assert(p1 == p1);
+assert(p1 != p2);
+assert(p1 == r1);
+assert(r1 == p1);
+assert(p1 != r2);
+assert(r2 != p1);
+}
+// Different types
+{
+std::experimental::pmr::monotonic_buffer_resource mono1;
+std::experimental::pmr::memory_resource & r1 = mono1;
+assert_on_compare c;
+std::experimental::pmr::memory_resource & r2 = c;
+assert(r1 != r2);
+assert(!(r1 == r2));
+}
+}
Index: 

r333387 - [X86] Merge the 3 different flavors of masked vpermi2var/vpermt2var builtins to a single version without masking. Use select builtins with appropriate operand instead.

[Craig Topper via cfe-commits]

Author: ctopper
Date: Mon May 28 20:26:38 2018
New Revision: 87

URL: http://llvm.org/viewvc/llvm-project?rev=87=rev
Log:
[X86] Merge the 3 different flavors of masked vpermi2var/vpermt2var builtins to 
a single version without masking. Use select builtins with appropriate operand 
instead.

Modified:
cfe/trunk/include/clang/Basic/BuiltinsX86.def
cfe/trunk/lib/Headers/avx512bwintrin.h
cfe/trunk/lib/Headers/avx512fintrin.h
cfe/trunk/lib/Headers/avx512vbmiintrin.h
cfe/trunk/lib/Headers/avx512vbmivlintrin.h
cfe/trunk/lib/Headers/avx512vlbwintrin.h
cfe/trunk/lib/Headers/avx512vlintrin.h
cfe/trunk/test/CodeGen/avx512bw-builtins.c
cfe/trunk/test/CodeGen/avx512f-builtins.c
cfe/trunk/test/CodeGen/avx512vbmi-builtins.c
cfe/trunk/test/CodeGen/avx512vbmivl-builtin.c
cfe/trunk/test/CodeGen/avx512vl-builtins.c
cfe/trunk/test/CodeGen/avx512vlbw-builtins.c

Modified: cfe/trunk/include/clang/Basic/BuiltinsX86.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsX86.def?rev=87=86=87=diff
==
--- cfe/trunk/include/clang/Basic/BuiltinsX86.def (original)
+++ cfe/trunk/include/clang/Basic/BuiltinsX86.def Mon May 28 20:26:38 2018
@@ -969,10 +969,6 @@ TARGET_BUILTIN(__builtin_ia32_storeupd51
 TARGET_BUILTIN(__builtin_ia32_storeapd512_mask, "vV8d*V8dUc", "n", "avx512f")
 TARGET_BUILTIN(__builtin_ia32_storeups512_mask, "vf*V16fUs", "n", "avx512f")
 TARGET_BUILTIN(__builtin_ia32_storeaps512_mask, "vV16f*V16fUs", "n", "avx512f")
-TARGET_BUILTIN(__builtin_ia32_vpermt2vard512_mask, "V16iV16iV16iV16iUs", "nc", 
"avx512f")
-TARGET_BUILTIN(__builtin_ia32_vpermt2varq512_mask, "V8LLiV8LLiV8LLiV8LLiUc", 
"nc", "avx512f")
-TARGET_BUILTIN(__builtin_ia32_vpermt2varps512_mask, "V16fV16iV16fV16fUs", 
"nc", "avx512f")
-TARGET_BUILTIN(__builtin_ia32_vpermt2varpd512_mask, "V8dV8LLiV8dV8dUc", "nc", 
"avx512f")
 
 TARGET_BUILTIN(__builtin_ia32_vpdpbusd128_mask, "V4iV4iV4iV4iUc", "nc", 
"avx512vl,avx512vnni")
 TARGET_BUILTIN(__builtin_ia32_vpdpbusd256_mask, "V8iV8iV8iV8iUc", "nc", 
"avx512vl,avx512vnni")
@@ -1092,10 +1088,6 @@ TARGET_BUILTIN(__builtin_ia32_psubsw512_
 TARGET_BUILTIN(__builtin_ia32_psubusb512_mask, "V64cV64cV64cV64cULLi", "nc", 
"avx512bw")
 TARGET_BUILTIN(__builtin_ia32_psubusw512_mask, "V32sV32sV32sV32sUi", "nc", 
"avx512bw")
 
-TARGET_BUILTIN(__builtin_ia32_vpermi2varhi512_mask, "V32sV32sV32sV32sUi", 
"nc", "avx512bw")
-TARGET_BUILTIN(__builtin_ia32_vpermt2varhi512_mask, "V32sV32sV32sV32sUi", 
"nc", "avx512bw")
-TARGET_BUILTIN(__builtin_ia32_vpermt2varhi512_maskz, "V32sV32sV32sV32sUi", 
"nc", "avx512bw")
-
 TARGET_BUILTIN(__builtin_ia32_vpconflictdi_128_mask, "V2LLiV2LLiV2LLiUc", 
"nc", "avx512cd,avx512vl")
 TARGET_BUILTIN(__builtin_ia32_vpconflictdi_256_mask, "V4LLiV4LLiV4LLiUc", 
"nc", "avx512cd,avx512vl")
 TARGET_BUILTIN(__builtin_ia32_vpconflictsi_128_mask, "V4iV4iV4iUc", "nc", 
"avx512cd,avx512vl")
@@ -1123,13 +1115,6 @@ TARGET_BUILTIN(__builtin_ia32_vpshufbitq
 TARGET_BUILTIN(__builtin_ia32_vpshufbitqmb256_mask, "UiV32cV32cUi", "nc", 
"avx512vl,avx512bitalg")
 TARGET_BUILTIN(__builtin_ia32_vpshufbitqmb512_mask, "ULLiV64cV64cULLi", "nc", 
"avx512bitalg")
 
-TARGET_BUILTIN(__builtin_ia32_vpermi2varhi128_mask, "V8sV8sV8sV8sUc", "nc", 
"avx512vl,avx512bw")
-TARGET_BUILTIN(__builtin_ia32_vpermi2varhi256_mask, "V16sV16sV16sV16sUs", 
"nc", "avx512vl,avx512bw")
-TARGET_BUILTIN(__builtin_ia32_vpermt2varhi128_mask, "V8sV8sV8sV8sUc", "nc", 
"avx512vl,avx512bw")
-TARGET_BUILTIN(__builtin_ia32_vpermt2varhi128_maskz, "V8sV8sV8sV8sUc", "nc", 
"avx512vl,avx512bw")
-TARGET_BUILTIN(__builtin_ia32_vpermt2varhi256_mask, "V16sV16sV16sV16sUs", 
"nc", "avx512vl,avx512bw")
-TARGET_BUILTIN(__builtin_ia32_vpermt2varhi256_maskz, "V16sV16sV16sV16sUs", 
"nc", "avx512vl,avx512bw")
-
 TARGET_BUILTIN(__builtin_ia32_pmulhrsw512, "V32sV32sV32s", "nc", "avx512bw")
 TARGET_BUILTIN(__builtin_ia32_pmulhuw512, "V32sV32sV32s", "nc", "avx512bw")
 TARGET_BUILTIN(__builtin_ia32_pmulhw512, "V32sV32sV32s", "nc", "avx512bw")
@@ -1266,30 +1251,24 @@ TARGET_BUILTIN(__builtin_ia32_scattersiv
 TARGET_BUILTIN(__builtin_ia32_scattersiv8sf, "vf*UcV8iV8fIi", "n", "avx512vl")
 TARGET_BUILTIN(__builtin_ia32_scattersiv8si, "vi*UcV8iV8iIi", "n", "avx512vl")
 
-TARGET_BUILTIN(__builtin_ia32_vpermi2vard128_mask, "V4iV4iV4iV4iUc", "nc", 
"avx512vl")
-TARGET_BUILTIN(__builtin_ia32_vpermi2vard256_mask, "V8iV8iV8iV8iUc", "nc", 
"avx512vl")
-TARGET_BUILTIN(__builtin_ia32_vpermi2varpd128_mask, "V2dV2dV2LLiV2dUc", "nc", 
"avx512vl")
-TARGET_BUILTIN(__builtin_ia32_vpermi2varpd256_mask, "V4dV4dV4LLiV4dUc", "nc", 
"avx512vl")
-TARGET_BUILTIN(__builtin_ia32_vpermi2varps128_mask, "V4fV4fV4iV4fUc", "nc", 
"avx512vl")
-TARGET_BUILTIN(__builtin_ia32_vpermi2varps256_mask, "V8fV8fV8iV8fUc", "nc", 
"avx512vl")
-TARGET_BUILTIN(__builtin_ia32_vpermi2varq128_mask, "V2LLiV2LLiV2LLiV2LLiUc", 
"nc", "avx512vl")

[PATCH] D47111: : Implement monotonic_buffer_resource.

[Eric Fiselier via Phabricator via cfe-commits]

EricWF added inline comments.



Comment at: src/experimental/memory_resource.cpp:237
+
+void *result = __res_->allocate(aligned_capacity, align);
+__monotonic_buffer_header *header = (__monotonic_buffer_header *)((char 
*)result + aligned_capacity - header_size);

Quuxplusone wrote:
> For reference, here is where we ask the upstream for a block aligned 
> according to the user's `align`.
> It occurs to me that the upstream might not be able to satisfy such a request 
> (actually, `new_delete_resource` works that way for me because libc++ doesn't 
> support aligned new and delete on OSX), which would make the upstream throw 
> `bad_alloc`. We handle this by passing along the exception. We //could// 
> conceivably handle it by retrying with
> ```
> aligned_capacity += align;
> __res_->allocate(aligned_capacity, alignof(max_align_t));
> header->__alignment_ = alignof(max_align_t);
> ```
> but I'm not sure that that's any of `monotonic_buffer_resource`'s business. 
> Happy to make the patch if you think it ought to be.
I was initially thinking of storing `lg(align)`.


Repository:
  rCXX libc++

https://reviews.llvm.org/D47111



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

[PATCH] D47111: : Implement monotonic_buffer_resource.

[Eric Fiselier via Phabricator via cfe-commits]

EricWF added inline comments.



Comment at: include/experimental/memory_resource:428
+__monotonic_buffer_header *__next_;
+size_t __capacity_;
+size_t __alignment_;

Can't we determine the capacity in most cases simply using 
`static_cast(this) - static_cast(__start_)`?



Comment at: include/experimental/memory_resource:429
+size_t __capacity_;
+size_t __alignment_;
+size_t __used_;

Quuxplusone wrote:
> EricWF wrote:
> > I can't imagine we'll need more than 1 byte to represent the alignment.
> Even assuming for the sake of argument that that's right, it wouldn't buy us 
> anything here because of padding, would it?
> 
> At the moment, `__alignment_` needs to have enough range to store the `align` 
> parameter passed to `monotonic_buffer_resource::do_allocate`. In an SSE world 
> we should not blithely assume that `align < 256`. We //could// store 
> `lg(align)` in a single byte since `align` is always a power of two and less 
> than 2^64 — but then we're back to the first argument, which is that it'll be 
> padded to 8 bytes anyway so what do we gain.
> Even assuming for the sake of argument that that's right, it wouldn't buy us 
> anything here because of padding, would it?

It could. (A) we don't actually need to include the types trailing padding when 
tail allocating it as a part of a buffer.
and less importantly (B) I'm not sure all ABI's require the trailing padding of 
a type be included when that type is a data member of another type (I might 
just be wrong on this).

I'm also looking into other ways of improving the way your implementation packs 
data, which may cause this to be beneficial. 



Comment at: include/experimental/memory_resource:474
+protected:
+void* do_allocate(size_t __bytes, size_t __alignment);
+

Quuxplusone wrote:
> EricWF wrote:
> > Lets add `override` to these functions.
> I grepped for "override" in `include/` and saw exactly one (accidental?) use 
> in `experimental/filesystem`, so I thought probably libc++ had a policy not 
> to use it for portability reasons. I'll add it throughout, but would like 
> someone to explicitly confirm that
> 
> (A) it's okay to use in this header and wouldn't need to be guarded with a 
> `_LIBCPP_OVERRIDE` macro or anything
> 
> (B) Arthur should //not// bother trying to add it to any //other// headers, 
> not even "for consistency"
The header already assumes a full C++11 implementation (it uses `std::tuple`), 
the `override` keyword will bi available. No need for a special macro.




Comment at: 
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic_buffer.pass.cpp:1
+//===--===//
+//

Instead of testing the whole of `monotonic_buffer` in a single file, this test 
should be broken up into separate files. Roughly one per function, or when it 
makes sense, one per subsection for the lowest level of heading (For example a 
single test for all constructors under `memory.buffer.ctor` ).


Repository:
  rCXX libc++

https://reviews.llvm.org/D47111



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

[PATCH] D47111: : Implement monotonic_buffer_resource.

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added inline comments.



Comment at: include/experimental/memory_resource:429
+size_t __capacity_;
+size_t __alignment_;
+size_t __used_;

EricWF wrote:
> I can't imagine we'll need more than 1 byte to represent the alignment.
Even assuming for the sake of argument that that's right, it wouldn't buy us 
anything here because of padding, would it?

At the moment, `__alignment_` needs to have enough range to store the `align` 
parameter passed to `monotonic_buffer_resource::do_allocate`. In an SSE world 
we should not blithely assume that `align < 256`. We //could// store 
`lg(align)` in a single byte since `align` is always a power of two and less 
than 2^64 — but then we're back to the first argument, which is that it'll be 
padded to 8 bytes anyway so what do we gain.



Comment at: include/experimental/memory_resource:474
+protected:
+void* do_allocate(size_t __bytes, size_t __alignment);
+

EricWF wrote:
> Lets add `override` to these functions.
I grepped for "override" in `include/` and saw exactly one (accidental?) use in 
`experimental/filesystem`, so I thought probably libc++ had a policy not to use 
it for portability reasons. I'll add it throughout, but would like someone to 
explicitly confirm that

(A) it's okay to use in this header and wouldn't need to be guarded with a 
`_LIBCPP_OVERRIDE` macro or anything

(B) Arthur should //not// bother trying to add it to any //other// headers, not 
even "for consistency"



Comment at: src/experimental/memory_resource.cpp:217
+{
+if (void *result = try_allocate_from_chunk(&__original_, bytes, align)) {
+return result;

EricWF wrote:
> Drop the braces for conditionals and loops with single statements.
*shiver* but okay.



Comment at: src/experimental/memory_resource.cpp:237
+
+void *result = __res_->allocate(aligned_capacity, align);
+__monotonic_buffer_header *header = (__monotonic_buffer_header *)((char 
*)result + aligned_capacity - header_size);

For reference, here is where we ask the upstream for a block aligned according 
to the user's `align`.
It occurs to me that the upstream might not be able to satisfy such a request 
(actually, `new_delete_resource` works that way for me because libc++ doesn't 
support aligned new and delete on OSX), which would make the upstream throw 
`bad_alloc`. We handle this by passing along the exception. We //could// 
conceivably handle it by retrying with
```
aligned_capacity += align;
__res_->allocate(aligned_capacity, alignof(max_align_t));
header->__alignment_ = alignof(max_align_t);
```
but I'm not sure that that's any of `monotonic_buffer_resource`'s business. 
Happy to make the patch if you think it ought to be.


Repository:
  rCXX libc++

https://reviews.llvm.org/D47111



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

[PATCH] D47341: [Sema] Disable creating new delayed typos while correcting existing.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

Some debugging details that can be useful but too specific for the commit 
message. When we have infinite loop, the steps are:

1. Detect typos `structVar1`, `structVar2`
2. Correct typos `structVar1`, `structVar2`
  1. Replace `structVar1` with `structVar`. Add corresponding object to 
`TransformTypos::TypoExprs`.
  2. Detect typo `fieldName1`. `NumTypos` becomes 0.
  3. Try to `CorrectDelayedTyposInExpr` for `fieldName1` TypoExpr. As there are 
no typos, no expression transformation.
  4. Replace `structVar2` with `structVar`. Add corresponding object to 
`TransformTypos::TypoExprs`.
  5. Detect typo `fieldName2`. `NumTypos` becomes 1.
1. Replace `fieldName2` with `fieldName`
  6. Figure out that `structVar.fieldName.member2` is invalid, 
CheckAndAdvanceTypoExprCorrectionStreams
3. Try different TypoCorrection for `structVar1`. As it is empty, the 
correction fails.
4. CheckAndAdvanceTypoExprCorrectionStreams. Reset correction stream for 
`structVar1` and keep going with typo correction because `structVar2` typo 
correction stream isn't finished.
5. Try different TypoCorrection for `structVar1`.
  1. Replace `structVar1` with `structVar`
  2. Detect typo `fieldName1`
1. Replace `fieldName1` with `fieldName`
  3. Figure out that `structVar.fieldName.member1` is invalid, 
CheckAndAdvanceTypoExprCorrectionStreams
6. Try different TypoCorrection for `structVar1`. As it is empty, the 
correction fails.
7. CheckAndAdvanceTypoExprCorrectionStreams over and over again.

After my fix the steps are:

1. Detect typos `structVar1`, `structVar2`
2. Correct typos `structVar1`, `structVar2`
  1. Replace `structVar1` with `structVar`. Add corresponding object to 
`TransformTypos::TypoExprs`.
  2. Detect typo `fieldName1`. `NumTypos` becomes 3.
1. Replace `fieldName1` with `fieldName`
  3. Figure out that `structVar.fieldName.member1` is invalid, 
CheckAndAdvanceTypoExprCorrectionStreams
3. Try different TypoCorrection for `structVar1`. As it is empty, the 
correction fails.
4. All available typo corrections were tried because 
`TransformTypos::TypoExprs` contains only `structVar1`. Complete the typo 
correction.


https://reviews.llvm.org/D47341



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

[PATCH] D47341: [Sema] Disable creating new delayed typos while correcting existing.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai updated this revision to Diff 148841.
vsapsai added a comment.

- Fix only infinite loop and don't touch the assertion failure.

New commit message should be:

[Sema] Fix infinite typo correction loop.

NumTypos guard value ~0U doesn't prevent from creating new delayed typos. When
you create new delayed typos during typo correction, value ~0U wraps around to
0. When NumTypos is 0 we can miss some typos and treat an expression as it can
be typo-corrected. But if the expression is still invalid after correction, we
can get stuck in infinite loop trying to correct it.

Fix by not using value ~0U so that NumTypos correctly reflects the number of
typos.

rdar://problem/38642201


https://reviews.llvm.org/D47341

Files:
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/Sema/typo-correction.c


Index: clang/test/Sema/typo-correction.c
===
--- clang/test/Sema/typo-correction.c
+++ clang/test/Sema/typo-correction.c
@@ -87,3 +87,16 @@
 void overloadable_callexpr(int arg) {
func_overloadable(ar); //expected-error{{use of undeclared identifier}}
 }
+
+// rdar://problem/38642201
+struct rdar38642201 {
+  int fieldName;
+};
+
+void rdar38642201_callee(int x, int y);
+void rdar38642201_caller() {
+  struct rdar38642201 structVar;
+  rdar38642201_callee(
+  structVar1.fieldName1.member1, //expected-error{{use of undeclared 
identifier 'structVar1'}}
+  structVar2.fieldName2.member2); //expected-error{{use of undeclared 
identifier 'structVar2'}}
+}
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -7668,12 +7668,8 @@
   if (E && !ExprEvalContexts.empty() && ExprEvalContexts.back().NumTypos &&
   (E->isTypeDependent() || E->isValueDependent() ||
E->isInstantiationDependent())) {
-auto TyposInContext = ExprEvalContexts.back().NumTypos;
-assert(TyposInContext < ~0U && "Recursive call of 
CorrectDelayedTyposInExpr");
-ExprEvalContexts.back().NumTypos = ~0U;
 auto TyposResolved = DelayedTypos.size();
 auto Result = TransformTypos(*this, InitDecl, Filter).Transform(E);
-ExprEvalContexts.back().NumTypos = TyposInContext;
 TyposResolved -= DelayedTypos.size();
 if (Result.isInvalid() || Result.get() != E) {
   ExprEvalContexts.back().NumTypos -= TyposResolved;


Index: clang/test/Sema/typo-correction.c
===
--- clang/test/Sema/typo-correction.c
+++ clang/test/Sema/typo-correction.c
@@ -87,3 +87,16 @@
 void overloadable_callexpr(int arg) {
 	func_overloadable(ar); //expected-error{{use of undeclared identifier}}
 }
+
+// rdar://problem/38642201
+struct rdar38642201 {
+  int fieldName;
+};
+
+void rdar38642201_callee(int x, int y);
+void rdar38642201_caller() {
+  struct rdar38642201 structVar;
+  rdar38642201_callee(
+  structVar1.fieldName1.member1, //expected-error{{use of undeclared identifier 'structVar1'}}
+  structVar2.fieldName2.member2); //expected-error{{use of undeclared identifier 'structVar2'}}
+}
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -7668,12 +7668,8 @@
   if (E && !ExprEvalContexts.empty() && ExprEvalContexts.back().NumTypos &&
   (E->isTypeDependent() || E->isValueDependent() ||
E->isInstantiationDependent())) {
-auto TyposInContext = ExprEvalContexts.back().NumTypos;
-assert(TyposInContext < ~0U && "Recursive call of CorrectDelayedTyposInExpr");
-ExprEvalContexts.back().NumTypos = ~0U;
 auto TyposResolved = DelayedTypos.size();
 auto Result = TransformTypos(*this, InitDecl, Filter).Transform(E);
-ExprEvalContexts.back().NumTypos = TyposInContext;
 TyposResolved -= DelayedTypos.size();
 if (Result.isInvalid() || Result.get() != E) {
   ExprEvalContexts.back().NumTypos -= TyposResolved;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44888: [RISCV] Add -mrelax/-mno-relax flags to enable/disable RISCV linker relaxation

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL85: [RISCV] Add -mrelax/-mno-relax flags to 
enable/disable RISCV linker relaxation (authored by shiva, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D44888?vs=148548=148840#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D44888

Files:
  cfe/trunk/include/clang/Driver/Options.td
  cfe/trunk/lib/Driver/ToolChains/Arch/RISCV.cpp
  cfe/trunk/test/Driver/riscv-features.c


Index: cfe/trunk/include/clang/Driver/Options.td
===
--- cfe/trunk/include/clang/Driver/Options.td
+++ cfe/trunk/include/clang/Driver/Options.td
@@ -151,6 +151,8 @@
 Group, DocName<"WebAssembly">;
 def m_x86_Features_Group : OptionGroup<"">,
Group, Flags<[CoreOption]>, DocName<"X86">;
+def m_riscv_Features_Group : OptionGroup<"">,
+ Group, DocName<"RISCV">;
 
 def m_libc_Group : OptionGroup<"">, Group,
Flags<[HelpHidden]>;
@@ -1947,6 +1949,11 @@
 def mno_soft_float : Flag<["-"], "mno-soft-float">, Group;
 def mno_stackrealign : Flag<["-"], "mno-stackrealign">, Group;
 
+def mrelax : Flag<["-"], "mrelax">, Group,
+  HelpText<"Enable linker relaxation">;
+def mno_relax : Flag<["-"], "mno-relax">, Group,
+  HelpText<"Disable linker relaxation">;
+
 def munaligned_access : Flag<["-"], "munaligned-access">, 
Group,
   HelpText<"Allow memory accesses to be unaligned (AArch32/AArch64 only)">;
 def mno_unaligned_access : Flag<["-"], "mno-unaligned-access">, 
Group,
Index: cfe/trunk/test/Driver/riscv-features.c
===
--- cfe/trunk/test/Driver/riscv-features.c
+++ cfe/trunk/test/Driver/riscv-features.c
@@ -2,3 +2,12 @@
 // RUN: %clang -target riscv64-unknown-elf -### %s -fsyntax-only 2>&1 | 
FileCheck %s
 
 // CHECK: fno-signed-char
+
+// RUN: %clang -target riscv32-unknown-elf -### %s -mrelax 2>&1 | FileCheck %s 
-check-prefix=RELAX
+// RUN: %clang -target riscv32-unknown-elf -### %s -mno-relax 2>&1 | FileCheck 
%s -check-prefix=NO-RELAX
+// RUN: %clang -target riscv32-unknown-elf -### %s 2>&1 | FileCheck %s 
-check-prefix=DEFAULT
+
+// RELAX: "-target-feature" "+relax"
+// NO-RELAX: "-target-feature" "-relax"
+// DEFAULT-NOT: "-target-feature" "+relax"
+// DEFAULT-NOT: "-target-feature" "-relax"
Index: cfe/trunk/lib/Driver/ToolChains/Arch/RISCV.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Arch/RISCV.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Arch/RISCV.cpp
@@ -15,6 +15,7 @@
 #include "llvm/Option/ArgList.h"
 #include "llvm/Support/TargetParser.h"
 #include "llvm/Support/raw_ostream.h"
+#include "ToolChains/CommonArgs.h"
 
 using namespace clang::driver;
 using namespace clang::driver::tools;
@@ -363,6 +364,10 @@
 // Handle all other types of extensions.
 getExtensionFeatures(D, Args, Features, MArch, OtherExts);
   }
+
+  // Now add any that the user explicitly requested on the command line,
+  // which may override the defaults.
+  handleTargetFeaturesGroup(Args, Features, 
options::OPT_m_riscv_Features_Group);
 }
 
 StringRef riscv::getRISCVABI(const ArgList , const llvm::Triple ) {


Index: cfe/trunk/include/clang/Driver/Options.td
===
--- cfe/trunk/include/clang/Driver/Options.td
+++ cfe/trunk/include/clang/Driver/Options.td
@@ -151,6 +151,8 @@
 Group, DocName<"WebAssembly">;
 def m_x86_Features_Group : OptionGroup<"">,
Group, Flags<[CoreOption]>, DocName<"X86">;
+def m_riscv_Features_Group : OptionGroup<"">,
+ Group, DocName<"RISCV">;
 
 def m_libc_Group : OptionGroup<"">, Group,
Flags<[HelpHidden]>;
@@ -1947,6 +1949,11 @@
 def mno_soft_float : Flag<["-"], "mno-soft-float">, Group;
 def mno_stackrealign : Flag<["-"], "mno-stackrealign">, Group;
 
+def mrelax : Flag<["-"], "mrelax">, Group,
+  HelpText<"Enable linker relaxation">;
+def mno_relax : Flag<["-"], "mno-relax">, Group,
+  HelpText<"Disable linker relaxation">;
+
 def munaligned_access : Flag<["-"], "munaligned-access">, Group,
   HelpText<"Allow memory accesses to be unaligned (AArch32/AArch64 only)">;
 def mno_unaligned_access : Flag<["-"], "mno-unaligned-access">, Group,
Index: cfe/trunk/test/Driver/riscv-features.c
===
--- cfe/trunk/test/Driver/riscv-features.c
+++ cfe/trunk/test/Driver/riscv-features.c
@@ -2,3 +2,12 @@
 // RUN: %clang -target riscv64-unknown-elf -### %s -fsyntax-only 2>&1 | FileCheck %s
 
 // CHECK: fno-signed-char
+
+// RUN: %clang -target riscv32-unknown-elf -### %s -mrelax 2>&1 | FileCheck %s -check-prefix=RELAX
+// RUN: %clang -target riscv32-unknown-elf -### %s -mno-relax 2>&1 | FileCheck %s -check-prefix=NO-RELAX

[PATCH] D47111: : Implement monotonic_buffer_resource.

[Eric Fiselier via Phabricator via cfe-commits]

EricWF added a comment.

FYI I have a full implementation of this laying around as 
https://reviews.llvm.org/D27402 (https://reviews.llvm.org/D27402). But I have 
never taken the time to resolve merge conflicts.
Feel free to steal any of the tests if they're still relevant.




Comment at: include/experimental/memory_resource:429
+size_t __capacity_;
+size_t __alignment_;
+size_t __used_;

I can't imagine we'll need more than 1 byte to represent the alignment.



Comment at: include/experimental/memory_resource:474
+protected:
+void* do_allocate(size_t __bytes, size_t __alignment);
+

Lets add `override` to these functions.



Comment at: src/experimental/memory_resource.cpp:217
+{
+if (void *result = try_allocate_from_chunk(&__original_, bytes, align)) {
+return result;

Drop the braces for conditionals and loops with single statements.


Repository:
  rCXX libc++

https://reviews.llvm.org/D47111



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

[PATCH] D47109: LWG 2969 "polymorphic_allocator::construct() shouldn't pass resource()"

[Eric Fiselier via Phabricator via cfe-commits]

EricWF closed this revision.
EricWF added a comment.

Committed as r84.


Repository:
  rCXX libc++

https://reviews.llvm.org/D47109



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

[libcxx] r333384 - LWG 2969 "polymorphic_allocator::construct() shouldn't pass resource()"

[Eric Fiselier via cfe-commits]

Author: ericwf
Date: Mon May 28 17:08:47 2018
New Revision: 84

URL: http://llvm.org/viewvc/llvm-project?rev=84=rev
Log:
LWG 2969 "polymorphic_allocator::construct() shouldn't pass resource()"

Patch from Arthur O'Dwyer.

In the TS, `uses_allocator` construction for `pair` tried to use an allocator
type of `memory_resource*`, which is incorrect because `memory_resource*` is
not an allocator type. LWG 2969 fixed it to use `polymorphic_allocator` as the
allocator type instead.

https://wg21.link/lwg2969

(D47090 included this in ``; at Eric's request, I've split
this out into its own patch applied to the existing
`` instead.)

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

Added:

libcxx/trunk/test/std/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/construct_piecewise_pair_evil.pass.cpp
Modified:
libcxx/trunk/include/experimental/memory_resource

libcxx/trunk/test/libcxx/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/construct_piecewise_pair.pass.cpp

libcxx/trunk/test/std/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/construct_pair_const_lvalue_pair.pass.cpp

libcxx/trunk/test/std/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/construct_pair_rvalue.pass.cpp

libcxx/trunk/test/std/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/construct_pair_values.pass.cpp

libcxx/trunk/test/std/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/construct_piecewise_pair.pass.cpp

libcxx/trunk/test/std/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/construct_types.pass.cpp
libcxx/trunk/test/support/test_memory_resource.hpp

Modified: libcxx/trunk/include/experimental/memory_resource
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/experimental/memory_resource?rev=84=83=84=diff
==
--- libcxx/trunk/include/experimental/memory_resource (original)
+++ libcxx/trunk/include/experimental/memory_resource Mon May 28 17:08:47 2018
@@ -206,7 +206,7 @@ public:
 void construct(_Tp* __p, _Ts &&... __args)
 {
 _VSTD_LFTS::__lfts_user_alloc_construct(
-__p, resource(), _VSTD::forward<_Ts>(__args)...
+__p, *this, _VSTD::forward<_Ts>(__args)...
   );
 }
 
@@ -218,14 +218,14 @@ public:
 ::new ((void*)__p) pair<_T1, _T2>(piecewise_construct
   , __transform_tuple(
   typename __lfts_uses_alloc_ctor<
-  _T1, memory_resource*, _Args1...
+  _T1, polymorphic_allocator&, _Args1...
   >::type()
 , _VSTD::move(__x)
 , typename __make_tuple_indices::type{}
   )
   , __transform_tuple(
   typename __lfts_uses_alloc_ctor<
-  _T2, memory_resource*, _Args2...
+  _T2, polymorphic_allocator&, _Args2...
   >::type()
 , _VSTD::move(__y)
 , typename __make_tuple_indices::type{}
@@ -289,23 +289,23 @@ private:
 
 template 
 _LIBCPP_INLINE_VISIBILITY
-tuple
+tuple
 __transform_tuple(integral_constant, tuple<_Args...> && __t,
-  __tuple_indices<_Idx...>) const
+  __tuple_indices<_Idx...>)
 {
-using _Tup = tuple;
-return _Tup(allocator_arg, resource(),
+using _Tup = tuple;
+return _Tup(allocator_arg, *this,
 _VSTD::get<_Idx>(_VSTD::move(__t))...);
 }
 
 template 
 _LIBCPP_INLINE_VISIBILITY
-tuple<_Args&&..., memory_resource*>
+tuple<_Args&&..., polymorphic_allocator&>
 __transform_tuple(integral_constant, tuple<_Args...> && __t,
-  __tuple_indices<_Idx...>) const
+  __tuple_indices<_Idx...>)
 {
-using _Tup = tuple<_Args&&..., memory_resource*>;
-return _Tup(_VSTD::get<_Idx>(_VSTD::move(__t))..., resource());
+using _Tup = tuple<_Args&&..., polymorphic_allocator&>;
+return _Tup(_VSTD::get<_Idx>(_VSTD::move(__t))..., *this);
 }
 
 _LIBCPP_INLINE_VISIBILITY

Modified: 
libcxx/trunk/test/libcxx/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/construct_piecewise_pair.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/libcxx/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/construct_piecewise_pair.pass.cpp?rev=84=83=84=diff
==
--- 
libcxx/trunk/test/libcxx/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/construct_piecewise_pair.pass.cpp
 (original)
+++ 

[PATCH] D45517: [analyzer] False positive refutation with Z3

[George Karpenkov via Phabricator via cfe-commits]

george.karpenkov added a comment.

Please resubmit with -U999 diff flag (or using arc)




Comment at: 
include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:362
 
+class FalsePositiveRefutationBRVisitor final
+: public BugReporterVisitorImpl {

Can we have the whole class inside the `.cpp` file? It's annoying to recompile 
half of the analyzer when an internal implementation detail changes



Comment at: 
include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:365
+  // Flag first node as seen by the visitor.
+  bool FirstNodeVisited = true;
+

I'm really not convinced we need this boolean field



Comment at: 
include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h:21
 #include "clang/StaticAnalyzer/Core/PathSensitive/SymExpr.h"
+#include "llvm/ADT/ImmutableMap.h"
 #include "llvm/ADT/Optional.h"

NB: diff should be resubmitted with -U999, as phabricator shows "context not 
available"



Comment at: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp:1261
+
+  for (ConstraintRangeTy::iterator I = CR.begin(), E = CR.end(); I != E; ++I) {
+SymbolRef Sym = I.getKey();

xbolva00 wrote:
> for (auto I : CR)?
@mikhail.ramalho yes please do fix this one


https://reviews.llvm.org/D45517



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

[PATCH] D47108: [CodeGenCXX] Add -fforce-emit-vtables

[Piotr Padlewski via Phabricator via cfe-commits]

Prazek updated this revision to Diff 148839.
Prazek marked an inline comment as done.
Prazek added a comment.

fixed test


Repository:
  rL LLVM

https://reviews.llvm.org/D47108

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/docs/ReleaseNotes.rst
  clang/docs/UsersManual.rst
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/include/clang/Frontend/CodeGenOptions.def
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CodeGenCXX/vtable-available-externally.cpp

Index: clang/test/CodeGenCXX/vtable-available-externally.cpp
===
--- clang/test/CodeGenCXX/vtable-available-externally.cpp
+++ clang/test/CodeGenCXX/vtable-available-externally.cpp
@@ -1,5 +1,6 @@
 // RUN: %clang_cc1 %s -I%S -triple=x86_64-apple-darwin10 -std=c++98 -emit-llvm -o %t
 // RUN: %clang_cc1 %s -I%S -triple=x86_64-apple-darwin10 -std=c++98 -O2 -disable-llvm-passes -emit-llvm -o %t.opt
+// RUN: %clang_cc1 %s -I%S -triple=x86_64-apple-darwin10 -std=c++98 -O2 -disable-llvm-passes -emit-llvm -o %t.vtable -fforce-emit-vtables -fstrict-vtable-pointers -mconstructor-aliases
 // RUN: FileCheck --check-prefix=CHECK-TEST1 %s < %t
 // RUN: FileCheck --check-prefix=CHECK-TEST2 %s < %t
 // RUN: FileCheck --check-prefix=CHECK-TEST5 %s < %t
@@ -13,10 +14,13 @@
 // RUN: FileCheck --check-prefix=CHECK-TEST15 %s < %t.opt
 // RUN: FileCheck --check-prefix=CHECK-TEST16 %s < %t.opt
 // RUN: FileCheck --check-prefix=CHECK-TEST17 %s < %t.opt
+// RUN: FileCheck --check-prefix=CHECK-FORCE-EMIT %s < %t.vtable
+
 
 #include 
 
 // CHECK-TEST1: @_ZTVN5Test11AE = external unnamed_addr constant
+// CHECK-FORCE-EMIT-DAG: @_ZTVN5Test11AE = available_externally unnamed_addr constant
 namespace Test1 {
 
 struct A {
@@ -213,14 +217,16 @@
 
 // because A's key function is defined here, vtable is generated in this TU
 // CHECK-TEST10-DAG: @_ZTVN6Test101AE = unnamed_addr constant
+// CHECK-FORCE-EMIT-DAG: @_ZTVN6Test101AE = unnamed_addr constant
 struct A {
   virtual void foo();
   virtual void bar();
 };
 void A::foo() {}
 
 // Because key function is inline we will generate vtable as linkonce_odr.
 // CHECK-TEST10-DAG: @_ZTVN6Test101DE = linkonce_odr unnamed_addr constant
+// CHECK-FORCE-EMIT-DAG: @_ZTVN6Test101DE = linkonce_odr unnamed_addr constant
 struct D : A {
   void bar();
 };
@@ -237,14 +243,17 @@
 // can't guarantee that we will be able to refer to bar from name
 // so (at the moment) we can't emit vtable available_externally.
 // CHECK-TEST10-DAG: @_ZTVN6Test101CE = external unnamed_addr constant
+// CHECK-FORCE-EMIT-DAG: @_ZTVN6Test101CE = available_externally unnamed_addr constant
 struct C : A {
   void bar() {}   // defined in body - not key function
   virtual inline void gar();  // inline in body - not key function
   virtual void car();
 };
 
 // no key function, vtable will be generated everywhere it will be used
 // CHECK-TEST10-DAG: @_ZTVN6Test101EE = linkonce_odr unnamed_addr constant
+// CHECK-FORCE-EMIT-DAG: @_ZTVN6Test101EE = linkonce_odr unnamed_addr constant
+
 struct E : A {};
 
 void g(A& a) {
@@ -298,11 +307,13 @@
 namespace Test12 {
 
 // CHECK-TEST12: @_ZTVN6Test121AE = external unnamed_addr constant
+// CHECK-FORCE-EMIT-DAG: @_ZTVN6Test121AE = available_externally unnamed_addr constant
 struct A {
   virtual void foo();
   virtual ~A() {}
 };
 // CHECK-TEST12: @_ZTVN6Test121BE = external unnamed_addr constant
+// CHECK-FORCE-EMIT-DAG: @_ZTVN6Test121BE = available_externally unnamed_addr constant
 struct B : A {
   void foo();
 };
@@ -319,6 +330,9 @@
 
 // CHECK-TEST13-DAG: @_ZTVN6Test131AE = available_externally unnamed_addr constant
 // CHECK-TEST13-DAG: @_ZTVN6Test131BE = external unnamed_addr constant
+// CHECK-FORCE-EMIT-DAG: @_ZTVN6Test131AE = available_externally unnamed_addr constant
+// CHECK-FORCE-EMIT-DAG: @_ZTVN6Test131BE = available_externally unnamed_addr constant
+
 struct A {
   virtual ~A();
 };
@@ -371,6 +385,8 @@
 // generate available_externally vtable for it.
 // CHECK-TEST16-DAG: @_ZTVN6Test161SE = external unnamed_addr constant
 // CHECK-TEST16-DAG: @_ZTVN6Test162S2E = available_externally
+// CHECK-FORCE-EMIT-DAG: @_ZTVN6Test161SE = external unnamed_addr constant
+// CHECK-FORCE-EMIT-DAG: @_ZTVN6Test162S2E = available_externally
 
 struct S {
   __attribute__((visibility("hidden"))) virtual void doStuff();
@@ -395,6 +411,10 @@
 // This test checks if we emit vtables opportunistically.
 // CHECK-TEST17-DAG: @_ZTVN6Test171AE = available_externally
 // CHECK-TEST17-DAG: @_ZTVN6Test171BE = external
+// CHECK-FORCE-EMIT-DAG: @_ZTVN6Test171AE = available_externally
+// CHECK-FORCE-EMIT-DAG: @_ZTVN6Test171BE = available_externally
+// CHECK-FORCE-EMIT-DAG: define linkonce_odr void @_ZN6Test171BD2Ev(
+// CHECK-FORCE-EMIT-DAG: define linkonce_odr void @_ZN6Test171BD0Ev(
 
 struct A {
  

Re: r333141 - Use zeroinitializer for (trailing zero portion of) large array initializers

[David Blaikie via cfe-commits]

Probably nice to mention in the commit message what the fix was (& if/where
there was there a test added for it?) so readers don't have to try to
eyeball diff this commit against the otherone.

On Wed, May 23, 2018 at 4:45 PM Richard Smith via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: rsmith
> Date: Wed May 23 16:41:38 2018
> New Revision: 333141
>
> URL: http://llvm.org/viewvc/llvm-project?rev=333141=rev
> Log:
> Use zeroinitializer for (trailing zero portion of) large array initializers
> more reliably.
>
> This re-commits r333044 with a fix for PR37560.
>
> Modified:
> cfe/trunk/lib/CodeGen/CGExprConstant.cpp
> cfe/trunk/lib/Sema/SemaInit.cpp
> cfe/trunk/test/CodeGen/init.c
> cfe/trunk/test/CodeGenCXX/cxx11-initializer-aggregate.cpp
> cfe/trunk/test/SemaCXX/aggregate-initialization.cpp
>
> Modified: cfe/trunk/lib/CodeGen/CGExprConstant.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprConstant.cpp?rev=333141=333140=333141=diff
>
> ==
> --- cfe/trunk/lib/CodeGen/CGExprConstant.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGExprConstant.cpp Wed May 23 16:41:38 2018
> @@ -635,6 +635,60 @@ static ConstantAddress tryEmitGlobalComp
>return ConstantAddress(GV, Align);
>  }
>
> +static llvm::Constant *
> +EmitArrayConstant(CodeGenModule , const ConstantArrayType *DestType,
> +  llvm::Type *CommonElementType, unsigned ArrayBound,
> +  SmallVectorImpl ,
> +  llvm::Constant *Filler) {
> +  // Figure out how long the initial prefix of non-zero elements is.
> +  unsigned NonzeroLength = ArrayBound;
> +  if (Elements.size() < NonzeroLength && Filler->isNullValue())
> +NonzeroLength = Elements.size();
> +  if (NonzeroLength == Elements.size()) {
> +while (NonzeroLength > 0 && Elements[NonzeroLength -
> 1]->isNullValue())
> +  --NonzeroLength;
> +  }
> +
> +  if (NonzeroLength == 0) {
> +return llvm::ConstantAggregateZero::get(
> +CGM.getTypes().ConvertType(QualType(DestType, 0)));
> +  }
> +
> +  // Add a zeroinitializer array filler if we have lots of trailing
> zeroes.
> +  unsigned TrailingZeroes = ArrayBound - NonzeroLength;
> +  if (TrailingZeroes >= 8) {
> +assert(Elements.size() >= NonzeroLength &&
> +   "missing initializer for non-zero element");
> +Elements.resize(NonzeroLength + 1);
> +auto *FillerType =
> +CommonElementType
> +? CommonElementType
> +: CGM.getTypes().ConvertType(DestType->getElementType());
> +FillerType = llvm::ArrayType::get(FillerType, TrailingZeroes);
> +Elements.back() = llvm::ConstantAggregateZero::get(FillerType);
> +CommonElementType = nullptr;
> +  } else if (Elements.size() != ArrayBound) {
> +// Otherwise pad to the right size with the filler if necessary.
> +Elements.resize(ArrayBound, Filler);
> +if (Filler->getType() != CommonElementType)
> +  CommonElementType = nullptr;
> +  }
> +
> +  // If all elements have the same type, just emit an array constant.
> +  if (CommonElementType)
> +return llvm::ConstantArray::get(
> +llvm::ArrayType::get(CommonElementType, ArrayBound), Elements);
> +
> +  // We have mixed types. Use a packed struct.
> +  llvm::SmallVector Types;
> +  Types.reserve(Elements.size());
> +  for (llvm::Constant *Elt : Elements)
> +Types.push_back(Elt->getType());
> +  llvm::StructType *SType =
> +  llvm::StructType::get(CGM.getLLVMContext(), Types, true);
> +  return llvm::ConstantStruct::get(SType, Elements);
> +}
> +
>  /// This class only needs to handle two cases:
>  /// 1) Literals (this is used by APValue emission to emit literals).
>  /// 2) Arrays, structs and unions (outside C++11 mode, we don't currently
> @@ -832,68 +886,47 @@ public:
>}
>
>llvm::Constant *EmitArrayInitialization(InitListExpr *ILE, QualType T) {
> -llvm::ArrayType *AType =
> -cast(ConvertType(ILE->getType()));
> -llvm::Type *ElemTy = AType->getElementType();
> +auto *CAT = CGM.getContext().getAsConstantArrayType(ILE->getType());
> +assert(CAT && "can't emit array init for non-constant-bound array");
>  unsigned NumInitElements = ILE->getNumInits();
> -unsigned NumElements = AType->getNumElements();
> +unsigned NumElements = CAT->getSize().getZExtValue();
>
>  // Initialising an array requires us to automatically
>  // initialise any elements that have not been initialised explicitly
>  unsigned NumInitableElts = std::min(NumInitElements, NumElements);
>
> -QualType EltType =
> CGM.getContext().getAsArrayType(T)->getElementType();
> +QualType EltType = CAT->getElementType();
>
>  // Initialize remaining array elements.
> -llvm::Constant *fillC;
> -if (Expr *filler = ILE->getArrayFiller())
> +llvm::Constant *fillC = nullptr;
> +if (Expr *filler = ILE->getArrayFiller()) {
>fillC = 

[PATCH] D44568: Fix emission of phony dependency targets when adding extra deps

[Volodymyr Sapsai via Phabricator via cfe-commits]

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

Looks good to me. Just watch the build bots in case some of them are strict 
with warnings and require `(void)AddFilename(Filename)`.

As for the case when the input file is also an extra dependency, I think we can 
ignore that for now because extra dependencies are supposed to be sanitizer 
blacklists. Even if that changes in the future, I expect extra dependencies to 
stay orthogonal to the input.




Comment at: test/Frontend/dependency-gen-extradeps-phony.c:6-7
+
+// CHECK: dependency-gen-extradeps-phony.o: 1.extra 2.extra
+// CHECK-NOT: .c:
+// CHECK: 1.extra:

dstenb wrote:
> vsapsai wrote:
> > I think it would be better to have a check
> > 
> > // CHECK:   dependency-gen-extradeps-phony.c
> > 
> > Because you expect it to be there and it's not that simple to notice the 
> > colon in `.c:`, so it's not immediately clear how CHECK-NOT is applied here.
> Do you mean replace the two checks with that? Isn't there a risk that that 
> would match with `dependency-gen-extradeps-phony.c:`, which the not-checks 
> would not pick up then?
> 
> I added a CHECK-NEXT check for the input file so that we match that whole 
> dependency entry at least.
You are right, you need to be careful to make sure that we match .c file only 
as a dependency and not as a target. Good solution with CHECK-NEXT. Now I'm 
happy with the test as expected output is clearly visible and we protect from 
undesired output.


https://reviews.llvm.org/D44568



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

[PATCH] D44480: [Sema] Don't skip function bodies with 'auto' without trailing return type

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Sorry for delays with this patch, I somehow missed the email notification about 
it.

> Expounding on my concerns a bit -- I'm worried about all the other places 
> calling isUndeducedType() and whether they're also at risk and thus a better 
> fix is to change isUndeducedType() to pay attention to the language mode.

I thought semantics of `isUndeducedType()` is actually what's most callers 
(e.g. diagnostics) want. That is "the type is undeduced **and** not dependent". 
But the name can definitely bring some confusion, so there might be other 
places that misinterpreted it.

@rsmith, it would be great if you could take another look when you have time.




Comment at: lib/Sema/SemaDecl.cpp:12609-12610
   return false;
+// We can't simply call Type::isUndeducedType here, because it returns
+// false on C++14's auto return type without trailing return type.
+DeducedType *DT = FD->getReturnType()->getContainedDeducedType();

rsmith wrote:
> I don't think this comment captures the problem. Rather, I think the problem 
> is that in a template we deduce the `auto` to a dependent type, so it's not 
> considered "undeduced" at the point when we ask this question.
Thanks! That definitely explains the problem in a proper manner.



Comment at: lib/Sema/SemaDecl.cpp:12612-12613
+DeducedType *DT = FD->getReturnType()->getContainedDeducedType();
+if (DT && DT->getDeducedType().isNull())
+  return false;
+  }

rsmith wrote:
> Is there any need to check whether the type has been deduced here? It seems 
> simpler (and equivalent) to turn off skipping for functions whose return type 
> involves a deduced type regardless of whether deduction has been performed 
> already (which it hasn't).
Thanks! I misinterpreted the deduced type's semantics. I thought it also 
appears in the following case:
`auto foo() -> int {}`
But it doesn't.


Repository:
  rC Clang

https://reviews.llvm.org/D44480



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

[libcxx] r333381 - Fix up the final bits of breakage due to clang v5 generating bad implicit template deduction guides - specifically for copy-ctors

[Marshall Clow via cfe-commits]

Author: marshall
Date: Mon May 28 12:20:21 2018
New Revision: 81

URL: http://llvm.org/viewvc/llvm-project?rev=81=rev
Log:
Fix up the final bits of breakage due to clang v5 generating bad implicit 
template deduction guides - specifically for copy-ctors

Modified:
libcxx/trunk/test/std/containers/sequences/array/array.cons/deduct.pass.cpp

libcxx/trunk/test/std/utilities/optional/optional.object/optional.object.ctor/deduct.fail.cpp

libcxx/trunk/test/std/utilities/optional/optional.object/optional.object.ctor/deduct.pass.cpp

Modified: 
libcxx/trunk/test/std/containers/sequences/array/array.cons/deduct.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/containers/sequences/array/array.cons/deduct.pass.cpp?rev=81=80=81=diff
==
--- libcxx/trunk/test/std/containers/sequences/array/array.cons/deduct.pass.cpp 
(original)
+++ libcxx/trunk/test/std/containers/sequences/array/array.cons/deduct.pass.cpp 
Mon May 28 12:20:21 2018
@@ -9,7 +9,10 @@
 
 // 
 // UNSUPPORTED: c++98, c++03, c++11, c++14
+// UNSUPPORTED: clang-5
 // UNSUPPORTED: libcpp-no-deduction-guides
+// Clang 5 will generate bad implicit deduction guides
+// Specifically, for the copy constructor.
 
 
 // template 
@@ -51,8 +54,6 @@ int main()
 }
 
 //  Test the implicit deduction guides
-// FIXME broken: no matching constructor
-#if 0
   {
   std::array source = {4.0, 5.0};
   std::array arr(source);   // array(array)
@@ -61,5 +62,4 @@ int main()
 assert(arr[0] == 4.0);
 assert(arr[1] == 5.0);
   }
-#endif
 }

Modified: 
libcxx/trunk/test/std/utilities/optional/optional.object/optional.object.ctor/deduct.fail.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/utilities/optional/optional.object/optional.object.ctor/deduct.fail.cpp?rev=81=80=81=diff
==
--- 
libcxx/trunk/test/std/utilities/optional/optional.object/optional.object.ctor/deduct.fail.cpp
 (original)
+++ 
libcxx/trunk/test/std/utilities/optional/optional.object/optional.object.ctor/deduct.fail.cpp
 Mon May 28 12:20:21 2018
@@ -9,7 +9,10 @@
 
 // 
 // UNSUPPORTED: c++98, c++03, c++11, c++14
+// UNSUPPORTED: clang-5
 // UNSUPPORTED: libcpp-no-deduction-guides
+// Clang 5 will generate bad implicit deduction guides
+//  Specifically, for the copy constructor.
 
 
 // template
@@ -28,7 +31,12 @@ int main()
 //  Test the implicit deduction guides
 {
 //  optional()
-std::optional opt;   // expected-error {{declaration of variable 'opt' 
with deduced type 'std::optional' requires an initializer}}
+std::optional opt;   // expected-error-re declaration of variable 
'opt' with deduced type 'std::optional' requires an initializer|no viable 
constructor or deduction guide for deduction of template arguments of 
'optional'
+//  clang-6 gives a bogus error here:
+//  declaration of variable 'opt' with deduced type 'std::optional' 
requires an initializer
+//  clang-7 (and later) give a better message:
+//  no viable constructor or deduction guide for deduction of template 
arguments of 'optional'
+//  So we check for one or the other.
 }
 
 {

Modified: 
libcxx/trunk/test/std/utilities/optional/optional.object/optional.object.ctor/deduct.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/utilities/optional/optional.object/optional.object.ctor/deduct.pass.cpp?rev=81=80=81=diff
==
--- 
libcxx/trunk/test/std/utilities/optional/optional.object/optional.object.ctor/deduct.pass.cpp
 (original)
+++ 
libcxx/trunk/test/std/utilities/optional/optional.object/optional.object.ctor/deduct.pass.cpp
 Mon May 28 12:20:21 2018
@@ -9,7 +9,10 @@
 
 // 
 // UNSUPPORTED: c++98, c++03, c++11, c++14
+// UNSUPPORTED: clang-5
 // UNSUPPORTED: libcpp-no-deduction-guides
+// Clang 5 will generate bad implicit deduction guides
+//  Specifically, for the copy constructor.
 
 
 // template
@@ -40,18 +43,12 @@ int main()
 }
 
 //  Test the implicit deduction guides
-
 {
-//  optional(const optional &);
-  // FIXME clang and GCC disagree about this!
-  // clang thinks opt is optional>, GCC thinks it's 
optional.
-#if 0
+//  optional(optional);
 std::optional source('A');
 std::optional opt(source);
-static_assert(std::is_same_v>>, "");
+static_assert(std::is_same_v>, "");
 assert(static_cast(opt) == static_cast(source));
 assert(*opt == *source);
-#endif
 }
-
 }


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

[PATCH] D44480: [Sema] Don't skip function bodies with 'auto' without trailing return type

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 148835.
ilya-biryukov marked 2 inline comments as done.
ilya-biryukov added a comment.

- Address review comments.
- Added one more test.


Repository:
  rC Clang

https://reviews.llvm.org/D44480

Files:
  lib/Sema/SemaDecl.cpp
  test/CodeCompletion/crash-skipped-bodies-template-inst.cpp
  test/CodeCompletion/skip-auto-funcs.cpp

Index: test/CodeCompletion/skip-auto-funcs.cpp
===
--- /dev/null
+++ test/CodeCompletion/skip-auto-funcs.cpp
@@ -0,0 +1,43 @@
+// We run clang in completion mode to force skipping of function bodies and
+// check if the function bodies were skipped by observing the warnings that
+// clang produces.
+// RUN: not %clang_cc1 -fsyntax-only -code-completion-at=%s:42:1 %s -o - 2>&1 | FileCheck %s
+template 
+auto not_skipped() {
+  int x;
+  if (x = 10) {}
+  // Check that this function is skipped.
+  // CHECK: 8:9: warning: using the result of an assignment as a condition without parentheses
+  return 1;
+}
+
+template 
+auto lambda_not_skipped = []() {
+  int x;
+  if (x = 10) {}
+  // Check that this function is skipped.
+  // CHECK: 17:9: warning: using the result of an assignment as a condition without parentheses
+  return 1;
+}
+
+template 
+auto skipped() -> T {
+  int x;
+  if (x = 10) {}
+  // Check that this function is skipped.
+  // CHECK-NOT: 26:9: warning: using the result of an assignment as a condition without parentheses
+  return 1;
+};
+
+auto lambda_skipped = []() -> int {
+  int x;
+  if (x = 10) {}
+  // This could potentially be skipped, but it isn't at the moment.
+  // CHECK: 34:9: warning: using the result of an assignment as a condition without parentheses
+  return 1;
+};
+
+int test() {
+  int complete_in_this_function;
+  // CHECK: COMPLETION: complete_in_this_function
+}
Index: test/CodeCompletion/crash-skipped-bodies-template-inst.cpp
===
--- /dev/null
+++ test/CodeCompletion/crash-skipped-bodies-template-inst.cpp
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:24:5 %s -o - 2>&1 | FileCheck %s
+template 
+auto make_func() {
+  struct impl {
+impl* func() {
+  int x;
+  if (x = 10) {}
+  // Check that body of this function is actually skipped.
+  // CHECK-NOT: crash-skipped-bodies-template-inst.cpp:7:{{[0-9]+}}: warning: using the result of an assignment as a condition without parentheses
+  return this;
+}
+  };
+
+  int x;
+  if (x = 10) {}
+  // Check that this function is not skipped.
+  // CHECK: crash-skipped-bodies-template-inst.cpp:15:9: warning: using the result of an assignment as a condition without parentheses
+  return impl();
+}
+
+void foo() {
+  []() {
+make_func();
+m
+// CHECK: COMPLETION: make_func : [#auto#]make_func<<#class T#>>()
+  };
+}
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -12658,9 +12658,15 @@
   // rest of the file.
   // We cannot skip the body of a function with an undeduced return type,
   // because any callers of that function need to know the type.
-  if (const FunctionDecl *FD = D->getAsFunction())
-if (FD->isConstexpr() || FD->getReturnType()->isUndeducedType())
+  if (const FunctionDecl *FD = D->getAsFunction()) {
+if (FD->isConstexpr())
   return false;
+// We can't simply call Type::isUndeducedType here, because inside template
+// auto can be deduced to a dependent type, which is not considered
+// "undeduced".
+if (FD->getReturnType()->getContainedDeducedType())
+  return false;
+  }
   return Consumer.shouldSkipFunctionBody(D);
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47454: [coroutines] Pass implicit object parameter to promise ctor (fix BUG37604)

[Brian Gesiak via Phabricator via cfe-commits]

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

Great! Thanks @GorNishanov!


https://reviews.llvm.org/D47454



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

[PATCH] D47454: [coroutines] Pass implicit object parameter to promise ctor (fix BUG37604)

[Gor Nishanov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL79: [coroutines] Pass implicit object parameter to 
promise ctor (fix BUG37604) (authored by GorNishanov, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D47454?vs=148831=148833#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D47454

Files:
  cfe/trunk/lib/Sema/SemaCoroutine.cpp
  cfe/trunk/test/CodeGenCoroutines/coro-params.cpp
  cfe/trunk/test/SemaCXX/coroutines.cpp

Index: cfe/trunk/test/SemaCXX/coroutines.cpp
===
--- cfe/trunk/test/SemaCXX/coroutines.cpp
+++ cfe/trunk/test/SemaCXX/coroutines.cpp
@@ -831,7 +831,7 @@
   };
 };
 
-extern "C" int f(mismatch_gro_type_tag1) { 
+extern "C" int f(mismatch_gro_type_tag1) {
   // expected-error@-1 {{cannot initialize return object of type 'int' with an rvalue of type 'void'}}
   co_return; //expected-note {{function is a coroutine due to use of 'co_return' here}}
 }
@@ -848,7 +848,7 @@
   };
 };
 
-extern "C" int f(mismatch_gro_type_tag2) { 
+extern "C" int f(mismatch_gro_type_tag2) {
   // expected-error@-1 {{cannot initialize return object of type 'int' with an lvalue of type 'void *'}}
   co_return; //expected-note {{function is a coroutine due to use of 'co_return' here}}
 }
@@ -866,7 +866,7 @@
   };
 };
 
-extern "C" int f(mismatch_gro_type_tag3) { 
+extern "C" int f(mismatch_gro_type_tag3) {
   // expected-error@-1 {{cannot initialize return object of type 'int' with an rvalue of type 'void'}}
   co_return; //expected-note {{function is a coroutine due to use of 'co_return' here}}
 }
@@ -885,7 +885,7 @@
   };
 };
 
-extern "C" int f(mismatch_gro_type_tag4) { 
+extern "C" int f(mismatch_gro_type_tag4) {
   // expected-error@-1 {{cannot initialize return object of type 'int' with an rvalue of type 'char *'}}
   co_return; //expected-note {{function is a coroutine due to use of 'co_return' here}}
 }
@@ -1246,7 +1246,10 @@
   co_return;
 }
 
+struct some_class;
+
 struct good_promise_custom_constructor {
+  good_promise_custom_constructor(some_class&, float, int);
   good_promise_custom_constructor(double, float, int);
   good_promise_custom_constructor() = delete;
   coro get_return_object();
@@ -1261,9 +1264,20 @@
   co_return;
 }
 
+struct some_class {
+  coro
+  good_coroutine_calls_custom_constructor(float, int) {
+co_return;
+  }
+  coro
+  static good_coroutine_calls_custom_constructor(double, float, int) {
+co_return;
+  }
+};
+
 struct bad_promise_no_matching_constructor {
   bad_promise_no_matching_constructor(int, int, int);
-  // expected-note@+1 {{'bad_promise_no_matching_constructor' has been explicitly marked deleted here}}
+  // expected-note@+1 2 {{'bad_promise_no_matching_constructor' has been explicitly marked deleted here}}
   bad_promise_no_matching_constructor() = delete;
   coro get_return_object();
   suspend_always initial_suspend();
@@ -1278,6 +1292,14 @@
   co_return;
 }
 
+struct some_class2 {
+coro
+bad_coroutine_calls_with_no_matching_constructor(int, int, int) {
+  // expected-error@-1 {{call to deleted constructor}}
+  co_return;
+}
+};
+
 } // namespace CoroHandleMemberFunctionTest
 
 class awaitable_no_unused_warn {
Index: cfe/trunk/test/CodeGenCoroutines/coro-params.cpp
===
--- cfe/trunk/test/CodeGenCoroutines/coro-params.cpp
+++ cfe/trunk/test/CodeGenCoroutines/coro-params.cpp
@@ -156,3 +156,28 @@
   // CHECK: invoke void @_ZNSt12experimental16coroutine_traitsIJv28promise_matching_constructorifdEE12promise_typeC1ES1_ifd(%"struct.std::experimental::coroutine_traits::promise_type"* %__promise, i32 %[[INT]], float %[[FLOAT]], double %[[DOUBLE]])
   co_return;
 }
+
+struct some_class;
+
+struct method {};
+
+template  struct std::experimental::coroutine_traits {
+  struct promise_type {
+promise_type(some_class&, float);
+method get_return_object();
+suspend_always initial_suspend();
+suspend_always final_suspend();
+void return_void();
+void unhandled_exception();
+  };
+};
+
+struct some_class {
+  method good_coroutine_calls_custom_constructor(float);
+};
+
+// CHECK-LABEL: define void @_ZN10some_class39good_coroutine_calls_custom_constructorEf(%struct.some_class*
+method some_class::good_coroutine_calls_custom_constructor(float) {
+  // CHECK: invoke void @_ZNSt12experimental16coroutine_traitsIJ6methodR10some_classfEE12promise_typeC1ES3_f(%"struct.std::experimental::coroutine_traits::promise_type"* %__promise, %struct.some_class* dereferenceable(1) %{{.+}}, float
+  co_return;
+}
Index: cfe/trunk/lib/Sema/SemaCoroutine.cpp
===
--- cfe/trunk/lib/Sema/SemaCoroutine.cpp
+++ cfe/trunk/lib/Sema/SemaCoroutine.cpp
@@ -510,6 +510,20 @@
   // Build a list of arguments, based on the coroutine functions arguments,
   // that will be passed to the promise 

[PATCH] D47454: [coroutines] Pass implicit object parameter to promise ctor (fix BUG37604)

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov created this revision.
GorNishanov added reviewers: modocache, rsmith, lewissbaker.
Herald added a subscriber: EricWF.

Complete the implementation of p0914r1.
Implicit object parameter should be passed to a promise constructor.

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


https://reviews.llvm.org/D47454

Files:
  lib/Sema/SemaCoroutine.cpp
  test/CodeGenCoroutines/coro-params.cpp
  test/SemaCXX/coroutines.cpp

Index: test/SemaCXX/coroutines.cpp
===
--- test/SemaCXX/coroutines.cpp
+++ test/SemaCXX/coroutines.cpp
@@ -831,7 +831,7 @@
   };
 };
 
-extern "C" int f(mismatch_gro_type_tag1) { 
+extern "C" int f(mismatch_gro_type_tag1) {
   // expected-error@-1 {{cannot initialize return object of type 'int' with an rvalue of type 'void'}}
   co_return; //expected-note {{function is a coroutine due to use of 'co_return' here}}
 }
@@ -848,7 +848,7 @@
   };
 };
 
-extern "C" int f(mismatch_gro_type_tag2) { 
+extern "C" int f(mismatch_gro_type_tag2) {
   // expected-error@-1 {{cannot initialize return object of type 'int' with an lvalue of type 'void *'}}
   co_return; //expected-note {{function is a coroutine due to use of 'co_return' here}}
 }
@@ -866,7 +866,7 @@
   };
 };
 
-extern "C" int f(mismatch_gro_type_tag3) { 
+extern "C" int f(mismatch_gro_type_tag3) {
   // expected-error@-1 {{cannot initialize return object of type 'int' with an rvalue of type 'void'}}
   co_return; //expected-note {{function is a coroutine due to use of 'co_return' here}}
 }
@@ -885,7 +885,7 @@
   };
 };
 
-extern "C" int f(mismatch_gro_type_tag4) { 
+extern "C" int f(mismatch_gro_type_tag4) {
   // expected-error@-1 {{cannot initialize return object of type 'int' with an rvalue of type 'char *'}}
   co_return; //expected-note {{function is a coroutine due to use of 'co_return' here}}
 }
@@ -1246,7 +1246,10 @@
   co_return;
 }
 
+struct some_class;
+
 struct good_promise_custom_constructor {
+  good_promise_custom_constructor(some_class&, float, int);
   good_promise_custom_constructor(double, float, int);
   good_promise_custom_constructor() = delete;
   coro get_return_object();
@@ -1261,9 +1264,20 @@
   co_return;
 }
 
+struct some_class {
+  coro
+  good_coroutine_calls_custom_constructor(float, int) {
+co_return;
+  }
+  coro
+  static good_coroutine_calls_custom_constructor(double, float, int) {
+co_return;
+  }
+};
+
 struct bad_promise_no_matching_constructor {
   bad_promise_no_matching_constructor(int, int, int);
-  // expected-note@+1 {{'bad_promise_no_matching_constructor' has been explicitly marked deleted here}}
+  // expected-note@+1 2 {{'bad_promise_no_matching_constructor' has been explicitly marked deleted here}}
   bad_promise_no_matching_constructor() = delete;
   coro get_return_object();
   suspend_always initial_suspend();
@@ -1278,6 +1292,14 @@
   co_return;
 }
 
+struct some_class2 {
+coro
+bad_coroutine_calls_with_no_matching_constructor(int, int, int) {
+  // expected-error@-1 {{call to deleted constructor}}
+  co_return;
+}
+};
+
 } // namespace CoroHandleMemberFunctionTest
 
 class awaitable_no_unused_warn {
Index: test/CodeGenCoroutines/coro-params.cpp
===
--- test/CodeGenCoroutines/coro-params.cpp
+++ test/CodeGenCoroutines/coro-params.cpp
@@ -156,3 +156,28 @@
   // CHECK: invoke void @_ZNSt12experimental16coroutine_traitsIJv28promise_matching_constructorifdEE12promise_typeC1ES1_ifd(%"struct.std::experimental::coroutine_traits::promise_type"* %__promise, i32 %[[INT]], float %[[FLOAT]], double %[[DOUBLE]])
   co_return;
 }
+
+struct some_class;
+
+struct method {};
+
+template  struct std::experimental::coroutine_traits {
+  struct promise_type {
+promise_type(some_class&, float);
+method get_return_object();
+suspend_always initial_suspend();
+suspend_always final_suspend();
+void return_void();
+void unhandled_exception();
+  };
+};
+
+struct some_class {
+  method good_coroutine_calls_custom_constructor(float);
+};
+
+// CHECK-LABEL: define void @_ZN10some_class39good_coroutine_calls_custom_constructorEf(%struct.some_class*
+method some_class::good_coroutine_calls_custom_constructor(float) {
+  // CHECK: invoke void @_ZNSt12experimental16coroutine_traitsIJ6methodR10some_classfEE12promise_typeC1ES3_f(%"struct.std::experimental::coroutine_traits::promise_type"* %__promise, %struct.some_class* dereferenceable(1) %{{.+}}, float
+  co_return;
+}
Index: lib/Sema/SemaCoroutine.cpp
===
--- lib/Sema/SemaCoroutine.cpp
+++ lib/Sema/SemaCoroutine.cpp
@@ -510,6 +510,20 @@
   // Build a list of arguments, based on the coroutine functions arguments,
   // that will be passed to the promise type's constructor.
   llvm::SmallVector CtorArgExprs;
+
+  // Add implicit object parameter.
+  if (auto *MD = dyn_cast(FD)) {
+if (MD->isInstance() && !isLambdaCallOperator(MD)) {
+   

[PATCH] D44954: [clangd] Add "member" symbols to the index

[Marc-Andre Laperle via Phabricator via cfe-commits]

malaperle added inline comments.



Comment at: unittests/clangd/SymbolCollectorTests.cpp:141
 Args.insert(Args.end(), ExtraArgs.begin(), ExtraArgs.end());
+Args.push_back(TestFileName);
+

This allows to override the "-xc++" with something else, i.e. -xobjective-c++


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44954



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

[PATCH] D44954: [clangd] Add "member" symbols to the index

[Marc-Andre Laperle via Phabricator via cfe-commits]

malaperle added inline comments.



Comment at: clangd/index/SymbolCollector.cpp:158
+  translationUnitDecl(), namespaceDecl(), linkageSpecDecl(), recordDecl(),
+  enumDecl(), objcProtocolDecl(), objcInterfaceDecl(), objcCategoryDecl(),
+  objcCategoryImplDecl(), objcImplementationDecl()));

ioeric wrote:
> (Disclaimer: I don't know obj-c...)
> 
> It seems that some objc contexts here are good for global code completion. If 
> we want to support objc symbols, it might also make sense to properly set 
> `SupportGlobalCompletion` for them.
Sounds good. Maybe it would be OK to do this in another (small) patch? I also 
know next to nothing about obj-c :)



Comment at: unittests/clangd/CodeCompleteTests.cpp:741
 
+TEST(CompletionTest, Enums) {
+  EXPECT_THAT(completions(R"cpp(

ioeric wrote:
> It's not clear to me what the following tests (`Enums`, `AnonymousNamespace`, 
> `InMainFile`) are testing. Do they test code completion or  symbol collector? 
> If these test symbol collection, they should be moved int 
> SymbolCollectorTest.cpp
They are testing that code completion works as intended regardless of how 
symbol collector is implemented. It's similar to our previous discussion in 
D44882 about "black box testing". I can remove them but it seems odd to me to 
not have code completion level tests for all cases because we assume that it 
will behave a certain way at the symbol collection and querying levels.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44954



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

[PATCH] D44954: [clangd] Add "member" symbols to the index

[Marc-Andre Laperle via Phabricator via cfe-commits]

malaperle updated this revision to Diff 148834.
malaperle marked 6 inline comments as done.
malaperle added a comment.

Address comments.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44954

Files:
  clangd/CodeComplete.cpp
  clangd/index/Index.h
  clangd/index/MemIndex.cpp
  clangd/index/SymbolCollector.cpp
  clangd/index/SymbolYAML.cpp
  unittests/clangd/CodeCompleteTests.cpp
  unittests/clangd/FileIndexTests.cpp
  unittests/clangd/FindSymbolsTests.cpp
  unittests/clangd/SymbolCollectorTests.cpp

Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -67,6 +67,9 @@
   Pos.end.character);
 }
 MATCHER_P(Refs, R, "") { return int(arg.References) == R; }
+MATCHER_P(Global, SupportGlobalCompletion, "") {
+  return arg.SupportGlobalCompletion == SupportGlobalCompletion;
+}
 
 namespace clang {
 namespace clangd {
@@ -132,9 +135,11 @@
 CollectorOpts, PragmaHandler.get());
 
 std::vector Args = {
-"symbol_collector", "-fsyntax-only", "-xc++", "-std=c++11",
-"-include", TestHeaderName,  TestFileName};
+"symbol_collector", "-fsyntax-only", "-xc++",
+"-std=c++11",   "-include",  TestHeaderName};
 Args.insert(Args.end(), ExtraArgs.begin(), ExtraArgs.end());
+Args.push_back(TestFileName);
+
 tooling::ToolInvocation Invocation(
 Args,
 Factory->create(), Files.get(),
@@ -163,8 +168,20 @@
 TEST_F(SymbolCollectorTest, CollectSymbols) {
   const std::string Header = R"(
 class Foo {
+  Foo() {}
+  Foo(int a) {}
   void f();
+  friend void f1();
+  friend class Friend;
+  Foo& operator=(const Foo&);
+  ~Foo();
+  class Nested {
+  void f();
+  };
+};
+class Friend {
 };
+
 void f1();
 inline void f2() {}
 static const int KInt = 2;
@@ -198,25 +215,79 @@
 } // namespace foo
   )";
   runSymbolCollector(Header, /*Main=*/"");
-  EXPECT_THAT(Symbols,
-  UnorderedElementsAreArray(
-  {QName("Foo"), QName("f1"), QName("f2"), QName("KInt"),
-   QName("kStr"), QName("foo"), QName("foo::bar"),
-   QName("foo::int32"), QName("foo::int32_t"), QName("foo::v1"),
-   QName("foo::bar::v2"), QName("foo::baz")}));
+  EXPECT_THAT(Symbols, UnorderedElementsAreArray(
+   {AllOf(QName("Foo"), Global(true)),
+AllOf(QName("Foo::Foo"), Global(false)),
+AllOf(QName("Foo::Foo"), Global(false)),
+AllOf(QName("Foo::f"), Global(false)),
+AllOf(QName("Foo::~Foo"), Global(false)),
+AllOf(QName("Foo::operator="), Global(false)),
+AllOf(QName("Foo::Nested"), Global(false)),
+AllOf(QName("Foo::Nested::f"), Global(false)),
+
+AllOf(QName("Friend"), Global(true)),
+AllOf(QName("f1"), Global(true)),
+AllOf(QName("f2"), Global(true)),
+AllOf(QName("KInt"), Global(true)),
+AllOf(QName("kStr"), Global(true)),
+AllOf(QName("foo"), Global(true)),
+AllOf(QName("foo::bar"), Global(true)),
+AllOf(QName("foo::int32"), Global(true)),
+AllOf(QName("foo::int32_t"), Global(true)),
+AllOf(QName("foo::v1"), Global(true)),
+AllOf(QName("foo::bar::v2"), Global(true)),
+AllOf(QName("foo::baz"), Global(true))}));
 }
 
 TEST_F(SymbolCollectorTest, Template) {
   Annotations Header(R"(
 // Template is indexed, specialization and instantiation is not.
-template  struct [[Tmpl]] {T x = 0;};
+template  struct [[Tmpl]] {T $xdecl[[x]] = 0;};
 template <> struct Tmpl {};
 extern template struct Tmpl;
 template struct Tmpl;
   )");
   runSymbolCollector(Header.code(), /*Main=*/"");
-  EXPECT_THAT(Symbols, UnorderedElementsAreArray({AllOf(
-   QName("Tmpl"), DeclRange(Header.range()))}));
+  EXPECT_THAT(Symbols,
+  UnorderedElementsAreArray(
+  {AllOf(QName("Tmpl"), DeclRange(Header.range())),
+   AllOf(QName("Tmpl::x"), DeclRange(Header.range("xdecl")))}));
+}
+
+TEST_F(SymbolCollectorTest, ObjCSymbols) {
+  const std::string Header = R"(
+@interface Person
+- (void)someMethodName:(void*)name1 lastName:(void*)lName;
+@end
+
+@implementation Person
+- (void)someMethodName:(void*)name1 lastName:(void*)lName{
+  int foo;
+  ^(int param){ int bar; };
+}
+@end
+
+@interface Person 

r333375 - Introduces --stats-only option to only show changes in statistics.

[Mikhail R. Gadelha via cfe-commits]

Author: mramalho
Date: Mon May 28 08:40:39 2018
New Revision: 75

URL: http://llvm.org/viewvc/llvm-project?rev=75=rev
Log:
Introduces --stats-only option to only show changes in statistics.

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

Modified: cfe/trunk/utils/analyzer/CmpRuns.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/analyzer/CmpRuns.py?rev=75=74=75=diff
==
--- cfe/trunk/utils/analyzer/CmpRuns.py (original)
+++ cfe/trunk/utils/analyzer/CmpRuns.py Mon May 28 08:40:39 2018
@@ -26,12 +26,25 @@ Usage:
 
 """
 
-import sys
-import os
-import plistlib
+from collections import defaultdict
+
 from math import log
 from optparse import OptionParser
+import json
+import os
+import plistlib
+import re
+import sys
+
+STATS_REGEXP = re.compile(r"Statistics: (\{.+\})", re.MULTILINE | re.DOTALL)
 
+class Colors:
+"""
+Color for terminal highlight.
+"""
+RED = '\x1b[2;30;41m'
+GREEN = '\x1b[6;30;42m'
+CLEAR = '\x1b[0m'
 
 # Information about analysis run:
 # path - the analysis output directory
@@ -120,12 +133,16 @@ class AnalysisRun:
 # Cumulative list of all diagnostics from all the reports.
 self.diagnostics = []
 self.clang_version = None
+self.stats = []
 
 def getClangVersion(self):
 return self.clang_version
 
 def readSingleFile(self, p, deleteEmpty):
 data = plistlib.readPlist(p)
+if 'statistics' in data:
+self.stats.append(json.loads(data['statistics']))
+data.pop('statistics')
 
 # We want to retrieve the clang version even if there are no
 # reports. Assume that all reports were created using the same
@@ -264,12 +281,53 @@ def compareResults(A, B, opts):
 
 return res
 
+def deriveStats(results):
+# Assume all keys are the same in each statistics bucket.
+combined_data = defaultdict(list)
+for stat in results.stats:
+for key, value in stat.iteritems():
+combined_data[key].append(value)
+combined_stats = {}
+for key, values in combined_data.iteritems():
+combined_stats[str(key)] = {
+"max": max(values),
+"min": min(values),
+"mean": sum(values) / len(values),
+"median": sorted(values)[len(values) / 2],
+"total": sum(values)
+}
+return combined_stats
+
+
+def compareStats(resultsA, resultsB):
+statsA = deriveStats(resultsA)
+statsB = deriveStats(resultsB)
+keys = sorted(statsA.keys())
+for key in keys:
+print key
+for kkey in statsA[key]:
+valA = float(statsA[key][kkey])
+valB = float(statsB[key][kkey])
+report = "%.3f -> %.3f" % (valA, valB)
+# Only apply highlighting when writing to TTY and it's not Windows
+if sys.stdout.isatty() and os.name != 'nt':
+if valA != 0:
+  ratio = (valB - valA) / valB
+  if ratio < -0.2:
+  report = Colors.GREEN + report + Colors.CLEAR
+  elif ratio > 0.2:
+  report = Colors.RED + report + Colors.CLEAR
+print "\t %s %s" % (kkey, report)
 
 def dumpScanBuildResultsDiff(dirA, dirB, opts, deleteEmpty=True,
  Stdout=sys.stdout):
 # Load the run results.
 resultsA = loadResults(dirA, opts, opts.rootA, deleteEmpty)
 resultsB = loadResults(dirB, opts, opts.rootB, deleteEmpty)
+if resultsA.stats:
+compareStats(resultsA, resultsB)
+if opts.stats_only:
+return
 
 # Open the verbose log, if given.
 if opts.verboseLog:
@@ -339,6 +397,8 @@ def generate_option_parser():
   default=False,
   help="Show histogram of absolute paths differences. \
 Requires matplotlib")
+parser.add_option("--stats-only", action="store_true", dest="stats_only",
+  default=False, help="Only show statistics on reports")
 return parser
 
 


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

r333379 - [coroutines] Pass implicit object parameter to promise ctor (fix BUG37604)

[Gor Nishanov via cfe-commits]

Author: gornishanov
Date: Mon May 28 11:08:47 2018
New Revision: 79

URL: http://llvm.org/viewvc/llvm-project?rev=79=rev
Log:
[coroutines] Pass implicit object parameter to promise ctor (fix BUG37604)

Summary:
Complete the implementation of p0914r1.
Implicit object parameter should be passed to a promise constructor.

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

Reviewers: modocache, rsmith, lewissbaker

Reviewed By: modocache

Subscribers: cfe-commits, EricWF

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

Modified:
cfe/trunk/lib/Sema/SemaCoroutine.cpp
cfe/trunk/test/CodeGenCoroutines/coro-params.cpp
cfe/trunk/test/SemaCXX/coroutines.cpp

Modified: cfe/trunk/lib/Sema/SemaCoroutine.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCoroutine.cpp?rev=79=78=79=diff
==
--- cfe/trunk/lib/Sema/SemaCoroutine.cpp (original)
+++ cfe/trunk/lib/Sema/SemaCoroutine.cpp Mon May 28 11:08:47 2018
@@ -510,6 +510,20 @@ VarDecl *Sema::buildCoroutinePromise(Sou
   // Build a list of arguments, based on the coroutine functions arguments,
   // that will be passed to the promise type's constructor.
   llvm::SmallVector CtorArgExprs;
+
+  // Add implicit object parameter.
+  if (auto *MD = dyn_cast(FD)) {
+if (MD->isInstance() && !isLambdaCallOperator(MD)) {
+  ExprResult ThisExpr = ActOnCXXThis(Loc);
+  if (ThisExpr.isInvalid())
+return nullptr;
+  ThisExpr = CreateBuiltinUnaryOp(Loc, UO_Deref, ThisExpr.get());
+  if (ThisExpr.isInvalid())
+return nullptr;
+  CtorArgExprs.push_back(ThisExpr.get());
+}
+  }
+
   auto  = ScopeInfo->CoroutineParameterMoves;
   for (auto *PD : FD->parameters()) {
 if (PD->getType()->isDependentType())

Modified: cfe/trunk/test/CodeGenCoroutines/coro-params.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCoroutines/coro-params.cpp?rev=79=78=79=diff
==
--- cfe/trunk/test/CodeGenCoroutines/coro-params.cpp (original)
+++ cfe/trunk/test/CodeGenCoroutines/coro-params.cpp Mon May 28 11:08:47 2018
@@ -156,3 +156,28 @@ void coroutine_matching_promise_construc
   // CHECK: invoke void 
@_ZNSt12experimental16coroutine_traitsIJv28promise_matching_constructorifdEE12promise_typeC1ES1_ifd(%"struct.std::experimental::coroutine_traits::promise_type"* %__promise, 
i32 %[[INT]], float %[[FLOAT]], double %[[DOUBLE]])
   co_return;
 }
+
+struct some_class;
+
+struct method {};
+
+template  struct std::experimental::coroutine_traits {
+  struct promise_type {
+promise_type(some_class&, float);
+method get_return_object();
+suspend_always initial_suspend();
+suspend_always final_suspend();
+void return_void();
+void unhandled_exception();
+  };
+};
+
+struct some_class {
+  method good_coroutine_calls_custom_constructor(float);
+};
+
+// CHECK-LABEL: define void 
@_ZN10some_class39good_coroutine_calls_custom_constructorEf(%struct.some_class*
+method some_class::good_coroutine_calls_custom_constructor(float) {
+  // CHECK: invoke void 
@_ZNSt12experimental16coroutine_traitsIJ6methodR10some_classfEE12promise_typeC1ES3_f(%"struct.std::experimental::coroutine_traits::promise_type"* %__promise, %struct.some_class* 
dereferenceable(1) %{{.+}}, float
+  co_return;
+}

Modified: cfe/trunk/test/SemaCXX/coroutines.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/coroutines.cpp?rev=79=78=79=diff
==
--- cfe/trunk/test/SemaCXX/coroutines.cpp (original)
+++ cfe/trunk/test/SemaCXX/coroutines.cpp Mon May 28 11:08:47 2018
@@ -831,7 +831,7 @@ struct std::experimental::coroutine_trai
   };
 };
 
-extern "C" int f(mismatch_gro_type_tag1) { 
+extern "C" int f(mismatch_gro_type_tag1) {
   // expected-error@-1 {{cannot initialize return object of type 'int' with an 
rvalue of type 'void'}}
   co_return; //expected-note {{function is a coroutine due to use of 
'co_return' here}}
 }
@@ -848,7 +848,7 @@ struct std::experimental::coroutine_trai
   };
 };
 
-extern "C" int f(mismatch_gro_type_tag2) { 
+extern "C" int f(mismatch_gro_type_tag2) {
   // expected-error@-1 {{cannot initialize return object of type 'int' with an 
lvalue of type 'void *'}}
   co_return; //expected-note {{function is a coroutine due to use of 
'co_return' here}}
 }
@@ -866,7 +866,7 @@ struct std::experimental::coroutine_trai
   };
 };
 
-extern "C" int f(mismatch_gro_type_tag3) { 
+extern "C" int f(mismatch_gro_type_tag3) {
   // expected-error@-1 {{cannot initialize return object of type 'int' with an 
rvalue of type 'void'}}
   co_return; //expected-note {{function is a coroutine due to use of 
'co_return' here}}
 }
@@ -885,7 +885,7 @@ struct std::experimental::coroutine_trai
   };
 };
 
-extern "C" int f(mismatch_gro_type_tag4) { 
+extern "C" 

[PATCH] D45517: [analyzer] False positive refutation with Z3

[Mikhail Ramalho via Phabricator via cfe-commits]

mikhail.ramalho updated this revision to Diff 148828.
mikhail.ramalho retitled this revision from "[analyzer] WIP: False positive 
refutation with Z3" to "[analyzer] False positive refutation with Z3".
mikhail.ramalho added a comment.

Added test cases and updated the analyzer-config tests with the new crosscheck 
flag.

Currently, there is one test failing that does not fail when building without 
the crosscheck:

  llvm/tools/clang/test/Driver/response-file.c:18:10: error: expected string 
not found in input
  // LONG: extern int it_works;
   ^
  :1:1: note: scanning from here
  clang version 7.0.0 (trunk 52) (llvm/trunk 74)
  ^
  :8:3: note: possible intended match here
  Selected GCC installation: /usr/lib/gcc/x86_64-redhat-linux/6.4.1
^


https://reviews.llvm.org/D45517

Files:
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
  include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
  include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
  lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
  lib/StaticAnalyzer/Core/BugReporter.cpp
  lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  lib/StaticAnalyzer/Core/ProgramState.cpp
  lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
  test/Analysis/analyzer-config.c
  test/Analysis/analyzer-config.cpp
  test/Analysis/z3-crosscheck.c

Index: test/Analysis/z3-crosscheck.c
===
--- /dev/null
+++ test/Analysis/z3-crosscheck.c
@@ -0,0 +1,41 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-config crosscheck-with-z3=true -verify %s
+// REQUIRES: z3
+// expected-no-diagnostics
+
+int foo(int x) 
+{
+  int *z = 0;
+  if ((x & 1) && ((x & 1) ^ 1))
+  return *z; // no-warning
+  return 0;
+}
+
+void g(int d);
+
+void f(int *a, int *b) {
+  int c = 5;
+  if ((a - b) == 0)
+c = 0;
+  if (a != b)
+g(3 / c); // no-warning
+}
+
+_Bool nondet_bool();
+
+void h(int d) {
+  int x, y, k, z = 1;
+  // FIXME: Should warn about 'k' being a garbage value
+  while (z < k) { // no-warning
+z = 2 * z;
+  }
+}
+
+void i() {
+  _Bool c = nondet_bool();
+  if (c) {
+h(1);
+  } else {
+h(2);
+  }
+}
+
Index: test/Analysis/analyzer-config.cpp
===
--- test/Analysis/analyzer-config.cpp
+++ test/Analysis/analyzer-config.cpp
@@ -32,6 +32,7 @@
 // CHECK-NEXT: cfg-rich-constructors = true
 // CHECK-NEXT: cfg-scopes = false
 // CHECK-NEXT: cfg-temporary-dtors = true
+// CHECK-NEXT: crosscheck-with-z3 = false
 // CHECK-NEXT: experimental-enable-naive-ctu-analysis = false
 // CHECK-NEXT: exploration_strategy = unexplored_first_queue
 // CHECK-NEXT: faux-bodies = true
@@ -50,4 +51,4 @@
 // CHECK-NEXT: unroll-loops = false
 // CHECK-NEXT: widen-loops = false
 // CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 30
+// CHECK-NEXT: num-entries = 31
Index: test/Analysis/analyzer-config.c
===
--- test/Analysis/analyzer-config.c
+++ test/Analysis/analyzer-config.c
@@ -18,6 +18,7 @@
 // CHECK-NEXT: cfg-rich-constructors = true
 // CHECK-NEXT: cfg-scopes = false
 // CHECK-NEXT: cfg-temporary-dtors = true
+// CHECK-NEXT: crosscheck-with-z3 = false
 // CHECK-NEXT: exploration_strategy = unexplored_first_queue
 // CHECK-NEXT: faux-bodies = true
 // CHECK-NEXT: graph-trim-interval = 1000
@@ -35,4 +36,4 @@
 // CHECK-NEXT: unroll-loops = false
 // CHECK-NEXT: widen-loops = false
 // CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 23
+// CHECK-NEXT: num-entries = 24
Index: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
===
--- lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
+++ lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
@@ -7,6 +7,7 @@
 //
 //===--===//
 
+#include "RangedConstraintManager.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
@@ -915,6 +916,13 @@
   void print(ProgramStateRef St, raw_ostream , const char *nl,
  const char *sep) override;
 
+  void reset() override;
+
+  bool isModelFeasible() override;
+
+  void addRangeConstraints(ConstraintRangeTy PrevCR, ConstraintRangeTy SuccCR,
+   bool OnlyPurged) override;
+
   //===--===//
   // Implementation for interface from SimpleConstraintManager.
   //===--===//
@@ -1235,6 +1243,58 @@
   return State->set(CZ);
 }
 
+void Z3ConstraintManager::reset() { Solver.reset(); }
+
+bool Z3ConstraintManager::isModelFeasible() {
+  return Solver.check() != Z3_L_FALSE;
+}
+
+void 

[PATCH] D47135: [analyzer] A checker for dangling internal buffer pointers in C++

[Reka Kovacs via Phabricator via cfe-commits]

rnkovacs updated this revision to Diff 148827.
rnkovacs added a comment.

Added a check for `UnknownVal` and two FIXMEs (one for the `OriginExpr` and one 
for the new `CheckKind`).


https://reviews.llvm.org/D47135

Files:
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  lib/StaticAnalyzer/Checkers/AllocationState.h
  lib/StaticAnalyzer/Checkers/CMakeLists.txt
  lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
  lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  test/Analysis/dangling-internal-buffer.cpp

Index: test/Analysis/dangling-internal-buffer.cpp
===
--- /dev/null
+++ test/Analysis/dangling-internal-buffer.cpp
@@ -0,0 +1,71 @@
+//RUN: %clang_analyze_cc1 -analyzer-checker=alpha.cplusplus.DanglingInternalBuffer %s -analyzer-output=text -verify
+
+namespace std {
+
+template< typename CharT >
+class basic_string {
+public:
+  ~basic_string();
+  const CharT *c_str();
+};
+
+typedef basic_string string;
+typedef basic_string wstring;
+typedef basic_string u16string;
+typedef basic_string u32string;
+
+} // end namespace std
+
+void consume(const char *) {}
+void consume(const wchar_t *) {}
+void consume(const char16_t *) {}
+void consume(const char32_t *) {}
+
+void deref_after_scope_char() {
+  const char *c;
+  {
+std::string s;
+c = s.c_str();
+  }
+  consume(c); // expected-warning {{Use of memory after it is freed}}
+  // expected-note@-1 {{Use of memory after it is freed}}
+}
+
+void deref_after_scope_wchar_t() {
+  const wchar_t *w;
+  {
+std::wstring ws;
+w = ws.c_str();
+  }
+  consume(w); // expected-warning {{Use of memory after it is freed}}
+  // expected-note@-1 {{Use of memory after it is freed}}
+}
+
+void deref_after_scope_char16_t() {
+  const char16_t *c16;
+  {
+std::u16string s16;
+c16 = s16.c_str();
+  }
+  consume(c16); // expected-warning {{Use of memory after it is freed}}
+  // expected-note@-1 {{Use of memory after it is freed}}
+}
+
+void deref_after_scope_char32_t() {
+  const char32_t *c32;
+  {
+std::u32string s32;
+c32 = s32.c_str();
+  }
+  consume(c32); // expected-warning {{Use of memory after it is freed}}
+  // expected-note@-1 {{Use of memory after it is freed}}
+}
+
+void deref_after_scope_ok() {
+  const char *c;
+  std::string s;
+  {
+c = s.c_str();
+  }
+  consume(c); // no-warning
+}
Index: lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -30,6 +30,7 @@
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringExtras.h"
+#include "AllocationState.h"
 #include 
 #include 
 
@@ -45,7 +46,8 @@
   AF_CXXNew,
   AF_CXXNewArray,
   AF_IfNameIndex,
-  AF_Alloca
+  AF_Alloca,
+  AF_InternalBuffer
 };
 
 class RefState {
@@ -1467,6 +1469,7 @@
 case AF_CXXNew: os << "'new'"; return;
 case AF_CXXNewArray: os << "'new[]'"; return;
 case AF_IfNameIndex: os << "'if_nameindex()'"; return;
+case AF_InternalBuffer: os << "container-specific allocator"; return;
 case AF_Alloca:
 case AF_None: llvm_unreachable("not a deallocation expression");
   }
@@ -1479,6 +1482,7 @@
 case AF_CXXNew: os << "'delete'"; return;
 case AF_CXXNewArray: os << "'delete[]'"; return;
 case AF_IfNameIndex: os << "'if_freenameindex()'"; return;
+case AF_InternalBuffer: os << "container-specific deallocator"; return;
 case AF_Alloca:
 case AF_None: llvm_unreachable("suspicious argument");
   }
@@ -1653,7 +1657,9 @@
 return Optional();
   }
   case AF_CXXNew:
-  case AF_CXXNewArray: {
+  case AF_CXXNewArray:
+  // FIXME: Add new CheckKind for AF_InternalBuffer.
+  case AF_InternalBuffer: {
 if (IsALeakCheck) {
   if (ChecksEnabled[CK_NewDeleteLeaksChecker])
 return CK_NewDeleteLeaksChecker;
@@ -2991,6 +2997,20 @@
   }
 }
 
+namespace clang {
+namespace ento {
+namespace allocation_state {
+
+ProgramStateRef
+markReleased(ProgramStateRef State, SymbolRef Sym, const Expr *Origin) {
+  AllocationFamily Family = AF_InternalBuffer;
+  return State->set(Sym, RefState::getReleased(Family, Origin));
+}
+
+} // end namespace allocation_state
+} // end namespace ento
+} // end namespace clang
+
 void ento::registerNewDeleteLeaksChecker(CheckerManager ) {
   registerCStringCheckerBasic(mgr);
   MallocChecker *checker = mgr.registerChecker();
Index: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
===
--- /dev/null
+++ lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
@@ -0,0 +1,88 @@
+//=== DanglingInternalBufferChecker.cpp ---*- C++ -*--//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//

[PATCH] D43341: [clang-doc] Implement reducer portion of the frontend framework

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: clang-doc/BitcodeReader.cpp:553
+
+#define READINFO(INFO) 
\
+  {
\

Convert this to a template function?



Comment at: clang-doc/Reducer.cpp:19
+
+std::unique_ptr reduceInfos(std::vector> ) {
+  return mergeInfos(Values);

Now that merging is implemented in the info library, `reduceInfos` doesn't seem 
to be necessary. 

 I'd suggest moving `writeInfo` closer to the bitcode writer library and get 
rid of the Reducer.h/cpp. 



Comment at: clang-doc/Reducer.cpp:23
+
+bool writeInfo(Info *I, SmallVectorImpl ) {
+  llvm::BitstreamWriter Stream(Buffer);

I think a more canonical form to pass in an output buffer is via 
`llvm::raw_ostream`, and then callers can use `llvm::raw_string_ostream`.



Comment at: clang-doc/Representation.cpp:30
+
+#define ASSERT_MERGEABLE   
\
+  assert(IT == Other.IT && (USR == EmptySID || USR == Other.USR))

Macros are generally discouraged. Could you make this a function in `Info` e.g. 
`Info::mergeable(const Info )`. And sub-classes can simply can 
`asssert(mergeable(Other))`.



Comment at: clang-doc/Representation.cpp:59
+  case InfoType::IT_default:
+llvm::errs() << "Unexpected info type in index.\n";
+return nullptr;

Use `llvm::Expected>` (e.g. 
`llvm::make_error(...)`) for error handling and let callers decide 
how to handle the error (e.g. `llvm::errs() << llvm::toString(Err)`).



Comment at: clang-doc/Representation.h:146
+protected:
+  bool mergeBase(Info &);
 };

juliehockett wrote:
> ioeric wrote:
> > It's a bit awkward that users have to dispatch from info types to the 
> > corresponding `merge` function (as in `Reducer.cpp`). I think it would make 
> > users' life easier if the library handles the dispatching.
> > 
> > I wonder if something like the following would be better:
> > ```
> > struct Info {
> > std::unique_ptr merge(const Indo& LHS, const Info& RHS);
> > };
> > // A standalone function.
> > std::unique_ptr mergeInfo(const Info , const Info& RHS) {
> >   // merge base info.
> >   ...
> >   // merge subclass infos.
> >   assert(LHS.IT == RHS.IT); // or return nullptr
> >   switch (LHS.IT) {
> >... 
> > return Namespace::merge(LHS, RHS);
> >   } 
> > }
> > 
> > struct NamespaceInfo : public Info {
> >   std::unique_ptr merge(LHS, RHS);
> > };
> > 
> > ```
> > 
> > The unhandled switch case warning in compiler would help you catch 
> > unimplemented `merge` when new info types are added.
> Sort of addressed in this update. There's an issue with where we allocate the 
> return pointer, because we need to know the type of info at allocation time 
> -- let me know if what's here now is too far off of what you were suggesting.
Thanks! 

I thin what you have is also fine, as long as it's easy to maintain when adding 
new info types. 



Comment at: clang-doc/Representation.h:216
 
+// A standalone function to call to merge a vector of infos into one.
+std::unique_ptr mergeInfos(std::vector> );

Please elaborate a bit on the contract e.g. all values should have the same 
type; otherweise ...



Comment at: clang-doc/Representation.h:217
+// A standalone function to call to merge a vector of infos into one.
+std::unique_ptr mergeInfos(std::vector> );
+

nit: s/Value/Values/ or Infos



Comment at: clang-doc/tool/ClangDocMain.cpp:181
+doc::writeInfo(I.get(), Buffer);
+  if (DumpResultToFile("bc", Group.getKey() + ".bc", Buffer))
+return 1;

juliehockett wrote:
> ioeric wrote:
> > (Sorry that I might be missing context here.)
> > 
> > Could you please explain the incentive for dumping each info group to one 
> > bc file? If the key identifies a symbol (e.g. USR), wouldn't this result in 
> > a bitcode file being created for each symbol? This doesn't seem very 
> > scalable.  
> Yes, it would. This is mostly for debugging, since there's not really any 
> tools outside the clang system that would actually want/be able to use this 
> information.
Ok, is there any plan to convert intermediate result to final result and output 
in a more accessible format? If so, please add a `FIXME` somewhere just to be 
clearer.



Comment at: clang-doc/tool/ClangDocMain.cpp:176
+Group.getValue().clear();
+Group.getValue().emplace_back(std::move(Reduced));
+

Rather then replace the values in `MapOutput`, it would probably be clearer if 
you store the reduced infos into a different container e.g. `Reduced`.



Comment at: docs/ReleaseNotes.rst:61
 
 - New module `abseil` for checks 

[PATCH] D47063: [clangd] Keep only a limited number of idle ASTs in memory

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

There are still a few nits I haven't addressed, but the other big change is now 
there: removing ASTBuilder, replacing it with free-standing functions. I'd say 
I liked the previous code better, since the use sites were a bit smaller and 
the functions have ridiculously many params now. But all of that seems 
manageable at this point.
@sammccall, let me know what you think. And I'll address the nits that are left 
tomorrow.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D47063



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

[PATCH] D47063: [clangd] Keep only a limited number of idle ASTs in memory

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 148824.
ilya-biryukov marked an inline comment as done.
ilya-biryukov added a comment.

- s/Params/Policy


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D47063

Files:
  clangd/ClangdUnit.cpp
  clangd/ClangdUnit.h
  clangd/TUScheduler.cpp
  clangd/TUScheduler.h
  test/clangd/trace.test
  unittests/clangd/FileIndexTests.cpp

Index: unittests/clangd/FileIndexTests.cpp
===
--- unittests/clangd/FileIndexTests.cpp
+++ unittests/clangd/FileIndexTests.cpp
@@ -11,6 +11,7 @@
 #include "TestFS.h"
 #include "TestTU.h"
 #include "index/FileIndex.h"
+#include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Frontend/PCHContainerOperations.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Tooling/CompilationDatabase.h"
@@ -208,18 +209,6 @@
 TEST(FileIndexTest, RebuildWithPreamble) {
   auto FooCpp = testPath("foo.cpp");
   auto FooH = testPath("foo.h");
-  FileIndex Index;
-  bool IndexUpdated = false;
-  CppFile File("foo.cpp", /*StorePreambleInMemory=*/true,
-   std::make_shared(),
-   [, ](PathRef FilePath, ASTContext ,
-   std::shared_ptr PP) {
- EXPECT_FALSE(IndexUpdated)
- << "Expected only a single index update";
- IndexUpdated = true;
- Index.update(FilePath, , std::move(PP));
-   });
-
   // Preparse ParseInputs.
   ParseInputs PI;
   PI.CompileCommand.Directory = testRoot();
@@ -243,7 +232,19 @@
   )cpp";
 
   // Rebuild the file.
-  File.rebuild(std::move(PI));
+  auto CI = buildCompilerInvocation(PI);
+
+  FileIndex Index;
+  bool IndexUpdated = false;
+  buildPreamble(
+  FooCpp, *CI, /*OldPreamble=*/nullptr, tooling::CompileCommand(), PI,
+  std::make_shared(), /*StoreInMemory=*/true,
+  [, ](PathRef FilePath, ASTContext ,
+  std::shared_ptr PP) {
+EXPECT_FALSE(IndexUpdated) << "Expected only a single index update";
+IndexUpdated = true;
+Index.update(FilePath, , std::move(PP));
+  });
   ASSERT_TRUE(IndexUpdated);
 
   // Check the index contains symbols from the preamble, but not from the main
Index: test/clangd/trace.test
===
--- test/clangd/trace.test
+++ test/clangd/trace.test
@@ -8,14 +8,14 @@
 # CHECK:   "args": {
 # CHECK: "File": "{{.*(/|\\)}}foo.c"
 # CHECK:   },
-# CHECK:   "name": "Preamble",
+# CHECK:   "name": "BuildPreamble",
 # CHECK:   "ph": "X",
 # CHECK: }
 # CHECK: {
 # CHECK:   "args": {
 # CHECK: "File": "{{.*(/|\\)}}foo.c"
 # CHECK:   },
-# CHECK:   "name": "Build",
+# CHECK:   "name": "BuildAST",
 # CHECK:   "ph": "X",
 # CHECK: }
 # CHECK: },
Index: clangd/TUScheduler.h
===
--- clangd/TUScheduler.h
+++ clangd/TUScheduler.h
@@ -42,6 +42,15 @@
 /// within a bounded amount of time.
 };
 
+/// Configuration of the AST retention policy. This only covers retention of
+/// *idle* ASTs. If queue has operations requiring the AST, they might be
+/// kept in memory.
+struct ASTRetentionPolicy {
+  /// Maximum number of ASTs to be retained in memory when there are no pending
+  /// requests for them.
+  unsigned MaxRetainedASTs = 3;
+};
+
 /// Handles running tasks for ClangdServer and managing the resources (e.g.,
 /// preambles and ASTs) for opened files.
 /// TUScheduler is not thread-safe, only one thread should be providing updates
@@ -53,7 +62,8 @@
 public:
   TUScheduler(unsigned AsyncThreadsCount, bool StorePreamblesInMemory,
   PreambleParsedCallback PreambleCallback,
-  std::chrono::steady_clock::duration UpdateDebounce);
+  std::chrono::steady_clock::duration UpdateDebounce,
+  ASTRetentionPolicy RetentionPolicy = {});
   ~TUScheduler();
 
   /// Returns estimated memory usage for each of the currently open files.
@@ -99,11 +109,18 @@
   /// This class stores per-file data in the Files map.
   struct FileData;
 
+public:
+  /// Responsible for retaining and rebuilding idle ASTs. An implementation is
+  /// an LRU cache.
+  class ASTCache;
+
+private:
   const bool StorePreamblesInMemory;
   const std::shared_ptr PCHOps;
   const PreambleParsedCallback PreambleCallback;
   Semaphore Barrier;
   llvm::StringMap Files;
+  std::unique_ptr IdleASTs;
   // None when running tasks synchronously and non-None when running tasks
   // asynchronously.
   llvm::Optional PreambleTasks;
Index: clangd/TUScheduler.cpp
===
--- clangd/TUScheduler.cpp
+++ clangd/TUScheduler.cpp
@@ -45,16 +45,88 @@
 #include "TUScheduler.h"
 #include "Logger.h"
 #include "Trace.h"
+#include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Frontend/PCHContainerOperations.h"
+#include 

[PATCH] D47063: [clangd] Keep only a limited number of idle ASTs in memory

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clangd/TUScheduler.cpp:71
+
+  /// Update the function used to compute the value.
+  void update(std::function ComputeF);

sammccall wrote:
> ilya-biryukov wrote:
> > sammccall wrote:
> > > I think I understand this more as "updates the value" but the value is 
> > > lazy...
> > > 
> > > I find this API somewhat hard to follow, maybe just because it's 
> > > unfamiliar.
> > > I've mostly seen cache APIs look like one of:
> > > 1. `Cache(function Compute)`, `Value Cache::get(Input)`
> > > 2. `void Cache::put(Key, Value)`, `Optional Cache::get(Key)`
> > > 3. `Handle Cache::put(Value)`, `Optional Handle::get()`
> > > 
> > > This one is `Slot Cache::create()`, `void Slot::update(function 
> > > LazyV)`, `Value Slot::get()`.
> > > 
> > > It's similar-ish to 3, but has 3 nontrivial operations instead of 2, and 
> > > the slot object is externally mutable instead of immutable, so it seems 
> > > more complex. What does it buy us in exchange?
> > > 
> > > (1 & 2 work well with a natural key or inputs that are easy to compare, 
> > > which we don't particularly have)
> > As discussed offline, now we have a simpler version that keeps 
> > `unique_ptr`s to idle ASTs and the clients are responsible for building the 
> > ASTs.
> > Note that it's not a "cache" per se, so we might want a different name for 
> > that.
> > @sammccall, you suggested to call it a pool, I find it reasonable.  Should 
> > we name it `ASTPool` instead of `ASTCache`?
> I think the name is actually fine, it's still mostly a cache.
> It does have things in common with a pool, but unrelated consumers can't 
> share a resource, so I think that name is at least as misleading.
SG, let's leave as is.



Comment at: clangd/TUScheduler.cpp:94
+Lock.unlock();
+ForCleanup.reset();
+  }

sammccall wrote:
> this line isn't actually needed right?
It isn't. But it makes an important operation (destructor of the AST) explicit, 
so I'd still keep it.



Comment at: clangd/TUScheduler.cpp:342
+if (!AST)
+  return Action(llvm::make_error(
+  "invalid AST", llvm::errc::invalid_argument));

sammccall wrote:
> This failure doesn't get cached, correct? That's bad for performance.
> 
> But if we think this is always a clangd bug, it's probably fine. (and 
> certainly simplifies things)
Thanks, good catch! I somehow missed it, since at some point the failure 
**was** cached.
I think we should cache it.

Failing ASTs is a clangd bug, of course.
However, they might be hard to fix if it's something inside clang, so I believe 
we should handle the failures gracefully in that case.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D47063



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

[PATCH] D47063: [clangd] Keep only a limited number of idle ASTs in memory

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 148822.
ilya-biryukov added a comment.

- Remove ASTBuilder, make everything a helper function


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D47063

Files:
  clangd/ClangdUnit.cpp
  clangd/ClangdUnit.h
  clangd/TUScheduler.cpp
  clangd/TUScheduler.h
  test/clangd/trace.test
  unittests/clangd/FileIndexTests.cpp

Index: unittests/clangd/FileIndexTests.cpp
===
--- unittests/clangd/FileIndexTests.cpp
+++ unittests/clangd/FileIndexTests.cpp
@@ -11,6 +11,7 @@
 #include "TestFS.h"
 #include "TestTU.h"
 #include "index/FileIndex.h"
+#include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Frontend/PCHContainerOperations.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Tooling/CompilationDatabase.h"
@@ -208,18 +209,6 @@
 TEST(FileIndexTest, RebuildWithPreamble) {
   auto FooCpp = testPath("foo.cpp");
   auto FooH = testPath("foo.h");
-  FileIndex Index;
-  bool IndexUpdated = false;
-  CppFile File("foo.cpp", /*StorePreambleInMemory=*/true,
-   std::make_shared(),
-   [, ](PathRef FilePath, ASTContext ,
-   std::shared_ptr PP) {
- EXPECT_FALSE(IndexUpdated)
- << "Expected only a single index update";
- IndexUpdated = true;
- Index.update(FilePath, , std::move(PP));
-   });
-
   // Preparse ParseInputs.
   ParseInputs PI;
   PI.CompileCommand.Directory = testRoot();
@@ -243,7 +232,19 @@
   )cpp";
 
   // Rebuild the file.
-  File.rebuild(std::move(PI));
+  auto CI = buildCompilerInvocation(PI);
+
+  FileIndex Index;
+  bool IndexUpdated = false;
+  buildPreamble(
+  FooCpp, *CI, /*OldPreamble=*/nullptr, tooling::CompileCommand(), PI,
+  std::make_shared(), /*StoreInMemory=*/true,
+  [, ](PathRef FilePath, ASTContext ,
+  std::shared_ptr PP) {
+EXPECT_FALSE(IndexUpdated) << "Expected only a single index update";
+IndexUpdated = true;
+Index.update(FilePath, , std::move(PP));
+  });
   ASSERT_TRUE(IndexUpdated);
 
   // Check the index contains symbols from the preamble, but not from the main
Index: test/clangd/trace.test
===
--- test/clangd/trace.test
+++ test/clangd/trace.test
@@ -8,14 +8,14 @@
 # CHECK:   "args": {
 # CHECK: "File": "{{.*(/|\\)}}foo.c"
 # CHECK:   },
-# CHECK:   "name": "Preamble",
+# CHECK:   "name": "BuildPreamble",
 # CHECK:   "ph": "X",
 # CHECK: }
 # CHECK: {
 # CHECK:   "args": {
 # CHECK: "File": "{{.*(/|\\)}}foo.c"
 # CHECK:   },
-# CHECK:   "name": "Build",
+# CHECK:   "name": "BuildAST",
 # CHECK:   "ph": "X",
 # CHECK: }
 # CHECK: },
Index: clangd/TUScheduler.h
===
--- clangd/TUScheduler.h
+++ clangd/TUScheduler.h
@@ -42,6 +42,15 @@
 /// within a bounded amount of time.
 };
 
+/// Configuration of the AST retention policy. This only covers retention of
+/// *idle* ASTs. If queue has operations requiring the AST, they might be
+/// kept in memory.
+struct ASTRetentionParams {
+  /// Maximum number of ASTs to be retained in memory when there are no pending
+  /// requests for them.
+  unsigned MaxRetainedASTs = 3;
+};
+
 /// Handles running tasks for ClangdServer and managing the resources (e.g.,
 /// preambles and ASTs) for opened files.
 /// TUScheduler is not thread-safe, only one thread should be providing updates
@@ -53,7 +62,8 @@
 public:
   TUScheduler(unsigned AsyncThreadsCount, bool StorePreamblesInMemory,
   PreambleParsedCallback PreambleCallback,
-  std::chrono::steady_clock::duration UpdateDebounce);
+  std::chrono::steady_clock::duration UpdateDebounce,
+  ASTRetentionParams RetentionConfig = {});
   ~TUScheduler();
 
   /// Returns estimated memory usage for each of the currently open files.
@@ -99,11 +109,18 @@
   /// This class stores per-file data in the Files map.
   struct FileData;
 
+public:
+  /// Responsible for retaining and rebuilding idle ASTs. An implementation is
+  /// an LRU cache.
+  class ASTCache;
+
+private:
   const bool StorePreamblesInMemory;
   const std::shared_ptr PCHOps;
   const PreambleParsedCallback PreambleCallback;
   Semaphore Barrier;
   llvm::StringMap Files;
+  std::unique_ptr IdleASTs;
   // None when running tasks synchronously and non-None when running tasks
   // asynchronously.
   llvm::Optional PreambleTasks;
Index: clangd/TUScheduler.cpp
===
--- clangd/TUScheduler.cpp
+++ clangd/TUScheduler.cpp
@@ -45,16 +45,88 @@
 #include "TUScheduler.h"
 #include "Logger.h"
 #include "Trace.h"
+#include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Frontend/PCHContainerOperations.h"
+#include "llvm/ADT/ScopeExit.h"
 #include 

[libcxx] r333376 - Mark the template deduction tests as UNSUPPORTED on clang 5, because it deduces the wrong type.

[Marshall Clow via cfe-commits]

Author: marshall
Date: Mon May 28 08:42:47 2018
New Revision: 76

URL: http://llvm.org/viewvc/llvm-project?rev=76=rev
Log:
Mark the template deduction tests as UNSUPPORTED on clang 5, because it deduces 
the wrong type.

Modified:

libcxx/trunk/test/std/containers/container.adaptors/queue/queue.cons/deduct.pass.cpp

libcxx/trunk/test/std/containers/container.adaptors/stack/stack.cons/deduct.pass.cpp

Modified: 
libcxx/trunk/test/std/containers/container.adaptors/queue/queue.cons/deduct.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/containers/container.adaptors/queue/queue.cons/deduct.pass.cpp?rev=76=75=76=diff
==
--- 
libcxx/trunk/test/std/containers/container.adaptors/queue/queue.cons/deduct.pass.cpp
 (original)
+++ 
libcxx/trunk/test/std/containers/container.adaptors/queue/queue.cons/deduct.pass.cpp
 Mon May 28 08:42:47 2018
@@ -9,8 +9,10 @@
 
 // 
 // UNSUPPORTED: c++98, c++03, c++11, c++14
+// UNSUPPORTED: clang-5
 // UNSUPPORTED: libcpp-no-deduction-guides
-
+// Clang 5 will generate bad implicit deduction guides
+//  Specifically, for the copy constructor.
 
 // template
 //   queue(Container) -> queue;

Modified: 
libcxx/trunk/test/std/containers/container.adaptors/stack/stack.cons/deduct.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/containers/container.adaptors/stack/stack.cons/deduct.pass.cpp?rev=76=75=76=diff
==
--- 
libcxx/trunk/test/std/containers/container.adaptors/stack/stack.cons/deduct.pass.cpp
 (original)
+++ 
libcxx/trunk/test/std/containers/container.adaptors/stack/stack.cons/deduct.pass.cpp
 Mon May 28 08:42:47 2018
@@ -9,7 +9,10 @@
 
 // 
 // UNSUPPORTED: c++98, c++03, c++11, c++14
+// UNSUPPORTED: clang-5
 // UNSUPPORTED: libcpp-no-deduction-guides
+// Clang 5 will generate bad implicit deduction guides
+// Specifically, for the copy constructor.
 
 
 // template


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

[PATCH] D44568: Fix emission of phony dependency targets when adding extra deps

[David Stenberg via Phabricator via cfe-commits]

dstenb added inline comments.



Comment at: test/Frontend/dependency-gen-extradeps-phony.c:6-7
+
+// CHECK: dependency-gen-extradeps-phony.o: 1.extra 2.extra
+// CHECK-NOT: .c:
+// CHECK: 1.extra:

vsapsai wrote:
> I think it would be better to have a check
> 
> // CHECK:   dependency-gen-extradeps-phony.c
> 
> Because you expect it to be there and it's not that simple to notice the 
> colon in `.c:`, so it's not immediately clear how CHECK-NOT is applied here.
Do you mean replace the two checks with that? Isn't there a risk that that 
would match with `dependency-gen-extradeps-phony.c:`, which the not-checks 
would not pick up then?

I added a CHECK-NEXT check for the input file so that we match that whole 
dependency entry at least.



Comment at: test/Frontend/dependency-gen-extradeps-phony.c:9-11
+// CHECK-NOT: .c:
+// CHECK: 2.extra:
+// CHECK-NOT: .c:

vsapsai wrote:
> For these repeated CHECK-NOT please consider using `FileCheck 
> --implicit-check-not`. In this case it's not that important as the test is 
> small but it can still help to capture your intention more clearly.
I'll add that in the next patch (and I'll keep that in mind for future 
changes). Thanks!


https://reviews.llvm.org/D44568



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

[PATCH] D47063: [clangd] Keep only a limited number of idle ASTs in memory

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 148820.
ilya-biryukov marked 2 inline comments as done.
ilya-biryukov added a comment.

- Address review comments.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D47063

Files:
  clangd/ClangdUnit.cpp
  clangd/ClangdUnit.h
  clangd/TUScheduler.cpp
  clangd/TUScheduler.h
  test/clangd/trace.test
  unittests/clangd/FileIndexTests.cpp

Index: unittests/clangd/FileIndexTests.cpp
===
--- unittests/clangd/FileIndexTests.cpp
+++ unittests/clangd/FileIndexTests.cpp
@@ -11,6 +11,7 @@
 #include "TestFS.h"
 #include "TestTU.h"
 #include "index/FileIndex.h"
+#include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Frontend/PCHContainerOperations.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Tooling/CompilationDatabase.h"
@@ -210,15 +211,15 @@
   auto FooH = testPath("foo.h");
   FileIndex Index;
   bool IndexUpdated = false;
-  CppFile File("foo.cpp", /*StorePreambleInMemory=*/true,
-   std::make_shared(),
-   [, ](PathRef FilePath, ASTContext ,
-   std::shared_ptr PP) {
- EXPECT_FALSE(IndexUpdated)
- << "Expected only a single index update";
- IndexUpdated = true;
- Index.update(FilePath, , std::move(PP));
-   });
+  ASTBuilder Builder("foo.cpp", /*StorePreambleInMemory=*/true,
+ std::make_shared(),
+ [, ](PathRef FilePath, ASTContext ,
+ std::shared_ptr PP) {
+   EXPECT_FALSE(IndexUpdated)
+   << "Expected only a single index update";
+   IndexUpdated = true;
+   Index.update(FilePath, , std::move(PP));
+ });
 
   // Preparse ParseInputs.
   ParseInputs PI;
@@ -243,7 +244,9 @@
   )cpp";
 
   // Rebuild the file.
-  File.rebuild(std::move(PI));
+  auto CI = Builder.buildCompilerInvocation(PI);
+  Builder.buildPreamble(*CI, /*OldPreamble=*/nullptr, tooling::CompileCommand(),
+PI);
   ASSERT_TRUE(IndexUpdated);
 
   // Check the index contains symbols from the preamble, but not from the main
Index: test/clangd/trace.test
===
--- test/clangd/trace.test
+++ test/clangd/trace.test
@@ -8,14 +8,14 @@
 # CHECK:   "args": {
 # CHECK: "File": "{{.*(/|\\)}}foo.c"
 # CHECK:   },
-# CHECK:   "name": "Preamble",
+# CHECK:   "name": "BuildPreamble",
 # CHECK:   "ph": "X",
 # CHECK: }
 # CHECK: {
 # CHECK:   "args": {
 # CHECK: "File": "{{.*(/|\\)}}foo.c"
 # CHECK:   },
-# CHECK:   "name": "Build",
+# CHECK:   "name": "BuildAST",
 # CHECK:   "ph": "X",
 # CHECK: }
 # CHECK: },
Index: clangd/TUScheduler.h
===
--- clangd/TUScheduler.h
+++ clangd/TUScheduler.h
@@ -42,6 +42,15 @@
 /// within a bounded amount of time.
 };
 
+/// Configuration of the AST retention policy. This only covers retention of
+/// *idle* ASTs. If queue has operations requiring the AST, they might be
+/// kept in memory.
+struct ASTRetentionParams {
+  /// Maximum number of ASTs to be retained in memory when there are no pending
+  /// requests for them.
+  unsigned MaxRetainedASTs = 3;
+};
+
 /// Handles running tasks for ClangdServer and managing the resources (e.g.,
 /// preambles and ASTs) for opened files.
 /// TUScheduler is not thread-safe, only one thread should be providing updates
@@ -53,7 +62,8 @@
 public:
   TUScheduler(unsigned AsyncThreadsCount, bool StorePreamblesInMemory,
   PreambleParsedCallback PreambleCallback,
-  std::chrono::steady_clock::duration UpdateDebounce);
+  std::chrono::steady_clock::duration UpdateDebounce,
+  ASTRetentionParams RetentionConfig = {});
   ~TUScheduler();
 
   /// Returns estimated memory usage for each of the currently open files.
@@ -99,11 +109,18 @@
   /// This class stores per-file data in the Files map.
   struct FileData;
 
+public:
+  /// Responsible for retaining and rebuilding idle ASTs. An implementation is
+  /// an LRU cache.
+  class ASTCache;
+
+private:
   const bool StorePreamblesInMemory;
   const std::shared_ptr PCHOps;
   const PreambleParsedCallback PreambleCallback;
   Semaphore Barrier;
   llvm::StringMap Files;
+  std::unique_ptr IdleASTs;
   // None when running tasks synchronously and non-None when running tasks
   // asynchronously.
   llvm::Optional PreambleTasks;
Index: clangd/TUScheduler.cpp
===
--- clangd/TUScheduler.cpp
+++ clangd/TUScheduler.cpp
@@ -45,16 +45,88 @@
 #include "TUScheduler.h"
 #include "Logger.h"
 #include "Trace.h"
+#include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Frontend/PCHContainerOperations.h"

[PATCH] D44568: Fix emission of phony dependency targets when adding extra deps

[David Stenberg via Phabricator via cfe-commits]

dstenb updated this revision to Diff 148819.
dstenb marked an inline comment as done.
dstenb added a comment.

Addressed vsapsai's comments.


https://reviews.llvm.org/D44568

Files:
  lib/Frontend/DependencyFile.cpp
  test/Frontend/dependency-gen-extradeps-phony.c


Index: test/Frontend/dependency-gen-extradeps-phony.c
===
--- /dev/null
+++ test/Frontend/dependency-gen-extradeps-phony.c
@@ -0,0 +1,10 @@
+// RUN: %clang -MM -MP -Xclang -fdepfile-entry=1.extra -Xclang 
-fdepfile-entry=2.extra -Xclang -fdepfile-entry=2.extra %s | \
+// RUN: FileCheck %s --implicit-check-not=.c:
+//
+// Verify that phony targets are only created for the extra dependency files,
+// and not the input file.
+
+// CHECK: dependency-gen-extradeps-phony.o: 1.extra 2.extra \
+// CHECK-NEXT: dependency-gen-extradeps-phony.c
+// CHECK: 1.extra:
+// CHECK: 2.extra:
Index: lib/Frontend/DependencyFile.cpp
===
--- lib/Frontend/DependencyFile.cpp
+++ lib/Frontend/DependencyFile.cpp
@@ -163,6 +163,7 @@
   bool SeenMissingHeader;
   bool IncludeModuleFiles;
   DependencyOutputFormat OutputFormat;
+  unsigned InputFileIndex;
 
 private:
   bool FileMatchesDepCriteria(const char *Filename,
@@ -177,9 +178,11 @@
   AddMissingHeaderDeps(Opts.AddMissingHeaderDeps),
   SeenMissingHeader(false),
   IncludeModuleFiles(Opts.IncludeModuleFiles),
-  OutputFormat(Opts.OutputFormat) {
+  OutputFormat(Opts.OutputFormat),
+  InputFileIndex(0) {
 for (const auto  : Opts.ExtraDeps) {
-  AddFilename(ExtraDep);
+  if (AddFilename(ExtraDep))
+++InputFileIndex;
 }
   }
 
@@ -201,7 +204,7 @@
 OutputDependencyFile();
   }
 
-  void AddFilename(StringRef Filename);
+  bool AddFilename(StringRef Filename);
   bool includeSystemHeaders() const { return IncludeSystemHeaders; }
   bool includeModuleFiles() const { return IncludeModuleFiles; }
 };
@@ -325,9 +328,12 @@
   }
 }
 
-void DFGImpl::AddFilename(StringRef Filename) {
-  if (FilesSet.insert(Filename).second)
+bool DFGImpl::AddFilename(StringRef Filename) {
+  if (FilesSet.insert(Filename).second) {
 Files.push_back(Filename);
+return true;
+  }
+  return false;
 }
 
 /// Print the filename, with escaping or quoting that accommodates the three
@@ -463,8 +469,10 @@
 
   // Create phony targets if requested.
   if (PhonyTarget && !Files.empty()) {
-// Skip the first entry, this is always the input file itself.
-for (auto I = Files.begin() + 1, E = Files.end(); I != E; ++I) {
+unsigned Index = 0;
+for (auto I = Files.begin(), E = Files.end(); I != E; ++I) {
+  if (Index++ == InputFileIndex)
+continue;
   OS << '\n';
   PrintFilename(OS, *I, OutputFormat);
   OS << ":\n";


Index: test/Frontend/dependency-gen-extradeps-phony.c
===
--- /dev/null
+++ test/Frontend/dependency-gen-extradeps-phony.c
@@ -0,0 +1,10 @@
+// RUN: %clang -MM -MP -Xclang -fdepfile-entry=1.extra -Xclang -fdepfile-entry=2.extra -Xclang -fdepfile-entry=2.extra %s | \
+// RUN: FileCheck %s --implicit-check-not=.c:
+//
+// Verify that phony targets are only created for the extra dependency files,
+// and not the input file.
+
+// CHECK: dependency-gen-extradeps-phony.o: 1.extra 2.extra \
+// CHECK-NEXT: dependency-gen-extradeps-phony.c
+// CHECK: 1.extra:
+// CHECK: 2.extra:
Index: lib/Frontend/DependencyFile.cpp
===
--- lib/Frontend/DependencyFile.cpp
+++ lib/Frontend/DependencyFile.cpp
@@ -163,6 +163,7 @@
   bool SeenMissingHeader;
   bool IncludeModuleFiles;
   DependencyOutputFormat OutputFormat;
+  unsigned InputFileIndex;
 
 private:
   bool FileMatchesDepCriteria(const char *Filename,
@@ -177,9 +178,11 @@
   AddMissingHeaderDeps(Opts.AddMissingHeaderDeps),
   SeenMissingHeader(false),
   IncludeModuleFiles(Opts.IncludeModuleFiles),
-  OutputFormat(Opts.OutputFormat) {
+  OutputFormat(Opts.OutputFormat),
+  InputFileIndex(0) {
 for (const auto  : Opts.ExtraDeps) {
-  AddFilename(ExtraDep);
+  if (AddFilename(ExtraDep))
+++InputFileIndex;
 }
   }
 
@@ -201,7 +204,7 @@
 OutputDependencyFile();
   }
 
-  void AddFilename(StringRef Filename);
+  bool AddFilename(StringRef Filename);
   bool includeSystemHeaders() const { return IncludeSystemHeaders; }
   bool includeModuleFiles() const { return IncludeModuleFiles; }
 };
@@ -325,9 +328,12 @@
   }
 }
 
-void DFGImpl::AddFilename(StringRef Filename) {
-  if (FilesSet.insert(Filename).second)
+bool DFGImpl::AddFilename(StringRef Filename) {
+  if (FilesSet.insert(Filename).second) {
 Files.push_back(Filename);
+return true;
+  }
+  return false;
 }
 
 /// Print the filename, with escaping or quoting that accommodates the three
@@ -463,8 +469,10 @@
 
   // Create phony targets if requested.
   

[PATCH] D47450: [ASTImporter] Use InjectedClassNameType at import of templated record.

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl added inline comments.



Comment at: lib/AST/ASTImporter.cpp:2139
+  CXXRecordDecl *Injected = nullptr;
+  for (NamedDecl *Found : D2CXX->noload_lookup(Name)) {
+auto *Record = dyn_cast(Found);

balazske wrote:
> r.stahl wrote:
> > The only thing I'm wondering is whether the Decls looked up here are always 
> > known to be already imported.
> These are in the 'To' context. It may be that the `Injected` is not found 
> here, probably because not yet imported (in this case the import may be part 
> of a not completed recursive process).
As far as I understand that corner case could be covered by doing the lookup on 
`DCXX` instead and then importing the injected decl. But then you wouldn't find 
it if it is only in the To context (if that is possible).

I mean if a user calls ImportDecl in another order specifically. But such a 
case is probably really artificial and I'm not sure if it's even makes sense or 
is already covered by ImportDeclParts.

It should be fine as it is.


Repository:
  rC Clang

https://reviews.llvm.org/D47450



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

[PATCH] D47451: [analyzer] Remove the redundant check about same state transition in `ArrayBoundCheckerV2.cpp`.

[Henry Wong via Phabricator via cfe-commits]

MTC created this revision.
MTC added reviewers: NoQ, xazax.hun, george.karpenkov.
Herald added subscribers: cfe-commits, a.sidorin, rnkovacs, szepet.

Since the `addTransitionImpl()` has a check about same state transition, there 
is no need to check it in `ArrayBoundCheckerV2.cpp`.


Repository:
  rC Clang

https://reviews.llvm.org/D47451

Files:
  lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp


Index: lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
===
--- lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
+++ lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
@@ -125,7 +125,6 @@
   // have some flexibility in defining the base region, we can achieve
   // various levels of conservatism in our buffer overflow checking.
   ProgramStateRef state = checkerContext.getState();
-  ProgramStateRef originalState = state;
 
   SValBuilder  = checkerContext.getSValBuilder();
   const RegionRawOffsetV2  =
@@ -224,8 +223,7 @@
   }
   while (false);
 
-  if (state != originalState)
-checkerContext.addTransition(state);
+  checkerContext.addTransition(state);
 }
 
 void ArrayBoundCheckerV2::reportOOB(


Index: lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
===
--- lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
+++ lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
@@ -125,7 +125,6 @@
   // have some flexibility in defining the base region, we can achieve
   // various levels of conservatism in our buffer overflow checking.
   ProgramStateRef state = checkerContext.getState();
-  ProgramStateRef originalState = state;
 
   SValBuilder  = checkerContext.getSValBuilder();
   const RegionRawOffsetV2  =
@@ -224,8 +223,7 @@
   }
   while (false);
 
-  if (state != originalState)
-checkerContext.addTransition(state);
+  checkerContext.addTransition(state);
 }
 
 void ArrayBoundCheckerV2::reportOOB(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47450: [ASTImporter] Use InjectedClassNameType at import of templated record.

[Balázs Kéri via Phabricator via cfe-commits]

balazske added inline comments.



Comment at: lib/AST/ASTImporter.cpp:2139
+  CXXRecordDecl *Injected = nullptr;
+  for (NamedDecl *Found : D2CXX->noload_lookup(Name)) {
+auto *Record = dyn_cast(Found);

r.stahl wrote:
> The only thing I'm wondering is whether the Decls looked up here are always 
> known to be already imported.
These are in the 'To' context. It may be that the `Injected` is not found here, 
probably because not yet imported (in this case the import may be part of a not 
completed recursive process).


Repository:
  rC Clang

https://reviews.llvm.org/D47450



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

[PATCH] D46823: [analyzer] const init: handle non-explicit cases more accurately

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl updated this revision to Diff 148809.
r.stahl added a comment.

Added FIXME tests.

I know my example didn't exercise your scenario, but it was the only one I 
could think of where returning zero would have been straight up wrong. Note 
that I default initialized `a` to 3 there, so `sarr` is not just 
zero-initialized.

I do not have access yet, but applied today!


https://reviews.llvm.org/D46823

Files:
  lib/StaticAnalyzer/Core/RegionStore.cpp
  test/Analysis/initialization.c
  test/Analysis/initialization.cpp

Index: test/Analysis/initialization.cpp
===
--- test/Analysis/initialization.cpp
+++ test/Analysis/initialization.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -triple i386-apple-darwin10 -analyze -analyzer-checker=core.builtin,debug.ExprInspection -verify %s
+
+void clang_analyzer_eval(int);
+
+struct S {
+  int a = 3;
+};
+S const sarr[2] = {};
+void definit() {
+  int i = 1;
+  // FIXME: Should recognize that it is 3.
+  clang_analyzer_eval(sarr[i].a); // expected-warning{{UNKNOWN}}
+}
+
+int const arr[2][2] = {};
+void arr2init() {
+  int i = 1;
+  // FIXME: Should recognize that it is 0.
+  clang_analyzer_eval(arr[i][0]); // expected-warning{{UNKNOWN}}
+}
Index: test/Analysis/initialization.c
===
--- test/Analysis/initialization.c
+++ test/Analysis/initialization.c
@@ -1,7 +1,28 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s
-// expected-no-diagnostics
+// RUN: %clang_cc1 -triple i386-apple-darwin10 -analyze -analyzer-checker=core.builtin,debug.ExprInspection -verify %s
+
+void clang_analyzer_eval(int);
 
 void initbug() {
   const union { float a; } u = {};
   (void)u.a; // no-crash
 }
+
+int const parr[2] = {1};
+void constarr() {
+  int i = 2;
+  clang_analyzer_eval(parr[i]); // expected-warning{{UNDEFINED}}
+  i = 1;
+  clang_analyzer_eval(parr[i] == 0); // expected-warning{{TRUE}}
+  i = -1;
+  clang_analyzer_eval(parr[i]); // expected-warning{{UNDEFINED}}
+}
+
+struct SM {
+  int a;
+  int b;
+};
+const struct SM sm = {.a = 1};
+void multinit() {
+  clang_analyzer_eval(sm.a == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(sm.b == 0); // expected-warning{{TRUE}}
+}
Index: lib/StaticAnalyzer/Core/RegionStore.cpp
===
--- lib/StaticAnalyzer/Core/RegionStore.cpp
+++ lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -1638,9 +1638,18 @@
   // The array index has to be known.
   if (auto CI = R->getIndex().getAs()) {
 int64_t i = CI->getValue().getSExtValue();
-// Return unknown value if index is out of bounds.
-if (i < 0 || i >= InitList->getNumInits())
-  return UnknownVal();
+// If it is known that the index is out of bounds, we can return
+// an undefined value.
+if (i < 0)
+  return UndefinedVal();
+
+if (auto CAT = Ctx.getAsConstantArrayType(VD->getType()))
+  if (CAT->getSize().sle(i))
+return UndefinedVal();
+
+// If there is a list, but no init, it must be zero.
+if (i >= InitList->getNumInits())
+  return svalBuilder.makeZeroVal(R->getElementType());
 
 if (const Expr *ElemInit = InitList->getInit(i))
   if (Optional V = svalBuilder.getConstantVal(ElemInit))
@@ -1715,11 +1724,15 @@
 // Either the record variable or the field has to be const qualified.
 if (RecordVarTy.isConstQualified() || Ty.isConstQualified())
   if (const Expr *Init = VD->getInit())
-if (const auto *InitList = dyn_cast(Init))
-  if (Index < InitList->getNumInits())
+if (const auto *InitList = dyn_cast(Init)) {
+  if (Index < InitList->getNumInits()) {
 if (const Expr *FieldInit = InitList->getInit(Index))
   if (Optional V = svalBuilder.getConstantVal(FieldInit))
 return *V;
+  } else {
+return svalBuilder.makeZeroVal(Ty);
+  }
+}
   }
 
   return getBindingForFieldOrElementCommon(B, R, Ty);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44888: [RISCV] Add -mrelax/-mno-relax flags to enable/disable RISCV linker relaxation

[Alex Bradbury via Phabricator via cfe-commits]

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

Looks good to me, thanks!


Repository:
  rL LLVM

https://reviews.llvm.org/D44888



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

[PATCH] D47450: [ASTImporter] Use InjectedClassNameType at import of templated record.

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl accepted this revision.
r.stahl added a comment.

LGTM!

I'm not really too confident to approve changes yet, but with a second opinion 
it should be fine.




Comment at: lib/AST/ASTImporter.cpp:2139
+  CXXRecordDecl *Injected = nullptr;
+  for (NamedDecl *Found : D2CXX->noload_lookup(Name)) {
+auto *Record = dyn_cast(Found);

The only thing I'm wondering is whether the Decls looked up here are always 
known to be already imported.


Repository:
  rC Clang

https://reviews.llvm.org/D47450



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

[PATCH] D46476: [HIP] Add action builder for HIP

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

ping

Any further changes are needed? Thanks.


https://reviews.llvm.org/D46476



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

[clang-tools-extra] r333373 - [clangd] Remove the outdated comment. NFC

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Mon May 28 05:43:20 2018
New Revision: 73

URL: http://llvm.org/viewvc/llvm-project?rev=73=rev
Log:
[clangd] Remove the outdated comment. NFC

Modified:
clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp

Modified: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp?rev=73=72=73=diff
==
--- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp Mon May 28 
05:43:20 2018
@@ -1000,7 +1000,6 @@ TEST(CompletionTest, NoIndexCompletionsI
   }
 }
 
-// FIXME: This still crashes under asan. Fix it and reenable the test.
 TEST(CompletionTest, DocumentationFromChangedFileCrash) {
   MockFSProvider FS;
   auto FooH = testPath("foo.h");


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

[PATCH] D47223: [clangd] Handle enumerators in named, unscoped enums similarly to scoped enums

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

So to keep completion working, maybe we can give add enumerators with full 
qualification and have a flag that determines whether the last component of the 
qualifier can be ignored?
This would give a simple implementation path for now, and will add a semantic 
signal that we need to support enumerators, at least.
For example,

  enum En {
A,
  };
  
  enum class ScopedEn {
A
  };

will produce symbols with

  [ { Scope = "En::", 
  InsideUnscopedEnum=true, 
  Name="A"}, 
{ Scope = "ScopedEn::",
  InsideUnscopedEnum = false,
  Name = "A" } ]

fuzzyFind will have 2 variants:

- For code completion, it will return `En::A` only when queries with `::A` 
(e.g. in global scope).
- For workspace symbol, it can return `En::A` for `A` and `En::A`. For `::A` it 
can return empty results.

Scoped enums work the same as before. Other case (using decls and inline 
namespaces) will also work as before.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D47223



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

[PATCH] D47445: [ASTImporter] Corrected diagnostic client handling in tests.

[Aleksei Sidorin via Phabricator via cfe-commits]

a.sidorin accepted this revision.
a.sidorin added a comment.

Looks good to me, but the approval from AST code owners is required, I think.


Repository:
  rC Clang

https://reviews.llvm.org/D47445



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

[PATCH] D47445: [ASTImporter] Corrected diagnostic client handling in tests.

[Gabor Marton via Phabricator via cfe-commits]

martong accepted this revision.
martong added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: rnkovacs.

LGTM! But let's wait the review of those people who have history in 
`ASTUnit.cpp`.


Repository:
  rC Clang

https://reviews.llvm.org/D47445



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

[PATCH] D47445: [ASTImporter] Corrected diagnostic client handling in tests.

[Gabor Marton via Phabricator via cfe-commits]

martong added reviewers: akyrtzi, ddunbar.
martong added a comment.

Adding Argyrios and Daniel as a reviewer for ASTUnit related changes.


Repository:
  rC Clang

https://reviews.llvm.org/D47445



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

[PATCH] D47450: [ASTImporter] Use InjectedClassNameType at import of templated record.

[Gabor Marton via Phabricator via cfe-commits]

martong added a reviewer: r.stahl.
martong added a comment.

Adding Rafael too as a reviewer, because he has been working also on the 
ASTImporter recently.


Repository:
  rC Clang

https://reviews.llvm.org/D47450



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

[PATCH] D47331: [clangd] Remove accessors for top-level decls from preamble

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE71: [clangd] Remove accessors for top-level decls from 
preamble (authored by ibiryukov, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D47331?vs=148605=148806#toc

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D47331

Files:
  clangd/ClangdUnit.cpp
  clangd/ClangdUnit.h
  clangd/XRefs.cpp
  unittests/clangd/TestTU.cpp

Index: unittests/clangd/TestTU.cpp
===
--- unittests/clangd/TestTU.cpp
+++ unittests/clangd/TestTU.cpp
@@ -1,5 +1,4 @@
-//===--- TestTU.cpp - Scratch source files for testing *-
-//C++-*-===//
+//===--- TestTU.cpp - Scratch source files for testing ===//
 //
 // The LLVM Compiler Infrastructure
 //
@@ -73,22 +72,24 @@
 }
 
 const NamedDecl (ParsedAST , llvm::StringRef QName) {
-  const NamedDecl *Result = nullptr;
-  for (const Decl *D : AST.getTopLevelDecls()) {
-auto *ND = dyn_cast(D);
-if (!ND || ND->getNameAsString() != QName)
-  continue;
-if (Result) {
-  ADD_FAILURE() << "Multiple Decls named " << QName;
-  assert(false && "QName is not unique");
-}
-Result = ND;
-  }
-  if (!Result) {
-ADD_FAILURE() << "No Decl named " << QName << " in AST";
-assert(false && "No Decl with QName");
+  llvm::SmallVector Components;
+  QName.split(Components, "::");
+
+  auto  = AST.getASTContext();
+  auto LookupDecl = [](const DeclContext ,
+   llvm::StringRef Name) -> const NamedDecl & {
+auto LookupRes = Scope.lookup(DeclarationName((Name)));
+assert(!LookupRes.empty() && "Lookup failed");
+assert(LookupRes.size() == 1 && "Lookup returned multiple results");
+return *LookupRes.front();
+  };
+
+  const DeclContext *Scope = Ctx.getTranslationUnitDecl();
+  for (auto NameIt = Components.begin(), End = Components.end() - 1;
+   NameIt != End; ++NameIt) {
+Scope = (LookupDecl(*Scope, *NameIt));
   }
-  return *Result;
+  return LookupDecl(*Scope, Components.back());
 }
 
 } // namespace clangd
Index: clangd/ClangdUnit.h
===
--- clangd/ClangdUnit.h
+++ clangd/ClangdUnit.h
@@ -44,12 +44,10 @@
 
 // Stores Preamble and associated data.
 struct PreambleData {
-  PreambleData(PrecompiledPreamble Preamble,
-   std::vector TopLevelDeclIDs,
-   std::vector Diags, std::vector Inclusions);
+  PreambleData(PrecompiledPreamble Preamble, std::vector Diags,
+   std::vector Inclusions);
 
   PrecompiledPreamble Preamble;
-  std::vector TopLevelDeclIDs;
   std::vector Diags;
   // Processes like code completions and go-to-definitions will need #include
   // information, and their compile action skips preamble range.
@@ -80,17 +78,19 @@
 
   ~ParsedAST();
 
+  /// Note that the returned ast will not contain decls from the preamble that
+  /// were not deserialized during parsing. Clients should expect only decls
+  /// from the main file to be in the AST.
   ASTContext ();
   const ASTContext () const;
 
   Preprocessor ();
   std::shared_ptr getPreprocessorPtr();
   const Preprocessor () const;
 
-  /// This function returns all top-level decls, including those that come
-  /// from Preamble. Decls, coming from Preamble, have to be deserialized, so
-  /// this call might be expensive.
-  ArrayRef getTopLevelDecls();
+  /// This function returns top-level decls present in the main file of the AST.
+  /// The result does not include the decls that come from the preamble.
+  ArrayRef getLocalTopLevelDecls();
 
   const std::vector () const;
 
@@ -103,11 +103,8 @@
   ParsedAST(std::shared_ptr Preamble,
 std::unique_ptr Clang,
 std::unique_ptr Action,
-std::vector TopLevelDecls, std::vector Diags,
-std::vector Inclusions);
-
-private:
-  void ensurePreambleDeclsDeserialized();
+std::vector LocalTopLevelDecls,
+std::vector Diags, std::vector Inclusions);
 
   // In-memory preambles must outlive the AST, it is important that this member
   // goes before Clang and Action.
@@ -122,8 +119,9 @@
 
   // Data, stored after parsing.
   std::vector Diags;
-  std::vector TopLevelDecls;
-  bool PreambleDeclsDeserialized;
+  // Top-level decls inside the current file. Not that this does not include
+  // top-level decls from the preamble.
+  std::vector LocalTopLevelDecls;
   std::vector Inclusions;
 };
 
Index: clangd/ClangdUnit.cpp
===
--- clangd/ClangdUnit.cpp
+++ clangd/ClangdUnit.cpp
@@ -90,37 +90,15 @@
   CppFilePreambleCallbacks(PathRef File, PreambleParsedCallback ParsedCallback)
   : File(File), ParsedCallback(ParsedCallback) {}
 
-  std::vector takeTopLevelDeclIDs() {
-return std::move(TopLevelDeclIDs);
-  }
-
   std::vector 

[PATCH] D47450: [ASTImporter] Use InjectedClassNameType at import of templated record.

[Gabor Marton via Phabricator via cfe-commits]

martong accepted this revision.
martong added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: rnkovacs.

LGTM! But let's wait for someone else's review too.

@a.sidorin 
We discovered this error during the CTU analysis of google/protobuf and we 
could reduce the erroneous C file with c-reduce to the minimal example 
presented in the test file.


Repository:
  rC Clang

https://reviews.llvm.org/D47450



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

[clang-tools-extra] r333371 - [clangd] Remove accessors for top-level decls from preamble

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Mon May 28 05:23:17 2018
New Revision: 71

URL: http://llvm.org/viewvc/llvm-project?rev=71=rev
Log:
[clangd] Remove accessors for top-level decls from preamble

Summary:
They cause lots of deserialization and are not actually used.
The ParsedAST accessor that previously returned those was renamed from
getTopLevelDecls to getLocalTopLevelDecls in order to avoid
confusion.

This change should considerably improve the speed of findDefinition
and other features that try to find AST nodes, corresponding to the
locations in the source code.

Reviewers: ioeric, sammccall

Reviewed By: sammccall

Subscribers: klimek, mehdi_amini, MaskRay, jkorous, cfe-commits

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

Modified:
clang-tools-extra/trunk/clangd/ClangdUnit.cpp
clang-tools-extra/trunk/clangd/ClangdUnit.h
clang-tools-extra/trunk/clangd/XRefs.cpp
clang-tools-extra/trunk/unittests/clangd/TestTU.cpp

Modified: clang-tools-extra/trunk/clangd/ClangdUnit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnit.cpp?rev=71=70=71=diff
==
--- clang-tools-extra/trunk/clangd/ClangdUnit.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp Mon May 28 05:23:17 2018
@@ -90,22 +90,8 @@ public:
   CppFilePreambleCallbacks(PathRef File, PreambleParsedCallback ParsedCallback)
   : File(File), ParsedCallback(ParsedCallback) {}
 
-  std::vector takeTopLevelDeclIDs() {
-return std::move(TopLevelDeclIDs);
-  }
-
   std::vector takeInclusions() { return std::move(Inclusions); }
 
-  void AfterPCHEmitted(ASTWriter ) override {
-TopLevelDeclIDs.reserve(TopLevelDecls.size());
-for (Decl *D : TopLevelDecls) {
-  // Invalid top-level decls may not have been serialized.
-  if (D->isInvalidDecl())
-continue;
-  TopLevelDeclIDs.push_back(Writer.getDeclID(D));
-}
-  }
-
   void AfterExecute(CompilerInstance ) override {
 if (!ParsedCallback)
   return;
@@ -113,14 +99,6 @@ public:
 ParsedCallback(File, CI.getASTContext(), CI.getPreprocessorPtr());
   }
 
-  void HandleTopLevelDecl(DeclGroupRef DG) override {
-for (Decl *D : DG) {
-  if (isa(D))
-continue;
-  TopLevelDecls.push_back(D);
-}
-  }
-
   void BeforeExecute(CompilerInstance ) override {
 SourceMgr = ();
   }
@@ -135,8 +113,6 @@ public:
 private:
   PathRef File;
   PreambleParsedCallback ParsedCallback;
-  std::vector TopLevelDecls;
-  std::vector TopLevelDeclIDs;
   std::vector Inclusions;
   SourceManager *SourceMgr = nullptr;
 };
@@ -204,28 +180,6 @@ ParsedAST::Build(std::unique_ptr Resolved;
-  Resolved.reserve(Preamble->TopLevelDeclIDs.size());
-
-  ExternalASTSource  = *getASTContext().getExternalSource();
-  for (serialization::DeclID TopLevelDecl : Preamble->TopLevelDeclIDs) {
-// Resolve the declaration ID to an actual declaration, possibly
-// deserializing the declaration in the process.
-if (Decl *D = Source.GetExternalDecl(TopLevelDecl))
-  Resolved.push_back(D);
-  }
-
-  TopLevelDecls.reserve(TopLevelDecls.size() +
-Preamble->TopLevelDeclIDs.size());
-  TopLevelDecls.insert(TopLevelDecls.begin(), Resolved.begin(), 
Resolved.end());
-
-  PreambleDeclsDeserialized = true;
-}
-
 ParsedAST::ParsedAST(ParsedAST &) = default;
 
 ParsedAST ::operator=(ParsedAST &) = default;
@@ -252,9 +206,8 @@ const Preprocessor ::getPrepro
   return Clang->getPreprocessor();
 }
 
-ArrayRef ParsedAST::getTopLevelDecls() {
-  ensurePreambleDeclsDeserialized();
-  return TopLevelDecls;
+ArrayRef ParsedAST::getLocalTopLevelDecls() {
+  return LocalTopLevelDecls;
 }
 
 const std::vector ::getDiagnostics() const { return Diags; }
@@ -264,7 +217,7 @@ std::size_t ParsedAST::getUsedBytes() co
   // FIXME(ibiryukov): we do not account for the dynamically allocated part of
   // Message and Fixes inside each diagnostic.
   return AST.getASTAllocatedMemory() + AST.getSideTableAllocatedMemory() +
- ::getUsedBytes(TopLevelDecls) + ::getUsedBytes(Diags);
+ ::getUsedBytes(LocalTopLevelDecls) + ::getUsedBytes(Diags);
 }
 
 const std::vector ::getInclusions() const {
@@ -272,21 +225,19 @@ const std::vector :
 }
 
 PreambleData::PreambleData(PrecompiledPreamble Preamble,
-   std::vector TopLevelDeclIDs,
std::vector Diags,
std::vector Inclusions)
-: Preamble(std::move(Preamble)),
-  TopLevelDeclIDs(std::move(TopLevelDeclIDs)), Diags(std::move(Diags)),
+: Preamble(std::move(Preamble)), Diags(std::move(Diags)),
   Inclusions(std::move(Inclusions)) {}
 
 ParsedAST::ParsedAST(std::shared_ptr Preamble,
  std::unique_ptr Clang,
  std::unique_ptr Action,
- std::vector TopLevelDecls,
+ std::vector LocalTopLevelDecls,

[PATCH] D47450: [ASTImporter] Use InjectedClassNameType at import of templated record.

[Balázs Kéri via Phabricator via cfe-commits]

balazske created this revision.
Herald added subscribers: cfe-commits, martong.
Herald added a reviewer: a.sidorin.

At import of a record describing a template set its type to
InjectedClassNameType (instead of RecordType).


Repository:
  rC Clang

https://reviews.llvm.org/D47450

Files:
  lib/AST/ASTImporter.cpp
  test/ASTMerge/injected-class-name-decl-1/Inputs/inject1.cpp
  test/ASTMerge/injected-class-name-decl-1/test.cpp
  test/ASTMerge/injected-class-name-decl/Inputs/inject1.cpp
  test/ASTMerge/injected-class-name-decl/Inputs/inject2.cpp
  test/ASTMerge/injected-class-name-decl/test.cpp

Index: test/ASTMerge/injected-class-name-decl/test.cpp
===
--- /dev/null
+++ test/ASTMerge/injected-class-name-decl/test.cpp
@@ -0,0 +1,3 @@
+// RUN: %clang_cc1 -std=c++1z -emit-pch -o %t.ast %S/Inputs/inject1.cpp
+// RUN: %clang_cc1 -std=c++1z -emit-obj -o /dev/null -ast-merge %t.ast %S/Inputs/inject2.cpp
+// expected-no-diagnostics
Index: test/ASTMerge/injected-class-name-decl/Inputs/inject2.cpp
===
--- /dev/null
+++ test/ASTMerge/injected-class-name-decl/Inputs/inject2.cpp
@@ -0,0 +1 @@
+template X C::x;
Index: test/ASTMerge/injected-class-name-decl/Inputs/inject1.cpp
===
--- /dev/null
+++ test/ASTMerge/injected-class-name-decl/Inputs/inject1.cpp
@@ -0,0 +1 @@
+template class C { static X x; };
Index: test/ASTMerge/injected-class-name-decl-1/test.cpp
===
--- /dev/null
+++ test/ASTMerge/injected-class-name-decl-1/test.cpp
@@ -0,0 +1,6 @@
+// Trigger a case when at import of template record and replacing its type
+// with InjectedClassNameType there is a PrevDecl of the record.
+
+// RUN: %clang_cc1 -std=c++1z -emit-pch -o %t.ast %S/Inputs/inject1.cpp
+// RUN: %clang_cc1 -std=c++1z -fsyntax-only -ast-merge %t.ast %s
+// expected-no-diagnostics
Index: test/ASTMerge/injected-class-name-decl-1/Inputs/inject1.cpp
===
--- /dev/null
+++ test/ASTMerge/injected-class-name-decl-1/Inputs/inject1.cpp
@@ -0,0 +1,43 @@
+namespace a {
+template  struct b;
+template  struct c;
+template  using e = d;
+class f;
+} // namespace a
+namespace google {
+namespace protobuf {
+namespace internal {
+class LogMessage {
+  LogMessage <<(const char *);
+};
+} // namespace internal
+} // namespace protobuf
+} // namespace google
+namespace a {
+template  class g;
+namespace i {
+struct h;
+template  struct F;
+struct G;
+template  struct j;
+using k = g;
+template  struct l {};
+} // namespace i
+} // namespace a
+namespace a {
+using n = c;
+template  class g : i::F> {
+  template  friend struct i::l;
+};
+} // namespace a
+namespace a {
+using i::G;
+}
+namespace google {
+namespace protobuf {
+namespace internal {
+LogMessage ::operator<<(const char *) { return *this; }
+a::f *o;
+} // namespace internal
+} // namespace protobuf
+} // namespace google
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -2128,6 +2128,29 @@
 if (!ToDescribed)
   return nullptr;
 D2CXX->setDescribedClassTemplate(ToDescribed);
+if (!DCXX->isInjectedClassName()) {
+  // In a record describing a template the type should be a
+  // InjectedClassNameType (see Sema::CheckClassTemplate). Update the
+  // previously set type to the correct value here (ToDescribed is not
+  // available at record create).
+  // FIXME: The previous type is cleared but not removed from
+  // ASTContext's internal storage.
+  CXXRecordDecl *Injected = nullptr;
+  for (NamedDecl *Found : D2CXX->noload_lookup(Name)) {
+auto *Record = dyn_cast(Found);
+if (Record && Record->isInjectedClassName()) {
+  Injected = Record;
+  break;
+}
+  }
+  D2CXX->setTypeForDecl(nullptr);
+  Importer.getToContext().getInjectedClassNameType(D2CXX,
+  ToDescribed->getInjectedClassNameSpecialization());
+  if (Injected) {
+Injected->setTypeForDecl(nullptr);
+Importer.getToContext().getTypeDeclType(Injected, D2CXX);
+  }
+}
   } else if (MemberSpecializationInfo *MemberInfo =
DCXX->getMemberSpecializationInfo()) {
 TemplateSpecializationKind SK =
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits