[PATCH] D47111: : Implement monotonic_buffer_resource.
Quuxplusone updated this revision to Diff 154050.
Quuxplusone marked 2 inline comments as done.
Quuxplusone added a comment.
Massive changes based on Eric's feedback. Moved constructors and destructors of
`monotonic_buffer_resource` into the header so they can be completely inlined.
Renamed `struct __monotonic_buffer_foo_header` to nested types
`monotonic_buffer_resource::__foo_header`. Refactored for 80-column lines.
Repository:
rCXX libc++
https://reviews.llvm.org/D47111
Files:
include/experimental/memory_resource
src/experimental/memory_resource.cpp
test/libcxx/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.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/allocate_with_initial_size.pass.cpp
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/equality.pass.cpp
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,62 @@
+//===--===//
+//
+// 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.
+//
+//===--===//
+
+// REQUIRES: c++experimental
+// 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_with_initial_size.pass.cpp
===
--- /dev/null
+++ test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_with_initial_size.pass.cpp
@@ -0,0 +1,51 @@
+//===--===//
+//
+// 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.
+//
+//===--===//
+
+// REQUIRES: c++experimental
+// UNSUPPORTED: c++98, c++03
+
+//
+
+// class monotonic_buffer_resource
+
+#include
[PATCH] D47111: : Implement monotonic_buffer_resource.
Quuxplusone marked 7 inline comments as done. Quuxplusone added a comment. > I'll take a pass at the tests tomorrow. Some pointers in general: > > - Tests should be named after the component they test, not how they're > testing it. > - All the tests for a single component should be in the same file. I'm certain I'm doing it fairly wrong and you'll have to give me specific handholds like "merge these two tests into one file", "rename this file to x.cpp", etc. The current large number of files is only because *last* iteration, you told me to split up the tests from "one big monotonic_buffer.pass.cpp" into one file per test! The problem with "one file per component" is that I don't understand what a "component" is in this context. From last iteration, I know that "all of `monotonic_buffer_resource`" is too big of a component! I also know that it's impossible to test `deallocate` without also calling `allocate`, but the reverse is not true. Comment at: include/experimental/memory_resource:471 + +void release(); + EricWF wrote: > I think it would be nice for `release` to be inline. What do you think? Good point. It does pull a ton of stuff into the public header, but it's probably worth it. I'm also moving the virtual destructor into the public header, so it can be inlined. (`do_allocate` becomes the new key function.) Comment at: test/std/experimental/memory/memory.resource.monotonic.buffer/nothing_to_do.pass.cpp:1 +// -*- C++ -*- +//===--===// EricWF wrote: > We don't actually need this file, though older libc++ tests will often > include it. > > It's only needed to keep empty directories around, but this one has children. I was starting to wonder about this! Removed. 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] D48753: [libcxx] Use custom allocator's `construct` in C++03 when available.
Quuxplusone added inline comments.
Comment at:
libcxx/test/std/containers/sequences/vector/vector.cons/construct_iter_iter_alloc.pass.cpp:60
+
+ void construct(pointer p, const value_type& val)
+ {
Per my comments on D48342, I think it would be fun to add a test allocator like
```
struct cpp03_fun_allocator : bare_allocator {
...
void construct(pointer p, const value_type& val) {
construct(p, val, std::is_class{});
}
void construct(pointer p, const value_type& val, std::true_type) {
::new(p) value_type(val);
}
void construct(pointer p, const value_type& val, std::false_type) {
::new(p) value_type(val);
}
```
and just see whether it passes the test suite. If it does, might as well add
that test. But if it doesn't, *I'm* not going to force anyone to fix it.
Allocators in C++03 seems like masochism to me. :)
https://reviews.llvm.org/D48753
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.
Quuxplusone added inline comments.
Comment at: libcxx/include/memory:1479
+struct __has_construct_missing
+: false_type
+{
vsapsai wrote:
> Quuxplusone wrote:
> > vsapsai wrote:
> > > vsapsai wrote:
> > > > vsapsai wrote:
> > > > > erik.pilkington wrote:
> > > > > > vsapsai wrote:
> > > > > > > erik.pilkington wrote:
> > > > > > > > Shouldn't this be true_type?
> > > > > > > I see this is confusing and I'm still struggling how to express
> > > > > > > it. The issue is that in C++03 `__has_construct` should be
> > > > > > > something like unknown, so that neither `__has_construct` nor `!
> > > > > > > __has_construct` evaluate to true because we don't really know if
> > > > > > > allocator has construct. This case is covered by the added test,
> > > > > > > in C++03 the memcpy specialization was enabled when
> > > > > > >
> > > > > > > ```
> > > > > > > is_same >
> > > > > > > || !false_type
> > > > > > > ```
> > > > > > >
> > > > > > > So `is_same` check had no effect and we were using memcpy to
> > > > > > > convert between int and float.
> > > > > > >
> > > > > > > I was considering using something like
> > > > > > >
> > > > > > > ```lang=c++
> > > > > > > typename enable_if
> > > > > > > <
> > > > > > > (is_same
> > > > > > > <
> > > > > > > typename _VSTD::remove_const > > > > > > allocator_type::value_type>::type,
> > > > > > > typename _VSTD::remove_const<_SourceTp>::type
> > > > > > > >::value
> > > > > > > #ifndef _LIBCPP_CXX03_LANG
> > > > > > > || !__has_construct > > > > > > _SourceTp>::value
> > > > > > > #endif
> > > > > > > ) &&
> > > > > > > is_trivially_move_constructible<_DestTp>::value,
> > > > > > > void
> > > > > > > >::type
> > > > > > > ```
> > > > > > >
> > > > > > > But that doesn't look readable to me, so I've introduced ad-hoc
> > > > > > > ternary logic with `__has_construct_missing`.
> > > > > > Oh I see, yikes! That's a pretty bad bug. I agree that this fix is
> > > > > > best then, but can you add a comment explaining this to
> > > > > > `__has_construct_missing` for future casual readers? Also, I think
> > > > > > we should move the __has_construct_missing bugfix into a different
> > > > > > (prerequisite) patch. Seems unrelated to the `const` related
> > > > > > optimization below.
> > > > > The bug as I described isn't really present now because function
> > > > > signature
> > > > >
> > > > > __construct_range_forward(allocator_type&, _Tp* __begin1, _Tp*
> > > > > __end1, _Tp*& __begin2)
> > > > >
> > > > > works as implicit `is_same` for `__begin1` and `__begin2` types. I
> > > > > think it is worth fixing separately and there is a bug with C++03 and
> > > > > custom allocators.
> > > > Instead of `__has_construct_missing` I've implemented real
> > > > `__has_construct` in D48753. But it is stricter in C++03 than in C++11
> > > > and later. So it made me think that absence of `construct` with exact
> > > > signature isn't a good reason to use memcpy.
> > > I was wrong. Now I think the logic for using memcpy should be
> > >
> > > if types are the same modulo constness
> > > and
> > > (
> > > using default allocator
> > > or
> > > using custom allocator without `construct`
> > > )
> > > and
> > > is_trivially_move_constructible
> > >
> > > The purpose of the allocator check is to cover cases when `static
> > > construct` would end up calling not user's code but libc++ code that we
> > > know can be replaced with memcpy.
> > I'd like to request the spelling `__has_trivial_construct_and_destroy > T&&>` as described here:
> > https://www.youtube.com/watch?v=MWBfmmg8-Yo=21m45s
> > I have an implementation in my fork that might be useful for comparison,
> > although even it doesn't add that last `T&&` parameter.
> > https://github.com/Quuxplusone/libcxx/commit/34eb0b5c8f03880b94d53b877cbca384783ad65a
> >
> > What we're *really* interested in, in most cases, is
> > `__has_trivial_construct && __has_trivial_destroy`. We
> > don't care if there happens to exist an obscure overload such as
> > `A::construct(T*, Widget, Gadget)` as long as it is not selected. This is
> > particularly relevant to `scoped_allocator_adaptor`, whose `construct` is
> > trivial for primitive types but non-trivial for allocator-aware types.
> >
> > Also, when we write out the template type parameters we care about, we can
> > do the decltype stuff really easily, without having to "fudge" the
> > metaprogramming and possibly get it wrong. For example, if `A::construct`
> > is an overload set, in which case the `&_Xp::construct` on this patch's
> > line 1492 will be ill-formed even though a non-trivial `construct` does
> > actually exist.
> I agree with benefits of using `__has_trivial_construct_and_destroy` in
>
[PATCH] D47111: : Implement monotonic_buffer_resource.
EricWF added a comment. In https://reviews.llvm.org/D47111#1114284, @Quuxplusone wrote: > 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. Woo! This is looking great! I'm done looking at the implementation, I'll take a pass at the tests tomorrow. Some pointers in general: 1. Tests should be named after the component they test, not how they're testing it. 2. All the tests for a single component should be in the same file. 3. You can use `LIBCPP_ASSERT` to check specific implementation details, like exactly how much our allocation sizes grow. (1) and (2) are important for maintaining the tests. (3) will probably be useful for actually writing meaningful tests despite most observable behavior being implementation specific. Comment at: include/experimental/memory_resource:425 + +struct __monotonic_buffer_chunk_header; + Can these classes be members of `monotonic_buffer_resource`? Comment at: include/experimental/memory_resource:451 + +monotonic_buffer_resource(void* __buffer, size_t __buffer_size, memory_resource* __upstream); + Lets keep these constructors inline. Comment at: include/experimental/memory_resource:467 + +~monotonic_buffer_resource() override; // key function + Need _LIBCPP_FUNC_VIS Comment at: include/experimental/memory_resource:471 + +void release(); + I think it would be nice for `release` to be inline. What do you think? Comment at: include/experimental/memory_resource:489 +private: +size_t __previous_allocation_size() const; + `__previous_allocation_size` isn't used in the header, so it shouldn't be declared there. We don't want to expose more symbols than we need to. Comment at: src/experimental/memory_resource.cpp:212 + +monotonic_buffer_resource::monotonic_buffer_resource(void* buffer, size_t buffer_size, memory_resource* upstream) +: __res_(upstream) In general, please wrap to 80 characters. Comment at: test/std/experimental/memory/memory.resource.monotonic.buffer/nothing_to_do.pass.cpp:1 +// -*- C++ -*- +//===--===// We don't actually need this file, though older libc++ tests will often include it. It's only needed to keep empty directories around, but this one has children. 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] D48894: [AST] Rename some Redeclarable functions to reduce confusion
MaskRay updated this revision to Diff 154044.
MaskRay added a comment.
Rename `Next` to `Link` as per rsmith
Repository:
rC Clang
https://reviews.llvm.org/D48894
Files:
include/clang/AST/Redeclarable.h
Index: include/clang/AST/Redeclarable.h
===
--- include/clang/AST/Redeclarable.h
+++ include/clang/AST/Redeclarable.h
@@ -36,7 +36,7 @@
//DeclLink that may point to one of 3 possible states:
// - the "previous" (temporal) element in the chain
// - the "latest" (temporal) element in the chain
-// - the an "uninitialized-latest" value (when newly-constructed)
+// - the "uninitialized-latest" value (when newly-constructed)
//
// - The first element is also often called the canonical element. Every
//element has a pointer to it so that "getCanonical" can be fast.
@@ -48,10 +48,8 @@
//"most-recent" when referring to temporal order: order of addition
//to the chain.
//
-// - To make matters confusing, the DeclLink type uses the term "next"
-//for its pointer-storage internally (thus functions like
-//NextIsPrevious). It's easiest to just ignore the implementation of
-//DeclLink when making sense of the redeclaration chain.
+// - It's easiest to just ignore the implementation of DeclLink when making
+//sense of the redeclaration chain.
//
// - There's also a "definition" link for several types of
//redeclarable, where only one definition should exist at any given
@@ -105,66 +103,64 @@
/// previous declaration.
using NotKnownLatest = llvm::PointerUnion;
-mutable llvm::PointerUnion Next;
+mutable llvm::PointerUnion Link;
public:
enum PreviousTag { PreviousLink };
enum LatestTag { LatestLink };
DeclLink(LatestTag, const ASTContext )
-: Next(NotKnownLatest(reinterpret_cast())) {}
-DeclLink(PreviousTag, decl_type *D) : Next(NotKnownLatest(Previous(D))) {}
+: Link(NotKnownLatest(reinterpret_cast())) {}
+DeclLink(PreviousTag, decl_type *D) : Link(NotKnownLatest(Previous(D))) {}
-bool NextIsPrevious() const {
- return Next.is() &&
+bool isFirst() const {
+ return Link.is() ||
// FIXME: 'template' is required on the next line due to an
// apparent clang bug.
- Next.get().template is();
+ Link.get().template is();
}
-bool NextIsLatest() const { return !NextIsPrevious(); }
-
-decl_type *getNext(const decl_type *D) const {
- if (Next.is()) {
-NotKnownLatest NKL = Next.get();
+decl_type *getPrevious(const decl_type *D) const {
+ if (Link.is()) {
+NotKnownLatest NKL = Link.get();
if (NKL.is())
return static_cast(NKL.get());
// Allocate the generational 'most recent' cache now, if needed.
-Next = KnownLatest(*reinterpret_cast(
+Link = KnownLatest(*reinterpret_cast(
NKL.get()),
const_cast(D));
}
- return static_cast(Next.get().get(D));
+ return static_cast(Link.get().get(D));
}
void setPrevious(decl_type *D) {
- assert(NextIsPrevious() && "decl became non-canonical unexpectedly");
- Next = Previous(D);
+ assert(!isFirst() && "decl became non-canonical unexpectedly");
+ Link = Previous(D);
}
void setLatest(decl_type *D) {
- assert(NextIsLatest() && "decl became canonical unexpectedly");
- if (Next.is()) {
-NotKnownLatest NKL = Next.get();
-Next = KnownLatest(*reinterpret_cast(
+ assert(isFirst() && "decl became canonical unexpectedly");
+ if (Link.is()) {
+NotKnownLatest NKL = Link.get();
+Link = KnownLatest(*reinterpret_cast(
NKL.get()),
D);
} else {
-auto Latest = Next.get();
+auto Latest = Link.get();
Latest.set(D);
-Next = Latest;
+Link = Latest;
}
}
-void markIncomplete() { Next.get().markIncomplete(); }
+void markIncomplete() { Link.get().markIncomplete(); }
Decl *getLatestNotUpdated() const {
- assert(NextIsLatest() && "expected a canonical decl");
- if (Next.is())
+ assert(isFirst() && "expected a canonical decl");
+ if (Link.is())
return nullptr;
- return Next.get().getNotUpdated();
+ return Link.get().getNotUpdated();
}
};
@@ -178,8 +174,8 @@
/// Points to the next redeclaration in the chain.
///
- /// If NextIsPrevious() is true, this is a link to the previous declaration
- /// of this same Decl. If NextIsLatest() is true, this is the first
+ /// If isFirst() is false, this is a link to the previous declaration
+ /// of this same Decl. If isFirst() is true, this is the first
/// declaration and Link points to the latest declaration. For example:
///
/// #1 int f(int x, int y
[PATCH] D47344: LWG 2843 "Unclear behavior of std::pmr::memory_resource::do_allocate()"
Quuxplusone added inline comments.
Comment at: src/experimental/memory_resource.cpp:48
+#ifdef _LIBCPP_HAS_NO_ALIGNED_ALLOCATION
+if (__size != 0 && !is_aligned_to(result, __align)) {
+_VSTD::__libcpp_deallocate(result, __align);
Quuxplusone wrote:
> EricWF wrote:
> > Also, I'm not sure about the `size != 0` check. The returned pointer may be
> > non-null and incorrectly aligned.
> This was covered in my commit message/summary: "If n == 0, return an
> unspecified result."
> However, I am chagrined to state that I have no idea where I got that idea! I
> can't find support for that anywhere in the current Standard (although I'm
> not sure if I missed it).
>
> We must choose here between "allocating zero bytes sometimes returns an
> underaligned pointer" and "allocating zero bytes sometimes throws bad_alloc."
> Neither one seems like good QoI. But as I no longer see why I thought
> "underaligned pointer" was permitted, I will change it to "sometimes throws
> bad_alloc" and re-upload.
Hm! This and your other comment play into each other unfortunately well. When
`size == 0`, the pointer we get back from `__libcpp_allocate` actually points
to zero bytes, which means that when I call `std::align` on it, I invoke
undefined behavior.
My new theory is "I don't care about that undefined behavior." I'd still
rather take undefined-behavior-in-a-rare-case over
implementation-defined-behavior-in-all-cases, and I'm too lazy to implement
implementation-defined-behavior-only-in-a-rare-case unless you tell me to. :P
Repository:
rCXX libc++
https://reviews.llvm.org/D47344
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47344: LWG 2843 "Unclear behavior of std::pmr::memory_resource::do_allocate()"
Quuxplusone updated this revision to Diff 154041.
Quuxplusone added a comment.
Updated to no longer check "size != 0".
Also rolled in some drive-by cosmetic refactoring that I'd done later in my
branch: these functions aren't in a public header and don't need to be uglified.
Repository:
rCXX libc++
https://reviews.llvm.org/D47344
Files:
src/experimental/memory_resource.cpp
Index: src/experimental/memory_resource.cpp
===
--- src/experimental/memory_resource.cpp
+++ src/experimental/memory_resource.cpp
@@ -8,6 +8,7 @@
//===--===//
#include "experimental/memory_resource"
+#include "memory"
#ifndef _LIBCPP_HAS_NO_ATOMIC_HEADER
#include "atomic"
@@ -23,38 +24,47 @@
// new_delete_resource()
+#ifdef _LIBCPP_HAS_NO_ALIGNED_ALLOCATION
+static bool is_aligned_to(void *ptr, size_t align)
+{
+void *p2 = ptr;
+size_t space = 1;
+void *result = _VSTD::align(align, 1, p2, space);
+return (result == ptr);
+}
+#endif
+
class _LIBCPP_TYPE_VIS __new_delete_memory_resource_imp
: public memory_resource
{
-public:
-~__new_delete_memory_resource_imp() = default;
-
-protected:
-virtual void* do_allocate(size_t __size, size_t __align)
-{ return _VSTD::__libcpp_allocate(__size, __align); /* FIXME */}
+void *do_allocate(size_t bytes, size_t align) override
+{
+void *result = _VSTD::__libcpp_allocate(bytes, align);
+#ifdef _LIBCPP_HAS_NO_ALIGNED_ALLOCATION
+if (!is_aligned_to(result, align)) {
+_VSTD::__libcpp_deallocate(result, align);
+__throw_bad_alloc();
+}
+#endif
+return result;
+}
-virtual void do_deallocate(void * __p, size_t, size_t __align)
-{ _VSTD::__libcpp_deallocate(__p, __align); /* FIXME */ }
+void do_deallocate(void *p, size_t, size_t align) override
+{ _VSTD::__libcpp_deallocate(p, align); }
-virtual bool do_is_equal(memory_resource const & __other) const _NOEXCEPT
-{ return &__other == this; }
+bool do_is_equal(const memory_resource& other) const _NOEXCEPT override
+{ return == this; }
};
// null_memory_resource()
class _LIBCPP_TYPE_VIS __null_memory_resource_imp
: public memory_resource
{
-public:
-~__null_memory_resource_imp() = default;
-
-protected:
-virtual void* do_allocate(size_t, size_t) {
-__throw_bad_alloc();
-}
-virtual void do_deallocate(void *, size_t, size_t) {}
-virtual bool do_is_equal(memory_resource const & __other) const _NOEXCEPT
-{ return &__other == this; }
+void *do_allocate(size_t, size_t) override { __throw_bad_alloc(); }
+void do_deallocate(void *, size_t, size_t) override {}
+bool do_is_equal(const memory_resource& other) const _NOEXCEPT override
+{ return == this; }
};
namespace {
Index: src/experimental/memory_resource.cpp
===
--- src/experimental/memory_resource.cpp
+++ src/experimental/memory_resource.cpp
@@ -8,6 +8,7 @@
//===--===//
#include "experimental/memory_resource"
+#include "memory"
#ifndef _LIBCPP_HAS_NO_ATOMIC_HEADER
#include "atomic"
@@ -23,38 +24,47 @@
// new_delete_resource()
+#ifdef _LIBCPP_HAS_NO_ALIGNED_ALLOCATION
+static bool is_aligned_to(void *ptr, size_t align)
+{
+void *p2 = ptr;
+size_t space = 1;
+void *result = _VSTD::align(align, 1, p2, space);
+return (result == ptr);
+}
+#endif
+
class _LIBCPP_TYPE_VIS __new_delete_memory_resource_imp
: public memory_resource
{
-public:
-~__new_delete_memory_resource_imp() = default;
-
-protected:
-virtual void* do_allocate(size_t __size, size_t __align)
-{ return _VSTD::__libcpp_allocate(__size, __align); /* FIXME */}
+void *do_allocate(size_t bytes, size_t align) override
+{
+void *result = _VSTD::__libcpp_allocate(bytes, align);
+#ifdef _LIBCPP_HAS_NO_ALIGNED_ALLOCATION
+if (!is_aligned_to(result, align)) {
+_VSTD::__libcpp_deallocate(result, align);
+__throw_bad_alloc();
+}
+#endif
+return result;
+}
-virtual void do_deallocate(void * __p, size_t, size_t __align)
-{ _VSTD::__libcpp_deallocate(__p, __align); /* FIXME */ }
+void do_deallocate(void *p, size_t, size_t align) override
+{ _VSTD::__libcpp_deallocate(p, align); }
-virtual bool do_is_equal(memory_resource const & __other) const _NOEXCEPT
-{ return &__other == this; }
+bool do_is_equal(const memory_resource& other) const _NOEXCEPT override
+{ return == this; }
};
// null_memory_resource()
class _LIBCPP_TYPE_VIS __null_memory_resource_imp
: public memory_resource
{
-public:
-~__null_memory_resource_imp() = default;
-
-protected:
-virtual void*
[PATCH] D47344: LWG 2843 "Unclear behavior of std::pmr::memory_resource::do_allocate()"
Quuxplusone added inline comments.
Comment at: src/experimental/memory_resource.cpp:29
+static bool is_aligned_to(void *ptr, size_t align)
+{
+void *p2 = ptr;
EricWF wrote:
> Wait, can't this be written `reinterpret_cast(ptr) % align == 0`?
Yes on sane platforms, but that's technically implementation-defined behavior:
the low bits of the uintptr_t may not correspond to the "alignment" of the
original pointer (for example, on some hypothetical tagged-pointer
architecture?). I don't know if libc++ cares about those platforms, but it
seems like `std::align` was added specifically to solve this problem in a
portable way, so I thought it would be best to use it.
If you reply and say "yeah but please change it anyway," I'll change it anyway.
:)
Comment at: src/experimental/memory_resource.cpp:48
+#ifdef _LIBCPP_HAS_NO_ALIGNED_ALLOCATION
+if (__size != 0 && !is_aligned_to(result, __align)) {
+_VSTD::__libcpp_deallocate(result, __align);
EricWF wrote:
> Also, I'm not sure about the `size != 0` check. The returned pointer may be
> non-null and incorrectly aligned.
This was covered in my commit message/summary: "If n == 0, return an
unspecified result."
However, I am chagrined to state that I have no idea where I got that idea! I
can't find support for that anywhere in the current Standard (although I'm not
sure if I missed it).
We must choose here between "allocating zero bytes sometimes returns an
underaligned pointer" and "allocating zero bytes sometimes throws bad_alloc."
Neither one seems like good QoI. But as I no longer see why I thought
"underaligned pointer" was permitted, I will change it to "sometimes throws
bad_alloc" and re-upload.
Repository:
rCXX libc++
https://reviews.llvm.org/D47344
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r336240 - PR33924: merge local declarations that have linkage of some kind within
Author: rsmith
Date: Tue Jul 3 19:25:38 2018
New Revision: 336240
URL: http://llvm.org/viewvc/llvm-project?rev=336240=rev
Log:
PR33924: merge local declarations that have linkage of some kind within
merged function definitions; also merge functions with deduced return
types.
This seems like two independent fixes, but unfortunately they are hard
to separate because it's challenging to reliably test either one of them
without also testing the other.
A complication arises with deduced return type support: we need the type
of the function in order to know how to merge it, but we can't load the
actual type of the function because it might reference an entity
declared within the function (and we need to have already merged the
function to correctly merge that entity, which we would need to do to
determine if the function types match). So we instead compare the
declared function type when merging functions, and defer loading the
actual type of a function with a deduced type until we've finished
loading and merging the function.
This reverts r336175, reinstating r336021, with one change (for PR38015):
we look at the TypeSourceInfo of the first-so-far declaration of each
function when considering whether to merge two functions. This works
around a problem where the calling convention in the TypeSourceInfo for
subsequent redeclarations may not match if it was implicitly adjusted.
Added:
cfe/trunk/test/Modules/merge-deduced-return.cpp
cfe/trunk/test/Modules/merge-lambdas.cpp
cfe/trunk/test/Modules/merge-static-locals.cpp
Modified:
cfe/trunk/include/clang/Serialization/ASTReader.h
cfe/trunk/lib/Serialization/ASTCommon.cpp
cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
cfe/trunk/test/Modules/cxx-dtor.cpp
cfe/trunk/test/Modules/friend-definition-2.cpp
cfe/trunk/test/Modules/pr27401.cpp
Modified: cfe/trunk/include/clang/Serialization/ASTReader.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTReader.h?rev=336240=336239=336240=diff
==
--- cfe/trunk/include/clang/Serialization/ASTReader.h (original)
+++ cfe/trunk/include/clang/Serialization/ASTReader.h Tue Jul 3 19:25:38 2018
@@ -546,7 +546,7 @@ private:
/// Mergeable declaration contexts that have anonymous declarations
/// within them, and those anonymous declarations.
- llvm::DenseMap>
+ llvm::DenseMap>
AnonymousDeclarationsForMerging;
struct FileDeclsInfo {
Modified: cfe/trunk/lib/Serialization/ASTCommon.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTCommon.cpp?rev=336240=336239=336240=diff
==
--- cfe/trunk/lib/Serialization/ASTCommon.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTCommon.cpp Tue Jul 3 19:25:38 2018
@@ -419,9 +419,21 @@ bool serialization::needsAnonymousDeclar
return true;
}
- // Otherwise, we only care about anonymous class members.
+ // At block scope, we number everything that we need to deduplicate, since we
+ // can't just use name matching to keep things lined up.
+ // FIXME: This is only necessary for an inline function or a template or
+ // similar.
+ if (D->getLexicalDeclContext()->isFunctionOrMethod()) {
+if (auto *VD = dyn_cast(D))
+ return VD->isStaticLocal();
+// FIXME: What about CapturedDecls (and declarations nested within them)?
+return isa(D) || isa(D);
+ }
+
+ // Otherwise, we only care about anonymous class members / block-scope decls.
+ // FIXME: We need to handle lambdas and blocks within inline / templated
+ // variables too.
if (D->getDeclName() || !isa(D->getLexicalDeclContext()))
return false;
return isa(D) || isa(D);
}
-
Modified: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=336240=336239=336240=diff
==
--- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Tue Jul 3 19:25:38 2018
@@ -87,7 +87,7 @@ namespace clang {
using RecordData = ASTReader::RecordData;
-TypeID TypeIDForTypeDecl = 0;
+TypeID DeferredTypeID = 0;
unsigned AnonymousDeclNumber;
GlobalDeclID NamedDeclForTagDecl = 0;
IdentifierInfo *TypedefNameForLinkage = nullptr;
@@ -177,6 +177,8 @@ namespace clang {
void MergeDefinitionData(ObjCProtocolDecl *D,
struct ObjCProtocolDecl::DefinitionData &);
+static DeclContext *getPrimaryDCForAnonymousDecl(DeclContext *LexicalDC);
+
static NamedDecl *getAnonymousDeclForMerging(ASTReader ,
DeclContext *DC,
unsigned Index);
@@ -528,7 +530,7 @@ void ASTDeclReader::Visit(Decl *D) {
if (auto *TD =
[PATCH] D48908: [clang-doc] Pass over function-internal declarations
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-doc/Mapper.cpp:32 + // Skip function-internal decls. + if (const auto *F = D->getParentFunctionOrMethod()) +return true; Type is not easily deducible, so please don't use auto. https://reviews.llvm.org/D48908 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47344: LWG 2843 "Unclear behavior of std::pmr::memory_resource::do_allocate()"
EricWF requested changes to this revision.
EricWF added a comment.
This revision now requires changes to proceed.
Actually, I spotted a couple of issues. Requesting changes.
Comment at: src/experimental/memory_resource.cpp:29
+static bool is_aligned_to(void *ptr, size_t align)
+{
+void *p2 = ptr;
Wait, can't this be written `reinterpret_cast(ptr) % align == 0`?
Comment at: src/experimental/memory_resource.cpp:48
+#ifdef _LIBCPP_HAS_NO_ALIGNED_ALLOCATION
+if (__size != 0 && !is_aligned_to(result, __align)) {
+_VSTD::__libcpp_deallocate(result, __align);
Also, I'm not sure about the `size != 0` check. The returned pointer may be
non-null and incorrectly aligned.
Repository:
rCXX libc++
https://reviews.llvm.org/D47344
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48909: [clang-doc] Update BitcodeReader to use llvm::Error
Eugene.Zelenko added inline comments.
Comment at: clang-tools-extra/clang-doc/BitcodeReader.cpp:13
#include "llvm/ADT/Optional.h"
#include "llvm/Support/raw_ostream.h"
Please include Support/Error.h and utility
Comment at: clang-tools-extra/clang-doc/BitcodeReader.h:25
#include "llvm/Bitcode/BitstreamReader.h"
namespace clang {
Please also include Support/Error.h
https://reviews.llvm.org/D48909
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48894: [AST] Rename some Redeclarable functions to reduce confusion
rsmith added inline comments. Comment at: include/clang/AST/Redeclarable.h:106 -mutable llvm::PointerUnion Next; +mutable llvm::PointerUnion Prev; I think this is still a confusing name, because it points either to the previous declaration or to the latest one. How about just calling this `Link`? Or if you want to be more explicit, `PrevOrLatest`. (But that name is still slightly inaccurate due to the `UninitializedLatest` case.) Comment at: include/clang/AST/Redeclarable.h:117-120 + return !(Prev.is() && + // FIXME: 'template' is required on the next line due to an + // apparent clang bug. + Prev.get().template is()); This would be simpler and more obvious as: ``` return Prev.is() || // FIXME: 'template' is required on the next line due to an // apparent clang bug. Prev.get().template is()); ``` Repository: rC Clang https://reviews.llvm.org/D48894 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47344: LWG 2843 "Unclear behavior of std::pmr::memory_resource::do_allocate()"
EricWF accepted this revision. EricWF added a comment. This revision is now accepted and ready to land. LGTM. I'll add a test when I commit this. Repository: rCXX libc++ https://reviews.llvm.org/D47344 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48734: [Sema] Consider all format_arg attributes.
This revision was automatically updated to reflect the committed changes.
Closed by commit rC336239: [Sema] Consider all format_arg attributes. (authored
by Meinersbur, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D48734?vs=153643=154038#toc
Repository:
rC Clang
https://reviews.llvm.org/D48734
Files:
lib/Sema/SemaChecking.cpp
test/Sema/attr-format_arg.c
Index: test/Sema/attr-format_arg.c
===
--- test/Sema/attr-format_arg.c
+++ test/Sema/attr-format_arg.c
@@ -4,10 +4,27 @@
const char* f(const char *s) __attribute__((format_arg(1)));
+const char *h(const char *msg1, const char *msg2)
+__attribute__((__format_arg__(1))) __attribute__((__format_arg__(2)));
+
void g(const char *s) {
printf("%d", 123);
printf("%d %d", 123); // expected-warning{{more '%' conversions than data
arguments}}
printf(f("%d"), 123);
printf(f("%d %d"), 123); // expected-warning{{more '%' conversions than data
arguments}}
+
+ printf(h(
+"", // expected-warning {{format string is empty}}
+"" // expected-warning {{format string is empty}}
+ ), 123);
+ printf(h(
+"%d",
+"" // expected-warning {{format string is empty}}
+ ), 123);
+ printf(h(
+"", // expected-warning {{format string is empty}}
+"%d"
+ ), 123);
+ printf(h("%d", "%d"), 123);
}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -5518,13 +5518,22 @@
case Stmt::CXXMemberCallExprClass: {
const CallExpr *CE = cast(E);
if (const NamedDecl *ND =
dyn_cast_or_null(CE->getCalleeDecl())) {
- if (const FormatArgAttr *FA = ND->getAttr()) {
+ bool IsFirst = true;
+ StringLiteralCheckType CommonResult;
+ for (const auto *FA : ND->specific_attrs()) {
const Expr *Arg = CE->getArg(FA->getFormatIdx().getASTIndex());
-return checkFormatStringExpr(S, Arg, Args,
- HasVAListArg, format_idx, firstDataArg,
- Type, CallType, InFunctionCall,
- CheckedVarArgs, UncoveredArg, Offset);
- } else if (const FunctionDecl *FD = dyn_cast(ND)) {
+StringLiteralCheckType Result = checkFormatStringExpr(
+S, Arg, Args, HasVAListArg, format_idx, firstDataArg, Type,
+CallType, InFunctionCall, CheckedVarArgs, UncoveredArg, Offset);
+if (IsFirst) {
+ CommonResult = Result;
+ IsFirst = false;
+}
+ }
+ if (!IsFirst)
+return CommonResult;
+
+ if (const auto *FD = dyn_cast(ND)) {
unsigned BuiltinID = FD->getBuiltinID();
if (BuiltinID == Builtin::BI__builtin___CFStringMakeConstantString ||
BuiltinID == Builtin::BI__builtin___NSStringMakeConstantString) {
Index: test/Sema/attr-format_arg.c
===
--- test/Sema/attr-format_arg.c
+++ test/Sema/attr-format_arg.c
@@ -4,10 +4,27 @@
const char* f(const char *s) __attribute__((format_arg(1)));
+const char *h(const char *msg1, const char *msg2)
+__attribute__((__format_arg__(1))) __attribute__((__format_arg__(2)));
+
void g(const char *s) {
printf("%d", 123);
printf("%d %d", 123); // expected-warning{{more '%' conversions than data arguments}}
printf(f("%d"), 123);
printf(f("%d %d"), 123); // expected-warning{{more '%' conversions than data arguments}}
+
+ printf(h(
+"", // expected-warning {{format string is empty}}
+"" // expected-warning {{format string is empty}}
+ ), 123);
+ printf(h(
+"%d",
+"" // expected-warning {{format string is empty}}
+ ), 123);
+ printf(h(
+"", // expected-warning {{format string is empty}}
+"%d"
+ ), 123);
+ printf(h("%d", "%d"), 123);
}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -5518,13 +5518,22 @@
case Stmt::CXXMemberCallExprClass: {
const CallExpr *CE = cast(E);
if (const NamedDecl *ND = dyn_cast_or_null(CE->getCalleeDecl())) {
- if (const FormatArgAttr *FA = ND->getAttr()) {
+ bool IsFirst = true;
+ StringLiteralCheckType CommonResult;
+ for (const auto *FA : ND->specific_attrs()) {
const Expr *Arg = CE->getArg(FA->getFormatIdx().getASTIndex());
-return checkFormatStringExpr(S, Arg, Args,
- HasVAListArg, format_idx, firstDataArg,
- Type, CallType, InFunctionCall,
- CheckedVarArgs, UncoveredArg, Offset);
- } else if (const FunctionDecl *FD = dyn_cast(ND)) {
+StringLiteralCheckType Result = checkFormatStringExpr(
+S, Arg, Args, HasVAListArg, format_idx, firstDataArg, Type,
+
[PATCH] D48661: [Fixed Point Arithmetic] Fixed Point Constant
rjmccall added a comment.
Thanks, much appreciated. A couple more style changes that I noticed and this
will be LGTM, although you should also make sure you have Bevin Hansson's
approval.
Comment at: include/clang/AST/ASTContext.h:33
#include "clang/Basic/AddressSpaces.h"
+#include "clang/Basic/FixedPoint.h"
#include "clang/Basic/IdentifierTable.h"
Please forward-declare `FixedPointSemantics` and `APFixedPoint` instead of
including them here. This file is `#include`d into almost the entire project,
but relatively few files will actually need to use any of the functionality of
`APFixedPoint`. Among other things, it will make your life much happier
bringing up this feature if every tiny change to the `APFixedPoint` interface
doesn't force a full recompile.
Comment at: include/clang/AST/ASTContext.h:1953
unsigned char getFixedPointIBits(QualType Ty) const;
+ FixedPointSemantics getFixedPointSema(QualType Ty) const;
+ APFixedPoint getFixedPointMax(QualType Ty) const;
Please spell out "semantics" here.
Comment at: include/clang/Basic/FixedPoint.h:41
+ APFixedPoint(const llvm::APSInt , unsigned Scale,
+ enum FixedPointSemantics Sema)
+ : Val(Val), Scale(Scale), Sema(Sema) {}
rjmccall wrote:
> rjmccall wrote:
> > Why the elaborated-type-specifier?
> Similarly, this should be renamed to `getIntegralBits` to follow the normal
> LLVM method-naming guidelines.
>
> Also, please go ahead and hide all the members here behind getters and
> setters. It's better to reserve flexibility in advance than to have to
> rewrite a bunch of code later.
All of these `inline`s are unnecessary.
Repository:
rC Clang
https://reviews.llvm.org/D48661
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48909: [clang-doc] Update BitcodeReader to use llvm::Error
juliehockett created this revision.
juliehockett added reviewers: ioeric, lebedev.ri.
juliehockett added a project: clang-tools-extra.
Replace booleans with the more descriptive llvm::Error or llvm::Expected
https://reviews.llvm.org/D48909
Files:
clang-tools-extra/clang-doc/BitcodeReader.cpp
clang-tools-extra/clang-doc/BitcodeReader.h
clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
Index: clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
===
--- clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
+++ clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
@@ -201,7 +201,11 @@
llvm::BitstreamCursor Stream(Value);
doc::ClangDocBitcodeReader Reader(Stream);
auto Infos = Reader.readBitcode();
-for (auto : Infos) {
+if (!Infos) {
+ llvm::errs() << llvm::toString(Infos.takeError()) << "\n";
+ return;
+}
+for (auto : Infos.get()) {
auto R =
MapOutput.try_emplace(Key, std::vector>());
R.first->second.emplace_back(std::move(I));
Index: clang-tools-extra/clang-doc/BitcodeReader.h
===
--- clang-tools-extra/clang-doc/BitcodeReader.h
+++ clang-tools-extra/clang-doc/BitcodeReader.h
@@ -19,6 +19,7 @@
#include "BitcodeWriter.h"
#include "Representation.h"
#include "clang/AST/AST.h"
+#include "llvm/ADT/Optional.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/Bitcode/BitstreamReader.h"
@@ -31,36 +32,37 @@
ClangDocBitcodeReader(llvm::BitstreamCursor ) : Stream(Stream) {}
// Main entry point, calls readBlock to read each block in the given stream.
- std::vector> readBitcode();
+ llvm::Expected>> readBitcode();
private:
enum class Cursor { BadBlock = 1, Record, BlockEnd, BlockBegin };
// Top level parsing
- bool validateStream();
- bool readVersion();
- bool readBlockInfoBlock();
+ llvm::Error validateStream();
+ llvm::Error readVersion();
+ llvm::Error readBlockInfoBlock();
// Read a block of records into a single Info struct, calls readRecord on each
// record found.
- template bool readBlock(unsigned ID, T I);
+ template llvm::Error readBlock(unsigned ID, T I);
// Step through a block of records to find the next data field.
- template bool readSubBlock(unsigned ID, T I);
+ template llvm::Error readSubBlock(unsigned ID, T I);
// Read record data into the given Info data field, calling the appropriate
// parseRecord functions to parse and store the data.
- template bool readRecord(unsigned ID, T I);
+ template llvm::Error readRecord(unsigned ID, T I);
// Allocate the relevant type of info and add read data to the object.
- template std::unique_ptr createInfo(unsigned ID);
+ template
+ llvm::Expected> createInfo(unsigned ID);
// Helper function to step through blocks to find and dispatch the next record
// or block to be read.
Cursor skipUntilRecordOrBlock(unsigned );
// Helper function to set up the approriate type of Info.
- std::unique_ptr readBlockToInfo(unsigned ID);
+ llvm::Expected> readBlockToInfo(unsigned ID);
llvm::BitstreamCursor
Optional BlockInfo;
Index: clang-tools-extra/clang-doc/BitcodeReader.cpp
===
--- clang-tools-extra/clang-doc/BitcodeReader.cpp
+++ clang-tools-extra/clang-doc/BitcodeReader.cpp
@@ -17,130 +17,143 @@
using Record = llvm::SmallVector;
-bool decodeRecord(Record R, llvm::SmallVectorImpl ,
- llvm::StringRef Blob) {
+llvm::Error decodeRecord(Record R, llvm::SmallVectorImpl ,
+ llvm::StringRef Blob) {
Field.assign(Blob.begin(), Blob.end());
- return true;
+ return llvm::Error::success();
}
-bool decodeRecord(Record R, SymbolID , llvm::StringRef Blob) {
+llvm::Error decodeRecord(Record R, SymbolID , llvm::StringRef Blob) {
if (R[0] != BitCodeConstants::USRHashSize)
-return false;
+return llvm::make_error("Incorrect USR size.\n",
+ llvm::inconvertibleErrorCode());
// First position in the record is the length of the following array, so we
// copy the following elements to the field.
for (int I = 0, E = R[0]; I < E; ++I)
Field[I] = R[I + 1];
- return true;
+ return llvm::Error::success();
}
-bool decodeRecord(Record R, bool , llvm::StringRef Blob) {
+llvm::Error decodeRecord(Record R, bool , llvm::StringRef Blob) {
Field = R[0] != 0;
- return true;
+ return llvm::Error::success();
}
-bool decodeRecord(Record R, int , llvm::StringRef Blob) {
+llvm::Error decodeRecord(Record R, int , llvm::StringRef Blob) {
if (R[0] > INT_MAX)
-return false;
+return llvm::make_error("Integer too large to parse.\n",
+ llvm::inconvertibleErrorCode());
Field = (int)R[0];
- return true;
+ return llvm::Error::success();
}
-bool decodeRecord(Record R,
[PATCH] D48908: [clang-doc] Pass over function-internal declarations
juliehockett created this revision.
juliehockett added reviewers: ioeric, lebedev.ri.
juliehockett added a project: clang-tools-extra.
Things declared internally to functions shouldn't be documented, and so bail
early if encountered.
https://reviews.llvm.org/D48908
Files:
clang-tools-extra/clang-doc/Mapper.cpp
clang-tools-extra/test/clang-doc/bc-record.cpp
clang-tools-extra/test/clang-doc/mapper-class-in-function.cpp
clang-tools-extra/test/clang-doc/yaml-record.cpp
Index: clang-tools-extra/test/clang-doc/yaml-record.cpp
===
--- clang-tools-extra/test/clang-doc/yaml-record.cpp
+++ clang-tools-extra/test/clang-doc/yaml-record.cpp
@@ -16,15 +16,14 @@
// RUN: cat %t/docs/X.yaml | FileCheck %s --check-prefix=CHECK-X
// RUN: cat %t/docs/X/Y.yaml | FileCheck %s --check-prefix=CHECK-Y
// RUN: cat %t/docs/H.yaml | FileCheck %s --check-prefix=CHECK-H
-// RUN: cat %t/docs/H/I.yaml | FileCheck %s --check-prefix=CHECK-I
union A { int X; int Y; };
// CHECK-A: ---
// CHECK-A-NEXT: USR: 'ACE81AFA6627B4CEF2B456FB6E1252925674AF7E'
// CHECK-A-NEXT: Name:'A'
// CHECK-A-NEXT: DefLocation:
-// CHECK-A-NEXT: LineNumber: 21
+// CHECK-A-NEXT: LineNumber: [[@LINE-6]]
// CHECK-A-NEXT: Filename:'{{.*}}'
// CHECK-A-NEXT: TagType: Union
// CHECK-A-NEXT: Members:
@@ -43,7 +42,7 @@
// CHECK-B-NEXT: USR: 'FC07BD34D5E77782C263FA97929EA8753740'
// CHECK-B-NEXT: Name:'B'
// CHECK-B-NEXT: DefLocation:
-// CHECK-B-NEXT: LineNumber: 40
+// CHECK-B-NEXT: LineNumber: [[@LINE-6]]
// CHECK-B-NEXT: Filename:'{{.*}}'
// CHECK-B-NEXT: Members:
// CHECK-B-NEXT: - 'X'
@@ -56,7 +55,7 @@
// CHECK-BC-NEXT: USR: '1E3438A08BA22025C0B46289FF0686F92C8924C5'
// CHECK-BC-NEXT: Name:'Bc'
// CHECK-BC-NEXT: DefLocation:
-// CHECK-BC-NEXT: LineNumber: 53
+// CHECK-BC-NEXT: LineNumber: [[@LINE-6]]
// CHECK-BC-NEXT: Filename:'{{.*}}'
// CHECK-BC-NEXT: Scoped: true
// CHECK-BC-NEXT: Members:
@@ -70,7 +69,7 @@
// CHECK-C-NEXT: USR: '06B5F6A19BA9F6A832E127C9968282B94619B210'
// CHECK-C-NEXT: Name:'C'
// CHECK-C-NEXT: DefLocation:
-// CHECK-C-NEXT: LineNumber: 67
+// CHECK-C-NEXT: LineNumber: [[@LINE-6]]
// CHECK-C-NEXT: Filename:'{{.*}}'
// CHECK-C-NEXT: Members:
// CHECK-C-NEXT: - Type:
@@ -84,7 +83,7 @@
// CHECK-D-NEXT: USR: '0921737541208B8FA9BB42B60F78AC1D779AA054'
// CHECK-D-NEXT: Name:'D'
// CHECK-D-NEXT: DefLocation:
-// CHECK-D-NEXT: LineNumber: 81
+// CHECK-D-NEXT: LineNumber: [[@LINE-6]]
// CHECK-D-NEXT: Filename:'{{.*}}'
// CHECK-D-NEXT: TagType: Class
// CHECK-D-NEXT: ...
@@ -101,7 +100,7 @@
// CHECK-ECON-NEXT: Name:'E'
// CHECK-ECON-NEXT: USR: '289584A8E0FF4178A794622A547AA622503967A1'
// CHECK-ECON-NEXT: DefLocation:
-// CHECK-ECON-NEXT: LineNumber: 94
+// CHECK-ECON-NEXT: LineNumber: [[@LINE-10]]
// CHECK-ECON-NEXT: Filename:'{{.*}}'
// CHECK-ECON-NEXT: IsMethod:true
// CHECK-ECON-NEXT: Parent:
@@ -123,7 +122,7 @@
// CHECK-EDES-NEXT: Name:'E'
// CHECK-EDES-NEXT: USR: '289584A8E0FF4178A794622A547AA622503967A1'
// CHECK-EDES-NEXT: DefLocation:
-// CHECK-EDES-NEXT: LineNumber: 116
+// CHECK-EDES-NEXT: LineNumber: [[@LINE-10]]
// CHECK-EDES-NEXT: Filename:'{{.*}}'
// CHECK-EDES-NEXT: IsMethod:true
// CHECK-EDES-NEXT: Parent:
@@ -144,7 +143,7 @@
// CHECK-E-NEXT: USR: '289584A8E0FF4178A794622A547AA622503967A1'
// CHECK-E-NEXT: Name:'E'
// CHECK-E-NEXT: DefLocation:
-// CHECK-E-NEXT: LineNumber: 92
+// CHECK-E-NEXT: LineNumber: [[@LINE-55]]
// CHECK-E-NEXT: Filename:'{{.*}}'
// CHECK-E-NEXT: TagType: Class
// CHECK-E-NEXT: ...
@@ -159,10 +158,10 @@
// CHECK-EPM-NEXT: Name:'E'
// CHECK-EPM-NEXT: USR: '289584A8E0FF4178A794622A547AA622503967A1'
// CHECK-EPM-NEXT: DefLocation:
-// CHECK-EPM-NEXT: LineNumber: 152
+// CHECK-EPM-NEXT: LineNumber: [[@LINE-10]]
// CHECK-EPM-NEXT: Filename:'{{.*}}'
// CHECK-EPM-NEXT: Location:
-// CHECK-EPM-NEXT: - LineNumber: 140
+// CHECK-EPM-NEXT: - LineNumber: [[@LINE-25]]
// CHECK-EPM-NEXT: Filename:'{{.*}}'
// CHECK-EPM-NEXT: IsMethod:true
// CHECK-EPM-NEXT: Parent:
@@ -180,7 +179,7 @@
// CHECK-F-NEXT: USR: 'E3B54702FABFF4037025BA194FC27C47006330B5'
// CHECK-F-NEXT: Name:'F'
// CHECK-F-NEXT: DefLocation:
-// CHECK-F-NEXT: LineNumber: 177
+// CHECK-F-NEXT: LineNumber: [[@LINE-6]]
//
[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.
vsapsai added inline comments.
Comment at: libcxx/include/memory:1479
+struct __has_construct_missing
+: false_type
+{
Quuxplusone wrote:
> vsapsai wrote:
> > vsapsai wrote:
> > > vsapsai wrote:
> > > > erik.pilkington wrote:
> > > > > vsapsai wrote:
> > > > > > erik.pilkington wrote:
> > > > > > > Shouldn't this be true_type?
> > > > > > I see this is confusing and I'm still struggling how to express it.
> > > > > > The issue is that in C++03 `__has_construct` should be something
> > > > > > like unknown, so that neither `__has_construct` nor `!
> > > > > > __has_construct` evaluate to true because we don't really know if
> > > > > > allocator has construct. This case is covered by the added test, in
> > > > > > C++03 the memcpy specialization was enabled when
> > > > > >
> > > > > > ```
> > > > > > is_same >
> > > > > > || !false_type
> > > > > > ```
> > > > > >
> > > > > > So `is_same` check had no effect and we were using memcpy to
> > > > > > convert between int and float.
> > > > > >
> > > > > > I was considering using something like
> > > > > >
> > > > > > ```lang=c++
> > > > > > typename enable_if
> > > > > > <
> > > > > > (is_same
> > > > > > <
> > > > > > typename _VSTD::remove_const > > > > > allocator_type::value_type>::type,
> > > > > > typename _VSTD::remove_const<_SourceTp>::type
> > > > > > >::value
> > > > > > #ifndef _LIBCPP_CXX03_LANG
> > > > > > || !__has_construct > > > > > _SourceTp>::value
> > > > > > #endif
> > > > > > ) &&
> > > > > > is_trivially_move_constructible<_DestTp>::value,
> > > > > > void
> > > > > > >::type
> > > > > > ```
> > > > > >
> > > > > > But that doesn't look readable to me, so I've introduced ad-hoc
> > > > > > ternary logic with `__has_construct_missing`.
> > > > > Oh I see, yikes! That's a pretty bad bug. I agree that this fix is
> > > > > best then, but can you add a comment explaining this to
> > > > > `__has_construct_missing` for future casual readers? Also, I think we
> > > > > should move the __has_construct_missing bugfix into a different
> > > > > (prerequisite) patch. Seems unrelated to the `const` related
> > > > > optimization below.
> > > > The bug as I described isn't really present now because function
> > > > signature
> > > >
> > > > __construct_range_forward(allocator_type&, _Tp* __begin1, _Tp*
> > > > __end1, _Tp*& __begin2)
> > > >
> > > > works as implicit `is_same` for `__begin1` and `__begin2` types. I
> > > > think it is worth fixing separately and there is a bug with C++03 and
> > > > custom allocators.
> > > Instead of `__has_construct_missing` I've implemented real
> > > `__has_construct` in D48753. But it is stricter in C++03 than in C++11
> > > and later. So it made me think that absence of `construct` with exact
> > > signature isn't a good reason to use memcpy.
> > I was wrong. Now I think the logic for using memcpy should be
> >
> > if types are the same modulo constness
> > and
> > (
> > using default allocator
> > or
> > using custom allocator without `construct`
> > )
> > and
> > is_trivially_move_constructible
> >
> > The purpose of the allocator check is to cover cases when `static
> > construct` would end up calling not user's code but libc++ code that we
> > know can be replaced with memcpy.
> I'd like to request the spelling `__has_trivial_construct_and_destroy T&&>` as described here: https://www.youtube.com/watch?v=MWBfmmg8-Yo=21m45s
> I have an implementation in my fork that might be useful for comparison,
> although even it doesn't add that last `T&&` parameter.
> https://github.com/Quuxplusone/libcxx/commit/34eb0b5c8f03880b94d53b877cbca384783ad65a
>
> What we're *really* interested in, in most cases, is
> `__has_trivial_construct && __has_trivial_destroy`. We don't
> care if there happens to exist an obscure overload such as `A::construct(T*,
> Widget, Gadget)` as long as it is not selected. This is particularly relevant
> to `scoped_allocator_adaptor`, whose `construct` is trivial for primitive
> types but non-trivial for allocator-aware types.
>
> Also, when we write out the template type parameters we care about, we can do
> the decltype stuff really easily, without having to "fudge" the
> metaprogramming and possibly get it wrong. For example, if `A::construct` is
> an overload set, in which case the `&_Xp::construct` on this patch's line
> 1492 will be ill-formed even though a non-trivial `construct` does actually
> exist.
I agree with benefits of using `__has_trivial_construct_and_destroy` in general
but in this case I don't see a need for `__has_trivial_destroy`.
`__construct_range_forward` only copies new elements and it isn't concerned
with destruction. Probably for some cases we can meld destroy+construct
together
[PATCH] D48661: [Fixed Point Arithmetic] Fixed Point Constant
leonardchan updated this revision to Diff 154020.
leonardchan marked 2 inline comments as done.
leonardchan added a comment.
- Renamed `fixedPointSemantics` to `FixedPointSemantics` and hid the members
behind getters
Repository:
rC Clang
https://reviews.llvm.org/D48661
Files:
include/clang/AST/ASTContext.h
include/clang/Basic/FixedPoint.h
include/clang/Basic/TargetInfo.h
lib/AST/ASTContext.cpp
lib/Basic/CMakeLists.txt
lib/Basic/FixedPoint.cpp
lib/Sema/SemaExpr.cpp
test/Frontend/fixed_point_declarations.c
unittests/Basic/CMakeLists.txt
unittests/Basic/FixedPointTest.cpp
Index: unittests/Basic/FixedPointTest.cpp
===
--- /dev/null
+++ unittests/Basic/FixedPointTest.cpp
@@ -0,0 +1,633 @@
+//===- unittests/Basic/FixedPointTest.cpp -- fixed point number tests -===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "clang/Basic/FixedPoint.h"
+#include "llvm/ADT/APSInt.h"
+#include "gtest/gtest.h"
+
+using clang::APFixedPoint;
+using clang::FixedPointSemantics;
+using llvm::APInt;
+using llvm::APSInt;
+
+namespace {
+
+FixedPointSemantics Saturated(FixedPointSemantics Sema) {
+ Sema.setSaturated(true);
+ return Sema;
+}
+
+FixedPointSemantics getSAccSema() {
+ return FixedPointSemantics(/*width=*/16, /*scale=*/7, /*isSigned=*/true,
+ /*isSaturated=*/false,
+ /*hasUnsignedPadding=*/false);
+}
+
+FixedPointSemantics getAccSema() {
+ return FixedPointSemantics(/*width=*/32, /*scale=*/15, /*isSigned=*/true,
+ /*isSaturated=*/false,
+ /*hasUnsignedPadding=*/false);
+}
+
+FixedPointSemantics getLAccSema() {
+ return FixedPointSemantics(/*width=*/64, /*scale=*/31, /*isSigned=*/true,
+ /*isSaturated=*/false,
+ /*hasUnsignedPadding=*/false);
+}
+
+FixedPointSemantics getSFractSema() {
+ return FixedPointSemantics(/*width=*/8, /*scale=*/7, /*isSigned=*/true,
+ /*isSaturated=*/false,
+ /*hasUnsignedPadding=*/false);
+}
+
+FixedPointSemantics getFractSema() {
+ return FixedPointSemantics(/*width=*/16, /*scale=*/15, /*isSigned=*/true,
+ /*isSaturated=*/false,
+ /*hasUnsignedPadding=*/false);
+}
+
+FixedPointSemantics getLFractSema() {
+ return FixedPointSemantics(/*width=*/32, /*scale=*/31, /*isSigned=*/true,
+ /*isSaturated=*/false,
+ /*hasUnsignedPadding=*/false);
+}
+
+FixedPointSemantics getUSAccSema() {
+ return FixedPointSemantics(/*width=*/16, /*scale=*/8, /*isSigned=*/false,
+ /*isSaturated=*/false,
+ /*hasUnsignedPadding=*/false);
+}
+
+FixedPointSemantics getUAccSema() {
+ return FixedPointSemantics(/*width=*/32, /*scale=*/16, /*isSigned=*/false,
+ /*isSaturated=*/false,
+ /*hasUnsignedPadding=*/false);
+}
+
+FixedPointSemantics getULAccSema() {
+ return FixedPointSemantics(/*width=*/64, /*scale=*/32, /*isSigned=*/false,
+ /*isSaturated=*/false,
+ /*hasUnsignedPadding=*/false);
+}
+
+FixedPointSemantics getUSFractSema() {
+ return FixedPointSemantics(/*width=*/8, /*scale=*/8, /*isSigned=*/false,
+ /*isSaturated=*/false,
+ /*hasUnsignedPadding=*/false);
+}
+
+FixedPointSemantics getUFractSema() {
+ return FixedPointSemantics(/*width=*/16, /*scale=*/16, /*isSigned=*/false,
+ /*isSaturated=*/false,
+ /*hasUnsignedPadding=*/false);
+}
+
+FixedPointSemantics getULFractSema() {
+ return FixedPointSemantics(/*width=*/32, /*scale=*/32, /*isSigned=*/false,
+ /*isSaturated=*/false,
+ /*hasUnsignedPadding=*/false);
+}
+
+FixedPointSemantics getPadUSAccSema() {
+ return FixedPointSemantics(/*width=*/16, /*scale=*/7, /*isSigned=*/false,
+ /*isSaturated=*/false,
+ /*hasUnsignedPadding=*/true);
+}
+
+FixedPointSemantics getPadUAccSema() {
+ return FixedPointSemantics(/*width=*/32, /*scale=*/15, /*isSigned=*/false,
+ /*isSaturated=*/false,
+ /*hasUnsignedPadding=*/true);
+}
+
+FixedPointSemantics getPadULAccSema() {
+ return FixedPointSemantics(/*width=*/64, /*scale=*/31, /*isSigned=*/false,
+ /*isSaturated=*/false,
+ /*hasUnsignedPadding=*/true);
+}
+
[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.
vsapsai updated this revision to Diff 154021.
vsapsai added a comment.
- Proper support for custom allocators without `construct`.
https://reviews.llvm.org/D48342
Files:
libcxx/include/memory
libcxx/test/std/containers/sequences/vector/vector.cons/construct_iter_iter_alloc.pass.cpp
libcxx/test/std/containers/sequences/vector/vector.cons/construct_iter_iter_different_value_type.pass.cpp
Index:
libcxx/test/std/containers/sequences/vector/vector.cons/construct_iter_iter_different_value_type.pass.cpp
===
--- /dev/null
+++
libcxx/test/std/containers/sequences/vector/vector.cons/construct_iter_iter_different_value_type.pass.cpp
@@ -0,0 +1,29 @@
+//===--===//
+//
+// 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.
+//
+//===--===//
+
+//
+
+// template vector(InputIter first, InputIter last);
+
+// Initialize a vector with a different value type. Make sure initialization
+// is performed with each element value, not with a memory blob.
+
+#include
+#include
+#include
+#include
+
+int main()
+{
+int array[3] = {0, 1, 2};
+std::vector v(array, array + 3);
+assert(std::fabs(v[0] - 0.0f) < FLT_EPSILON);
+assert(std::fabs(v[1] - 1.0f) < FLT_EPSILON);
+assert(std::fabs(v[2] - 2.0f) < FLT_EPSILON);
+}
Index:
libcxx/test/std/containers/sequences/vector/vector.cons/construct_iter_iter_alloc.pass.cpp
===
---
libcxx/test/std/containers/sequences/vector/vector.cons/construct_iter_iter_alloc.pass.cpp
+++
libcxx/test/std/containers/sequences/vector/vector.cons/construct_iter_iter_alloc.pass.cpp
@@ -195,6 +195,17 @@
//C v(It(arr2), It(std::end(arr2)), a);
}
}
+ {
+using C = std::vector >;
+{
+ ExpectConstructGuard G(1);
+ C v(arr1, arr1 + 1);
+}
+{
+ ExpectConstructGuard G(3);
+ C v(arr2, arr2 + 3);
+}
+ }
#endif
}
Index: libcxx/include/memory
===
--- libcxx/include/memory
+++ libcxx/include/memory
@@ -1651,23 +1651,29 @@
construct(__a, _VSTD::__to_raw_pointer(__begin2), *__begin1);
}
-template
+template
_LIBCPP_INLINE_VISIBILITY
static
typename enable_if
<
-(is_same >::value
-|| !__has_construct::value) &&
- is_trivially_move_constructible<_Tp>::value,
+is_same
+<
+typename _VSTD::remove_const<_SourceTp>::type,
+typename _VSTD::remove_const<_DestTp>::type
+>::value &&
+(is_same::type> >::value
+|| is_same >::value
+|| !__has_construct::value) &&
+ is_trivially_move_constructible<_DestTp>::value,
void
>::type
-__construct_range_forward(allocator_type&, _Tp* __begin1, _Tp* __end1,
_Tp*& __begin2)
+__construct_range_forward(allocator_type&, _SourceTp* __begin1,
_SourceTp* __end1, _DestTp*& __begin2)
{
-typedef typename remove_const<_Tp>::type _Vp;
+typedef typename remove_const<_DestTp>::type _Vp;
ptrdiff_t _Np = __end1 - __begin1;
if (_Np > 0)
{
-_VSTD::memcpy(const_cast<_Vp*>(__begin2), __begin1, _Np *
sizeof(_Tp));
+_VSTD::memcpy(const_cast<_Vp*>(__begin2), __begin1, _Np *
sizeof(_DestTp));
__begin2 += _Np;
}
}
Index: libcxx/test/std/containers/sequences/vector/vector.cons/construct_iter_iter_different_value_type.pass.cpp
===
--- /dev/null
+++ libcxx/test/std/containers/sequences/vector/vector.cons/construct_iter_iter_different_value_type.pass.cpp
@@ -0,0 +1,29 @@
+//===--===//
+//
+// 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.
+//
+//===--===//
+
+//
+
+// template vector(InputIter first, InputIter last);
+
+// Initialize a vector with a different value type. Make sure initialization
+// is performed with each element value, not with a memory blob.
+
+#include
+#include
+#include
+#include
+
+int main()
+{
+int array[3] = {0, 1, 2};
+std::vector v(array, array + 3);
+assert(std::fabs(v[0] - 0.0f) < FLT_EPSILON);
+assert(std::fabs(v[1] - 1.0f) < FLT_EPSILON);
+assert(std::fabs(v[2]
[PATCH] D48845: [Sema] Add fixit for unused lambda captures
acomminos planned changes to this revision. acomminos added a comment. Ah yes, thanks for pointing this out. Some additional logic is going to be necessary to handle capture initializers correctly- I'll look into exposing full source ranges in LambdaCapture to make this more consistent across capture types. Repository: rC Clang https://reviews.llvm.org/D48845 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48863: [Sema] Explain coroutine_traits template in diag
rsmith added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:9051 def err_implied_coroutine_type_not_found : Error< "%0 type was not found; include before defining " "a coroutine">; Maybe we should also remove the "%0 type was not found; " here to avoid the suggestion that declaring that type yourself would be a reasonable way to solve the problem. (We don't say "std::type_info was not found" when a user uses `typeid` without a `#include ` and that doesn't seem to confuse anyone.) For symmetry with what we say for `typeid`, maybe this should be worded "you need to include before defining a coroutine" Repository: rC Clang https://reviews.llvm.org/D48863 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48863: [Sema] Explain coroutine_traits template in diag
rsmith added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:9053 "a coroutine">; +def note_coroutine_types_for_traits_here : Note< + "the coroutine traits class template is being instantiated using the return " modocache wrote: > GorNishanov wrote: > > I am wondering what is the use case here. > > Is it to guard against badly designed standard library? > > > > I played around a little and tried to see how I could get to trigger this > > error with a user-code, but, failed. If I did not specify enough parameters > > it will default to primary template defined in the > That's fair. Here's my rationale: SemaCoroutune.cpp contains diagnostics, > even before this change, in case of a "badly designed standard library". > There's a diagnostic for if coroutine_traits is defined as non-template > class, for example. > > A user would not encounter that diagnostic under any normal circumstance. If > they follow the instructions the compiler gives them, they'll include > `` and be done with it. But since we have the code to > emit the (unlikely to ever be seen) diagnostic anyway, why not make it as > helpful as we can? > > If a user for some reason defines their own `coroutine_traits`, and their > definition only takes a single template argument, and if they define a > coroutine with a single return type and a single parameter, then they'll see > the diagnostic "too many template arguments for class template". At this > point to learn why the heck a `co_return` statement is instantiating a > template, they'd have to either read the compiler source code or the > standard. My rationale is that we might as well save them this step. > > All that being said, this change is just a suggestion, I'm not insisting we > put it in! I drew some inspiration for this change from your Naked Coroutines > talk, in which you used the `co_return` keyword and then followed the > instructions the compiler gave you until you had a working program. Since the > Clang diagnostic tells me in that case that > "std::experimental::coroutine_traits is not defined, you must include > ", I wondered what would happen if a user decided to > just define their own `std::experimental::coroutine_traits` instead. If they > do that, and correct the errors that `coroutine_traits` must be a class and > it must be a template, eventually they could find their way to this > diagnostic. I agree that it's an unlikely case, so feel free to reject this > and I'll close it :) Rather than try to explain after the fact what happened, I think we should do some up-front checking that the `coroutine_traits` template we found looks reasonable. Specifically: walk its template parameter list, and check that it has a non-pack type parameter followed by a pack type parameter. If not, diagnose that we don't support that declaration for `coroutine_traits` with some kind of "unsupported standard library implementation" error. But the purpose of this would be to check that we're being used with a standard library implementation that we understand, not to allow arbitrary end users to declare `coroutine_traits` themselves. (Maybe one day we should add a diagnostic for users writing code in namespace `std`...) Repository: rC Clang https://reviews.llvm.org/D48863 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48753: [libcxx] Use custom allocator's `construct` in C++03 when available.
vsapsai updated this revision to Diff 154018.
vsapsai added a comment.
- Clean up functionality not required by the Standard.
https://reviews.llvm.org/D48753
Files:
libcxx/include/memory
libcxx/test/std/containers/sequences/vector/vector.cons/construct_iter_iter_alloc.pass.cpp
Index: libcxx/test/std/containers/sequences/vector/vector.cons/construct_iter_iter_alloc.pass.cpp
===
--- libcxx/test/std/containers/sequences/vector/vector.cons/construct_iter_iter_alloc.pass.cpp
+++ libcxx/test/std/containers/sequences/vector/vector.cons/construct_iter_iter_alloc.pass.cpp
@@ -50,6 +50,26 @@
#endif
+template
+struct cpp03_allocator : bare_allocator {
+ typedef T value_type;
+ typedef value_type* pointer;
+
+ static bool construct_called;
+
+ void construct(pointer p, const value_type& val)
+ {
+::new(p) value_type(val);
+construct_called = true;
+ }
+
+ std::size_t max_size() const
+ {
+return UINT_MAX / sizeof(T);
+ }
+};
+template bool cpp03_allocator::construct_called = false;
+
void basic_tests() {
{
int a[] = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 8, 7, 6, 5, 4, 3, 1, 0};
@@ -129,9 +149,22 @@
}
void test_ctor_under_alloc() {
-#if TEST_STD_VER >= 11
int arr1[] = {42};
int arr2[] = {1, 101, 42};
+ {
+typedef std::vector > C;
+{
+ cpp03_allocator::construct_called = false;
+ C v(arr1, arr1 + 1);
+ assert(cpp03_allocator::construct_called);
+}
+{
+ cpp03_allocator::construct_called = false;
+ C v(arr2, arr2 + 3);
+ assert(cpp03_allocator::construct_called);
+}
+ }
+#if TEST_STD_VER >= 11
{
using C = TCT::vector<>;
using T = typename C::value_type;
Index: libcxx/include/memory
===
--- libcxx/include/memory
+++ libcxx/include/memory
@@ -1459,24 +1459,28 @@
#else // _LIBCPP_CXX03_LANG
-#ifndef _LIBCPP_HAS_NO_VARIADICS
-
-template
-struct __has_construct
-: false_type
+template
+struct __has_construct_test
{
-};
+private:
+template static true_type __test(void (_Xp::*)(_Pointer, _Args));
+template static false_type __test(...);
-#else // _LIBCPP_HAS_NO_VARIADICS
+template
+static decltype(__test(&_Xp::construct)) __test_exist(decltype(&_Xp::construct));
+template static false_type __test_exist(...);
+
+typedef decltype(__test_exist<_Alloc>(0)) type;
+public:
+static const bool value = type::value;
+};
template
struct __has_construct
-: false_type
+: integral_constant::value>
{
};
-#endif // _LIBCPP_HAS_NO_VARIADICS
-
template
struct __has_destroy
: false_type
@@ -1570,9 +1574,10 @@
}
template
_LIBCPP_INLINE_VISIBILITY
-static void construct(allocator_type&, _Tp* __p, const _A0& __a0)
+static void construct(allocator_type& __a, _Tp* __p, const _A0& __a0)
{
-::new ((void*)__p) _Tp(__a0);
+__construct(__has_construct(),
+__a, __p, __a0);
}
template
_LIBCPP_INLINE_VISIBILITY
@@ -1720,6 +1725,19 @@
{
::new ((void*)__p) _Tp(_VSTD::forward<_Args>(__args)...);
}
+#else // _LIBCPP_HAS_NO_VARIADICS
+template
+_LIBCPP_INLINE_VISIBILITY
+static void __construct(true_type, allocator_type& __a, _Tp* __p,
+const _A0& __a0)
+{__a.construct(__p, __a0);}
+template
+_LIBCPP_INLINE_VISIBILITY
+static void __construct(false_type, allocator_type&, _Tp* __p,
+const _A0& __a0)
+{
+::new ((void*)__p) _Tp(__a0);
+}
#endif // _LIBCPP_HAS_NO_VARIADICS
template
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48863: [Sema] Explain coroutine_traits template in diag
modocache added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:9053 "a coroutine">; +def note_coroutine_types_for_traits_here : Note< + "the coroutine traits class template is being instantiated using the return " GorNishanov wrote: > I am wondering what is the use case here. > Is it to guard against badly designed standard library? > > I played around a little and tried to see how I could get to trigger this > error with a user-code, but, failed. If I did not specify enough parameters > it will default to primary template defined in the That's fair. Here's my rationale: SemaCoroutune.cpp contains diagnostics, even before this change, in case of a "badly designed standard library". There's a diagnostic for if coroutine_traits is defined as non-template class, for example. A user would not encounter that diagnostic under any normal circumstance. If they follow the instructions the compiler gives them, they'll include `` and be done with it. But since we have the code to emit the (unlikely to ever be seen) diagnostic anyway, why not make it as helpful as we can? If a user for some reason defines their own `coroutine_traits`, and their definition only takes a single template argument, and if they define a coroutine with a single return type and a single parameter, then they'll see the diagnostic "too many template arguments for class template". At this point to learn why the heck a `co_return` statement is instantiating a template, they'd have to either read the compiler source code or the standard. My rationale is that we might as well save them this step. All that being said, this change is just a suggestion, I'm not insisting we put it in! I drew some inspiration for this change from your Naked Coroutines talk, in which you used the `co_return` keyword and then followed the instructions the compiler gave you until you had a working program. Since the Clang diagnostic tells me in that case that "std::experimental::coroutine_traits is not defined, you must include ", I wondered what would happen if a user decided to just define their own `std::experimental::coroutine_traits` instead. If they do that, and correct the errors that `coroutine_traits` must be a class and it must be a template, eventually they could find their way to this diagnostic. I agree that it's an unlikely case, so feel free to reject this and I'll close it :) Repository: rC Clang https://reviews.llvm.org/D48863 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48617: [Builtins][Attributes][X86] Tag all X86 builtins with their required vector width. Add a min_vector_width function attribute and tag all x86 instrinsics with it.
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. Aside from a new round of minor doc nits, I think this is looking good. One remaining question I have is whether the attribute should diagnose an argument for a width that's not supported by the target and drop the attribute explicitly. e.g., if a user picks a min width that's 1024 and the target cannot handle it, I think it's better to diagnose than to silently truncate to a value the user wasn't expecting. However, this can be handled in a follow-up patch if you want to land this and not have to deal with constant rebasing headaches. Comment at: include/clang/Basic/AttrDocs.td:1512 +vectors to be limited to using 256-bit vectors to avoid frequency penalties. +This is currently enabled with the -prefer-vector-width=256 command line option. +The min_vector_width attribute can be used to prevent the backend from trying Backticks around the command line option; same below (and for the attribute name as well). Basically, put the code things into the code font. https://reviews.llvm.org/D48617 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48687: [clangd] Avoid duplicates in findDefinitions response
simark added a comment. An update, I traced the difference in behavior to the difference in how `RealFileSystem` and `InMemoryFileSystem` return `Status`es. I uploaded a patch to change `InMemoryFileSystem` to work like `RealFileSystem`: https://reviews.llvm.org/D48903 With this patch applied on clang, it is now possible to write a test that reproduces the issue. As @hokein said, if I keep the dynamic disabled, I get a single result with the ".." in it. If I enable the dynamic index, I get two results, one with ".." and one without. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48687 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48903: [VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the requested name
simark updated this revision to Diff 154010.
simark added a comment.
Update commit message
Repository:
rC Clang
https://reviews.llvm.org/D48903
Files:
lib/Basic/VirtualFileSystem.cpp
Index: lib/Basic/VirtualFileSystem.cpp
===
--- lib/Basic/VirtualFileSystem.cpp
+++ lib/Basic/VirtualFileSystem.cpp
@@ -508,10 +508,18 @@
class InMemoryFileAdaptor : public File {
InMemoryFile
+ // The name to use when returning a Status for this file.
+ std::string RequestedName;
+
public:
- explicit InMemoryFileAdaptor(InMemoryFile ) : Node(Node) {}
+ explicit InMemoryFileAdaptor(InMemoryFile , std::string RequestedName)
+ : Node(Node), RequestedName (std::move (RequestedName))
+ {}
- llvm::ErrorOr status() override { return Node.getStatus(); }
+ llvm::ErrorOr status() override {
+Status St = Node.getStatus();
+return Status::copyWithNewName(St, RequestedName);
+ }
llvm::ErrorOr>
getBuffer(const Twine , int64_t FileSize, bool RequiresNullTerminator,
@@ -711,7 +719,10 @@
llvm::ErrorOr InMemoryFileSystem::status(const Twine ) {
auto Node = lookupInMemoryNode(*this, Root.get(), Path);
if (Node)
-return (*Node)->getStatus();
+{
+ Status St = (*Node)->getStatus();
+ return Status::copyWithNewName(St, Path.str());
+}
return Node.getError();
}
@@ -724,7 +735,7 @@
// When we have a file provide a heap-allocated wrapper for the memory buffer
// to match the ownership semantics for File.
if (auto *F = dyn_cast(*Node))
-return std::unique_ptr(new detail::InMemoryFileAdaptor(*F));
+return std::unique_ptr(new detail::InMemoryFileAdaptor(*F,
Path.str()));
// FIXME: errc::not_a_file?
return make_error_code(llvm::errc::invalid_argument);
Index: lib/Basic/VirtualFileSystem.cpp
===
--- lib/Basic/VirtualFileSystem.cpp
+++ lib/Basic/VirtualFileSystem.cpp
@@ -508,10 +508,18 @@
class InMemoryFileAdaptor : public File {
InMemoryFile
+ // The name to use when returning a Status for this file.
+ std::string RequestedName;
+
public:
- explicit InMemoryFileAdaptor(InMemoryFile ) : Node(Node) {}
+ explicit InMemoryFileAdaptor(InMemoryFile , std::string RequestedName)
+ : Node(Node), RequestedName (std::move (RequestedName))
+ {}
- llvm::ErrorOr status() override { return Node.getStatus(); }
+ llvm::ErrorOr status() override {
+Status St = Node.getStatus();
+return Status::copyWithNewName(St, RequestedName);
+ }
llvm::ErrorOr>
getBuffer(const Twine , int64_t FileSize, bool RequiresNullTerminator,
@@ -711,7 +719,10 @@
llvm::ErrorOr InMemoryFileSystem::status(const Twine ) {
auto Node = lookupInMemoryNode(*this, Root.get(), Path);
if (Node)
-return (*Node)->getStatus();
+{
+ Status St = (*Node)->getStatus();
+ return Status::copyWithNewName(St, Path.str());
+}
return Node.getError();
}
@@ -724,7 +735,7 @@
// When we have a file provide a heap-allocated wrapper for the memory buffer
// to match the ownership semantics for File.
if (auto *F = dyn_cast(*Node))
-return std::unique_ptr(new detail::InMemoryFileAdaptor(*F));
+return std::unique_ptr(new detail::InMemoryFileAdaptor(*F, Path.str()));
// FIXME: errc::not_a_file?
return make_error_code(llvm::errc::invalid_argument);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48863: [Sema] Explain coroutine_traits template in diag
GorNishanov added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:9053 "a coroutine">; +def note_coroutine_types_for_traits_here : Note< + "the coroutine traits class template is being instantiated using the return " I am wondering what is the use case here. Is it to guard against badly designed standard library? I played around a little and tried to see how I could get to trigger this error with a user-code, but, failed. If I did not specify enough parameters it will default to primary template defined in the Repository: rC Clang https://reviews.llvm.org/D48863 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48903: [VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the requested name
simark created this revision.
Herald added subscribers: cfe-commits, ioeric, ilya-biryukov.
InMemoryFileSystem::status behaves differently than
RealFileSystem::status. The Name contained in the Status returned by
RealFileSystem::status will be the path as requested by the caller,
whereas InMemoryFileSystem::status returns the normalized path.
For example, when requested the status for "../src/first.h",
RealFileSystem returns a Status with "../src/first.h" as the Name.
InMemoryFileSystem returns "/absolute/path/to/src/first.h".
The reason for this change is that I want to make a unit test in the
clangd testsuite (where we use an InMemoryFileSystem) to reproduce a
bug I get with the clangd program (where a RealFileSystem is used).
This difference in behavior "hides" the bug in the unit test version.
Repository:
rC Clang
https://reviews.llvm.org/D48903
Files:
lib/Basic/VirtualFileSystem.cpp
Index: lib/Basic/VirtualFileSystem.cpp
===
--- lib/Basic/VirtualFileSystem.cpp
+++ lib/Basic/VirtualFileSystem.cpp
@@ -508,10 +508,18 @@
class InMemoryFileAdaptor : public File {
InMemoryFile
+ // The name to use when returning a Status for this file.
+ std::string RequestedName;
+
public:
- explicit InMemoryFileAdaptor(InMemoryFile ) : Node(Node) {}
+ explicit InMemoryFileAdaptor(InMemoryFile , std::string RequestedName)
+ : Node(Node), RequestedName (std::move (RequestedName))
+ {}
- llvm::ErrorOr status() override { return Node.getStatus(); }
+ llvm::ErrorOr status() override {
+Status St = Node.getStatus();
+return Status::copyWithNewName(St, RequestedName);
+ }
llvm::ErrorOr>
getBuffer(const Twine , int64_t FileSize, bool RequiresNullTerminator,
@@ -711,7 +719,10 @@
llvm::ErrorOr InMemoryFileSystem::status(const Twine ) {
auto Node = lookupInMemoryNode(*this, Root.get(), Path);
if (Node)
-return (*Node)->getStatus();
+{
+ Status St = (*Node)->getStatus();
+ return Status::copyWithNewName(St, Path.str());
+}
return Node.getError();
}
@@ -724,7 +735,7 @@
// When we have a file provide a heap-allocated wrapper for the memory buffer
// to match the ownership semantics for File.
if (auto *F = dyn_cast(*Node))
-return std::unique_ptr(new detail::InMemoryFileAdaptor(*F));
+return std::unique_ptr(new detail::InMemoryFileAdaptor(*F,
Path.str()));
// FIXME: errc::not_a_file?
return make_error_code(llvm::errc::invalid_argument);
Index: lib/Basic/VirtualFileSystem.cpp
===
--- lib/Basic/VirtualFileSystem.cpp
+++ lib/Basic/VirtualFileSystem.cpp
@@ -508,10 +508,18 @@
class InMemoryFileAdaptor : public File {
InMemoryFile
+ // The name to use when returning a Status for this file.
+ std::string RequestedName;
+
public:
- explicit InMemoryFileAdaptor(InMemoryFile ) : Node(Node) {}
+ explicit InMemoryFileAdaptor(InMemoryFile , std::string RequestedName)
+ : Node(Node), RequestedName (std::move (RequestedName))
+ {}
- llvm::ErrorOr status() override { return Node.getStatus(); }
+ llvm::ErrorOr status() override {
+Status St = Node.getStatus();
+return Status::copyWithNewName(St, RequestedName);
+ }
llvm::ErrorOr>
getBuffer(const Twine , int64_t FileSize, bool RequiresNullTerminator,
@@ -711,7 +719,10 @@
llvm::ErrorOr InMemoryFileSystem::status(const Twine ) {
auto Node = lookupInMemoryNode(*this, Root.get(), Path);
if (Node)
-return (*Node)->getStatus();
+{
+ Status St = (*Node)->getStatus();
+ return Status::copyWithNewName(St, Path.str());
+}
return Node.getError();
}
@@ -724,7 +735,7 @@
// When we have a file provide a heap-allocated wrapper for the memory buffer
// to match the ownership semantics for File.
if (auto *F = dyn_cast(*Node))
-return std::unique_ptr(new detail::InMemoryFileAdaptor(*F));
+return std::unique_ptr(new detail::InMemoryFileAdaptor(*F, Path.str()));
// FIXME: errc::not_a_file?
return make_error_code(llvm::errc::invalid_argument);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48734: [Sema] Consider all format_arg attributes.
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Repository: rC Clang https://reviews.llvm.org/D48734 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45131: [AST] Refactor UnaryTransformType into TransformTraitType supporting non-unary transforms
EricWF updated this revision to Diff 154005.
EricWF marked 2 inline comments as done.
EricWF added a comment.
Address main review comments.
The `TransformTraitType` is now built early, and the `DeclSpec` refers to it
and not the list of argument types.
Additionally, the mangling formulation `u []` has
been implemented as suggested, despite being non-standard. How do we suggest
this extension to Itanium?
I believe this patch should be good to go.
https://reviews.llvm.org/D45131
Files:
include/clang/AST/ASTContext.h
include/clang/AST/CanonicalType.h
include/clang/AST/RecursiveASTVisitor.h
include/clang/AST/Type.h
include/clang/AST/TypeLoc.h
include/clang/AST/TypeNodes.def
include/clang/ASTMatchers/ASTMatchers.h
include/clang/Basic/DiagnosticCommonKinds.td
include/clang/Basic/DiagnosticParseKinds.td
include/clang/Parse/Parser.h
include/clang/Sema/Sema.h
include/clang/Serialization/ASTBitCodes.h
lib/AST/ASTContext.cpp
lib/AST/ASTDumper.cpp
lib/AST/ASTImporter.cpp
lib/AST/ASTStructuralEquivalence.cpp
lib/AST/ItaniumMangle.cpp
lib/AST/MicrosoftMangle.cpp
lib/AST/Type.cpp
lib/AST/TypeLoc.cpp
lib/AST/TypePrinter.cpp
lib/ASTMatchers/ASTMatchersInternal.cpp
lib/CodeGen/CGDebugInfo.cpp
lib/CodeGen/CodeGenFunction.cpp
lib/Parse/ParseDecl.cpp
lib/Parse/ParseDeclCXX.cpp
lib/Sema/DeclSpec.cpp
lib/Sema/SemaExpr.cpp
lib/Sema/SemaTemplate.cpp
lib/Sema/SemaTemplateDeduction.cpp
lib/Sema/SemaType.cpp
lib/Sema/TreeTransform.h
lib/Serialization/ASTReader.cpp
lib/Serialization/ASTWriter.cpp
test/CodeGenCXX/mangle.cpp
test/SemaCXX/underlying_type.cpp
Index: test/SemaCXX/underlying_type.cpp
===
--- test/SemaCXX/underlying_type.cpp
+++ test/SemaCXX/underlying_type.cpp
@@ -10,7 +10,8 @@
struct is_same_type {
static const bool value = true;
};
-
+__underlying_type() x; // expected-error {{type trait requires 1 argument; have 0 arguments}}
+__underlying_type(int, int) y; // expected-error {{type trait requires 1 argument; have 2 arguments}}
__underlying_type(int) a; // expected-error {{only enumeration types}}
__underlying_type(struct b) c; // expected-error {{only enumeration types}}
@@ -26,6 +27,31 @@
static_assert(is_same_type::value,
"h has the wrong type");
+template struct TypeList {};
+
+template
+struct TestParse;
+
+template struct MyTest {};
+
+template
+struct TestParse> {
+ using type = __underlying_type(Args...);
+};
+
+template
+struct TestParse, TypeList> {
+ // expected-error@+2 2 {{type trait requires 1 argument; have 2 arguments}}
+ // expected-error@+1 {{type trait requires 1 argument; have 0 arguments}}
+ using type = __underlying_type(Args1..., Args2...);
+};
+static_assert(is_same_type>::type, char>::value, "wrong type");
+static_assert(is_same_type, TypeList<>>::type, char>::value, "wrong type");
+template struct TestParse, TypeList<>>; // expected-note {{requested here}}
+template struct TestParse, TypeList<>>; // expected-note {{requested here}}
+template struct TestParse, TypeList>; // expected-note {{requested here}}
+
+
template
struct underlying_type {
typedef __underlying_type(T) type; // expected-error {{only enumeration types}}
@@ -38,7 +64,7 @@
using uint = unsigned;
enum class foo : uint { bar };
-
+
static_assert(is_same_type::type, unsigned>::value,
"foo has the wrong underlying type");
Index: test/CodeGenCXX/mangle.cpp
===
--- test/CodeGenCXX/mangle.cpp
+++ test/CodeGenCXX/mangle.cpp
@@ -1110,9 +1110,13 @@
void fn(T, __underlying_type(T)) {}
template void fn(E, __underlying_type(E));
-// CHECK-LABEL: @_ZN6test552fnINS_1vT_U3eutS2_
+// CHECK-LABEL: @_ZN6test552fnINS_1vT_u17__underlying_typeIS2_E
+
+void fn2(E, __underlying_type(E)) {}
+// CHECK-LABEL: @_ZN6test553fn2ENS_1EEj
}
+
namespace test56 {
struct A { A *operator->(); int n; } a;
template void f(decltype(a->n + N)) {}
Index: lib/Serialization/ASTWriter.cpp
===
--- lib/Serialization/ASTWriter.cpp
+++ lib/Serialization/ASTWriter.cpp
@@ -358,11 +358,14 @@
Code = TYPE_DECLTYPE;
}
-void ASTTypeWriter::VisitUnaryTransformType(const UnaryTransformType *T) {
- Record.AddTypeRef(T->getBaseType());
- Record.AddTypeRef(T->getUnderlyingType());
- Record.push_back(T->getUTTKind());
- Code = TYPE_UNARY_TRANSFORM;
+void ASTTypeWriter::VisitTransformTraitType(const TransformTraitType *T) {
+ auto ArgTys = T->getArgs();
+ Record.push_back(ArgTys.size());
+ for (auto Ty : ArgTys)
+Record.AddTypeRef(Ty);
+ Record.AddTypeRef(T->getTransformedType());
+ Record.push_back(T->getTTKind());
+ Code = TYPE_TRANSFORM_TRAIT;
}
void ASTTypeWriter::VisitAutoType(const AutoType *T) {
@@ -731,11 +734,14 @@
Record.AddSourceLocation(TL.getNameLoc());
}
-void
[PATCH] D44143: [clang-tidy] Create properly seeded random generator check
aaron.ballman added a comment. This still LGTM; do you need someone to commit on your behalf? https://reviews.llvm.org/D44143 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48322: [Sema] Discarded statment should be an evaluatable context
This revision was automatically updated to reflect the committed changes.
Closed by commit rL336233: [Sema] Discarded statment should be an evaluatable
context. (authored by epilk, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D48322?vs=151910=154002#toc
Repository:
rL LLVM
https://reviews.llvm.org/D48322
Files:
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/test/SemaCXX/constant-expression-cxx1z.cpp
Index: cfe/trunk/test/SemaCXX/constant-expression-cxx1z.cpp
===
--- cfe/trunk/test/SemaCXX/constant-expression-cxx1z.cpp
+++ cfe/trunk/test/SemaCXX/constant-expression-cxx1z.cpp
@@ -46,3 +46,16 @@
const int = 0;
constexpr int n = r;
}
+
+namespace PR37585 {
+template struct S { static constexpr bool value = true; };
+template constexpr bool f() { return true; }
+template constexpr bool v = true;
+
+void test() {
+ if constexpr (true) {}
+ else if constexpr (f()) {}
+ else if constexpr (S::value) {}
+ else if constexpr (v) {}
+}
+}
Index: cfe/trunk/lib/Sema/SemaExpr.cpp
===
--- cfe/trunk/lib/Sema/SemaExpr.cpp
+++ cfe/trunk/lib/Sema/SemaExpr.cpp
@@ -14237,13 +14237,13 @@
switch (SemaRef.ExprEvalContexts.back().Context) {
case Sema::ExpressionEvaluationContext::Unevaluated:
case Sema::ExpressionEvaluationContext::UnevaluatedAbstract:
-case Sema::ExpressionEvaluationContext::DiscardedStatement:
// Expressions in this context are never evaluated.
return false;
case Sema::ExpressionEvaluationContext::UnevaluatedList:
case Sema::ExpressionEvaluationContext::ConstantEvaluated:
case Sema::ExpressionEvaluationContext::PotentiallyEvaluated:
+case Sema::ExpressionEvaluationContext::DiscardedStatement:
// Expressions in this context could be evaluated.
return true;
Index: cfe/trunk/test/SemaCXX/constant-expression-cxx1z.cpp
===
--- cfe/trunk/test/SemaCXX/constant-expression-cxx1z.cpp
+++ cfe/trunk/test/SemaCXX/constant-expression-cxx1z.cpp
@@ -46,3 +46,16 @@
const int = 0;
constexpr int n = r;
}
+
+namespace PR37585 {
+template struct S { static constexpr bool value = true; };
+template constexpr bool f() { return true; }
+template constexpr bool v = true;
+
+void test() {
+ if constexpr (true) {}
+ else if constexpr (f()) {}
+ else if constexpr (S::value) {}
+ else if constexpr (v) {}
+}
+}
Index: cfe/trunk/lib/Sema/SemaExpr.cpp
===
--- cfe/trunk/lib/Sema/SemaExpr.cpp
+++ cfe/trunk/lib/Sema/SemaExpr.cpp
@@ -14237,13 +14237,13 @@
switch (SemaRef.ExprEvalContexts.back().Context) {
case Sema::ExpressionEvaluationContext::Unevaluated:
case Sema::ExpressionEvaluationContext::UnevaluatedAbstract:
-case Sema::ExpressionEvaluationContext::DiscardedStatement:
// Expressions in this context are never evaluated.
return false;
case Sema::ExpressionEvaluationContext::UnevaluatedList:
case Sema::ExpressionEvaluationContext::ConstantEvaluated:
case Sema::ExpressionEvaluationContext::PotentiallyEvaluated:
+case Sema::ExpressionEvaluationContext::DiscardedStatement:
// Expressions in this context could be evaluated.
return true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r336233 - [Sema] Discarded statment should be an evaluatable context.
Author: epilk
Date: Tue Jul 3 15:15:36 2018
New Revision: 336233
URL: http://llvm.org/viewvc/llvm-project?rev=336233=rev
Log:
[Sema] Discarded statment should be an evaluatable context.
The constexpr evaluator was erroring out because these templates weren't
defined. Despite being used in a discarded statement, we still need to constexpr
evaluate them, which means that we need to instantiate them. Fixes PR37585.
Differential revision: https://reviews.llvm.org/D48322
Modified:
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/test/SemaCXX/constant-expression-cxx1z.cpp
Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=336233=336232=336233=diff
==
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Tue Jul 3 15:15:36 2018
@@ -14237,13 +14237,13 @@ static bool isEvaluatableContext(Sema
switch (SemaRef.ExprEvalContexts.back().Context) {
case Sema::ExpressionEvaluationContext::Unevaluated:
case Sema::ExpressionEvaluationContext::UnevaluatedAbstract:
-case Sema::ExpressionEvaluationContext::DiscardedStatement:
// Expressions in this context are never evaluated.
return false;
case Sema::ExpressionEvaluationContext::UnevaluatedList:
case Sema::ExpressionEvaluationContext::ConstantEvaluated:
case Sema::ExpressionEvaluationContext::PotentiallyEvaluated:
+case Sema::ExpressionEvaluationContext::DiscardedStatement:
// Expressions in this context could be evaluated.
return true;
Modified: cfe/trunk/test/SemaCXX/constant-expression-cxx1z.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/constant-expression-cxx1z.cpp?rev=336233=336232=336233=diff
==
--- cfe/trunk/test/SemaCXX/constant-expression-cxx1z.cpp (original)
+++ cfe/trunk/test/SemaCXX/constant-expression-cxx1z.cpp Tue Jul 3 15:15:36
2018
@@ -46,3 +46,16 @@ namespace Cxx17CD_NB_GB19 {
const int = 0;
constexpr int n = r;
}
+
+namespace PR37585 {
+template struct S { static constexpr bool value = true; };
+template constexpr bool f() { return true; }
+template constexpr bool v = true;
+
+void test() {
+ if constexpr (true) {}
+ else if constexpr (f()) {}
+ else if constexpr (S::value) {}
+ else if constexpr (v) {}
+}
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45131: [AST] Refactor UnaryTransformType into TransformTraitType supporting non-unary transforms
EricWF marked 13 inline comments as done.
EricWF added inline comments.
Comment at: lib/AST/ItaniumMangle.cpp:3250-3251
- mangleType(T->getBaseType());
+ for (auto Ty : T->getArgs())
+mangleType(Ty);
}
rsmith wrote:
> EricWF wrote:
> > EricWF wrote:
> > > rsmith wrote:
> > > > We need manglings to be self-delimiting, and we can't tell where the
> > > > end of the argument list is with this mangling approach. :(
> > > >
> > > > (Eg, `f(__raw_invocation_type(T1, T2, T3))` and
> > > > `f(__raw_invocation_type(T1, T2), T3)` would mangle the same with this
> > > > mangling.)
> > > >
> > > > Maybe drop an `E` on the end? (Or maybe do that only for traits that
> > > > don't require exactly one argument, but that would create pain for
> > > > demanglers)
> > > Makes sense. Thanks for the explanation. I'll go ahead and drop and E on
> > > the end.
> > >
> > > However, will this cause ABI issues? Either with existing code or with
> > > GCC? If so, perhaps we should maintain the current mangling for
> > > `__underlying_type`.
> > To answer my own question, GCC doesn't mangle `__underlying_type` yet. And
> > I doubt that people are currently depending on the dependent mangling of
> > `__underlying_type`. Perhaps I'm wrong about that last assumption though.
> The existing `U3eut` mangling (with no terminating `E`) was approximately OK,
> following
> http://itanium-cxx-abi.github.io/cxx-abi/abi.html#mangle.qualified-type. It
> would be better to use `U17__underlying_type`, though. This is not exactly
> right, since it treats `__underlying_type` as a type qualifier rather than a
> type function, but that's not too far off.
>
> The new mangling doesn't match the Itanium ABI rule for vendor extensions. We
> basically have a choice between `u` (which doesn't give us a
> good way to encode the arguments) or `U`
> (which could work, but we'd need to make a somewhat-arbitrary choice of which
> type we consider to be the "main" type to which the qualifier applies).
>
> We could arbitrarily say:
>
> * the first type in the trait is the "main" type
> * the rest of the trait is mangled as a qualifier
> * the qualifier is given template-args if and only if there is more than one
> argument
>
> So:
>
> * `__underlying_type(T)` would mangle as `U17__underlying_type1T` (or
> approximately `T __underlying_type`)
> * `__raw_invocation_type(F, A1, A2)` would mangle as
> `U21__raw_invocation_typeI2A12A2E1F` (or approximately `F
> __raw_invocation_type`)
> * `__raw_invocation_type(F)` would mangle as `U21__raw_invocation_type1F`
> (or approximately `F __raw_invocation_type`)
>
> Or we could track here whether the trait can accept >1 argument, and always
> use the template-args formulation if so. I have no strong opinions either way.
>
> Or we could suggest an Itanium ABI extension to permit for
> the `u...` vendor extension type form, for vendor type traits. That'd lead to:
>
> * `__underlying_type(T)` would mangle as `u17__underlying_typeI1TE` (or
> approximately `__underlying_type`)
> * `__raw_invocation_type(F, A1, A2)` would mangle as
> `u21__raw_invocation_typeI1F2A12A2E` (or approximately
> `__raw_invocation_type`)
> * `__raw_invocation_type(F)` would mangle as `u21__raw_invocation_typeI1FE`
> (or approximately `__raw_invocation_type`)
>
> ... and we could encourage demanglers to use parens instead of angles for
> those arguments.
>
>
> The final of the above options seems best to me. What do you think?
> Or we could suggest an Itanium ABI extension to permit for
> the u... vendor extension type form,
> [...]
> The final of the above options seems best to me. What do you think?
I think the final form would also be preferable. I'll go ahead and implement it.
What would it involve to suggest the Itanium ABI extension?
Comment at: lib/Sema/DeclSpec.cpp:369-370
case TST_underlyingType:
+ return false;
+
rsmith wrote:
> This is going to be problematic down the line -- I expect there are going to
> be type transform traits that can produce function types. I think this
> strongly argues that we need to have already converted the trait into a
> `QualType` by the time we get here, rather than representing it as a trait
> kind and a list of types in the `DeclSpec` and deferring the conversion to
> later.
Ack. Done.
Comment at: lib/Sema/SemaType.cpp:1508-1509
+ break;
+default:
+ llvm_unreachable("unhandled case");
+}
EricWF wrote:
> rsmith wrote:
> > EricWF wrote:
> > > rsmith wrote:
> > > > Try to avoid `default:` cases like this, as they suppress the warning
> > > > notifying the user to update this switch when new traits are added.
> > > Understood, but surely this is a case where `default:` is warranted.
> > > We're switching over a range of values much larger than
> > > `TransformTraitType::TTKind` in order to
[PATCH] D48322: [Sema] Discarded statment should be an evaluatable context
rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. In https://reviews.llvm.org/D48322#1150316, @erik.pilkington wrote: > Is this what you were concerned about? Yes, exactly. Thank you for checking. Repository: rC Clang https://reviews.llvm.org/D48322 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48880: [Sema] Fix crash in getConstructorName.
rsmith added a comment. Looks fine, but please put the test case somewhere more appropriate (under SemaCXX, for instance). Comment at: lib/Sema/SemaExprCXX.cpp:117 } assert(InjectedClassName && "couldn't find injected class name"); We would be able to recover in more cases if we only bail out when the `InjectedClassName` is not found. Maybe instead of this change, you could weaken the assertion to only fail if the class is valid, and return `ParsedType` here if we don't find an `InjectedClassName`. Repository: rC Clang https://reviews.llvm.org/D48880 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48395: Added PublicOnly flag
juliehockett added inline comments.
Comment at: clang-tools-extra/clang-doc/Serialize.cpp:322-324
+ if(PublicOnly && ! isPublic(D->getAccess(), D->getLinkageInternal())){
+return "";
+ }
anniecherk wrote:
> juliehockett wrote:
> > Since this is the same for Record/Function/Enum, can we move this to
> > `populateSymbolInfo()`?
> >
> > Also, elide braces on single-line ifs.
> I don't see a good way to put it into populateSymbolInfo because if the
> condition passes then the emitInfo method needs to bail out and I don't see a
> clean way to do that if the check is in populateSymbolInfo. A clunky way to
> do this would be to either have populateSymbolInfo set a flag that emitInfo
> checks or emitInfo can assume that populateSymbolInfo bailed if all the info
> is unpopulated, but that seems like a worse way to do it. Am I missing
> something?
>
> I can refactor the condition into a function if that would be better for
> understandability / maintainability.
Hmm I suppose this is fine then.
Still elide braces on single-line ifs.
https://reviews.llvm.org/D48395
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r336231 - Factor out Clang's desired 8MB stack size constant from the various
Author: rsmith
Date: Tue Jul 3 14:34:13 2018
New Revision: 336231
URL: http://llvm.org/viewvc/llvm-project?rev=336231=rev
Log:
Factor out Clang's desired 8MB stack size constant from the various
places we hardcode it.
Added:
cfe/trunk/include/clang/Basic/Stack.h
Modified:
cfe/trunk/lib/Frontend/CompilerInstance.cpp
cfe/trunk/lib/StaticAnalyzer/Frontend/ModelInjector.cpp
cfe/trunk/tools/driver/cc1_main.cpp
cfe/trunk/tools/libclang/CIndex.cpp
Added: cfe/trunk/include/clang/Basic/Stack.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Stack.h?rev=336231=auto
==
--- cfe/trunk/include/clang/Basic/Stack.h (added)
+++ cfe/trunk/include/clang/Basic/Stack.h Tue Jul 3 14:34:13 2018
@@ -0,0 +1,25 @@
+//===--- Stack.h - Utilities for dealing with stack space ---*- C++
-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+///
+/// \file
+/// Defines utilities for dealing with stack allocation and stack space.
+///
+//===--===//
+
+#ifndef LLVM_CLANG_BASIC_STACK_H
+#define LLVM_CLANG_BASIC_STACK_H
+
+namespace clang {
+ /// The amount of stack space that Clang would like to be provided with.
+ /// If less than this much is available, we may be unable to reach our
+ /// template instantiation depth limit and other similar limits.
+ constexpr size_t DesiredStackSize = 8 << 20;
+} // end namespace clang
+
+#endif // LLVM_CLANG_BASIC_STACK_H
Modified: cfe/trunk/lib/Frontend/CompilerInstance.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInstance.cpp?rev=336231=336230=336231=diff
==
--- cfe/trunk/lib/Frontend/CompilerInstance.cpp (original)
+++ cfe/trunk/lib/Frontend/CompilerInstance.cpp Tue Jul 3 14:34:13 2018
@@ -16,6 +16,7 @@
#include "clang/Basic/FileManager.h"
#include "clang/Basic/MemoryBufferCache.h"
#include "clang/Basic/SourceManager.h"
+#include "clang/Basic/Stack.h"
#include "clang/Basic/TargetInfo.h"
#include "clang/Basic/Version.h"
#include "clang/Config/config.h"
@@ -1164,14 +1165,13 @@ compileModuleImpl(CompilerInstance
// Execute the action to actually build the module in-place. Use a separate
// thread so that we get a stack large enough.
- const unsigned ThreadStackSize = 8 << 20;
llvm::CrashRecoveryContext CRC;
CRC.RunSafelyOnThread(
[&]() {
GenerateModuleFromModuleMapAction Action;
Instance.ExecuteAction(Action);
},
- ThreadStackSize);
+ DesiredStackSize);
PostBuildStep(Instance);
Modified: cfe/trunk/lib/StaticAnalyzer/Frontend/ModelInjector.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Frontend/ModelInjector.cpp?rev=336231=336230=336231=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Frontend/ModelInjector.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Frontend/ModelInjector.cpp Tue Jul 3 14:34:13
2018
@@ -10,6 +10,7 @@
#include "ModelInjector.h"
#include "clang/AST/Decl.h"
#include "clang/Basic/IdentifierTable.h"
+#include "clang/Basic/Stack.h"
#include "clang/Frontend/ASTUnit.h"
#include "clang/Frontend/CompilerInstance.h"
#include "clang/Frontend/FrontendAction.h"
@@ -95,11 +96,10 @@ void ModelInjector::onBodySynthesis(cons
ParseModelFileAction parseModelFile(Bodies);
- const unsigned ThreadStackSize = 8 << 20;
llvm::CrashRecoveryContext CRC;
CRC.RunSafelyOnThread([&]() { Instance.ExecuteAction(parseModelFile); },
-ThreadStackSize);
+DesiredStackSize);
Instance.getPreprocessor().FinalizeForModelFile();
Modified: cfe/trunk/tools/driver/cc1_main.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/driver/cc1_main.cpp?rev=336231=336230=336231=diff
==
--- cfe/trunk/tools/driver/cc1_main.cpp (original)
+++ cfe/trunk/tools/driver/cc1_main.cpp Tue Jul 3 14:34:13 2018
@@ -16,6 +16,7 @@
#include "llvm/Option/Arg.h"
#include "clang/CodeGen/ObjectFilePCHContainerOperations.h"
#include "clang/Config/config.h"
+#include "clang/Basic/Stack.h"
#include "clang/Driver/DriverDiagnostic.h"
#include "clang/Driver/Options.h"
#include "clang/Frontend/CompilerInstance.h"
@@ -73,13 +74,6 @@ void initializePollyPasses(llvm::PassReg
#endif
#ifdef CLANG_HAVE_RLIMITS
-// The amount of stack we think is "sufficient". If less than this much is
-// available, we may be unable to reach our template instantiation depth
-// limit and other similar limits.
-// FIXME:
[PATCH] D48845: [Sema] Add fixit for unused lambda captures
bkramer added inline comments.
Comment at: test/FixIt/fixit-unused-lambda-capture.cpp:31
+ // CHECK: [=,] { return i; };
+}
This needs tests for:
* capture initializers `[c = foo()] {};`
* Capturing this `[this] {};`
* Capturing *this `[*this] {};`
* VLA capture `int a; int c[a]; [] {};`
Repository:
rC Clang
https://reviews.llvm.org/D48845
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48845: [Sema] Add fixit for unused lambda captures
acomminos updated this revision to Diff 153968.
acomminos retitled this revision from "[Sema] Add fixit for
-Wno-unused-lambda-capture" to "[Sema] Add fixit for unused lambda captures".
acomminos edited the summary of this revision.
acomminos changed the visibility from "Custom Policy" to "Public (No Login
Required)".
acomminos added a subscriber: alexshap.
acomminos added a comment.
Herald added a subscriber: cfe-commits.
Added tests, add logic for removing a comma forward for beginning edge case.
Repository:
rC Clang
https://reviews.llvm.org/D48845
Files:
include/clang/Sema/Sema.h
lib/Sema/SemaLambda.cpp
test/FixIt/fixit-unused-lambda-capture.cpp
Index: test/FixIt/fixit-unused-lambda-capture.cpp
===
--- /dev/null
+++ test/FixIt/fixit-unused-lambda-capture.cpp
@@ -0,0 +1,31 @@
+// RUN: cp %s %t
+// RUN: %clang_cc1 -x c++ -fsyntax-only -Wunused-lambda-capture -std=c++1z -fixit %t
+// RUN: grep -v CHECK %t | FileCheck %s
+
+void test() {
+ int i = 0;
+ int j = 0;
+ int k = 0;
+ [i,j] { return i; };
+ // CHECK: [i] { return i; };
+ [i,j] { return j; };
+ // CHECK: [j] { return j; };
+ [i,j,k] {};
+ // CHECK: [] {};
+ [i,j,k] { return i + j; };
+ // CHECK: [i,j] { return i + j; };
+ [i,j,k] { return j + k; };
+ // CHECK: [j,k] { return j + k; };
+ [i,j,k] { return i + k; };
+ // CHECK: [i,k] { return i + k; };
+ [i,j,k] { return i + j + k; };
+ // CHECK: [i,j,k] { return i + j + k; };
+ [&,i] { return k; };
+ // CHECK: [&] { return k; };
+ [=,] { return k; };
+ // CHECK: [=] { return k; };
+ [=,,] { return j; };
+ // CHECK: [=,] { return j; };
+ [=,,] { return i; };
+ // CHECK: [=,] { return i; };
+}
Index: lib/Sema/SemaLambda.cpp
===
--- lib/Sema/SemaLambda.cpp
+++ lib/Sema/SemaLambda.cpp
@@ -1478,7 +1478,8 @@
return false;
}
-void Sema::DiagnoseUnusedLambdaCapture(const Capture ) {
+void Sema::DiagnoseUnusedLambdaCapture(const SourceRange CaptureRange,
+ const Capture ) {
if (CaptureHasSideEffects(From))
return;
@@ -1491,6 +1492,7 @@
else
diag << From.getVariable();
diag << From.isNonODRUsed();
+ diag << FixItHint::CreateRemoval(CaptureRange);
}
ExprResult Sema::BuildLambdaExpr(SourceLocation StartLoc, SourceLocation EndLoc,
@@ -1532,19 +1534,40 @@
// Translate captures.
auto CurField = Class->field_begin();
+// True if the current capture has an initializer or default before it.
+bool CurHasPreviousInitializer = CaptureDefault != LCD_None;
+SourceLocation PrevCaptureLoc = CurHasPreviousInitializer ?
+CaptureDefaultLoc : IntroducerRange.getBegin();
+
for (unsigned I = 0, N = LSI->Captures.size(); I != N; ++I, ++CurField) {
const Capture = LSI->Captures[I];
assert(!From.isBlockCapture() && "Cannot capture __block variables");
bool IsImplicit = I >= LSI->NumExplicitCaptures;
// Warn about unused explicit captures.
+ bool IsCaptureUsed = true;
if (!CurContext->isDependentContext() && !IsImplicit && !From.isODRUsed()) {
// Initialized captures that are non-ODR used may not be eliminated.
bool NonODRUsedInitCapture =
IsGenericLambda && From.isNonODRUsed() && From.getInitExpr();
-if (!NonODRUsedInitCapture)
- DiagnoseUnusedLambdaCapture(From);
+if (!NonODRUsedInitCapture) {
+ // Delete either the preceding or next comma in the explicit capture
+ // list, depending on whether or not elements follow.
+ SourceRange FixItRange;
+ bool IsLast = I + 1 == LSI->NumExplicitCaptures;
+ if (!CurHasPreviousInitializer && !IsLast) {
+FixItRange = SourceRange(From.getLocation(),
+ getLocForEndOfToken(From.getLocation()));
+ } else {
+FixItRange = SourceRange(getLocForEndOfToken(PrevCaptureLoc),
+ From.getLocation());
+ }
+
+ DiagnoseUnusedLambdaCapture(FixItRange, From);
+ IsCaptureUsed = false;
+}
}
+ CurHasPreviousInitializer |= IsCaptureUsed;
// Handle 'this' capture.
if (From.isThisCapture()) {
@@ -1574,6 +1597,8 @@
Init = InitResult.get();
}
CaptureInits.push_back(Init);
+
+ PrevCaptureLoc = From.getLocation();
}
// C++11 [expr.prim.lambda]p6:
Index: include/clang/Sema/Sema.h
===
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -5604,7 +5604,8 @@
bool CaptureHasSideEffects(const sema::Capture );
/// Diagnose if an explicit lambda capture is unused.
- void DiagnoseUnusedLambdaCapture(const sema::Capture );
+ void DiagnoseUnusedLambdaCapture(const SourceRange CaptureRange,
+ const
[clang-tools-extra] r336228 - [clangd] Replace UniqueFunction with llvm::unique_function.
Author: d0k
Date: Tue Jul 3 13:59:33 2018
New Revision: 336228
URL: http://llvm.org/viewvc/llvm-project?rev=336228=rev
Log:
[clangd] Replace UniqueFunction with llvm::unique_function.
One implementation of this ought to be enough for everyone.
Modified:
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/ClangdServer.h
clang-tools-extra/trunk/clangd/CodeComplete.cpp
clang-tools-extra/trunk/clangd/Function.h
clang-tools-extra/trunk/clangd/TUScheduler.cpp
clang-tools-extra/trunk/clangd/TUScheduler.h
clang-tools-extra/trunk/clangd/Threading.cpp
clang-tools-extra/trunk/clangd/Threading.h
clang-tools-extra/trunk/unittests/clangd/SyncAPI.cpp
Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=336228=336227=336228=diff
==
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Tue Jul 3 13:59:33 2018
@@ -280,7 +280,7 @@ void ClangdServer::rename(PathRef File,
}
void ClangdServer::dumpAST(PathRef File,
- UniqueFunction Callback) {
+ llvm::unique_function Callback) {
auto Action = [](decltype(Callback) Callback,
llvm::Expected InpAST) {
if (!InpAST) {
Modified: clang-tools-extra/trunk/clangd/ClangdServer.h
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.h?rev=336228=336227=336228=diff
==
--- clang-tools-extra/trunk/clangd/ClangdServer.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.h Tue Jul 3 13:59:33 2018
@@ -188,7 +188,7 @@ public:
/// Only for testing purposes.
/// Waits until all requests to worker thread are finished and dumps AST for
/// \p File. \p File must be in the list of added documents.
- void dumpAST(PathRef File, UniqueFunction Callback);
+ void dumpAST(PathRef File, llvm::unique_function
Callback);
/// Called when an event occurs for a watched file in the workspace.
void onFileEvent(const DidChangeWatchedFilesParams );
Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=336228=336227=336228=diff
==
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Tue Jul 3 13:59:33 2018
@@ -566,7 +566,7 @@ static bool isBlacklistedMember(const Na
// within the callback.
struct CompletionRecorder : public CodeCompleteConsumer {
CompletionRecorder(const CodeCompleteOptions ,
- UniqueFunction ResultsCallback)
+ llvm::unique_function ResultsCallback)
: CodeCompleteConsumer(Opts.getClangCompleteOpts(),
/*OutputIsBinary=*/false),
CCContext(CodeCompletionContext::CCC_Other), Opts(Opts),
@@ -657,7 +657,7 @@ private:
CodeCompleteOptions Opts;
std::shared_ptr CCAllocator;
CodeCompletionTUInfo CCTUInfo;
- UniqueFunction ResultsCallback;
+ llvm::unique_function ResultsCallback;
};
class SignatureHelpCollector final : public CodeCompleteConsumer {
Modified: clang-tools-extra/trunk/clangd/Function.h
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Function.h?rev=336228=336227=336228=diff
==
--- clang-tools-extra/trunk/clangd/Function.h (original)
+++ clang-tools-extra/trunk/clangd/Function.h Tue Jul 3 13:59:33 2018
@@ -7,83 +7,25 @@
//
//===--===//
//
-// This file provides an analogue to std::function that supports move
semantics.
+// This file provides utilities for callable objects.
//
//===--===//
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_FUNCTION_H
#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_FUNCTION_H
-#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/FunctionExtras.h"
#include "llvm/Support/Error.h"
-#include
-#include
#include
-#include
#include
namespace clang {
namespace clangd {
-/// A move-only type-erasing function wrapper. Similar to `std::function`, but
-/// allows to store move-only callables.
-template class UniqueFunction;
/// A Callback is a void function that accepts Expected.
/// This is accepted by ClangdServer functions that logically return T.
-template using Callback = UniqueFunction)>;
-
-template class UniqueFunction {
-public:
- UniqueFunction() = default;
- UniqueFunction(std::nullptr_t) : UniqueFunction(){};
-
- UniqueFunction(UniqueFunction const &) = delete;
-
[PATCH] D48896: [libcxx][c++17] P0083R5: Splicing Maps and Sets Part 2: merge
erik.pilkington created this revision.
erik.pilkington added reviewers: EricWF, mclow.lists.
Herald added subscribers: dexonsmith, ldionne, christof.
This was originally part of https://reviews.llvm.org/D46845, but I decided to
split it out to clean up the diff. From that patch's description:
> In `__hash_table`, I split up the functions `__node_insert_unqiue` and
> `__node_insert_multi` into 2 parts in order to implement merge. The first
> part, `__node_insert_{multi,unique}_prepare` checks to see if a node already
> is present in the container (as needed), and rehashes the container. This
> function can forward exceptions, but doesn't mutate the node it's preparing
> to insert. the `__node_insert_{multi,unique}_perform` is noexcept, but
> mutates the pointers in the node to actually insert it into the container.
> This allows merge to call a `_prepare` function on a node while it is in the
> source container, without invalidating anything or losing nodes if an
> exception is thrown.
Thanks for taking a look!
Erik
Repository:
rCXX libc++
https://reviews.llvm.org/D48896
Files:
libcxx/include/__hash_table
libcxx/include/__tree
libcxx/include/map
libcxx/include/set
libcxx/include/unordered_map
libcxx/include/unordered_set
libcxx/test/std/containers/associative/map/map.modifiers/merge.pass.cpp
libcxx/test/std/containers/associative/multimap/multimap.modifiers/merge.pass.cpp
libcxx/test/std/containers/associative/multiset/merge.pass.cpp
libcxx/test/std/containers/associative/set/merge.pass.cpp
libcxx/test/std/containers/unord/unord.map/unord.map.modifiers/merge.pass.cpp
libcxx/test/std/containers/unord/unord.multimap/unord.multimap.modifiers/merge.pass.cpp
libcxx/test/std/containers/unord/unord.multiset/merge.pass.cpp
libcxx/test/std/containers/unord/unord.set/merge.pass.cpp
Index: libcxx/test/std/containers/unord/unord.set/merge.pass.cpp
===
--- /dev/null
+++ libcxx/test/std/containers/unord/unord.set/merge.pass.cpp
@@ -0,0 +1,135 @@
+//===--===//
+//
+// 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, c++11, c++14
+
+//
+
+// class unordered_set
+
+// template
+// void merge(unordered_set& source);
+
+#include
+#include "test_macros.h"
+#include "Counter.h"
+
+template
+bool set_equal(const Set& set, Set other)
+{
+return set == other;
+}
+
+#ifndef TEST_HAS_NO_EXCEPTIONS
+template
+struct throw_hasher
+{
+bool& should_throw_;
+
+throw_hasher(bool& should_throw) : should_throw_(should_throw) {}
+
+typedef size_t result_type;
+typedef T argument_type;
+
+size_t operator()(const T& p) const
+{
+if (should_throw_)
+throw 0;
+return std::hash()(p);
+}
+};
+#endif
+
+int main()
+{
+{
+std::unordered_set src{1, 3, 5};
+std::unordered_set dst{2, 4, 5};
+dst.merge(src);
+assert(set_equal(src, {5}));
+assert(set_equal(dst, {1, 2, 3, 4, 5}));
+}
+
+#ifndef TEST_HAS_NO_EXCEPTIONS
+{
+bool do_throw = false;
+typedef std::unordered_set, throw_hasher>> set_type;
+set_type src({1, 3, 5}, 0, throw_hasher>(do_throw));
+set_type dst({2, 4, 5}, 0, throw_hasher>(do_throw));
+
+assert(Counter_base::gConstructed == 6);
+
+do_throw = true;
+try
+{
+dst.merge(src);
+}
+catch (int)
+{
+do_throw = false;
+}
+assert(!do_throw);
+assert(set_equal(src, set_type({1, 3, 5}, 0, throw_hasher>(do_throw;
+assert(set_equal(dst, set_type({2, 4, 5}, 0, throw_hasher>(do_throw;
+}
+#endif
+assert(Counter_base::gConstructed == 0);
+struct equal
+{
+equal() = default;
+
+bool operator()(const Counter& lhs, const Counter& rhs) const
+{
+return lhs == rhs;
+}
+};
+struct hasher
+{
+hasher() = default;
+typedef Counter argument_type;
+typedef size_t result_type;
+size_t operator()(const Counter& p) const { return std::hash>()(p); }
+};
+{
+typedef std::unordered_set, std::hash>, std::equal_to>> first_set_type;
+typedef std::unordered_set, hasher, equal> second_set_type;
+typedef std::unordered_multiset, hasher, equal> third_set_type;
+
+first_set_type first{1, 2, 3};
+second_set_type second{2, 3, 4};
+third_set_type third{1, 3};
+
+assert(Counter_base::gConstructed == 8);
+
+first.merge(second);
+first.merge(std::move(second));
+first.merge(third);
+
[PATCH] D46845: [libcxx][c++17] P0083R5: Splicing Maps and Sets
erik.pilkington updated this revision to Diff 153964.
erik.pilkington added a comment.
Herald added a subscriber: dexonsmith.
Split this up. I'm moving the `merge` stuff to a follow-up, that makes this
diff a lot easier to read.
https://reviews.llvm.org/D46845
Files:
libcxx/include/__hash_table
libcxx/include/__node_handle
libcxx/include/__tree
libcxx/include/map
libcxx/include/module.modulemap
libcxx/include/set
libcxx/include/unordered_map
libcxx/include/unordered_set
libcxx/test/std/containers/associative/map/map.modifiers/insert_extract.pass.cpp
libcxx/test/std/containers/associative/multimap/multimap.modifiers/insert_extract.pass.cpp
libcxx/test/std/containers/associative/multiset/insert_extract.pass.cpp
libcxx/test/std/containers/associative/set/insert_extract.pass.cpp
libcxx/test/std/containers/container.node/node_handle.pass.cpp
libcxx/test/std/containers/unord/unord.map/unord.map.modifiers/insert_extract.pass.cpp
libcxx/test/std/containers/unord/unord.multimap/unord.multimap.modifiers/insert_extract.pass.cpp
libcxx/test/std/containers/unord/unord.multiset/insert_extract.pass.cpp
libcxx/test/std/containers/unord/unord.set/insert_extract.pass.cpp
libcxx/test/support/Counter.h
Index: libcxx/test/support/Counter.h
===
--- libcxx/test/support/Counter.h
+++ libcxx/test/support/Counter.h
@@ -49,7 +49,7 @@
typedef Counter argument_type;
typedef std::size_t result_type;
-std::size_t operator()(const Counter& x) const {return std::hash(x.get());}
+std::size_t operator()(const Counter& x) const {return std::hash()(x.get());}
};
}
Index: libcxx/test/std/containers/unord/unord.set/insert_extract.pass.cpp
===
--- /dev/null
+++ libcxx/test/std/containers/unord/unord.set/insert_extract.pass.cpp
@@ -0,0 +1,220 @@
+//===--===//
+//
+// 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, c++11, c++14
+
+//
+
+// class unordered_set
+
+#include
+#include
+#include
+#include "min_allocator.h"
+#include "Counter.h"
+
+namespace test_irt
+{
+typedef std::unordered_set int_set_type;
+using intIRT = int_set_type::insert_return_type;
+static_assert(std::is_move_constructible::value &&
+ std::is_move_assignable::value &&
+ std::is_default_constructible::value &&
+ std::is_destructible::value, "");
+
+typedef std::unordered_set, std::hash>,
+ std::equal_to>,
+ min_allocator>> set_type;
+using minIRT = set_type::insert_return_type;
+
+static_assert(std::is_move_constructible::value &&
+ std::is_move_assignable::value &&
+ std::is_default_constructible::value &&
+ std::is_destructible::value, "");
+
+static void doit()
+{
+{
+intIRT a, b;
+std::swap(a, b);
+
+minIRT c, d;
+std::swap(c, d);
+}
+
+{
+set_type st = {0};
+set_type::node_type nh = st.extract(st.begin());
+set_type::insert_return_type irt = st.insert(std::move(nh));
+assert(irt.inserted);
+assert(irt.position == st.begin());
+assert(irt.node.empty());
+}
+
+{
+set_type st = {0};
+set_type::node_type nh = st.extract(st.begin());
+st.insert(0);
+set_type::insert_return_type irt = st.insert(std::move(nh));
+assert(!irt.inserted);
+assert(irt.position == st.begin());
+assert(!irt.node.empty());
+}
+
+{
+set_type st;
+set_type::node_type nh;
+set_type::insert_return_type irt = st.insert(std::move(nh));
+assert(!irt.inserted);
+assert(irt.position == st.end());
+assert(irt.node.empty());
+}
+
+assert(Counter_base::gConstructed == 0);
+}
+}
+
+namespace test_extract
+{
+using the_set = std::unordered_set;
+
+static void doit()
+{
+the_set s;
+s.insert(1);
+s.insert(2);
+s.insert(3);
+assert(s.size() == 3);
+
+the_set::node_type handle = s.extract(2);
+assert(s.size() == 2);
+assert(s.count(1) == 1 && s.count(3) == 1);
+the_set::iterator i = s.begin();
+std::advance(i, 2);
+assert(i == s.end());
+}
+}
+
+namespace test_count_dtors
+{
+typedef std::unordered_set> set_type;
+
+static void doit()
+{
+{
+set_type dtc;
+for (int i = 0; i != 10; ++i)
+dtc.emplace(i);
+
+assert(Counter_base::gConstructed == 10);
+assert(dtc.size() == 10);
+
+set_type::node_type nh3 = dtc.extract(Counter(3));
+set_type::node_type nh0
Re: r336225 - Fix allocation of Nullability attribute.
On Tue, Jul 3, 2018 at 4:30 PM, Erich Keane via cfe-commits
wrote:
> Author: erichkeane
> Date: Tue Jul 3 13:30:34 2018
> New Revision: 336225
>
> URL: http://llvm.org/viewvc/llvm-project?rev=336225=rev
> Log:
> Fix allocation of Nullability attribute.
>
> Existing code always allocates for on the declarator's attribute pool,
> but sometimes adds it to the declspec. This patch ensures that the
> correct pool is used.
>
>
> Discovered while testing: https://reviews.llvm.org/D48788
Can you devise a testcase for this change, or is that hard to get the
original behavior to fail in a consistent way?
~Aaron
>
> Modified:
> cfe/trunk/lib/Parse/ParseObjc.cpp
>
> Modified: cfe/trunk/lib/Parse/ParseObjc.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseObjc.cpp?rev=336225=336224=336225=diff
> ==
> --- cfe/trunk/lib/Parse/ParseObjc.cpp (original)
> +++ cfe/trunk/lib/Parse/ParseObjc.cpp Tue Jul 3 13:30:34 2018
> @@ -381,25 +381,23 @@ static void addContextSensitiveTypeNulla
> SourceLocation nullabilityLoc,
> bool ) {
>// Create the attribute.
> - auto getNullabilityAttr = [&]() -> AttributeList * {
> -return D.getAttributePool().create(
> - P.getNullabilityKeyword(nullability),
> - SourceRange(nullabilityLoc),
> - nullptr, SourceLocation(),
> - nullptr, 0,
> - AttributeList::AS_ContextSensitiveKeyword);
> + auto getNullabilityAttr = [&](AttributePool ) -> AttributeList * {
> +return Pool.create(P.getNullabilityKeyword(nullability),
> + SourceRange(nullabilityLoc), nullptr,
> SourceLocation(),
> + nullptr, 0,
> AttributeList::AS_ContextSensitiveKeyword);
>};
>
>if (D.getNumTypeObjects() > 0) {
> // Add the attribute to the declarator chunk nearest the declarator.
> -auto nullabilityAttr = getNullabilityAttr();
> +auto nullabilityAttr = getNullabilityAttr(D.getAttributePool());
> DeclaratorChunk = D.getTypeObject(0);
> nullabilityAttr->setNext(chunk.getAttrListRef());
> chunk.getAttrListRef() = nullabilityAttr;
>} else if (!addedToDeclSpec) {
> // Otherwise, just put it on the declaration specifiers (if one
> // isn't there already).
> -D.getMutableDeclSpec().addAttributes(getNullabilityAttr());
> +D.getMutableDeclSpec().addAttributes(
> +getNullabilityAttr(D.getDeclSpec().getAttributePool()));
> addedToDeclSpec = true;
>}
> }
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r336225 - Fix allocation of Nullability attribute.
Author: erichkeane
Date: Tue Jul 3 13:30:34 2018
New Revision: 336225
URL: http://llvm.org/viewvc/llvm-project?rev=336225=rev
Log:
Fix allocation of Nullability attribute.
Existing code always allocates for on the declarator's attribute pool,
but sometimes adds it to the declspec. This patch ensures that the
correct pool is used.
Discovered while testing: https://reviews.llvm.org/D48788
Modified:
cfe/trunk/lib/Parse/ParseObjc.cpp
Modified: cfe/trunk/lib/Parse/ParseObjc.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseObjc.cpp?rev=336225=336224=336225=diff
==
--- cfe/trunk/lib/Parse/ParseObjc.cpp (original)
+++ cfe/trunk/lib/Parse/ParseObjc.cpp Tue Jul 3 13:30:34 2018
@@ -381,25 +381,23 @@ static void addContextSensitiveTypeNulla
SourceLocation nullabilityLoc,
bool ) {
// Create the attribute.
- auto getNullabilityAttr = [&]() -> AttributeList * {
-return D.getAttributePool().create(
- P.getNullabilityKeyword(nullability),
- SourceRange(nullabilityLoc),
- nullptr, SourceLocation(),
- nullptr, 0,
- AttributeList::AS_ContextSensitiveKeyword);
+ auto getNullabilityAttr = [&](AttributePool ) -> AttributeList * {
+return Pool.create(P.getNullabilityKeyword(nullability),
+ SourceRange(nullabilityLoc), nullptr, SourceLocation(),
+ nullptr, 0, AttributeList::AS_ContextSensitiveKeyword);
};
if (D.getNumTypeObjects() > 0) {
// Add the attribute to the declarator chunk nearest the declarator.
-auto nullabilityAttr = getNullabilityAttr();
+auto nullabilityAttr = getNullabilityAttr(D.getAttributePool());
DeclaratorChunk = D.getTypeObject(0);
nullabilityAttr->setNext(chunk.getAttrListRef());
chunk.getAttrListRef() = nullabilityAttr;
} else if (!addedToDeclSpec) {
// Otherwise, just put it on the declaration specifiers (if one
// isn't there already).
-D.getMutableDeclSpec().addAttributes(getNullabilityAttr());
+D.getMutableDeclSpec().addAttributes(
+getNullabilityAttr(D.getDeclSpec().getAttributePool()));
addedToDeclSpec = true;
}
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48894: [AST] Rename some Redeclarable functions to reduce confusion
MaskRay created this revision.
MaskRay added reviewers: rsmith, akyrtzi, Eugene.Zelenko.
Herald added a subscriber: cfe-commits.
Repository:
rC Clang
https://reviews.llvm.org/D48894
Files:
include/clang/AST/Redeclarable.h
Index: include/clang/AST/Redeclarable.h
===
--- include/clang/AST/Redeclarable.h
+++ include/clang/AST/Redeclarable.h
@@ -36,7 +36,7 @@
//DeclLink that may point to one of 3 possible states:
// - the "previous" (temporal) element in the chain
// - the "latest" (temporal) element in the chain
-// - the an "uninitialized-latest" value (when newly-constructed)
+// - the "uninitialized-latest" value (when newly-constructed)
//
// - The first element is also often called the canonical element. Every
//element has a pointer to it so that "getCanonical" can be fast.
@@ -48,10 +48,8 @@
//"most-recent" when referring to temporal order: order of addition
//to the chain.
//
-// - To make matters confusing, the DeclLink type uses the term "next"
-//for its pointer-storage internally (thus functions like
-//NextIsPrevious). It's easiest to just ignore the implementation of
-//DeclLink when making sense of the redeclaration chain.
+// - It's easiest to just ignore the implementation of DeclLink when making
+//sense of the redeclaration chain.
//
// - There's also a "definition" link for several types of
//redeclarable, where only one definition should exist at any given
@@ -105,66 +103,64 @@
/// previous declaration.
using NotKnownLatest = llvm::PointerUnion;
-mutable llvm::PointerUnion Next;
+mutable llvm::PointerUnion Prev;
public:
enum PreviousTag { PreviousLink };
enum LatestTag { LatestLink };
DeclLink(LatestTag, const ASTContext )
-: Next(NotKnownLatest(reinterpret_cast())) {}
-DeclLink(PreviousTag, decl_type *D) : Next(NotKnownLatest(Previous(D))) {}
+: Prev(NotKnownLatest(reinterpret_cast())) {}
+DeclLink(PreviousTag, decl_type *D) : Prev(NotKnownLatest(Previous(D))) {}
-bool NextIsPrevious() const {
- return Next.is() &&
- // FIXME: 'template' is required on the next line due to an
- // apparent clang bug.
- Next.get().template is();
+bool isFirst() const {
+ return !(Prev.is() &&
+ // FIXME: 'template' is required on the next line due to an
+ // apparent clang bug.
+ Prev.get().template is());
}
-bool NextIsLatest() const { return !NextIsPrevious(); }
-
-decl_type *getNext(const decl_type *D) const {
- if (Next.is()) {
-NotKnownLatest NKL = Next.get();
+decl_type *getPrevious(const decl_type *D) const {
+ if (Prev.is()) {
+NotKnownLatest NKL = Prev.get();
if (NKL.is())
return static_cast(NKL.get());
// Allocate the generational 'most recent' cache now, if needed.
-Next = KnownLatest(*reinterpret_cast(
+Prev = KnownLatest(*reinterpret_cast(
NKL.get()),
const_cast(D));
}
- return static_cast(Next.get().get(D));
+ return static_cast(Prev.get().get(D));
}
void setPrevious(decl_type *D) {
- assert(NextIsPrevious() && "decl became non-canonical unexpectedly");
- Next = Previous(D);
+ assert(!isFirst() && "decl became non-canonical unexpectedly");
+ Prev = Previous(D);
}
void setLatest(decl_type *D) {
- assert(NextIsLatest() && "decl became canonical unexpectedly");
- if (Next.is()) {
-NotKnownLatest NKL = Next.get();
-Next = KnownLatest(*reinterpret_cast(
+ assert(isFirst() && "decl became canonical unexpectedly");
+ if (Prev.is()) {
+NotKnownLatest NKL = Prev.get();
+Prev = KnownLatest(*reinterpret_cast(
NKL.get()),
D);
} else {
-auto Latest = Next.get();
+auto Latest = Prev.get();
Latest.set(D);
-Next = Latest;
+Prev = Latest;
}
}
-void markIncomplete() { Next.get().markIncomplete(); }
+void markIncomplete() { Prev.get().markIncomplete(); }
Decl *getLatestNotUpdated() const {
- assert(NextIsLatest() && "expected a canonical decl");
- if (Next.is())
+ assert(isFirst() && "expected a canonical decl");
+ if (Prev.is())
return nullptr;
- return Next.get().getNotUpdated();
+ return Prev.get().getNotUpdated();
}
};
@@ -178,8 +174,8 @@
/// Points to the next redeclaration in the chain.
///
- /// If NextIsPrevious() is true, this is a link to the previous declaration
- /// of this same Decl. If NextIsLatest() is true, this is the first
+ /// If isFirst() is false, this is a link to the previous declaration
+ /// of this same Decl. If isFirst()
[PATCH] D48886: VS 2017 clang integration
dtarditi added a subscriber: hans. dtarditi added a comment. It's great to see your work on Visual Studio extension integration via VSIX. I'm happy to try out your change. I have a few questions: 1. How am I supposed to try this out? Given that actually building a VSIX installer isn't part of this change, what should I do? 2. Have you looked into scripting the VSIX creation? (Yes, I work at Microsoft, but I too am doing compiler development, not working on installers etc.) Repository: rL LLVM https://reviews.llvm.org/D48886 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
RE: r334650 - Implement constexpr __builtin_*_overflow
Yikes! Thanks for the heads up! I’ll start looking into this.
Thanks for letting me know.
From: Evgenii Stepanov [mailto:eugeni.stepa...@gmail.com]
Sent: Tuesday, July 3, 2018 12:59 PM
To: Keane, Erich
Cc: cfe-commits
Subject: Re: r334650 - Implement constexpr __builtin_*_overflow
Hi,
with this change, the following compiles to "ret i32 undef":
int main(int argc, char **argv) {
constexpr int x = 1;
constexpr int y = 2;
int z;
__builtin_sadd_overflow(x, y, );
return z;
}
On Wed, Jun 13, 2018 at 1:43 PM, Erich Keane via cfe-commits
mailto:cfe-commits@lists.llvm.org>> wrote:
Author: erichkeane
Date: Wed Jun 13 13:43:27 2018
New Revision: 334650
URL: http://llvm.org/viewvc/llvm-project?rev=334650=rev
Log:
Implement constexpr __builtin_*_overflow
As requested here:https://bugs.llvm.org/show_bug.cgi?id=37633
permit the __builtin_*_overflow builtins in constexpr functions.
Differential Revision: https://reviews.llvm.org/D48040
Modified:
cfe/trunk/lib/AST/ExprConstant.cpp
cfe/trunk/test/SemaCXX/builtins-overflow.cpp
Modified: cfe/trunk/lib/AST/ExprConstant.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=334650=334649=334650=diff
==
--- cfe/trunk/lib/AST/ExprConstant.cpp (original)
+++ cfe/trunk/lib/AST/ExprConstant.cpp Wed Jun 13 13:43:27 2018
@@ -8155,6 +8155,124 @@ bool IntExprEvaluator::VisitBuiltinCallE
case Builtin::BIomp_is_initial_device:
// We can decide statically which value the runtime would return if called.
return Success(Info.getLangOpts().OpenMPIsDevice ? 0 : 1, E);
+ case Builtin::BI__builtin_add_overflow:
+ case Builtin::BI__builtin_sub_overflow:
+ case Builtin::BI__builtin_mul_overflow:
+ case Builtin::BI__builtin_sadd_overflow:
+ case Builtin::BI__builtin_uadd_overflow:
+ case Builtin::BI__builtin_uaddl_overflow:
+ case Builtin::BI__builtin_uaddll_overflow:
+ case Builtin::BI__builtin_usub_overflow:
+ case Builtin::BI__builtin_usubl_overflow:
+ case Builtin::BI__builtin_usubll_overflow:
+ case Builtin::BI__builtin_umul_overflow:
+ case Builtin::BI__builtin_umull_overflow:
+ case Builtin::BI__builtin_umulll_overflow:
+ case Builtin::BI__builtin_saddl_overflow:
+ case Builtin::BI__builtin_saddll_overflow:
+ case Builtin::BI__builtin_ssub_overflow:
+ case Builtin::BI__builtin_ssubl_overflow:
+ case Builtin::BI__builtin_ssubll_overflow:
+ case Builtin::BI__builtin_smul_overflow:
+ case Builtin::BI__builtin_smull_overflow:
+ case Builtin::BI__builtin_smulll_overflow: {
+LValue ResultLValue;
+APSInt LHS, RHS;
+
+QualType ResultType = E->getArg(2)->getType()->getPointeeType();
+if (!EvaluateInteger(E->getArg(0), LHS, Info) ||
+!EvaluateInteger(E->getArg(1), RHS, Info) ||
+!EvaluatePointer(E->getArg(2), ResultLValue, Info))
+ return false;
+
+APSInt Result;
+bool DidOverflow = false;
+
+// If the types don't have to match, enlarge all 3 to the largest of them.
+if (BuiltinOp == Builtin::BI__builtin_add_overflow ||
+BuiltinOp == Builtin::BI__builtin_sub_overflow ||
+BuiltinOp == Builtin::BI__builtin_mul_overflow) {
+ bool IsSigned = LHS.isSigned() || RHS.isSigned() ||
+ ResultType->isSignedIntegerOrEnumerationType();
+ bool AllSigned = LHS.isSigned() && RHS.isSigned() &&
+ ResultType->isSignedIntegerOrEnumerationType();
+ uint64_t LHSSize = LHS.getBitWidth();
+ uint64_t RHSSize = RHS.getBitWidth();
+ uint64_t ResultSize = Info.Ctx.getTypeSize(ResultType);
+ uint64_t MaxBits = std::max(std::max(LHSSize, RHSSize), ResultSize);
+
+ // Add an additional bit if the signedness isn't uniformly agreed to. We
+ // could do this ONLY if there is a signed and an unsigned that both have
+ // MaxBits, but the code to check that is pretty nasty. The issue will
be
+ // caught in the shrink-to-result later anyway.
+ if (IsSigned && !AllSigned)
+++MaxBits;
+
+ LHS = APSInt(IsSigned ? LHS.sextOrSelf(MaxBits) :
LHS.zextOrSelf(MaxBits),
+ !IsSigned);
+ RHS = APSInt(IsSigned ? RHS.sextOrSelf(MaxBits) :
RHS.zextOrSelf(MaxBits),
+ !IsSigned);
+ Result = APSInt(MaxBits, !IsSigned);
+}
+
+// Find largest int.
+switch (BuiltinOp) {
+default:
+ llvm_unreachable("Invalid value for BuiltinOp");
+case Builtin::BI__builtin_add_overflow:
+case Builtin::BI__builtin_sadd_overflow:
+case Builtin::BI__builtin_saddl_overflow:
+case Builtin::BI__builtin_saddll_overflow:
+case Builtin::BI__builtin_uadd_overflow:
+case Builtin::BI__builtin_uaddl_overflow:
+case Builtin::BI__builtin_uaddll_overflow:
+ Result = LHS.isSigned() ? LHS.sadd_ov(RHS, DidOverflow)
+ : LHS.uadd_ov(RHS, DidOverflow);
+ break;
+case Builtin::BI__builtin_sub_overflow:
+case
Re: r334650 - Implement constexpr __builtin_*_overflow
Discovered by MemorySanitizer, btw.
On Tue, Jul 3, 2018 at 12:59 PM, Evgenii Stepanov wrote:
> Hi,
>
> with this change, the following compiles to "ret i32 undef":
>
> int main(int argc, char **argv) {
>
> constexpr int x = 1;
>
> constexpr int y = 2;
>
> int z;
>
>
>
> __builtin_sadd_overflow(x, y, );
>
> return z;
>
> }
>
>
> On Wed, Jun 13, 2018 at 1:43 PM, Erich Keane via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: erichkeane
>> Date: Wed Jun 13 13:43:27 2018
>> New Revision: 334650
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=334650=rev
>> Log:
>> Implement constexpr __builtin_*_overflow
>>
>> As requested here:https://bugs.llvm.org/show_bug.cgi?id=37633
>> permit the __builtin_*_overflow builtins in constexpr functions.
>>
>> Differential Revision: https://reviews.llvm.org/D48040
>>
>> Modified:
>> cfe/trunk/lib/AST/ExprConstant.cpp
>> cfe/trunk/test/SemaCXX/builtins-overflow.cpp
>>
>> Modified: cfe/trunk/lib/AST/ExprConstant.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprCo
>> nstant.cpp?rev=334650=334649=334650=diff
>>
>> ==
>> --- cfe/trunk/lib/AST/ExprConstant.cpp (original)
>> +++ cfe/trunk/lib/AST/ExprConstant.cpp Wed Jun 13 13:43:27 2018
>> @@ -8155,6 +8155,124 @@ bool IntExprEvaluator::VisitBuiltinCallE
>>case Builtin::BIomp_is_initial_device:
>> // We can decide statically which value the runtime would return if
>> called.
>> return Success(Info.getLangOpts().OpenMPIsDevice ? 0 : 1, E);
>> + case Builtin::BI__builtin_add_overflow:
>> + case Builtin::BI__builtin_sub_overflow:
>> + case Builtin::BI__builtin_mul_overflow:
>> + case Builtin::BI__builtin_sadd_overflow:
>> + case Builtin::BI__builtin_uadd_overflow:
>> + case Builtin::BI__builtin_uaddl_overflow:
>> + case Builtin::BI__builtin_uaddll_overflow:
>> + case Builtin::BI__builtin_usub_overflow:
>> + case Builtin::BI__builtin_usubl_overflow:
>> + case Builtin::BI__builtin_usubll_overflow:
>> + case Builtin::BI__builtin_umul_overflow:
>> + case Builtin::BI__builtin_umull_overflow:
>> + case Builtin::BI__builtin_umulll_overflow:
>> + case Builtin::BI__builtin_saddl_overflow:
>> + case Builtin::BI__builtin_saddll_overflow:
>> + case Builtin::BI__builtin_ssub_overflow:
>> + case Builtin::BI__builtin_ssubl_overflow:
>> + case Builtin::BI__builtin_ssubll_overflow:
>> + case Builtin::BI__builtin_smul_overflow:
>> + case Builtin::BI__builtin_smull_overflow:
>> + case Builtin::BI__builtin_smulll_overflow: {
>> +LValue ResultLValue;
>> +APSInt LHS, RHS;
>> +
>> +QualType ResultType = E->getArg(2)->getType()->getPointeeType();
>> +if (!EvaluateInteger(E->getArg(0), LHS, Info) ||
>> +!EvaluateInteger(E->getArg(1), RHS, Info) ||
>> +!EvaluatePointer(E->getArg(2), ResultLValue, Info))
>> + return false;
>> +
>> +APSInt Result;
>> +bool DidOverflow = false;
>> +
>> +// If the types don't have to match, enlarge all 3 to the largest of
>> them.
>> +if (BuiltinOp == Builtin::BI__builtin_add_overflow ||
>> +BuiltinOp == Builtin::BI__builtin_sub_overflow ||
>> +BuiltinOp == Builtin::BI__builtin_mul_overflow) {
>> + bool IsSigned = LHS.isSigned() || RHS.isSigned() ||
>> + ResultType->isSignedIntegerOrEnumerationType();
>> + bool AllSigned = LHS.isSigned() && RHS.isSigned() &&
>> + ResultType->isSignedIntegerOrEnumerationType();
>> + uint64_t LHSSize = LHS.getBitWidth();
>> + uint64_t RHSSize = RHS.getBitWidth();
>> + uint64_t ResultSize = Info.Ctx.getTypeSize(ResultType);
>> + uint64_t MaxBits = std::max(std::max(LHSSize, RHSSize),
>> ResultSize);
>> +
>> + // Add an additional bit if the signedness isn't uniformly agreed
>> to. We
>> + // could do this ONLY if there is a signed and an unsigned that
>> both have
>> + // MaxBits, but the code to check that is pretty nasty. The issue
>> will be
>> + // caught in the shrink-to-result later anyway.
>> + if (IsSigned && !AllSigned)
>> +++MaxBits;
>> +
>> + LHS = APSInt(IsSigned ? LHS.sextOrSelf(MaxBits) :
>> LHS.zextOrSelf(MaxBits),
>> + !IsSigned);
>> + RHS = APSInt(IsSigned ? RHS.sextOrSelf(MaxBits) :
>> RHS.zextOrSelf(MaxBits),
>> + !IsSigned);
>> + Result = APSInt(MaxBits, !IsSigned);
>> +}
>> +
>> +// Find largest int.
>> +switch (BuiltinOp) {
>> +default:
>> + llvm_unreachable("Invalid value for BuiltinOp");
>> +case Builtin::BI__builtin_add_overflow:
>> +case Builtin::BI__builtin_sadd_overflow:
>> +case Builtin::BI__builtin_saddl_overflow:
>> +case Builtin::BI__builtin_saddll_overflow:
>> +case Builtin::BI__builtin_uadd_overflow:
>> +case Builtin::BI__builtin_uaddl_overflow:
>> +case Builtin::BI__builtin_uaddll_overflow:
>> + Result = LHS.isSigned() ?
Re: r334650 - Implement constexpr __builtin_*_overflow
Hi,
with this change, the following compiles to "ret i32 undef":
int main(int argc, char **argv) {
constexpr int x = 1;
constexpr int y = 2;
int z;
__builtin_sadd_overflow(x, y, );
return z;
}
On Wed, Jun 13, 2018 at 1:43 PM, Erich Keane via cfe-commits <
cfe-commits@lists.llvm.org> wrote:
> Author: erichkeane
> Date: Wed Jun 13 13:43:27 2018
> New Revision: 334650
>
> URL: http://llvm.org/viewvc/llvm-project?rev=334650=rev
> Log:
> Implement constexpr __builtin_*_overflow
>
> As requested here:https://bugs.llvm.org/show_bug.cgi?id=37633
> permit the __builtin_*_overflow builtins in constexpr functions.
>
> Differential Revision: https://reviews.llvm.org/D48040
>
> Modified:
> cfe/trunk/lib/AST/ExprConstant.cpp
> cfe/trunk/test/SemaCXX/builtins-overflow.cpp
>
> Modified: cfe/trunk/lib/AST/ExprConstant.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/
> ExprConstant.cpp?rev=334650=334649=334650=diff
>
> ==
> --- cfe/trunk/lib/AST/ExprConstant.cpp (original)
> +++ cfe/trunk/lib/AST/ExprConstant.cpp Wed Jun 13 13:43:27 2018
> @@ -8155,6 +8155,124 @@ bool IntExprEvaluator::VisitBuiltinCallE
>case Builtin::BIomp_is_initial_device:
> // We can decide statically which value the runtime would return if
> called.
> return Success(Info.getLangOpts().OpenMPIsDevice ? 0 : 1, E);
> + case Builtin::BI__builtin_add_overflow:
> + case Builtin::BI__builtin_sub_overflow:
> + case Builtin::BI__builtin_mul_overflow:
> + case Builtin::BI__builtin_sadd_overflow:
> + case Builtin::BI__builtin_uadd_overflow:
> + case Builtin::BI__builtin_uaddl_overflow:
> + case Builtin::BI__builtin_uaddll_overflow:
> + case Builtin::BI__builtin_usub_overflow:
> + case Builtin::BI__builtin_usubl_overflow:
> + case Builtin::BI__builtin_usubll_overflow:
> + case Builtin::BI__builtin_umul_overflow:
> + case Builtin::BI__builtin_umull_overflow:
> + case Builtin::BI__builtin_umulll_overflow:
> + case Builtin::BI__builtin_saddl_overflow:
> + case Builtin::BI__builtin_saddll_overflow:
> + case Builtin::BI__builtin_ssub_overflow:
> + case Builtin::BI__builtin_ssubl_overflow:
> + case Builtin::BI__builtin_ssubll_overflow:
> + case Builtin::BI__builtin_smul_overflow:
> + case Builtin::BI__builtin_smull_overflow:
> + case Builtin::BI__builtin_smulll_overflow: {
> +LValue ResultLValue;
> +APSInt LHS, RHS;
> +
> +QualType ResultType = E->getArg(2)->getType()->getPointeeType();
> +if (!EvaluateInteger(E->getArg(0), LHS, Info) ||
> +!EvaluateInteger(E->getArg(1), RHS, Info) ||
> +!EvaluatePointer(E->getArg(2), ResultLValue, Info))
> + return false;
> +
> +APSInt Result;
> +bool DidOverflow = false;
> +
> +// If the types don't have to match, enlarge all 3 to the largest of
> them.
> +if (BuiltinOp == Builtin::BI__builtin_add_overflow ||
> +BuiltinOp == Builtin::BI__builtin_sub_overflow ||
> +BuiltinOp == Builtin::BI__builtin_mul_overflow) {
> + bool IsSigned = LHS.isSigned() || RHS.isSigned() ||
> + ResultType->isSignedIntegerOrEnumerationType();
> + bool AllSigned = LHS.isSigned() && RHS.isSigned() &&
> + ResultType->isSignedIntegerOrEnumerationType();
> + uint64_t LHSSize = LHS.getBitWidth();
> + uint64_t RHSSize = RHS.getBitWidth();
> + uint64_t ResultSize = Info.Ctx.getTypeSize(ResultType);
> + uint64_t MaxBits = std::max(std::max(LHSSize, RHSSize), ResultSize);
> +
> + // Add an additional bit if the signedness isn't uniformly agreed
> to. We
> + // could do this ONLY if there is a signed and an unsigned that
> both have
> + // MaxBits, but the code to check that is pretty nasty. The issue
> will be
> + // caught in the shrink-to-result later anyway.
> + if (IsSigned && !AllSigned)
> +++MaxBits;
> +
> + LHS = APSInt(IsSigned ? LHS.sextOrSelf(MaxBits) :
> LHS.zextOrSelf(MaxBits),
> + !IsSigned);
> + RHS = APSInt(IsSigned ? RHS.sextOrSelf(MaxBits) :
> RHS.zextOrSelf(MaxBits),
> + !IsSigned);
> + Result = APSInt(MaxBits, !IsSigned);
> +}
> +
> +// Find largest int.
> +switch (BuiltinOp) {
> +default:
> + llvm_unreachable("Invalid value for BuiltinOp");
> +case Builtin::BI__builtin_add_overflow:
> +case Builtin::BI__builtin_sadd_overflow:
> +case Builtin::BI__builtin_saddl_overflow:
> +case Builtin::BI__builtin_saddll_overflow:
> +case Builtin::BI__builtin_uadd_overflow:
> +case Builtin::BI__builtin_uaddl_overflow:
> +case Builtin::BI__builtin_uaddll_overflow:
> + Result = LHS.isSigned() ? LHS.sadd_ov(RHS, DidOverflow)
> + : LHS.uadd_ov(RHS, DidOverflow);
> + break;
> +case Builtin::BI__builtin_sub_overflow:
> +case Builtin::BI__builtin_ssub_overflow:
> +case
[PATCH] D48863: [Sema] Explain coroutine_traits template in diag
modocache updated this revision to Diff 153956.
modocache added a comment.
Oops, apologies, I included a line I shouldn't have in the previous diff.
Repository:
rC Clang
https://reviews.llvm.org/D48863
Files:
include/clang/Basic/DiagnosticSemaKinds.td
lib/Sema/SemaCoroutine.cpp
test/SemaCXX/coroutine-traits-incorrect-argument-types.cpp
Index: test/SemaCXX/coroutine-traits-incorrect-argument-types.cpp
===
--- /dev/null
+++ test/SemaCXX/coroutine-traits-incorrect-argument-types.cpp
@@ -0,0 +1,21 @@
+// This file contains references to sections of the Coroutines TS, which can be
+// found at http://wg21.link/coroutines.
+
+// RUN: %clang_cc1 -std=c++14 -fcoroutines-ts -verify %s -fcxx-exceptions
-fexceptions -Wunused-result
+
+namespace std {
+namespace experimental {
+
+template
+struct coroutine_traits { // expected-note {{template is declared here}}
+ struct promise_type {}; // expected-note@-1 {{template is declared here}}
+};
+}} // namespace std::experimental
+
+void too_few_types_for_coroutine_traits() { // expected-note {{the coroutine
traits class template is being instantiated using the return type, followed by
the parameter types, of this coroutine}}
+ co_return; // expected-error {{too few template arguments for class template
'coroutine_traits'}}
+}
+
+void too_many_types_for_coroutine_traits(int x, float y) { // expected-note
{{the coroutine traits class template is being instantiated using the return
type, followed by the parameter types, of this coroutine}}
+ co_return; // expected-error {{too many template arguments for class
template 'coroutine_traits'}}
+}
Index: lib/Sema/SemaCoroutine.cpp
===
--- lib/Sema/SemaCoroutine.cpp
+++ lib/Sema/SemaCoroutine.cpp
@@ -107,13 +107,21 @@
for (QualType T : FnType->getParamTypes())
AddArg(T);
- // Build the template-id.
+ // Try to instantiate the coroutine traits template specialization, referred
+ // to in [temp.names]p1 as a template-id.
QualType CoroTrait =
S.CheckTemplateIdType(TemplateName(CoroTraits), KwLoc, Args);
- if (CoroTrait.isNull())
+ if (CoroTrait.isNull()) {
+// The instantiation failed; maybe the user defined a coroutine_traits that
+// did something strange, like one that takes a non-type template
parameter.
+S.Diag(FD->getLocation(), diag::note_coroutine_types_for_traits_here);
return QualType();
+ }
+
if (S.RequireCompleteType(KwLoc, CoroTrait,
diag::err_coroutine_type_missing_specialization))
+// The particular specialization is missing. For example, the user may have
+// forward-declared it.
return QualType();
auto *RD = CoroTrait->getAsCXXRecordDecl();
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -9050,6 +9050,9 @@
def err_implied_coroutine_type_not_found : Error<
"%0 type was not found; include before defining "
"a coroutine">;
+def note_coroutine_types_for_traits_here : Note<
+ "the coroutine traits class template is being instantiated using the return "
+ "type, followed by the parameter types, of this coroutine">;
def err_implicit_coroutine_std_nothrow_type_not_found : Error<
"std::nothrow was not found; include before defining a coroutine which
"
"uses get_return_object_on_allocation_failure()">;
Index: test/SemaCXX/coroutine-traits-incorrect-argument-types.cpp
===
--- /dev/null
+++ test/SemaCXX/coroutine-traits-incorrect-argument-types.cpp
@@ -0,0 +1,21 @@
+// This file contains references to sections of the Coroutines TS, which can be
+// found at http://wg21.link/coroutines.
+
+// RUN: %clang_cc1 -std=c++14 -fcoroutines-ts -verify %s -fcxx-exceptions -fexceptions -Wunused-result
+
+namespace std {
+namespace experimental {
+
+template
+struct coroutine_traits { // expected-note {{template is declared here}}
+ struct promise_type {}; // expected-note@-1 {{template is declared here}}
+};
+}} // namespace std::experimental
+
+void too_few_types_for_coroutine_traits() { // expected-note {{the coroutine traits class template is being instantiated using the return type, followed by the parameter types, of this coroutine}}
+ co_return; // expected-error {{too few template arguments for class template 'coroutine_traits'}}
+}
+
+void too_many_types_for_coroutine_traits(int x, float y) { // expected-note {{the coroutine traits class template is being instantiated using the return type, followed by the parameter types, of this coroutine}}
+ co_return; // expected-error {{too many template arguments for class template 'coroutine_traits'}}
+}
Index: lib/Sema/SemaCoroutine.cpp
[PATCH] D48687: [clangd] Avoid duplicates in findDefinitions response
simark added a comment. In https://reviews.llvm.org/D48687#1150960, @hokein wrote: > After taking a look closely, I figured why there are two candidates here -- > one is from AST (the one with ".." path); the other one is from dynamic > index, the deduplication failed because of the different paths :( > > I think the fixing way is to normalize the file path from AST (making it > absolute). > > > +1 to sharing the code. I guess we're struggling with similar problems > > here. Any pointers to the functions we should use? > > Yeah, we struggled this problem in SymbolCollector. We cleanup the file path > manually. I'd suggest abstract that logic from `toURI` > (https://github.com/llvm-mirror/clang-tools-extra/blob/master/clangd/index/SymbolCollector.cpp#L80), > and use it here. > > > I tried to make it as close as possible to the example in the bug report, > > but don't manage to reproduce the bug. > > To reproduce the issue, I think you'd also need to enable the dynamic index. Thanks for looking into it. In my unit test, the only path I receive is one without "..", although I don't use the dynamic index. Still, I tried to enable the dynamic index using: auto Opts = ClangdServer::optsForTest(); Opts.BuildDynamicSymbolIndex = true; ClangdServer Server(CDB, FS, DiagConsumer, Opts); but it didn't change the result. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48687 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48249: [analyzer] Add stubs for argument construction contexts for arguments of C++ constructors and Objective-C messages.
george.karpenkov requested changes to this revision.
george.karpenkov added a comment.
This revision now requires changes to proceed.
Minor nit: request for code reuse.
Comment at: lib/Analysis/CFG.cpp:694
+std::is_same::value ||
+std::is_same::value>>
+ void findConstructionContextsForArguments(CallLikeExpr *E) {
Who needs inheritance if one has templates.
This is pretty bad though, I wish we could add another interface to those
three, if anything just to have `arguments()`
Comment at: lib/Analysis/ConstructionContext.cpp:115
// This is a constructor into a function argument. Not implemented yet.
-if (isa(ParentLayer->getTriggerStmt()))
+if (isa(ParentLayer->getTriggerStmt()) ||
+isa(ParentLayer->getTriggerStmt()) ||
Could we refactor the check into `isCallable` or whatever would be the
appropriate name here?
https://reviews.llvm.org/D48249
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r336219 - Fix crash in clang.
Author: zturner
Date: Tue Jul 3 11:12:39 2018
New Revision: 336219
URL: http://llvm.org/viewvc/llvm-project?rev=336219=rev
Log:
Fix crash in clang.
This happened during a recent refactor. toStringRefArray() returns
a vector, which was being implicitly converted to an
ArrayRef, and then the vector was immediately being
destroyed, so the ArrayRef<> was losing its backing storage.
Fix this by making sure the vector gets permanent storage.
Modified:
cfe/trunk/lib/Driver/Job.cpp
Modified: cfe/trunk/lib/Driver/Job.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Job.cpp?rev=336219=336218=336219=diff
==
--- cfe/trunk/lib/Driver/Job.cpp (original)
+++ cfe/trunk/lib/Driver/Job.cpp Tue Jul 3 11:12:39 2018
@@ -318,10 +318,12 @@ int Command::Execute(ArrayRef Argv;
Optional> Env;
+ std::vector ArgvVectorStorage;
if (!Environment.empty()) {
assert(Environment.back() == nullptr &&
"Environment vector should be null-terminated by now");
-Env = llvm::toStringRefArray(Environment.data());
+ArgvVectorStorage = llvm::toStringRefArray(Environment.data());
+Env = makeArrayRef(ArgvVectorStorage);
}
if (ResponseFile == nullptr) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48395: Added PublicOnly flag
anniecherk marked 11 inline comments as done.
anniecherk added inline comments.
Comment at: clang-tools-extra/clang-doc/Serialize.cpp:322-324
+ if(PublicOnly && ! isPublic(D->getAccess(), D->getLinkageInternal())){
+return "";
+ }
juliehockett wrote:
> Since this is the same for Record/Function/Enum, can we move this to
> `populateSymbolInfo()`?
>
> Also, elide braces on single-line ifs.
I don't see a good way to put it into populateSymbolInfo because if the
condition passes then the emitInfo method needs to bail out and I don't see a
clean way to do that if the check is in populateSymbolInfo. A clunky way to do
this would be to either have populateSymbolInfo set a flag that emitInfo checks
or emitInfo can assume that populateSymbolInfo bailed if all the info is
unpopulated, but that seems like a worse way to do it. Am I missing something?
I can refactor the condition into a function if that would be better for
understandability / maintainability.
https://reviews.llvm.org/D48395
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45898: [SemaCXX] Mark destructor as referenced
rjmccall added a comment. LGTM, but I'd like Richard to sign off, too. Repository: rC Clang https://reviews.llvm.org/D45898 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
RE: [clang-tools-extra] r336177 - [clangd] Incorporate transitive #includes into code complete proximity scoring.
Hi Sam,
This commit is causing failures on windows buildbots.
Testing Time: 211.19s
Failing Tests (1):
Extra Tools Unit Tests ::
clangd/Checking/./ClangdTests.exe/FileDistanceTests.URI
Example build:
http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/18166
These failures are also occurring on our internal bots. Could you take a look
at this?
Thanks,
Matthew
-Original Message-
From: cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] On Behalf Of Sam
McCall via cfe-commits
Sent: Tuesday, July 3, 2018 1:09 AM
To: cfe-commits@lists.llvm.org
Subject: [clang-tools-extra] r336177 - [clangd] Incorporate transitive
#includes into code complete proximity scoring.
Author: sammccall
Date: Tue Jul 3 01:09:29 2018
New Revision: 336177
URL: http://llvm.org/viewvc/llvm-project?rev=336177=rev
Log:
[clangd] Incorporate transitive #includes into code complete proximity scoring.
Summary:
We now compute a distance from the main file to the symbol header, which
is a weighted count of:
- some number of #include traversals from source file --> included file
- some number of FS traversals from file --> parent directory
- some number of FS traversals from parent directory --> child file/dir
This calculation is performed in the appropriate URI scheme.
This means we'll get some proximity boost from header files in main-file
contexts, even when these are in different directory trees.
This extended file proximity model is not yet incorporated in the index
interface/implementation.
Reviewers: ioeric
Subscribers: mgorny, ilya-biryukov, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D48441
Added:
clang-tools-extra/trunk/clangd/FileDistance.cpp
clang-tools-extra/trunk/clangd/FileDistance.h
clang-tools-extra/trunk/unittests/clangd/FileDistanceTests.cpp
Modified:
clang-tools-extra/trunk/clangd/CMakeLists.txt
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/ClangdUnit.cpp
clang-tools-extra/trunk/clangd/ClangdUnit.h
clang-tools-extra/trunk/clangd/CodeComplete.cpp
clang-tools-extra/trunk/clangd/CodeComplete.h
clang-tools-extra/trunk/clangd/Headers.cpp
clang-tools-extra/trunk/clangd/Headers.h
clang-tools-extra/trunk/clangd/Quality.cpp
clang-tools-extra/trunk/clangd/Quality.h
clang-tools-extra/trunk/clangd/XRefs.cpp
clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt
clang-tools-extra/trunk/unittests/clangd/HeadersTests.cpp
clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp
Modified: clang-tools-extra/trunk/clangd/CMakeLists.txt
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CMakeLists.txt?rev=336177=336176=336177=diff
==
--- clang-tools-extra/trunk/clangd/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clangd/CMakeLists.txt Tue Jul 3 01:09:29 2018
@@ -19,6 +19,7 @@ add_clang_library(clangDaemon
Diagnostics.cpp
DraftStore.cpp
FindSymbols.cpp
+ FileDistance.cpp
FuzzyMatch.cpp
GlobalCompilationDatabase.cpp
Headers.cpp
Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=336177=336176=336177=diff
==
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Tue Jul 3 01:09:29 2018
@@ -167,7 +167,7 @@ void ClangdServer::codeComplete(PathRef
// both the old and the new version in case only one of them matches.
CodeCompleteResult Result = clangd::codeComplete(
File, IP->Command, PreambleData ? >Preamble : nullptr,
-PreambleData ? PreambleData->Inclusions : std::vector(),
+PreambleData ? PreambleData->Includes : IncludeStructure(),
IP->Contents, Pos, FS, PCHs, CodeCompleteOpts);
CB(std::move(Result));
};
Modified: clang-tools-extra/trunk/clangd/ClangdUnit.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnit.cpp?rev=336177=336176=336177=diff
==
--- clang-tools-extra/trunk/clangd/ClangdUnit.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp Tue Jul 3 01:09:29 2018
@@ -88,7 +88,7 @@ public:
CppFilePreambleCallbacks(PathRef File, PreambleParsedCallback ParsedCallback)
: File(File), ParsedCallback(ParsedCallback) {}
- std::vector takeInclusions() { return std::move(Inclusions); }
+ IncludeStructure takeIncludes() { return std::move(Includes); }
void AfterExecute(CompilerInstance ) override {
if (!ParsedCallback)
@@ -103,15 +103,13 @@ public:
std::unique_ptr createPPCallbacks() override {
assert(SourceMgr && "SourceMgr must be set at this point");
-
[PATCH] D48661: [Fixed Point Arithmetic] Fixed Point Constant
rjmccall added inline comments.
Comment at: include/clang/Basic/FixedPoint.h:31
+ SatNoPadding,
+};
+
leonardchan wrote:
> ebevhan wrote:
> > rjmccall wrote:
> > > I figured you'd want this to be a struct which include the scale, width,
> > > signed-ness, and saturating-ness; and then `APFixedPoint` can just store
> > > one of these next to a bare `APInt`.
> > That's what I was expecting too, similar to the APFloat version.
> >
> > What width would it contain? The width with or without padding? Probably
> > the one with padding as the APInt itself has the width without padding.
> Woops my bad, I thought you were referring to the `APFloatSemantics`. I
> actually didn't know about `fltSemantics` until now.
Thanks, this looks a lot better to me. Please do capitalize
`FixedPointSemantics`, though; `fltSemantics` is a bizarre deviation from
LLVM's style guidelines that should be fixed, not something to emulate.
Comment at: include/clang/Basic/FixedPoint.h:41
+ APFixedPoint(const llvm::APSInt , unsigned Scale,
+ enum FixedPointSemantics Sema)
+ : Val(Val), Scale(Scale), Sema(Sema) {}
rjmccall wrote:
> Why the elaborated-type-specifier?
Similarly, this should be renamed to `getIntegralBits` to follow the normal
LLVM method-naming guidelines.
Also, please go ahead and hide all the members here behind getters and setters.
It's better to reserve flexibility in advance than to have to rewrite a bunch
of code later.
Repository:
rC Clang
https://reviews.llvm.org/D48661
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48886: VS 2017 clang integration
dtarditi created this revision.
dtarditi added a reviewer: hans.
Herald added subscribers: delcypher, mgorny.
Add support for integrating clang as a platform toolset for Visual Studio 2017.
In prior versions of Visual Studio, MSBuild was installed separately in its own
known directory location and the platform toolset information was placed within
that direcotry. In VS2017, MSBuild is installed in the VS 2017 directory and
the platform toolset information is placed in a different directory within the
VS 2017 directory. Use the vswhere program
(https://github.com/Microsoft/vswhere) to locate the installation of VS 2017.
vswhere is guaranteed to be placed in a known location on a machine, and can
then be used to locate VS 2017 installs. With that information we can install
the .props and .targets files for MSBuild. These are slightly modified
versions of the (misnamed) VS 2014 files.
Visual Studio allows side-by-side installations, including installations of
different products (Community, Professional, and Enterprise) with the same
version and the same product with the same version. For now, just use the
latest version of VS 2017 in the install and uninstall scripts, so we get
predictable behavior. We leave it as future work for someone else to deal
with side-by-side installs.
Testing: this change is being backported from
https://github.com/Microsoft/checkedc-llvm.Other people have tried out the
change there and it has worked for them. I locally built a clean version of
the Windows LLVM installer with the backported change and tried it out. It
worked fine.
Repository:
rC Clang
https://reviews.llvm.org/D48886
Files:
tools/msbuild/CMakeLists.txt
tools/msbuild/install.bat
tools/msbuild/toolset-vs2017.targets
tools/msbuild/toolset-vs2017_xp.targets
tools/msbuild/uninstall.bat
utils/lit/lit/TestingConfig.py
Index: utils/lit/lit/TestingConfig.py
===
--- utils/lit/lit/TestingConfig.py
+++ utils/lit/lit/TestingConfig.py
@@ -37,6 +37,7 @@
environment.update({
'INCLUDE' : os.environ.get('INCLUDE',''),
'PATHEXT' : os.environ.get('PATHEXT',''),
+'PROGRAMDATA' : os.environ.get('PROGRAMDATA',''),
'PYTHONUNBUFFERED' : '1',
'TEMP' : os.environ.get('TEMP',''),
'TMP' : os.environ.get('TMP',''),
Index: tools/msbuild/uninstall.bat
===
--- tools/msbuild/uninstall.bat
+++ tools/msbuild/uninstall.bat
@@ -66,6 +66,26 @@
IF EXIST %D%\LLVM-vs2014_xp del %D%\LLVM-vs2014_xp\toolset.targets
IF EXIST %D%\LLVM-vs2014_xp rmdir %D%\LLVM-vs2014_xp
+REM MSBuild is now under the Visual Studio installation. VSWhere is a new executable placed in a known
+REM location that can be used to find the VS installation, starting with VS 2017 SP1
+IF EXIST "%ProgramFiles(x86)%\Microsoft Visual Studio\Installer\vswhere.exe" (
+ FOR /f "usebackq delims=" %%i IN (`"%ProgramFiles(x86)%\Microsoft Visual Studio\Installer\vswhere.exe" -latest -prerelease -version 15 -property installationPath`) DO (
+SET D="%%i\Common7\IDE\VC\VCTargets\Platforms\%PLATFORM%\PlatformToolsets"
+ )
+)
+
+REM On 32-bit Windows OSes before Windows 10, vswhere will be installed under %ProgramFiles%
+IF EXIST "%ProgramFiles%\Microsoft Visual Studio\Installer\vswhere.exe" (
+ FOR /f "usebackq delims=" %%i IN (`"%ProgramFiles%\Microsoft Visual Studio\Installer\vswhere.exe" -latest -prerelease -version 15 -property installationPath`) DO (
+SET D="%%i\Common7\IDE\VC\VCTargets\Platforms\%PLATFORM%\PlatformToolsets"
+ )
+)
+IF EXIST %D%\LLVM-vs2017 del %D%\LLVM-vs2017\toolset.props
+IF EXIST %D%\LLVM-vs2017 del %D%\LLVM-vs2017\toolset.targets
+IF EXIST %D%\LLVM-vs2017 rmdir %D%\LLVM-vs2017
+IF EXIST %D%\LLVM-vs2017_xp del %D%\LLVM-vs2017_xp\toolset.props
+IF EXIST %D%\LLVM-vs2017_xp del %D%\LLVM-vs2017_xp\toolset.targets
+IF EXIST %D%\LLVM-vs2017_xp rmdir %D%\LLVM-vs2017_xp
GOTO LOOPHEAD
Index: tools/msbuild/toolset-vs2017_xp.targets
===
--- tools/msbuild/toolset-vs2017_xp.targets
+++ tools/msbuild/toolset-vs2017_xp.targets
@@ -0,0 +1,21 @@
+http://schemas.microsoft.com/developer/msbuild/2003;>
+
+
+v4.0
+NoSupportCodeAnalysisXP;$(BeforeClCompileTargets)
+
+
+
+
+
+
+
+
+
+CheckWindowsSDK71A;$(PrepareForBuildDependsOn)
+
+
+
+
+
+
Index: tools/msbuild/toolset-vs2017.targets
===
--- tools/msbuild/toolset-vs2017.targets
+++ tools/msbuild/toolset-vs2017.targets
@@ -0,0 +1,3 @@
+http://schemas.microsoft.com/developer/msbuild/2003;>
+
+
Index: tools/msbuild/install.bat
===
--- tools/msbuild/install.bat
+++
[PATCH] D46919: [libclang] Deprecate CXPrintingPolicy_IncludeTagDefinition
jdenny updated this revision to Diff 153930.
jdenny added a comment.
Ping. Rebased.
https://reviews.llvm.org/D46919
Files:
include/clang-c/Index.h
include/clang/AST/PrettyPrinter.h
lib/AST/DeclPrinter.cpp
lib/AST/TypePrinter.cpp
tools/c-index-test/c-index-test.c
Index: tools/c-index-test/c-index-test.c
===
--- tools/c-index-test/c-index-test.c
+++ tools/c-index-test/c-index-test.c
@@ -99,8 +99,6 @@
CXPrintingPolicy_SuppressSpecifiers},
{"CINDEXTEST_PRINTINGPOLICY_SUPPRESSTAGKEYWORD",
CXPrintingPolicy_SuppressTagKeyword},
- {"CINDEXTEST_PRINTINGPOLICY_INCLUDETAGDEFINITION",
- CXPrintingPolicy_IncludeTagDefinition},
{"CINDEXTEST_PRINTINGPOLICY_SUPPRESSSCOPE",
CXPrintingPolicy_SuppressScope},
{"CINDEXTEST_PRINTINGPOLICY_SUPPRESSUNWRITTENSCOPE",
Index: lib/AST/TypePrinter.cpp
===
--- lib/AST/TypePrinter.cpp
+++ lib/AST/TypePrinter.cpp
@@ -454,7 +454,7 @@
OS << '(';
PrintingPolicy InnerPolicy(Policy);
- InnerPolicy.IncludeTagDefinition = false;
+ InnerPolicy.State.PrintOwnedTagDecl = false;
TypePrinter(InnerPolicy).print(QualType(T->getClass(), 0), OS, StringRef());
OS << "::*";
@@ -1054,9 +1054,9 @@
}
void TypePrinter::printTag(TagDecl *D, raw_ostream ) {
- if (Policy.IncludeTagDefinition) {
+ if (Policy.State.PrintOwnedTagDecl) {
PrintingPolicy SubPolicy = Policy;
-SubPolicy.IncludeTagDefinition = false;
+SubPolicy.State.PrintOwnedTagDecl = false;
D->print(OS, SubPolicy, Indentation);
spaceBeforePlaceHolder(OS);
return;
@@ -1209,35 +1209,34 @@
void TypePrinter::printElaboratedBefore(const ElaboratedType *T,
raw_ostream ) {
- if (Policy.IncludeTagDefinition && T->getOwnedTagDecl()) {
+ if (Policy.State.PrintOwnedTagDecl && T->getOwnedTagDecl()) {
TagDecl *OwnedTagDecl = T->getOwnedTagDecl();
assert(OwnedTagDecl->getTypeForDecl() == T->getNamedType().getTypePtr() &&
"OwnedTagDecl expected to be a declaration for the type");
PrintingPolicy SubPolicy = Policy;
-SubPolicy.IncludeTagDefinition = false;
+SubPolicy.State.PrintOwnedTagDecl = false;
OwnedTagDecl->print(OS, SubPolicy, Indentation);
spaceBeforePlaceHolder(OS);
return;
}
// The tag definition will take care of these.
- if (!Policy.IncludeTagDefinition)
- {
+ if (!Policy.State.PrintOwnedTagDecl) {
OS << TypeWithKeyword::getKeywordName(T->getKeyword());
if (T->getKeyword() != ETK_None)
OS << " ";
NestedNameSpecifier *Qualifier = T->getQualifier();
if (Qualifier)
Qualifier->print(OS, Policy);
}
-
+
ElaboratedTypePolicyRAII PolicyRAII(Policy);
printBefore(T->getNamedType(), OS);
}
void TypePrinter::printElaboratedAfter(const ElaboratedType *T,
raw_ostream ) {
- if (Policy.IncludeTagDefinition && T->getOwnedTagDecl())
+ if (Policy.State.PrintOwnedTagDecl && T->getOwnedTagDecl())
return;
ElaboratedTypePolicyRAII PolicyRAII(Policy);
printAfter(T->getNamedType(), OS);
Index: lib/AST/DeclPrinter.cpp
===
--- lib/AST/DeclPrinter.cpp
+++ lib/AST/DeclPrinter.cpp
@@ -178,12 +178,12 @@
for ( ; Begin != End; ++Begin) {
if (isFirst) {
if(TD)
-SubPolicy.IncludeTagDefinition = true;
+SubPolicy.State.PrintOwnedTagDecl = true;
SubPolicy.SuppressSpecifiers = false;
isFirst = false;
} else {
if (!isFirst) Out << ", ";
- SubPolicy.IncludeTagDefinition = false;
+ SubPolicy.State.PrintOwnedTagDecl = false;
SubPolicy.SuppressSpecifiers = true;
}
@@ -849,7 +849,7 @@
}
PrintingPolicy SubPolicy(Policy);
SubPolicy.SuppressSpecifiers = false;
- SubPolicy.IncludeTagDefinition = false;
+ SubPolicy.State.PrintOwnedTagDecl = false;
Init->printPretty(Out, nullptr, SubPolicy, Indentation);
if ((D->getInitStyle() == VarDecl::CallInit) && !isa(Init))
Out << ")";
Index: include/clang/AST/PrettyPrinter.h
===
--- include/clang/AST/PrettyPrinter.h
+++ include/clang/AST/PrettyPrinter.h
@@ -93,14 +93,7 @@
/// \endcode
bool SuppressTagKeyword : 1;
- /// When true, include the body of a tag definition.
- ///
- /// This is used to place the definition of a struct
- /// in the middle of another declaration as with:
- ///
- /// \code
- /// typedef struct { int x, y; } Point;
- /// \endcode
+ /// This flag is deprecated and no longer has any effect.
bool IncludeTagDefinition : 1;
/// Suppresses printing of scope specifiers.
@@ -225,6 +218,25 @@
/// When true, print the fully qualified name of function declarations.
/// This is the opposite of SuppressScope and
[PATCH] D48687: [clangd] Avoid duplicates in findDefinitions response
hokein added a comment. After taking a look closely, I figured why there are two candidates here -- one is from AST (the one with ".." path); the other one is from dynamic index, the deduplication failed because of the different paths :( I think the fixing way is to normalize the file path from AST (making it absolute). > +1 to sharing the code. I guess we're struggling with similar problems here. > Any pointers to the functions we should use? Yeah, we struggled this problem in SymbolCollector. We cleanup the file path manually. I'd suggest abstract that logic from `toURI` (https://github.com/llvm-mirror/clang-tools-extra/blob/master/clangd/index/SymbolCollector.cpp#L80), and use it here. > I tried to make it as close as possible to the example in the bug report, but > don't manage to reproduce the bug. To reproduce the issue, I think you'd also need to enable the dynamic index. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48687 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48661: [Fixed Point Arithmetic] Fixed Point Constant
leonardchan added inline comments.
Comment at: lib/Basic/FixedPoint.cpp:40
+ if (DstWidth > Val.getBitWidth())
+Val = Val.extend(DstWidth);
+ if (Upscaling)
ebevhan wrote:
> It should be possible to replace this with `extOrTrunc` and move it below the
> saturation.
I don't think we can move the extension below saturation since saturation
requires checking the bits above the dst width which may require extending and
shifting beforehand.
Still leaving it above saturation may require checking for the width over using
`extOrTrunc` to prevent truncating early before right shifting.
Comment at: lib/Basic/FixedPoint.cpp:58
+ if (!DstSema.isSigned && Val.isNegative()) {
+Val = 0;
+ } else if (!(Masked == Mask || Masked == 0)) {
ebevhan wrote:
> If the value is unsigned and negative, we always zero it?
>
> Also, this code always saturates the value regardless of whether the
> destination semantics call for it, no?
You're right. I did not test for this since I thought this would fall under
undefined behavior converting a signed negative number to an unsigned number.
I'll add a check for saturation since I imagine most people would expect
modular wrap around, but do you think I should add a test case for this?
Comment at: lib/Basic/FixedPoint.cpp:73
+ if (DstWidth < Val.getBitWidth())
+Val = Val.trunc(DstWidth);
+
ebevhan wrote:
> When we say 'width' do we mean the width - padding or the width including
> padding? What information does the semantics structure enshrine?
This width includes the padding. This is commented on over `struct
fixedPointSema` where this width represents that of the underlying APInt, so if
the sema is unsigned and hasUnsignedPadding is true, then in a provided APInt
with this width, the `width-1` bit is padding.
Repository:
rC Clang
https://reviews.llvm.org/D48661
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48661: [Fixed Point Arithmetic] Fixed Point Constant
leonardchan updated this revision to Diff 153924.
leonardchan marked 7 inline comments as done.
Repository:
rC Clang
https://reviews.llvm.org/D48661
Files:
include/clang/AST/ASTContext.h
include/clang/Basic/FixedPoint.h
include/clang/Basic/TargetInfo.h
lib/AST/ASTContext.cpp
lib/Basic/CMakeLists.txt
lib/Basic/FixedPoint.cpp
lib/Sema/SemaExpr.cpp
test/Frontend/fixed_point_declarations.c
unittests/Basic/CMakeLists.txt
unittests/Basic/FixedPointTest.cpp
Index: unittests/Basic/FixedPointTest.cpp
===
--- /dev/null
+++ unittests/Basic/FixedPointTest.cpp
@@ -0,0 +1,618 @@
+//===- unittests/Basic/FixedPointTest.cpp -- fixed point number tests -===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "clang/Basic/FixedPoint.h"
+#include "llvm/ADT/APSInt.h"
+#include "gtest/gtest.h"
+
+using clang::APFixedPoint;
+using clang::fixedPointSemantics;
+using llvm::APInt;
+using llvm::APSInt;
+
+namespace {
+
+struct fixedPointSemantics Saturated(struct fixedPointSemantics Sema) {
+ Sema.isSaturated = true;
+ return Sema;
+}
+
+struct fixedPointSemantics getSAccSema() {
+ return {/*width=*/16, /*scale=*/7, /*isSigned=*/true,
+ /*isSaturated=*/false, /*hasUnsignedPadding=*/false};
+}
+
+struct fixedPointSemantics getAccSema() {
+ return {/*width=*/32, /*scale=*/15, /*isSigned=*/true,
+ /*isSaturated=*/false, /*hasUnsignedPadding=*/false};
+}
+
+struct fixedPointSemantics getLAccSema() {
+ return {/*width=*/64, /*scale=*/31, /*isSigned=*/true,
+ /*isSaturated=*/false, /*hasUnsignedPadding=*/false};
+}
+
+struct fixedPointSemantics getSFractSema() {
+ return {/*width=*/8, /*scale=*/7, /*isSigned=*/true,
+ /*isSaturated=*/false, /*hasUnsignedPadding=*/false};
+}
+
+struct fixedPointSemantics getFractSema() {
+ return {/*width=*/16, /*scale=*/15, /*isSigned=*/true,
+ /*isSaturated=*/false, /*hasUnsignedPadding=*/false};
+}
+
+struct fixedPointSemantics getLFractSema() {
+ return {/*width=*/32, /*scale=*/31, /*isSigned=*/true,
+ /*isSaturated=*/false, /*hasUnsignedPadding=*/false};
+}
+
+struct fixedPointSemantics getUSAccSema() {
+ return {/*width=*/16, /*scale=*/8, /*isSigned=*/false,
+ /*isSaturated=*/false, /*hasUnsignedPadding=*/false};
+}
+
+struct fixedPointSemantics getUAccSema() {
+ return {/*width=*/32, /*scale=*/16, /*isSigned=*/false,
+ /*isSaturated=*/false, /*hasUnsignedPadding=*/false};
+}
+
+struct fixedPointSemantics getULAccSema() {
+ return {/*width=*/64, /*scale=*/32, /*isSigned=*/false,
+ /*isSaturated=*/false, /*hasUnsignedPadding=*/false};
+}
+
+struct fixedPointSemantics getUSFractSema() {
+ return {/*width=*/8, /*scale=*/8, /*isSigned=*/false,
+ /*isSaturated=*/false, /*hasUnsignedPadding=*/false};
+}
+
+struct fixedPointSemantics getUFractSema() {
+ return {/*width=*/16, /*scale=*/16, /*isSigned=*/false,
+ /*isSaturated=*/false, /*hasUnsignedPadding=*/false};
+}
+
+struct fixedPointSemantics getULFractSema() {
+ return {/*width=*/32, /*scale=*/32, /*isSigned=*/false,
+ /*isSaturated=*/false, /*hasUnsignedPadding=*/false};
+}
+
+struct fixedPointSemantics getPadUSAccSema() {
+ return {/*width=*/16, /*scale=*/7, /*isSigned=*/false,
+ /*isSaturated=*/false, /*hasUnsignedPadding=*/true};
+}
+
+struct fixedPointSemantics getPadUAccSema() {
+ return {/*width=*/32, /*scale=*/15, /*isSigned=*/false,
+ /*isSaturated=*/false, /*hasUnsignedPadding=*/true};
+}
+
+struct fixedPointSemantics getPadULAccSema() {
+ return {/*width=*/64, /*scale=*/31, /*isSigned=*/false,
+ /*isSaturated=*/false, /*hasUnsignedPadding=*/true};
+}
+
+struct fixedPointSemantics getPadUSFractSema() {
+ return {/*width=*/8, /*scale=*/7, /*isSigned=*/false,
+ /*isSaturated=*/false, /*hasUnsignedPadding=*/true};
+}
+
+struct fixedPointSemantics getPadUFractSema() {
+ return {/*width=*/16, /*scale=*/15, /*isSigned=*/false,
+ /*isSaturated=*/false, /*hasUnsignedPadding=*/true};
+}
+
+struct fixedPointSemantics getPadULFractSema() {
+ return {/*width=*/32, /*scale=*/31, /*isSigned=*/false,
+ /*isSaturated=*/false, /*hasUnsignedPadding=*/true};
+}
+
+void CheckUnpaddedMax(const fixedPointSemantics ) {
+ ASSERT_EQ(APFixedPoint::getMax(Sema).getValue(),
+APSInt::getMaxValue(Sema.width, !Sema.isSigned));
+}
+
+void CheckPaddedMax(const fixedPointSemantics ) {
+ ASSERT_EQ(APFixedPoint::getMax(Sema).getValue(),
+APSInt::getMaxValue(Sema.width, !Sema.isSigned) >> 1);
+}
+
+void CheckMin(const fixedPointSemantics ) {
+ ASSERT_EQ(APFixedPoint::getMin(Sema).getValue(),
+APSInt::getMinValue(Sema.width, !Sema.isSigned));
+}
[PATCH] D48773: [ASTImporter] Fix import of objects with anonymous types
balazske added inline comments.
Comment at: lib/AST/ASTImporter.cpp:2085
}
+ } else {
+if (!IsStructuralMatch(D, FoundRecord, false))
martong wrote:
> a_sidorin wrote:
> > Is it possible to use the added code for the entire condition `if (auto
> > *FoundRecord = dyn_cast(Found))`, replacing its body? Our
> > structural matching already knows how to handle unnamed structures, and the
> > upper code partially duplicates
> > `IsStructurallyEquivalent(StructuralEquivalenceContext ,RecordDecl
> > *D1, RecordDecl *D2)`. Can we change structural matching to handle this
> > stuff instead of doing it in ASTNodeImporter?
> Yes, this is a good point, updated the patch accordingly.
My idea was to remove this whole `if (!SearchName)` branch, but some
restructuring of the next conditions may be needed. Setting of `PrevDecl` in
the `if` branch does not look correct (if `!SearchName` is false it is never
set, unlike in the old code).
Repository:
rC Clang
https://reviews.llvm.org/D48773
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48854: Use ExprMutationAnalyzer in performance-for-range-copy
hokein accepted this revision. hokein added a comment. This revision is now accepted and ready to land. LGTM. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48854 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48866: [clang-tidy] Add incorrect-pointer-cast checker
Eugene.Zelenko added a comment.
//bugprone// seems to be proper category for this check.
Comment at: clang-tidy/misc/IncorrectPointerCastCheck.cpp:35
+ const ASTContext = *Result.Context;
+ const CStyleCastExpr *CastExpr =
+ Result.Nodes.getNodeAs("cast");
const auto * could be used instead.
Comment at: clang-tidy/misc/IncorrectPointerCastCheck.h:19
+
+/// This checker warns for cases when pointer is cast and the pointed to type
is
+/// incompatible with allocated memory area type. This may lead to access
memory
Please make sentence same as in Release Notes.
Comment at: docs/clang-tidy/checks/misc-incorrect-pointer-cast.rst:6
+
+This checker warns for cases when pointer is cast and the pointed to type is
+incompatible with allocated memory area type. This may lead to access memory
Please make this sentence same as in Release Notes.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D48866
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42682: [clang-tidy] Add io-functions-misused checker
aaron.ballman added inline comments.
Comment at: clang-tidy/bugprone/BugproneTidyModule.cpp:84
+CheckFactories.registerCheck(
+"bugprone-io-functions-misused");
CheckFactories.registerCheck(
This name reads a bit awkwardly because usually "misused" would come before "io
functions". However, given that we've already said it's prone to bugs, I think
"misuse" is somewhat implied. Perhaps `bugprone-io-functions` or
`bugprone-misused-io-functions'?
However, I think this is actually checking an existing CERT rule and perhaps
should be added to the CERT module as `cert-fio34-c` and perhaps that's the
only module it should live in?
Comment at: clang-tidy/bugprone/IoFunctionsMisusedCheck.cpp:25-31
+ has(callExpr(callee(
+ functionDecl(hasAnyName("::getwchar", "::getwc", "::fgetwc"))
+ .bind("FuncDecl",
+ has(callExpr(callee(
+ functionDecl(hasAnyName("::std::getwchar", "::std::getwc",
+ "::std::fgetwc"))
+ .bind("FuncDecl",
You can combine these `hasAnyName()` calls into one given that they are
otherwise identical matchers.
Comment at: clang-tidy/bugprone/IoFunctionsMisusedCheck.cpp:26
+ has(callExpr(callee(
+ functionDecl(hasAnyName("::getwchar", "::getwc", "::fgetwc"))
+ .bind("FuncDecl",
Why only the wide char versions and not the narrow char versions as well? For
instance, those are problematic on systems where `sizeof(int) == sizeof(char)`.
See
https://wiki.sei.cmu.edu/confluence/display/c/FIO34-C.+Distinguish+between+characters+read+from+a+file+and+EOF+or+WEOF
for more details.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D42682
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48773: [ASTImporter] Fix import of objects with anonymous types
martong added inline comments.
Comment at: lib/AST/ASTImporter.cpp:2085
}
+ } else {
+if (!IsStructuralMatch(D, FoundRecord, false))
a_sidorin wrote:
> Is it possible to use the added code for the entire condition `if (auto
> *FoundRecord = dyn_cast(Found))`, replacing its body? Our
> structural matching already knows how to handle unnamed structures, and the
> upper code partially duplicates
> `IsStructurallyEquivalent(StructuralEquivalenceContext ,RecordDecl
> *D1, RecordDecl *D2)`. Can we change structural matching to handle this stuff
> instead of doing it in ASTNodeImporter?
Yes, this is a good point, updated the patch accordingly.
Repository:
rC Clang
https://reviews.llvm.org/D48773
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r336203 - [clangd] Use default format style and fallback style. NFC
Author: ioeric
Date: Tue Jul 3 07:51:23 2018
New Revision: 336203
URL: http://llvm.org/viewvc/llvm-project?rev=336203=rev
Log:
[clangd] Use default format style and fallback style. NFC
Modified:
clang-tools-extra/trunk/clangd/CodeComplete.cpp
Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=336203=336202=336203=diff
==
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Tue Jul 3 07:51:23 2018
@@ -955,10 +955,10 @@ public:
CodeCompleteResult Output;
auto RecorderOwner = llvm::make_unique(Opts, [&]() {
assert(Recorder && "Recorder is not set");
- // FIXME(ioeric): needs more consistent style support in clangd server.
auto Style =
- format::getStyle("file", SemaCCInput.FileName, "LLVM",
- SemaCCInput.Contents, SemaCCInput.VFS.get());
+ format::getStyle(format::DefaultFormatStyle, SemaCCInput.FileName,
+ format::DefaultFallbackStyle, SemaCCInput.Contents,
+ SemaCCInput.VFS.get());
if (!Style) {
log("Failed to get FormatStyle for file" + SemaCCInput.FileName + ": "
+
llvm::toString(Style.takeError()) + ". Fallback is LLVM style.");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48773: [ASTImporter] Fix import of objects with anonymous types
martong updated this revision to Diff 153917.
martong marked an inline comment as done.
martong added a comment.
Remove redundant code and use only StructurlaEquivalence
Repository:
rC Clang
https://reviews.llvm.org/D48773
Files:
lib/AST/ASTImporter.cpp
unittests/AST/ASTImporterTest.cpp
Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -1622,6 +1622,35 @@
.match(ToTU, classTemplateSpecializationDecl()));
}
+TEST_P(ASTImporterTestBase, ObjectsWithUnnamedStructType) {
+ Decl *FromTU = getTuDecl(
+ R"(
+ struct { int a; int b; } object0 = { 2, 3 };
+ struct { int x; int y; int z; } object1;
+ )",
+ Lang_CXX, "input0.cc");
+
+ auto getRecordDecl = [](VarDecl *VD) {
+auto *ET = cast(VD->getType().getTypePtr());
+return cast(ET->getNamedType().getTypePtr())->getDecl();
+ };
+
+ auto *Obj0 =
+ FirstDeclMatcher().match(FromTU, varDecl(hasName("object0")));
+ auto *From0 = getRecordDecl(Obj0);
+ auto *Obj1 =
+ FirstDeclMatcher().match(FromTU, varDecl(hasName("object1")));
+ auto *From1 = getRecordDecl(Obj1);
+
+ auto *To0 = Import(From0, Lang_CXX);
+ auto *To1 = Import(From1, Lang_CXX);
+
+ EXPECT_TRUE(To0);
+ EXPECT_TRUE(To1);
+ EXPECT_NE(To0, To1);
+ EXPECT_NE(To0->getCanonicalDecl(), To1->getCanonicalDecl());
+}
+
struct ImportFunctions : ASTImporterTestBase {};
TEST_P(ImportFunctions,
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -2072,21 +2072,12 @@
if (auto *FoundRecord = dyn_cast(Found)) {
if (!SearchName) {
- // If both unnamed structs/unions are in a record context, make sure
- // they occur in the same location in the context records.
- if (Optional Index1 =
- StructuralEquivalenceContext::findUntaggedStructOrUnionIndex(
- D)) {
-if (Optional Index2 = StructuralEquivalenceContext::
-findUntaggedStructOrUnionIndex(FoundRecord)) {
- if (*Index1 != *Index2)
-continue;
-}
- }
+ if (IsStructuralMatch(D, FoundRecord, false))
+PrevDecl = FoundRecord;
+ else
+continue;
}
-PrevDecl = FoundRecord;
-
if (RecordDecl *FoundDef = FoundRecord->getDefinition()) {
if ((SearchName && !D->isCompleteDefinition())
|| (D->isCompleteDefinition() &&
Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -1622,6 +1622,35 @@
.match(ToTU, classTemplateSpecializationDecl()));
}
+TEST_P(ASTImporterTestBase, ObjectsWithUnnamedStructType) {
+ Decl *FromTU = getTuDecl(
+ R"(
+ struct { int a; int b; } object0 = { 2, 3 };
+ struct { int x; int y; int z; } object1;
+ )",
+ Lang_CXX, "input0.cc");
+
+ auto getRecordDecl = [](VarDecl *VD) {
+auto *ET = cast(VD->getType().getTypePtr());
+return cast(ET->getNamedType().getTypePtr())->getDecl();
+ };
+
+ auto *Obj0 =
+ FirstDeclMatcher().match(FromTU, varDecl(hasName("object0")));
+ auto *From0 = getRecordDecl(Obj0);
+ auto *Obj1 =
+ FirstDeclMatcher().match(FromTU, varDecl(hasName("object1")));
+ auto *From1 = getRecordDecl(Obj1);
+
+ auto *To0 = Import(From0, Lang_CXX);
+ auto *To1 = Import(From1, Lang_CXX);
+
+ EXPECT_TRUE(To0);
+ EXPECT_TRUE(To1);
+ EXPECT_NE(To0, To1);
+ EXPECT_NE(To0->getCanonicalDecl(), To1->getCanonicalDecl());
+}
+
struct ImportFunctions : ASTImporterTestBase {};
TEST_P(ImportFunctions,
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -2072,21 +2072,12 @@
if (auto *FoundRecord = dyn_cast(Found)) {
if (!SearchName) {
- // If both unnamed structs/unions are in a record context, make sure
- // they occur in the same location in the context records.
- if (Optional Index1 =
- StructuralEquivalenceContext::findUntaggedStructOrUnionIndex(
- D)) {
-if (Optional Index2 = StructuralEquivalenceContext::
-findUntaggedStructOrUnionIndex(FoundRecord)) {
- if (*Index1 != *Index2)
-continue;
-}
- }
+ if (IsStructuralMatch(D, FoundRecord, false))
+PrevDecl = FoundRecord;
+ else
+continue;
}
-PrevDecl = FoundRecord;
-
if (RecordDecl *FoundDef = FoundRecord->getDefinition()) {
if ((SearchName && !D->isCompleteDefinition())
r336202 - [Driver] Add PPC64 as supported for Scudo
Author: cryptoad
Date: Tue Jul 3 07:39:29 2018
New Revision: 336202
URL: http://llvm.org/viewvc/llvm-project?rev=336202=rev
Log:
[Driver] Add PPC64 as supported for Scudo
Summary:
Scudo works on PPC64 as is, so mark the architecture as supported for it. This
will also require a change to config-ix.cmake on the compiler-rt side.
Update the tests accordingly.
Reviewers: eugenis, alekseyshl
Reviewed By: alekseyshl
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D48833
Modified:
cfe/trunk/lib/Driver/ToolChains/Linux.cpp
cfe/trunk/test/Driver/fsanitize.c
Modified: cfe/trunk/lib/Driver/ToolChains/Linux.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Linux.cpp?rev=336202=336201=336202=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Linux.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Linux.cpp Tue Jul 3 07:39:29 2018
@@ -931,7 +931,8 @@ SanitizerMask Linux::getSupportedSanitiz
Res |= SanitizerKind::Efficiency;
if (IsX86 || IsX86_64)
Res |= SanitizerKind::Function;
- if (IsX86_64 || IsMIPS64 || IsAArch64 || IsX86 || IsMIPS || IsArmArch)
+ if (IsX86_64 || IsMIPS64 || IsAArch64 || IsX86 || IsMIPS || IsArmArch ||
+ IsPowerPC64)
Res |= SanitizerKind::Scudo;
if (IsX86_64 || IsAArch64) {
Res |= SanitizerKind::HWAddress;
Modified: cfe/trunk/test/Driver/fsanitize.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/fsanitize.c?rev=336202=336201=336202=diff
==
--- cfe/trunk/test/Driver/fsanitize.c (original)
+++ cfe/trunk/test/Driver/fsanitize.c Tue Jul 3 07:39:29 2018
@@ -670,6 +670,8 @@
// RUN: %clang -target mips64el-unknown-linux-gnu -fsanitize=scudo %s -###
2>&1 | FileCheck %s --check-prefix=CHECK-SCUDO
// RUN: %clang -target mips-unknown-linux-gnu -fsanitize=scudo %s -### 2>&1 |
FileCheck %s --check-prefix=CHECK-SCUDO
// RUN: %clang -target mipsel-unknown-linux-gnu -fsanitize=scudo %s -### 2>&1
| FileCheck %s --check-prefix=CHECK-SCUDO
+// RUN: %clang -target powerpc64-unknown-linux -fsanitize=scudo %s -### 2>&1 |
FileCheck %s --check-prefix=CHECK-SCUDO
+// RUN: %clang -target powerpc64le-unknown-linux -fsanitize=scudo %s -### 2>&1
| FileCheck %s --check-prefix=CHECK-SCUDO
// CHECK-SCUDO: "-fsanitize=scudo"
// RUN: %clang -target x86_64-linux-gnu -fsanitize=scudo %s -### 2>&1 |
FileCheck %s --check-prefix=CHECK-SCUDO-PIE
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48833: [Driver] Add PPC64 as supported for Scudo
This revision was automatically updated to reflect the committed changes.
Closed by commit rC336202: [Driver] Add PPC64 as supported for Scudo (authored
by cryptoad, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D48833?vs=153727=153915#toc
Repository:
rC Clang
https://reviews.llvm.org/D48833
Files:
lib/Driver/ToolChains/Linux.cpp
test/Driver/fsanitize.c
Index: test/Driver/fsanitize.c
===
--- test/Driver/fsanitize.c
+++ test/Driver/fsanitize.c
@@ -670,6 +670,8 @@
// RUN: %clang -target mips64el-unknown-linux-gnu -fsanitize=scudo %s -###
2>&1 | FileCheck %s --check-prefix=CHECK-SCUDO
// RUN: %clang -target mips-unknown-linux-gnu -fsanitize=scudo %s -### 2>&1 |
FileCheck %s --check-prefix=CHECK-SCUDO
// RUN: %clang -target mipsel-unknown-linux-gnu -fsanitize=scudo %s -### 2>&1
| FileCheck %s --check-prefix=CHECK-SCUDO
+// RUN: %clang -target powerpc64-unknown-linux -fsanitize=scudo %s -### 2>&1 |
FileCheck %s --check-prefix=CHECK-SCUDO
+// RUN: %clang -target powerpc64le-unknown-linux -fsanitize=scudo %s -### 2>&1
| FileCheck %s --check-prefix=CHECK-SCUDO
// CHECK-SCUDO: "-fsanitize=scudo"
// RUN: %clang -target x86_64-linux-gnu -fsanitize=scudo %s -### 2>&1 |
FileCheck %s --check-prefix=CHECK-SCUDO-PIE
Index: lib/Driver/ToolChains/Linux.cpp
===
--- lib/Driver/ToolChains/Linux.cpp
+++ lib/Driver/ToolChains/Linux.cpp
@@ -931,7 +931,8 @@
Res |= SanitizerKind::Efficiency;
if (IsX86 || IsX86_64)
Res |= SanitizerKind::Function;
- if (IsX86_64 || IsMIPS64 || IsAArch64 || IsX86 || IsMIPS || IsArmArch)
+ if (IsX86_64 || IsMIPS64 || IsAArch64 || IsX86 || IsMIPS || IsArmArch ||
+ IsPowerPC64)
Res |= SanitizerKind::Scudo;
if (IsX86_64 || IsAArch64) {
Res |= SanitizerKind::HWAddress;
Index: test/Driver/fsanitize.c
===
--- test/Driver/fsanitize.c
+++ test/Driver/fsanitize.c
@@ -670,6 +670,8 @@
// RUN: %clang -target mips64el-unknown-linux-gnu -fsanitize=scudo %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SCUDO
// RUN: %clang -target mips-unknown-linux-gnu -fsanitize=scudo %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SCUDO
// RUN: %clang -target mipsel-unknown-linux-gnu -fsanitize=scudo %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SCUDO
+// RUN: %clang -target powerpc64-unknown-linux -fsanitize=scudo %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SCUDO
+// RUN: %clang -target powerpc64le-unknown-linux -fsanitize=scudo %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SCUDO
// CHECK-SCUDO: "-fsanitize=scudo"
// RUN: %clang -target x86_64-linux-gnu -fsanitize=scudo %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SCUDO-PIE
Index: lib/Driver/ToolChains/Linux.cpp
===
--- lib/Driver/ToolChains/Linux.cpp
+++ lib/Driver/ToolChains/Linux.cpp
@@ -931,7 +931,8 @@
Res |= SanitizerKind::Efficiency;
if (IsX86 || IsX86_64)
Res |= SanitizerKind::Function;
- if (IsX86_64 || IsMIPS64 || IsAArch64 || IsX86 || IsMIPS || IsArmArch)
+ if (IsX86_64 || IsMIPS64 || IsAArch64 || IsX86 || IsMIPS || IsArmArch ||
+ IsPowerPC64)
Res |= SanitizerKind::Scudo;
if (IsX86_64 || IsAArch64) {
Res |= SanitizerKind::HWAddress;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48694: [libc++abi] Limit libc++ header search to specified paths
ldionne added a comment. In https://reviews.llvm.org/D48694#1148718, @EricWF wrote: > As an aside, libc++abi should really live in the libc++ repository. That way > it would always have the correct headers available. But every time I pitch > that idea I get a ton of push back. FWIW, I think I would support that. I'd be curious to hear other people's concerns about this. Repository: rL LLVM https://reviews.llvm.org/D48694 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48881: [clangd] Avoid collecting symbols from broken TUs in global-symbol-builder.
ioeric created this revision.
ioeric added a reviewer: ilya-biryukov.
Herald added subscribers: cfe-commits, jkorous, MaskRay.
For example, template parameter might not be resolved in a broken TU,
which can result in wrong USR/SymbolID.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D48881
Files:
clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
Index: clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
===
--- clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
+++ clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
@@ -82,6 +82,14 @@
void EndSourceFileAction() override {
WrapperFrontendAction::EndSourceFileAction();
+const auto = getCompilerInstance();
+if (CI.hasDiagnostics() &&
+(CI.getDiagnosticClient().getNumErrors() > 0)) {
+ llvm::errs() << "Found errors in the translation unit. Igoring "
+ "collected symbols...\n";
+ return;
+}
+
auto Symbols = Collector->takeSymbols();
for (const auto : Symbols) {
Ctx->reportResult(Sym.ID.str(), SymbolToYAML(Sym));
Index: clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
===
--- clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
+++ clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
@@ -82,6 +82,14 @@
void EndSourceFileAction() override {
WrapperFrontendAction::EndSourceFileAction();
+const auto = getCompilerInstance();
+if (CI.hasDiagnostics() &&
+(CI.getDiagnosticClient().getNumErrors() > 0)) {
+ llvm::errs() << "Found errors in the translation unit. Igoring "
+ "collected symbols...\n";
+ return;
+}
+
auto Symbols = Collector->takeSymbols();
for (const auto : Symbols) {
Ctx->reportResult(Sym.ID.str(), SymbolToYAML(Sym));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48880: [Sema] Fix crash in getConstructorName.
ilya-biryukov created this revision.
ilya-biryukov added reviewers: bkramer, rsmith.
Can happen when getConstructorName is called on invalid decls,
specifically the ones that do not have the injected class name.
Repository:
rC Clang
https://reviews.llvm.org/D48880
Files:
lib/Sema/SemaExprCXX.cpp
test/Sema/injected-class-name-crash.cpp
Index: test/Sema/injected-class-name-crash.cpp
===
--- /dev/null
+++ test/Sema/injected-class-name-crash.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+template
+struct X : public Foo
+X::X() {
+}
Index: lib/Sema/SemaExprCXX.cpp
===
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -96,7 +96,8 @@
return ParsedType::make(T);
}
- if (SS.isNotEmpty() && RequireCompleteDeclContext(SS, CurClass))
+ if ((SS.isNotEmpty() && RequireCompleteDeclContext(SS, CurClass)) ||
+ CurClass->isInvalidDecl())
return ParsedType();
// Find the injected-class-name declaration. Note that we make no attempt to
Index: test/Sema/injected-class-name-crash.cpp
===
--- /dev/null
+++ test/Sema/injected-class-name-crash.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+template
+struct X : public Foo
+X::X() {
+}
Index: lib/Sema/SemaExprCXX.cpp
===
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -96,7 +96,8 @@
return ParsedType::make(T);
}
- if (SS.isNotEmpty() && RequireCompleteDeclContext(SS, CurClass))
+ if ((SS.isNotEmpty() && RequireCompleteDeclContext(SS, CurClass)) ||
+ CurClass->isInvalidDecl())
return ParsedType();
// Find the injected-class-name declaration. Note that we make no attempt to
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47537: [clang-tools-extra] Cleanup documentation routine
ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. lgtm. Thanks for doing this! https://reviews.llvm.org/D47537 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48427: [Analyzer] Fix for D47417 to make the tests pass
baloghadamsoftware updated this revision to Diff 153907.
baloghadamsoftware added a comment.
Instead of marking the container alive, now we defer deletion of the container
data until all its iterators are cleaned up.
https://reviews.llvm.org/D48427
Files:
lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
Index: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
+++ lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
@@ -291,6 +291,7 @@
const MemRegion *Cont);
ProgramStateRef setContainerData(ProgramStateRef State, const MemRegion *Cont,
const ContainerData );
+bool hasLiveIterators(ProgramStateRef State, const MemRegion *Cont);
bool isOutOfRange(ProgramStateRef State, const IteratorPosition );
bool isZero(ProgramStateRef State, const NonLoc );
} // namespace
@@ -532,7 +533,9 @@
auto ContMap = State->get();
for (const auto Cont : ContMap) {
if (!SR.isLiveRegion(Cont.first)) {
- State = State->remove(Cont.first);
+ if (!hasLiveIterators(State, Cont.first)) {
+State = State->remove(Cont.first);
+ }
}
}
@@ -542,6 +545,8 @@
State = State->remove(Comp.first);
}
}
+
+ C.addTransition(State);
}
ProgramStateRef IteratorChecker::evalAssume(ProgramStateRef State, SVal Cond,
@@ -1174,6 +1179,22 @@
return State;
}
+bool hasLiveIterators(ProgramStateRef State, const MemRegion *Cont) {
+ auto RegionMap = State->get();
+ for (const auto Reg : RegionMap) {
+if (Reg.second.getContainer() == Cont)
+ return true;
+ }
+
+ auto SymbolMap = State->get();
+ for (const auto Sym : SymbolMap) {
+if (Sym.second.getContainer() == Cont)
+ return true;
+ }
+
+ return false;
+}
+
bool isZero(ProgramStateRef State, const NonLoc ) {
auto = State->getBasicVals();
return compare(State, Val,
Index: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
+++ lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
@@ -291,6 +291,7 @@
const MemRegion *Cont);
ProgramStateRef setContainerData(ProgramStateRef State, const MemRegion *Cont,
const ContainerData );
+bool hasLiveIterators(ProgramStateRef State, const MemRegion *Cont);
bool isOutOfRange(ProgramStateRef State, const IteratorPosition );
bool isZero(ProgramStateRef State, const NonLoc );
} // namespace
@@ -532,7 +533,9 @@
auto ContMap = State->get();
for (const auto Cont : ContMap) {
if (!SR.isLiveRegion(Cont.first)) {
- State = State->remove(Cont.first);
+ if (!hasLiveIterators(State, Cont.first)) {
+State = State->remove(Cont.first);
+ }
}
}
@@ -542,6 +545,8 @@
State = State->remove(Comp.first);
}
}
+
+ C.addTransition(State);
}
ProgramStateRef IteratorChecker::evalAssume(ProgramStateRef State, SVal Cond,
@@ -1174,6 +1179,22 @@
return State;
}
+bool hasLiveIterators(ProgramStateRef State, const MemRegion *Cont) {
+ auto RegionMap = State->get();
+ for (const auto Reg : RegionMap) {
+if (Reg.second.getContainer() == Cont)
+ return true;
+ }
+
+ auto SymbolMap = State->get();
+ for (const auto Sym : SymbolMap) {
+if (Sym.second.getContainer() == Cont)
+ return true;
+ }
+
+ return false;
+}
+
bool isZero(ProgramStateRef State, const NonLoc ) {
auto = State->getBasicVals();
return compare(State, Val,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48831: alpha.unix.cstring.OutOfBounds checker enable/disable fix
dkrupp marked 2 inline comments as done. dkrupp added inline comments. Comment at: lib/StaticAnalyzer/Checkers/CStringChecker.cpp:311 +if (!Filter.CheckCStringOutOfBounds) + return StOutBound; NoQ wrote: > Could we preserve the other portion of the assertion on this branch? I.e., > `assert(Filter.CheckCStringNullArg)`. > > Additionally, do you really want to continue analysis on this path? Maybe > `return nullptr` to sink? I was unsure whether to return nullptr or StOutBound. I thought that alpha.unix.cstring.OutOfBounds is in alpha because it may falsely detect buffer overflows and then we would cut the path unnecessarily. But OK, it is safer this way. I could not put back the assertion, because if only unix.Malloc checker is enabled (and CStringOutOfBounds and CStringNullArg are not) the assertion is not true. https://reviews.llvm.org/D48831 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48831: alpha.unix.cstring.OutOfBounds checker enable/disable fix
dkrupp updated this revision to Diff 153905.
dkrupp added a comment.
The patch has been updated.
Changes:
-The analysis path is cut if overvlow is detected even if CStringOutOfBounds is
disabled
The assert(Filter.CheckCStringOutOfBounds || Filter.CheckCStringNullArg);
cannot be put back, because if both checkers are disabled, the assertion is not
true.
https://reviews.llvm.org/D48831
Files:
lib/StaticAnalyzer/Checkers/CStringChecker.cpp
test/Analysis/cstring-plist.c
test/Analysis/malloc.c
test/Analysis/string.c
Index: test/Analysis/string.c
===
--- test/Analysis/string.c
+++ test/Analysis/string.c
@@ -31,6 +31,8 @@
void clang_analyzer_eval(int);
int scanf(const char *restrict format, ...);
+void *malloc(size_t);
+void free(void *);
//===--===
// strlen()
@@ -110,7 +112,7 @@
if (a == 0) {
clang_analyzer_eval(b == 0); // expected-warning{{TRUE}}
// Make sure clang_analyzer_eval does not invalidate globals.
-clang_analyzer_eval(strlen(global_str) == 0); // expected-warning{{TRUE}}
+clang_analyzer_eval(strlen(global_str) == 0); // expected-warning{{TRUE}}
}
// Call a function with unknown effects, which should invalidate globals.
@@ -309,11 +311,13 @@
clang_analyzer_eval(globalInt == 42); // expected-warning{{TRUE}}
}
+#ifndef SUPPRESS_OUT_OF_BOUND
void strcpy_overflow(char *y) {
char x[4];
if (strlen(y) == 4)
strcpy(x, y); // expected-warning{{String copy function overflows destination buffer}}
}
+#endif
void strcpy_no_overflow(char *y) {
char x[4];
@@ -348,11 +352,13 @@
clang_analyzer_eval(a == x[0]); // expected-warning{{UNKNOWN}}
}
+#ifndef SUPPRESS_OUT_OF_BOUND
void stpcpy_overflow(char *y) {
char x[4];
if (strlen(y) == 4)
stpcpy(x, y); // expected-warning{{String copy function overflows destination buffer}}
}
+#endif
void stpcpy_no_overflow(char *y) {
char x[4];
@@ -403,6 +409,7 @@
clang_analyzer_eval((int)strlen(x) == (orig_len + strlen(y))); // expected-warning{{TRUE}}
}
+#ifndef SUPPRESS_OUT_OF_BOUND
void strcat_overflow_0(char *y) {
char x[4] = "12";
if (strlen(y) == 4)
@@ -420,6 +427,7 @@
if (strlen(y) == 2)
strcat(x, y); // expected-warning{{String copy function overflows destination buffer}}
}
+#endif
void strcat_no_overflow(char *y) {
char x[5] = "12";
@@ -496,6 +504,15 @@
clang_analyzer_eval(a == x[0]); // expected-warning{{UNKNOWN}}
}
+#ifndef SUPPRESS_OUT_OF_BOUND
+// Enabling the malloc checker enables some of the buffer-checking portions
+// of the C-string checker.
+void cstringchecker_bounds_nocrash() {
+ char *p = malloc(2);
+ strncpy(p, "AAA", sizeof("AAA")); // expected-warning {{Size argument is greater than the length of the destination buffer}}
+ free(p);
+}
+
void strncpy_overflow(char *y) {
char x[4];
if (strlen(y) == 4)
@@ -516,6 +533,7 @@
if (strlen(y) == 3)
strncpy(x, y, n); // expected-warning{{Size argument is greater than the length of the destination buffer}}
}
+#endif
void strncpy_truncate(char *y) {
char x[4];
@@ -592,6 +610,7 @@
clang_analyzer_eval(strlen(x) == (orig_len + strlen(y))); // expected-warning{{TRUE}}
}
+#ifndef SUPPRESS_OUT_OF_BOUND
void strncat_overflow_0(char *y) {
char x[4] = "12";
if (strlen(y) == 4)
@@ -615,6 +634,8 @@
if (strlen(y) == 4)
strncat(x, y, 2); // expected-warning{{Size argument is greater than the free space in the destination buffer}}
}
+#endif
+
void strncat_no_overflow_1(char *y) {
char x[5] = "12";
if (strlen(y) == 2)
@@ -632,6 +653,7 @@
clang_analyzer_eval(strlen(dst) >= 4); // expected-warning{{TRUE}}
}
+#ifndef SUPPRESS_OUT_OF_BOUND
void strncat_symbolic_src_length(char *src) {
char dst[8] = "1234";
strncat(dst, src, 3);
@@ -649,6 +671,7 @@
char dst2[8] = "1234";
strncat(dst2, [offset], 4); // expected-warning{{Size argument is greater than the free space in the destination buffer}}
}
+#endif
// There is no strncat_unknown_dst_length because if we can't get a symbolic
// length for the "before" strlen, we won't be able to set one for "after".
Index: test/Analysis/malloc.c
===
--- test/Analysis/malloc.c
+++ test/Analysis/malloc.c
@@ -375,16 +375,16 @@
// or inter-procedural analysis, this is a conservative answer.
int *f3() {
static int *p = 0;
- p = malloc(12);
+ p = malloc(12);
return p; // no-warning
}
// This case tests that storing malloc'ed memory to a static global variable
// which is then returned is not leaked. In the absence of known contracts for
// functions or inter-procedural analysis, this is a conservative answer.
static int *p_f4 = 0;
int *f4() {
- p_f4 = malloc(12);
+ p_f4 = malloc(12);
return p_f4; // no-warning
}
@@ -1232,7 +1232,7 @@
if (myValueSize <=
[PATCH] D48854: Use ExprMutationAnalyzer in performance-for-range-copy
JonasToth added inline comments.
Comment at: test/clang-tidy/performance-for-range-copy.cpp:120
+struct Point {
+ ~Point() {}
I feel that `const` methods should be added as a test as well.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D48854
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48873: [AST] Use llvm::TrailingObjects in clang::CXXTryStmt
bricci created this revision.
bricci added a reviewer: rsmith.
bricci added a project: clang.
Herald added a subscriber: cfe-commits.
Use TrailingObjects for CXXTryStmt instead of hand-rolling it.
This hides the reinterpret_casts and make it more consistent with the other
classes.
Repository:
rC Clang
https://reviews.llvm.org/D48873
Files:
include/clang/AST/StmtCXX.h
lib/AST/StmtCXX.cpp
Index: lib/AST/StmtCXX.cpp
===
--- lib/AST/StmtCXX.cpp
+++ lib/AST/StmtCXX.cpp
@@ -25,26 +25,22 @@
CXXTryStmt *CXXTryStmt::Create(const ASTContext , SourceLocation tryLoc,
Stmt *tryBlock, ArrayRef handlers) {
- std::size_t Size = sizeof(CXXTryStmt);
- Size += ((handlers.size() + 1) * sizeof(Stmt *));
-
+ const size_t Size = totalSizeToAlloc(handlers.size() + 1);
void *Mem = C.Allocate(Size, alignof(CXXTryStmt));
return new (Mem) CXXTryStmt(tryLoc, tryBlock, handlers);
}
CXXTryStmt *CXXTryStmt::Create(const ASTContext , EmptyShell Empty,
unsigned numHandlers) {
- std::size_t Size = sizeof(CXXTryStmt);
- Size += ((numHandlers + 1) * sizeof(Stmt *));
-
+ const size_t Size = totalSizeToAlloc(numHandlers + 1);
void *Mem = C.Allocate(Size, alignof(CXXTryStmt));
return new (Mem) CXXTryStmt(Empty, numHandlers);
}
CXXTryStmt::CXXTryStmt(SourceLocation tryLoc, Stmt *tryBlock,
ArrayRef handlers)
: Stmt(CXXTryStmtClass), TryLoc(tryLoc), NumHandlers(handlers.size()) {
- Stmt **Stmts = reinterpret_cast(this + 1);
+ Stmt **Stmts = getTrailingObjects();
Stmts[0] = tryBlock;
std::copy(handlers.begin(), handlers.end(), Stmts + 1);
}
Index: include/clang/AST/StmtCXX.h
===
--- include/clang/AST/StmtCXX.h
+++ include/clang/AST/StmtCXX.h
@@ -62,21 +62,21 @@
/// CXXTryStmt - A C++ try block, including all handlers.
///
-class CXXTryStmt : public Stmt {
+class CXXTryStmt final : public Stmt,
+ private llvm::TrailingObjects {
+
+ friend TrailingObjects;
+
SourceLocation TryLoc;
unsigned NumHandlers;
+ size_t numTrailingObjects(OverloadToken) const { return NumHandlers; }
CXXTryStmt(SourceLocation tryLoc, Stmt *tryBlock, ArrayRef handlers);
-
CXXTryStmt(EmptyShell Empty, unsigned numHandlers)
: Stmt(CXXTryStmtClass), NumHandlers(numHandlers) { }
- Stmt const * const *getStmts() const {
-return reinterpret_cast(this + 1);
- }
- Stmt **getStmts() {
-return reinterpret_cast(this + 1);
- }
+ Stmt * const *getStmts() const { return getTrailingObjects(); }
+ Stmt **getStmts() { return getTrailingObjects(); }
public:
static CXXTryStmt *Create(const ASTContext , SourceLocation tryLoc,
@@ -90,30 +90,31 @@
SourceLocation getTryLoc() const { return TryLoc; }
SourceLocation getEndLoc() const {
-return getStmts()[NumHandlers]->getLocEnd();
+return getTrailingObjects()[NumHandlers]->getLocEnd();
}
CompoundStmt *getTryBlock() {
-return cast(getStmts()[0]);
+return cast(getTrailingObjects()[0]);
}
const CompoundStmt *getTryBlock() const {
-return cast(getStmts()[0]);
+return cast(getTrailingObjects()[0]);
}
unsigned getNumHandlers() const { return NumHandlers; }
CXXCatchStmt *getHandler(unsigned i) {
-return cast(getStmts()[i + 1]);
+return cast(getTrailingObjects()[i + 1]);
}
const CXXCatchStmt *getHandler(unsigned i) const {
-return cast(getStmts()[i + 1]);
+return cast(getTrailingObjects()[i + 1]);
}
static bool classof(const Stmt *T) {
return T->getStmtClass() == CXXTryStmtClass;
}
child_range children() {
-return child_range(getStmts(), getStmts() + getNumHandlers() + 1);
+return child_range(getTrailingObjects(),
+ getTrailingObjects() + getNumHandlers() + 1);
}
friend class ASTStmtReader;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48721: Patch to fix pragma metadata for do-while loops
bjope added a comment. In https://reviews.llvm.org/D48721#1150361, @hfinkel wrote: > In https://reviews.llvm.org/D48721#1150333, @bjope wrote: > > > Is the fault that the metadata only should be put on the back edge, not the > > branch in the preheader? > > > Yea. Our past thinking has been that any backedge in the loop is valid. The > metadata shouldn't end up other places, although it's benign unless those > other places are (or may later become) a backedge for some different loop. I'm no expert on clang. The patch seems to fix the problem if the goal is to only add the loop-metadata on the backedge , but I'll leave it to someone else to approve it. I'm a little bit concerned about opt not detecting this kind of problems though. Would it be possible for some verifier to detect if we have loop metadata on some branch that aren't in the latch block? And/or should the optimization that "merges" two branches with different loop metadata), be smarter about which loop metadata to keep? Or maybe we should be defensive and discard loop metadata on the merged branch instruction? Repository: rC Clang https://reviews.llvm.org/D48721 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35068: [analyzer] Detect usages of unsafe I/O functions
koldaniel added a comment. Herald added a subscriber: mikhail.ramalho. Hi, could you please take a look at this issue? https://reviews.llvm.org/D35068 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41458: [libc++][C++17] Elementary string conversions for integral types
lichray updated this revision to Diff 153888.
lichray added a comment.
Install the header file
Repository:
rCXX libc++
https://reviews.llvm.org/D41458
Files:
.gitignore
include/CMakeLists.txt
include/charconv
include/module.modulemap
src/charconv.cpp
test/libcxx/double_include.sh.cpp
test/std/utilities/charconv/
test/std/utilities/charconv/charconv.from.chars/
test/std/utilities/charconv/charconv.from.chars/integral.bool.fail.cpp
test/std/utilities/charconv/charconv.from.chars/integral.pass.cpp
test/std/utilities/charconv/charconv.to.chars/
test/std/utilities/charconv/charconv.to.chars/integral.bool.fail.cpp
test/std/utilities/charconv/charconv.to.chars/integral.pass.cpp
test/support/charconv_test_helpers.h
Index: test/support/charconv_test_helpers.h
===
--- /dev/null
+++ test/support/charconv_test_helpers.h
@@ -0,0 +1,233 @@
+//===--===//
+//
+// 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.
+//
+//===--===//
+
+#ifndef SUPPORT_CHARCONV_TEST_HELPERS_H
+#define SUPPORT_CHARCONV_TEST_HELPERS_H
+
+#include
+#include
+#include
+#include
+#include
+
+#include "test_macros.h"
+
+using std::false_type;
+using std::true_type;
+
+template
+constexpr auto
+is_non_narrowing(From a) -> decltype(To{a}, true_type())
+{
+return {};
+}
+
+template
+constexpr auto
+is_non_narrowing(...) -> false_type
+{
+return {};
+}
+
+template
+constexpr bool
+_fits_in(T, true_type /* non-narrowing*/, ...)
+{
+return true;
+}
+
+template >
+constexpr bool
+_fits_in(T v, false_type, true_type /* T signed*/, true_type /* X signed */)
+{
+return xl::lowest() <= v && v <= (xl::max)();
+}
+
+template >
+constexpr bool
+_fits_in(T v, false_type, true_type /* T signed */, false_type /* X unsigned*/)
+{
+return 0 <= v && typename std::make_unsigned::type(v) <= (xl::max)();
+}
+
+template >
+constexpr bool
+_fits_in(T v, false_type, false_type /* T unsigned */, ...)
+{
+return v <= typename std::make_unsigned::type((xl::max)());
+}
+
+template
+constexpr bool
+fits_in(T v)
+{
+return _fits_in(v, is_non_narrowing(v), std::is_signed(),
+ std::is_signed());
+}
+
+template
+struct to_chars_test_base
+{
+template
+void test(T v, char const ()[N], Ts... args)
+{
+using std::to_chars;
+std::to_chars_result r;
+
+constexpr size_t len = N - 1;
+static_assert(len > 0, "expected output won't be empty");
+
+if (!fits_in(v))
+return;
+
+r = to_chars(buf, buf + len - 1, X(v), args...);
+assert(r.ptr == buf + len - 1);
+assert(r.ec == std::errc::value_too_large);
+
+r = to_chars(buf, buf + sizeof(buf), X(v), args...);
+assert(r.ptr == buf + len);
+assert(r.ec == std::errc{});
+assert(memcmp(buf, expect, len) == 0);
+}
+
+template
+void test_value(X v, Ts... args)
+{
+using std::to_chars;
+std::to_chars_result r;
+
+r = to_chars(buf, buf + sizeof(buf), v, args...);
+assert(r.ec == std::errc{});
+*r.ptr = '\0';
+
+auto a = fromchars(buf, r.ptr, args...);
+assert(v == a);
+
+auto ep = r.ptr - 1;
+r = to_chars(buf, ep, v, args...);
+assert(r.ptr == ep);
+assert(r.ec == std::errc::value_too_large);
+}
+
+private:
+using max_t = typename std::conditional::value, long long,
+unsigned long long>::type;
+
+static auto fromchars(char const* p, char const* ep, int base, true_type)
+-> long long
+{
+char* last;
+auto r = strtoll(p, , base);
+assert(last == ep);
+
+return r;
+}
+
+static auto fromchars(char const* p, char const* ep, int base, false_type)
+-> unsigned long long
+{
+char* last;
+auto r = strtoull(p, , base);
+assert(last == ep);
+
+return r;
+}
+
+static auto fromchars(char const* p, char const* ep, int base = 10) -> max_t
+{
+return fromchars(p, ep, base, std::is_signed());
+}
+
+char buf[100];
+};
+
+template
+struct roundtrip_test_base
+{
+template
+void test(T v, Ts... args)
+{
+using std::from_chars;
+using std::to_chars;
+std::from_chars_result r2;
+std::to_chars_result r;
+X x = 0xc;
+
+if (fits_in(v))
+{
+r = to_chars(buf, buf + sizeof(buf), v, args...);
+assert(r.ec == std::errc{});
+
+r2 = from_chars(buf, r.ptr, x, args...);
+assert(r2.ptr == r.ptr);
+assert(x == X(v));
+}
+
[PATCH] D48722: [ASTImporter] Update isUsed flag at Decl import.
martong added a comment. > I have a strong feeling of duplication with attribute and flags merging move > in https://reviews.llvm.org/D47632. Maybe it is better to be resolved in that > review by using the same code for attr/flag merging for both newly-created > and mapped decls? Ok, then I'll integrate this into https://reviews.llvm.org/D47632. Thanks for the review! Repository: rC Clang https://reviews.llvm.org/D48722 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45045: [DebugInfo] Generate debug information for labels.
hans added a comment. In https://reviews.llvm.org/D45045#1150525, @HsiangKai wrote: > In https://reviews.llvm.org/D45045#1114280, @HsiangKai wrote: > > > 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. > > > https://reviews.llvm.org/rL336176 has fixed the bug. Could I revert the > commit? > You could refer to PR37395. I'm not familiar with your patch, but if you're confident that it works (for example, a clang bootstrap works), then committing again sounds good to me. 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] D47358: : Implement {un, }synchronized_pool_resource.
Quuxplusone updated this revision to Diff 153882.
Herald added subscribers: ldionne, christof.
Repository:
rCXX libc++
https://reviews.llvm.org/D47358
Files:
include/experimental/memory_resource
src/experimental/memory_resource.cpp
test/libcxx/experimental/memory/memory.resource.pool/memory.resource.pool.mem/unsynchronized_buffer.pass.cpp
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.ctor/ctor_does_not_allocate.pass.cpp
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.ctor/sync_with_default_resource.pass.cpp
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.ctor/unsync_with_default_resource.pass.cpp
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.mem/equality.pass.cpp
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.mem/sync_allocate.pass.cpp
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.mem/sync_allocate_overaligned_request.pass.cpp
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.mem/sync_allocate_reuse_blocks.pass.cpp
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.mem/sync_deallocate_matches_allocate.pass.cpp
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.mem/unsync_allocate.pass.cpp
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.mem/unsync_allocate_overaligned_request.pass.cpp
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.mem/unsync_allocate_reuse_blocks.pass.cpp
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.mem/unsync_deallocate_matches_allocate.pass.cpp
test/std/experimental/memory/memory.resource.pool/pool_options.pass.cpp
test/support/count_new.hpp
Index: test/support/count_new.hpp
===
--- test/support/count_new.hpp
+++ test/support/count_new.hpp
@@ -211,6 +211,11 @@
return disable_checking || n != delete_called;
}
+bool checkDeleteCalledGreaterThan(int n) const
+{
+return disable_checking || delete_called > n;
+}
+
bool checkAlignedNewCalledEq(int n) const
{
return disable_checking || n == aligned_new_called;
Index: test/std/experimental/memory/memory.resource.pool/pool_options.pass.cpp
===
--- /dev/null
+++ test/std/experimental/memory/memory.resource.pool/pool_options.pass.cpp
@@ -0,0 +1,26 @@
+//===--===//
+//
+// 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.
+//
+//===--===//
+
+// REQUIRES: c++experimental
+// UNSUPPORTED: c++98, c++03, c++11, c++14
+// UNSUPPORTED: apple-clang-7
+
+//
+
+// struct pool_options
+
+#include
+#include
+
+int main()
+{
+const std::experimental::pmr::pool_options p;
+assert(p.max_blocks_per_chunk == 0);
+assert(p.largest_required_pool_block == 0);
+}
Index: test/std/experimental/memory/memory.resource.pool/memory.resource.pool.mem/unsync_deallocate_matches_allocate.pass.cpp
===
--- /dev/null
+++ test/std/experimental/memory/memory.resource.pool/memory.resource.pool.mem/unsync_deallocate_matches_allocate.pass.cpp
@@ -0,0 +1,110 @@
+//===--===//
+//
+// 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.
+//
+//===--===//
+
+// REQUIRES: c++experimental
+// UNSUPPORTED: c++98, c++03
+
+//
+
+// class unsynchronized_pool_resource
+
+#include
+#include
+#include
+
+struct allocation_record {
+size_t bytes;
+size_t align;
+explicit allocation_record(size_t b, size_t a) : bytes(b), align(a) {}
+bool operator==(const allocation_record& rhs) const {
+return (bytes == rhs.bytes) && (align == rhs.align);
+}
+bool operator<(const allocation_record& rhs) const {
+if (bytes != rhs.bytes) return (bytes < rhs.bytes);
+return (align < rhs.align);
+}
+};
+
+class test_resource : public std::experimental::pmr::memory_resource {
+void *do_allocate(size_t bytes, size_t align) override {
+void *result = std::experimental::pmr::new_delete_resource()->allocate(bytes, align);
+successful_allocations.emplace_back(bytes, align);
+return result;
+}
+void do_deallocate(void *p, size_t bytes, size_t align) override {
+deallocations.emplace_back(bytes, align);
+
[PATCH] D47111: : Implement monotonic_buffer_resource.
Quuxplusone updated this revision to Diff 153881.
Repository:
rCXX libc++
https://reviews.llvm.org/D47111
Files:
include/experimental/memory_resource
src/experimental/memory_resource.cpp
test/libcxx/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.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/allocate_with_initial_size.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,62 @@
+//===--===//
+//
+// 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.
+//
+//===--===//
+
+// REQUIRES: c++experimental
+// 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_with_initial_size.pass.cpp
===
--- /dev/null
+++ test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_with_initial_size.pass.cpp
@@ -0,0 +1,51 @@
+//===--===//
+//
+//
[PATCH] D44609: [Clang-Format] New option BreakBeforeLambdaBody to manage lambda line break inside function parameter call
klimek added inline comments.
Comment at: lib/Format/TokenAnnotator.cpp:627
}
+if(Style.BraceWrapping.BeforeLambdaBody && Current->is(TT_LambdaLSquare)) {
+++Left->BlockParameterCount;
Wawha wrote:
> klimek wrote:
> > Why do we want to increase the parameter count here? Specifically, why is
> > it not enough to || this condition to the first is in the function?
> I'm not sure to understand. You want to move this test in the first "if" of
> updateParameterCount()?
> Like :
>
> ```
> if ((Current->is(tok::l_brace) && Current->BlockKind == BK_Block) ||
> Style.BraceWrapping.BeforeLambdaBody && Current->is(TT_LambdaLSquare))
> ++Left->BlockParameterCount;
> ```
> If it's the case, doing that won't work, because updateParameterCount() is
> call only for brace, not square, so the condition while be true only once and
> the BlockParameterCount while be equal to 1.
> And ContinuationIndenter::moveStatePastScopeOpener() while require a value
> greater than 1 to flag true 'HasMultipleNestedBlocks'.
> Same for ParameterCount.
>
> I had this code because I was thinking that you want me to have this
> BlockParameterCount equal to 2 in case a lambda is alone inside a function.
Ok, sorry, I'll need to look at this on more detail again. If we only have one
nested block Blockparametercount should probably be one. I'm unfortunately out
for the next two weeks. If Sam or Daniel or somebody else can look at that it
might work earlier, otherwise we'll need to wait.
Repository:
rC Clang
https://reviews.llvm.org/D44609
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48661: [Fixed Point Arithmetic] Fixed Point Constant
ebevhan added inline comments.
Comment at: include/clang/Basic/FixedPoint.h:67
+ // Convert this number to match the semantics provided.
+ void convert(const struct fixedPointSemantics );
+
If this class is supposed to be used like APInt and APSInt, perhaps this
function should return a copy instead of modifying in place.
The reason that the APFloat `convert` function modifies in place is because it
needs to return a status code, but I don't think that is necessary for this.
Comment at: lib/Basic/FixedPoint.cpp:53
+// We can overflow here
+unsigned ShiftAmt = DstScale - Scale;
+if (Val < 0 && Val.countLeadingOnes() >= ShiftAmt)
leonardchan wrote:
> leonardchan wrote:
> > ebevhan wrote:
> > > ebevhan wrote:
> > > > I think saturation can be modeled a bit better. It should be possible
> > > > to do the overflow check/saturation once instead of twice, and also
> > > > have it fit in better with the other conversions.
> > > >
> > > > Saturation on a value is essentially checking whether all bits above
> > > > and including a certain bit are all the same, and 'clamping' to either
> > > > the minimum (the mask) or maximum (inverse of the mask) if not.
> > > > ```
> > > > // Saturate Value to SatWidth. This will clamp Value at the signed
> > > > min/max of a value that is SatWidth long.
> > > > Saturate(SatWidth):
> > > > Mask = highBits(Width, SatWidth + 1)
> > > > Masked = Value & Mask
> > > > Result = Value
> > > > if (Masked == Mask || Masked == 0) {
> > > > if (Masked.isNegative())
> > > > Result = Mask
> > > > else
> > > > Result = ~Mask
> > > > }
> > > > ```
> > > > This cannot be done on the value in the source scale, since upscaling
> > > > after clamping would produce an incorrect value - we would shift in 0's
> > > > from the right even though we should have saturated all bits
> > > > completely. Also, we cannot upscale without first extending or we might
> > > > shift out bits on the left that could affect saturation.
> > > >
> > > > One thing to note is that (I'm ***pretty sure*** this is the case)
> > > > fractional bits cannot affect saturation. This means that we can find a
> > > > common scale, then perform the saturation operation, and then resize,
> > > > and the value should just fall into place. Basically:
> > > > # Upscale if necessary, but extend first to ensure that we don't drop
> > > > any bits on the left. Do this by resizing to `SrcWidth - SrcScale +
> > > > std::max(SrcScale, DstScale)` and then upscaling normally by `DstScale
> > > > - SrcScale`.
> > > > # Downscale if necessary by `SrcScale - DstScale`. (I think; in our
> > > > downstream, we saturate first and then downscale, but we also assume
> > > > that saturation can only occur on _Fract, and doing saturation first
> > > > makes the saturation width calculation a bit messier because it will be
> > > > a `max` expression. I'm unsure if the order can be changed.)
> > > > # At this point, the scale of the value should be `DstScale`.
> > > > Saturate with `Saturate(DstWidth)`.
> > > > # Now the value should be in range for the new width, and will be at
> > > > the right scale as well. Resize with `extOrTrunc(DstWidth)`.
> > > > # (Also; if the result was negative and the dest type is unsigned,
> > > > just make the result zero here instead.)
> > > >
> > > > Note that the order of operations is different than what I've mentioned
> > > > before with non-saturated conversion (downscale if needed, resize,
> > > > upscale if needed), but it works because we only do the upscale as a
> > > > resize+upscale. Somewhere in here you also need to change the
> > > > signedness of the value, but I'm not entirely certain where. Likely
> > > > after 4.
> > > >
> > > > Everything I've mentioned here is semi-conjectural, since our
> > > > implementation has different type widths than the defaults in this one,
> > > > we can only saturate on _Fract, and our unsigned types have padding.
> > > > It's possible that there are some assumptions that cause this method to
> > > > fail for unsigned without padding, or perhaps for some other type
> > > > conversion, but I haven't come up with a counterexample yet.
> > > Now that I think about it a bit more, it's clear that the Saturate
> > > routine does not work for unsigned fixed-point without padding. That
> > > would need to be taken into consideration, but the rest should work.
> > Thanks for the deeper explanation. The implementation is essentially the
> > same and catching the unsigned fixed point without padding is just checking
> > if the value is negative and we are casting to an unsigned.
> Also I wasn't sure if this was a typo, but I have
>
> ```
> if (!(Masked == Mask || Masked == 0)) {
> ```
>
> instead of
>
> ```
> if (Masked == Mask || Masked == 0) {
> ```
>
> because I think the mask comparison checks if the bits above
[PATCH] D44609: [Clang-Format] New option BreakBeforeLambdaBody to manage lambda line break inside function parameter call
Wawha added inline comments.
Comment at: lib/Format/TokenAnnotator.cpp:627
}
+if(Style.BraceWrapping.BeforeLambdaBody && Current->is(TT_LambdaLSquare)) {
+++Left->BlockParameterCount;
klimek wrote:
> Why do we want to increase the parameter count here? Specifically, why is it
> not enough to || this condition to the first is in the function?
I'm not sure to understand. You want to move this test in the first "if" of
updateParameterCount()?
Like :
```
if ((Current->is(tok::l_brace) && Current->BlockKind == BK_Block) ||
Style.BraceWrapping.BeforeLambdaBody && Current->is(TT_LambdaLSquare))
++Left->BlockParameterCount;
```
If it's the case, doing that won't work, because updateParameterCount() is call
only for brace, not square, so the condition while be true only once and the
BlockParameterCount while be equal to 1.
And ContinuationIndenter::moveStatePastScopeOpener() while require a value
greater than 1 to flag true 'HasMultipleNestedBlocks'.
Same for ParameterCount.
I had this code because I was thinking that you want me to have this
BlockParameterCount equal to 2 in case a lambda is alone inside a function.
Repository:
rC Clang
https://reviews.llvm.org/D44609
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48441: [clangd] Incorporate transitive #includes into code complete proximity scoring.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL336177: [clangd] Incorporate transitive #includes into code
complete proximity scoring. (authored by sammccall, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D48441
Files:
clang-tools-extra/trunk/clangd/CMakeLists.txt
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/ClangdUnit.cpp
clang-tools-extra/trunk/clangd/ClangdUnit.h
clang-tools-extra/trunk/clangd/CodeComplete.cpp
clang-tools-extra/trunk/clangd/CodeComplete.h
clang-tools-extra/trunk/clangd/FileDistance.cpp
clang-tools-extra/trunk/clangd/FileDistance.h
clang-tools-extra/trunk/clangd/Headers.cpp
clang-tools-extra/trunk/clangd/Headers.h
clang-tools-extra/trunk/clangd/Quality.cpp
clang-tools-extra/trunk/clangd/Quality.h
clang-tools-extra/trunk/clangd/XRefs.cpp
clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt
clang-tools-extra/trunk/unittests/clangd/FileDistanceTests.cpp
clang-tools-extra/trunk/unittests/clangd/HeadersTests.cpp
clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp
Index: clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp
@@ -17,6 +17,7 @@
//
//===--===//
+#include "FileDistance.h"
#include "Quality.h"
#include "TestFS.h"
#include "TestTU.h"
@@ -162,9 +163,22 @@
PoorNameMatch.NameMatch = 0.2f;
EXPECT_LT(PoorNameMatch.evaluate(), Default.evaluate());
- SymbolRelevanceSignals WithProximity;
- WithProximity.SemaProximityScore = 0.2f;
- EXPECT_GT(WithProximity.evaluate(), Default.evaluate());
+ SymbolRelevanceSignals WithSemaProximity;
+ WithSemaProximity.SemaProximityScore = 0.2f;
+ EXPECT_GT(WithSemaProximity.evaluate(), Default.evaluate());
+
+ SymbolRelevanceSignals IndexProximate;
+ IndexProximate.SymbolURI = "unittest:/foo/bar.h";
+ llvm::StringMap ProxSources;
+ ProxSources.try_emplace(testPath("foo/baz.h"));
+ URIDistance Distance(ProxSources);
+ IndexProximate.FileProximityMatch =
+ EXPECT_GT(IndexProximate.evaluate(), Default.evaluate());
+ SymbolRelevanceSignals IndexDistant = IndexProximate;
+ IndexDistant.SymbolURI = "unittest:/elsewhere/path.h";
+ EXPECT_GT(IndexProximate.evaluate(), IndexDistant.evaluate())
+ << IndexProximate << IndexDistant;
+ EXPECT_GT(IndexDistant.evaluate(), Default.evaluate());
SymbolRelevanceSignals Scoped;
Scoped.Scope = SymbolRelevanceSignals::FileScope;
@@ -185,59 +199,6 @@
EXPECT_LT(sortText(0, "a"), sortText(0, "z"));
}
-// {a,b,c} becomes /clangd-test/a/b/c
-std::string joinPaths(llvm::ArrayRef Parts) {
- return testPath(
- llvm::join(Parts.begin(), Parts.end(), llvm::sys::path::get_separator()));
-}
-
-static constexpr float ProximityBase = 0.7f;
-
-// Calculates a proximity score for an index symbol with declaration file
-// SymPath with the given URI scheme.
-float URIProximity(const FileProximityMatcher , StringRef SymPath,
- StringRef Scheme = "file") {
- auto U = URI::create(SymPath, Scheme);
- EXPECT_TRUE(static_cast(U)) << llvm::toString(U.takeError());
- return Matcher.uriProximity(U->toString());
-}
-
-TEST(QualityTests, URIProximityScores) {
- FileProximityMatcher Matcher(
- /*ProximityPaths=*/{joinPaths({"a", "b", "c", "d", "x"})});
-
- EXPECT_FLOAT_EQ(URIProximity(Matcher, joinPaths({"a", "b", "c", "d", "x"})),
- 1);
- EXPECT_FLOAT_EQ(URIProximity(Matcher, joinPaths({"a", "b", "c", "d", "y"})),
- ProximityBase);
- EXPECT_FLOAT_EQ(URIProximity(Matcher, joinPaths({"a", "y", "z"})),
- std::pow(ProximityBase, 5));
- EXPECT_FLOAT_EQ(
- URIProximity(Matcher, joinPaths({"a", "b", "c", "d", "e", "y"})),
- std::pow(ProximityBase, 2));
- EXPECT_FLOAT_EQ(
- URIProximity(Matcher, joinPaths({"a", "b", "m", "n", "o", "y"})),
- std::pow(ProximityBase, 5));
- EXPECT_FLOAT_EQ(
- URIProximity(Matcher, joinPaths({"a", "t", "m", "n", "o", "y"})),
- std::pow(ProximityBase, 6));
- // Note the common directory is /clang-test/
- EXPECT_FLOAT_EQ(URIProximity(Matcher, joinPaths({"m", "n", "o", "p", "y"})),
- std::pow(ProximityBase, 7));
-}
-
-TEST(QualityTests, URIProximityScoresWithTestURI) {
- FileProximityMatcher Matcher(
- /*ProximityPaths=*/{joinPaths({"b", "c", "x"})});
- EXPECT_FLOAT_EQ(URIProximity(Matcher, joinPaths({"b", "c", "x"}), "unittest"),
- 1);
- EXPECT_FLOAT_EQ(URIProximity(Matcher, joinPaths({"b", "y"}), "unittest"),
- std::pow(ProximityBase, 2));
- // unittest:///b/c/x vs unittest:///m/n/y. No common directory.
- EXPECT_FLOAT_EQ(URIProximity(Matcher, joinPaths({"m",
[clang-tools-extra] r336177 - [clangd] Incorporate transitive #includes into code complete proximity scoring.
Author: sammccall
Date: Tue Jul 3 01:09:29 2018
New Revision: 336177
URL: http://llvm.org/viewvc/llvm-project?rev=336177=rev
Log:
[clangd] Incorporate transitive #includes into code complete proximity scoring.
Summary:
We now compute a distance from the main file to the symbol header, which
is a weighted count of:
- some number of #include traversals from source file --> included file
- some number of FS traversals from file --> parent directory
- some number of FS traversals from parent directory --> child file/dir
This calculation is performed in the appropriate URI scheme.
This means we'll get some proximity boost from header files in main-file
contexts, even when these are in different directory trees.
This extended file proximity model is not yet incorporated in the index
interface/implementation.
Reviewers: ioeric
Subscribers: mgorny, ilya-biryukov, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D48441
Added:
clang-tools-extra/trunk/clangd/FileDistance.cpp
clang-tools-extra/trunk/clangd/FileDistance.h
clang-tools-extra/trunk/unittests/clangd/FileDistanceTests.cpp
Modified:
clang-tools-extra/trunk/clangd/CMakeLists.txt
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/ClangdUnit.cpp
clang-tools-extra/trunk/clangd/ClangdUnit.h
clang-tools-extra/trunk/clangd/CodeComplete.cpp
clang-tools-extra/trunk/clangd/CodeComplete.h
clang-tools-extra/trunk/clangd/Headers.cpp
clang-tools-extra/trunk/clangd/Headers.h
clang-tools-extra/trunk/clangd/Quality.cpp
clang-tools-extra/trunk/clangd/Quality.h
clang-tools-extra/trunk/clangd/XRefs.cpp
clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt
clang-tools-extra/trunk/unittests/clangd/HeadersTests.cpp
clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp
Modified: clang-tools-extra/trunk/clangd/CMakeLists.txt
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CMakeLists.txt?rev=336177=336176=336177=diff
==
--- clang-tools-extra/trunk/clangd/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clangd/CMakeLists.txt Tue Jul 3 01:09:29 2018
@@ -19,6 +19,7 @@ add_clang_library(clangDaemon
Diagnostics.cpp
DraftStore.cpp
FindSymbols.cpp
+ FileDistance.cpp
FuzzyMatch.cpp
GlobalCompilationDatabase.cpp
Headers.cpp
Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=336177=336176=336177=diff
==
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Tue Jul 3 01:09:29 2018
@@ -167,7 +167,7 @@ void ClangdServer::codeComplete(PathRef
// both the old and the new version in case only one of them matches.
CodeCompleteResult Result = clangd::codeComplete(
File, IP->Command, PreambleData ? >Preamble : nullptr,
-PreambleData ? PreambleData->Inclusions : std::vector(),
+PreambleData ? PreambleData->Includes : IncludeStructure(),
IP->Contents, Pos, FS, PCHs, CodeCompleteOpts);
CB(std::move(Result));
};
Modified: clang-tools-extra/trunk/clangd/ClangdUnit.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnit.cpp?rev=336177=336176=336177=diff
==
--- clang-tools-extra/trunk/clangd/ClangdUnit.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp Tue Jul 3 01:09:29 2018
@@ -88,7 +88,7 @@ public:
CppFilePreambleCallbacks(PathRef File, PreambleParsedCallback ParsedCallback)
: File(File), ParsedCallback(ParsedCallback) {}
- std::vector takeInclusions() { return std::move(Inclusions); }
+ IncludeStructure takeIncludes() { return std::move(Includes); }
void AfterExecute(CompilerInstance ) override {
if (!ParsedCallback)
@@ -103,15 +103,13 @@ public:
std::unique_ptr createPPCallbacks() override {
assert(SourceMgr && "SourceMgr must be set at this point");
-return collectInclusionsInMainFileCallback(
-*SourceMgr,
-[this](Inclusion Inc) { Inclusions.push_back(std::move(Inc)); });
+return collectIncludeStructureCallback(*SourceMgr, );
}
private:
PathRef File;
PreambleParsedCallback ParsedCallback;
- std::vector Inclusions;
+ IncludeStructure Includes;
SourceManager *SourceMgr = nullptr;
};
@@ -153,15 +151,11 @@ ParsedAST::Build(std::unique_ptr Inclusions;
// Copy over the includes from the preamble, then combine with the
// non-preamble includes below.
- if (Preamble)
-Inclusions = Preamble->Inclusions;
-
- Clang->getPreprocessor().addPPCallbacks(collectInclusionsInMainFileCallback(
- Clang->getSourceManager(),
- [](Inclusion Inc) {
[PATCH] D48827: [clang-format ]Extend IncludeCategories regex documentation
krasimir requested changes to this revision. krasimir added a comment. This revision now requires changes to proceed. This file is automatically generated from `Format.h` using `clang/docs/tools/dump_format_style.py`. Please update. Repository: rC Clang https://reviews.llvm.org/D48827 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48441: [clangd] Incorporate transitive #includes into code complete proximity scoring.
sammccall added a comment. Performance on a transitively big file. Stress test - global scope completion with index turned off. Benchmarking codeComplete() call 100x per trial, trials randomly ordered. Before this patch (427ms 421ms 430ms) After this patch (418ms 420ms 417ms) So it seems this is slightly faster, maybe 2%. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48441 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45045: [DebugInfo] Generate debug information for labels.
HsiangKai added a comment. In https://reviews.llvm.org/D45045#1114280, @HsiangKai wrote: > 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. https://reviews.llvm.org/rL336176 has fixed the bug. Could I revert the commit? You could refer to PR37395. 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
