[PATCH] D54391: Fix compatibility with z3-4.8.1
This revision was automatically updated to reflect the committed changes.
Closed by commit rL346635: Fix compatibility with z3-4.8.1 (authored by
jankratochvil, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D54391?vs=173555=173618#toc
Repository:
rL LLVM
https://reviews.llvm.org/D54391
Files:
cfe/trunk/lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
Index: cfe/trunk/lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
@@ -46,7 +46,7 @@
// Function used to report errors
void Z3ErrorHandler(Z3_context Context, Z3_error_code Error) {
llvm::report_fatal_error("Z3 error: " +
- llvm::Twine(Z3_get_error_msg_ex(Context, Error)));
+ llvm::Twine(Z3_get_error_msg(Context, Error)));
}
/// Wrapper for Z3 context
Index: cfe/trunk/lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
@@ -46,7 +46,7 @@
// Function used to report errors
void Z3ErrorHandler(Z3_context Context, Z3_error_code Error) {
llvm::report_fatal_error("Z3 error: " +
- llvm::Twine(Z3_get_error_msg_ex(Context, Error)));
+ llvm::Twine(Z3_get_error_msg(Context, Error)));
}
/// Wrapper for Z3 context
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D54414: [Sema] Make sure we substitute an instantiation-dependent default template parameter
erik.pilkington created this revision.
erik.pilkington added a reviewer: rsmith.
Herald added a subscriber: dexonsmith.
Previously, we'd accept the attached test because we didn't substitute into
`void_t` when we really ought to of.
Fixes llvm.org/PR39623
Thanks!
Repository:
rC Clang
https://reviews.llvm.org/D54414
Files:
clang/lib/Sema/SemaTemplate.cpp
clang/test/SemaCXX/alias-template.cpp
Index: clang/test/SemaCXX/alias-template.cpp
===
--- clang/test/SemaCXX/alias-template.cpp
+++ clang/test/SemaCXX/alias-template.cpp
@@ -179,3 +179,13 @@
};
static_assert(__is_same(S<3>::U, X[2]), ""); // expected-error {{static_assert
failed}}
}
+
+namespace PR39623 {
+template
+using void_t = void;
+
+template >
+int sfinae_me() { return 0; } // expected-note{{candidate template ignored:
substitution failure}}
+
+int g = sfinae_me(); // expected-error{{no matching function for call to
'sfinae_me'}}
+}
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -4434,7 +4434,7 @@
// If the argument type is dependent, instantiate it now based
// on the previously-computed template arguments.
- if (ArgType->getType()->isDependentType()) {
+ if (ArgType->getType()->isInstantiationDependentType()) {
Sema::InstantiatingTemplate Inst(SemaRef, TemplateLoc,
Param, Template, Converted,
SourceRange(TemplateLoc, RAngleLoc));
Index: clang/test/SemaCXX/alias-template.cpp
===
--- clang/test/SemaCXX/alias-template.cpp
+++ clang/test/SemaCXX/alias-template.cpp
@@ -179,3 +179,13 @@
};
static_assert(__is_same(S<3>::U, X[2]), ""); // expected-error {{static_assert failed}}
}
+
+namespace PR39623 {
+template
+using void_t = void;
+
+template >
+int sfinae_me() { return 0; } // expected-note{{candidate template ignored: substitution failure}}
+
+int g = sfinae_me(); // expected-error{{no matching function for call to 'sfinae_me'}}
+}
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -4434,7 +4434,7 @@
// If the argument type is dependent, instantiate it now based
// on the previously-computed template arguments.
- if (ArgType->getType()->isDependentType()) {
+ if (ArgType->getType()->isInstantiationDependentType()) {
Sema::InstantiatingTemplate Inst(SemaRef, TemplateLoc,
Param, Template, Converted,
SourceRange(TemplateLoc, RAngleLoc));
___
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 173613.
Quuxplusone added a comment.
Herald added a subscriber: libcxx-commits.
Rebased on master. @ericwf @ldionne ping please?
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
[PATCH] D47344: LWG 2843 "Unclear behavior of std::pmr::memory_resource::do_allocate()"
Quuxplusone updated this revision to Diff 173612.
Repository:
rCXX libc++
https://reviews.llvm.org/D47344
Files:
include/experimental/memory_resource
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,39 +24,51 @@
// 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 */}
-
-virtual void do_deallocate(void* __p, size_t __n, size_t __align) {
- _VSTD::__libcpp_deallocate(__p, __n, __align); /* FIXME */
+void *do_allocate(size_t bytes, size_t align) override
+{
+#ifndef _LIBCPP_HAS_NO_ALIGNED_ALLOCATION
+return _VSTD::__libcpp_allocate(bytes, align);
+#else
+if (bytes == 0)
+bytes = 1;
+void *result = _VSTD::__libcpp_allocate(bytes, align);
+if (!is_aligned_to(result, align)) {
+_VSTD::__libcpp_deallocate(result, bytes, align);
+__throw_bad_alloc();
+}
+return result;
+#endif
}
-virtual bool do_is_equal(memory_resource const & __other) const _NOEXCEPT
-{ return &__other == this; }
+void do_deallocate(void *p, size_t bytes, size_t align) override
+{ _VSTD::__libcpp_deallocate(p, bytes, align); }
+
+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: include/experimental/memory_resource
===
--- include/experimental/memory_resource
+++ include/experimental/memory_resource
@@ -195,7 +195,7 @@
}
_LIBCPP_INLINE_VISIBILITY
-void deallocate(_ValueType * __p, size_t __n) _NOEXCEPT {
+void deallocate(_ValueType * __p, size_t __n) {
_LIBCPP_ASSERT(__n <= __max_size(),
"deallocate called for size which exceeds max_size()");
__res_->deallocate(__p, __n * sizeof(_ValueType), alignof(_ValueType));
@@ -265,7 +265,7 @@
template
_LIBCPP_INLINE_VISIBILITY
-void destroy(_Tp * __p) _NOEXCEPT
+void destroy(_Tp * __p)
{ __p->~_Tp(); }
_LIBCPP_INLINE_VISIBILITY
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47111: : Implement monotonic_buffer_resource.
Quuxplusone updated this revision to Diff 173611.
Quuxplusone added a comment.
Rebased on master. @ericwf @ldionne ping please?
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
+#include
+
+#include "count_new.hpp"
+
+void test(size_t initial_buffer_size)
+{
+globalMemCounter.reset();
+
+std::experimental::pmr::monotonic_buffer_resource mono1(
+initial_buffer_size,
+std::experimental::pmr::new_delete_resource()
+);
+
[PATCH] D47344: LWG 2843 "Unclear behavior of std::pmr::memory_resource::do_allocate()"
Quuxplusone updated this revision to Diff 173610.
Quuxplusone added a comment.
Rebased on master. @EricWF @ldionne ping please?
Repository:
rCXX libc++
https://reviews.llvm.org/D47344
Files:
include/experimental/memory_resource
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,39 +24,51 @@
// 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 */}
-
-virtual void do_deallocate(void* __p, size_t __n, size_t __align) {
- _VSTD::__libcpp_deallocate(__p, __n, __align); /* FIXME */
+void *do_allocate(size_t bytes, size_t align) override
+{
+#ifndef _LIBCPP_HAS_NO_ALIGNED_ALLOCATION
+return _VSTD::__libcpp_allocate(bytes, align);
+#else
+if (bytes == 0)
+bytes = 1;
+void *result = _VSTD::__libcpp_allocate(bytes, align);
+if (!is_aligned_to(result, align)) {
+_VSTD::__libcpp_deallocate(result, align);
+__throw_bad_alloc();
+}
+return result;
+#endif
}
-virtual bool do_is_equal(memory_resource const & __other) const _NOEXCEPT
-{ return &__other == this; }
+void do_deallocate(void *p, size_t, size_t align) override
+{ _VSTD::__libcpp_deallocate(p, align); }
+
+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: include/experimental/memory_resource
===
--- include/experimental/memory_resource
+++ include/experimental/memory_resource
@@ -195,7 +195,7 @@
}
_LIBCPP_INLINE_VISIBILITY
-void deallocate(_ValueType * __p, size_t __n) _NOEXCEPT {
+void deallocate(_ValueType * __p, size_t __n) {
_LIBCPP_ASSERT(__n <= __max_size(),
"deallocate called for size which exceeds max_size()");
__res_->deallocate(__p, __n * sizeof(_ValueType), alignof(_ValueType));
@@ -265,7 +265,7 @@
template
_LIBCPP_INLINE_VISIBILITY
-void destroy(_Tp * __p) _NOEXCEPT
+void destroy(_Tp * __p)
{ __p->~_Tp(); }
_LIBCPP_INLINE_VISIBILITY
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50318: Support Swift in platform availability attribute
This revision was automatically updated to reflect the committed changes.
Closed by commit rC346633: Support Swift in platform availability attribute
(authored by mwu, committed by ).
Repository:
rC Clang
https://reviews.llvm.org/D50318
Files:
include/clang/Basic/Attr.td
include/clang/Basic/DiagnosticSemaKinds.td
include/clang/Basic/Features.def
lib/Parse/ParseDecl.cpp
lib/Sema/SemaDeclAttr.cpp
test/Index/availability.c
test/Sema/attr-availability-swift.c
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -2410,6 +2410,15 @@
if (const auto *SE = dyn_cast_or_null(AL.getReplacementExpr()))
Replacement = SE->getString();
+ if (II->isStr("swift")) {
+if (Introduced.isValid() || Obsoleted.isValid() ||
+(!IsUnavailable && !Deprecated.isValid())) {
+ S.Diag(AL.getLoc(),
+ diag::warn_availability_swift_unavailable_deprecated_only);
+ return;
+}
+ }
+
AvailabilityAttr *NewAttr = S.mergeAvailabilityAttr(ND, AL.getRange(), II,
false/*Implicit*/,
Introduced.Version,
Index: lib/Parse/ParseDecl.cpp
===
--- lib/Parse/ParseDecl.cpp
+++ lib/Parse/ParseDecl.cpp
@@ -1001,6 +1001,21 @@
continue;
}
+if (Keyword == Ident_deprecated && Platform->Ident &&
+Platform->Ident->isStr("swift")) {
+ // For swift, we deprecate for all versions.
+ if (Changes[Deprecated].KeywordLoc.isValid()) {
+Diag(KeywordLoc, diag::err_availability_redundant)
+ << Keyword
+ << SourceRange(Changes[Deprecated].KeywordLoc);
+ }
+
+ Changes[Deprecated].KeywordLoc = KeywordLoc;
+ // Use a fake version here.
+ Changes[Deprecated].Version = VersionTuple(1);
+ continue;
+}
+
if (Tok.isNot(tok::equal)) {
Diag(Tok, diag::err_expected_after) << Keyword << tok::equal;
SkipUntil(tok::r_paren, StopAtSemi);
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -2993,6 +2993,9 @@
InGroup;
def note_overridden_method : Note<
"overridden method is here">;
+def warn_availability_swift_unavailable_deprecated_only : Warning<
+ "only 'unavailable' and 'deprecated' are supported for Swift availability">,
+ InGroup;
def note_protocol_method : Note<
"protocol method is here">;
Index: include/clang/Basic/Attr.td
===
--- include/clang/Basic/Attr.td
+++ include/clang/Basic/Attr.td
@@ -723,6 +723,7 @@
.Case("macos_app_extension", "macOS (App Extension)")
.Case("tvos_app_extension", "tvOS (App Extension)")
.Case("watchos_app_extension", "watchOS (App Extension)")
+ .Case("swift", "Swift")
.Default(llvm::StringRef());
}
static llvm::StringRef getPlatformNameSourceSpelling(llvm::StringRef Platform) {
Index: include/clang/Basic/Features.def
===
--- include/clang/Basic/Features.def
+++ include/clang/Basic/Features.def
@@ -51,6 +51,7 @@
FEATURE(attribute_availability_with_strict, true)
FEATURE(attribute_availability_with_replacement, true)
FEATURE(attribute_availability_in_templates, true)
+FEATURE(attribute_availability_swift, true)
FEATURE(attribute_cf_returns_not_retained, true)
FEATURE(attribute_cf_returns_retained, true)
FEATURE(attribute_cf_returns_on_parameters, true)
Index: test/Sema/attr-availability-swift.c
===
--- test/Sema/attr-availability-swift.c
+++ test/Sema/attr-availability-swift.c
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin9 -fsyntax-only -fblocks -verify %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin9 -fsyntax-only -ast-dump %s | FileCheck %s
+//
+
+#if !__has_feature(attribute_availability_with_message)
+# error "Missing __has_feature"
+#endif
+
+#if __has_feature(attribute_availability_swift)
+# warning "okay"
+// expected-warning@-1{{okay}}
+#else
+# error "Missing __has_feature"
+#endif
+
+extern int noSwiftGlobal1 __attribute__((availability(swift, unavailable)));
+// CHECK: AvailabilityAttr {{.*}}swift 0 0 0 Unavailable "" ""
+extern int noSwiftGlobal1 __attribute__((availability(macosx, introduced=10.1))); // okay
+// CHECK: AvailabilityAttr {{.*}}Inherited swift 0 0 0 Unavailable "" ""
+// CHECK: AvailabilityAttr {{.*}}macos 10.1 0 0 "" ""
+extern int noSwiftGlobal1 __attribute__((availability(swift, unavailable, message="and this one has a message"))); // okay
+// CHECK: AvailabilityAttr {{.*}}Inherited macos 10.1 0 0 "" ""
+// CHECK:
r346633 - Support Swift in platform availability attribute
Author: mwu
Date: Sun Nov 11 18:44:33 2018
New Revision: 346633
URL: http://llvm.org/viewvc/llvm-project?rev=346633=rev
Log:
Support Swift in platform availability attribute
Summary: This adds support for Swift platform availability attributes. It's
largely a port of the changes made to https://github.com/apple/swift-clang/ for
Swift availability attributes. Specifically,
https://github.com/apple/swift-clang/commit/84b5a21c31cb5b0d7d958a478bc01964939b6952
and
https://github.com/apple/swift-clang/commit/e5b87f265aede41c8381094bbf54e2715c8293b0
. The implementation of attribute_availability_swift is a little different and
additional tests in test/Index/availability.c were added.
Reviewers: manmanren, friss, doug.gregor, arphaman, jfb, erik.pilkington,
aaron.ballman
Reviewed By: aaron.ballman
Subscribers: aaron.ballman, ColinKinloch, jrmuizel, cfe-commits
Differential Revision: https://reviews.llvm.org/D50318
Added:
cfe/trunk/test/Sema/attr-availability-swift.c
Modified:
cfe/trunk/include/clang/Basic/Attr.td
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Basic/Features.def
cfe/trunk/lib/Parse/ParseDecl.cpp
cfe/trunk/lib/Sema/SemaDeclAttr.cpp
cfe/trunk/test/Index/availability.c
Modified: cfe/trunk/include/clang/Basic/Attr.td
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=346633=346632=346633=diff
==
--- cfe/trunk/include/clang/Basic/Attr.td (original)
+++ cfe/trunk/include/clang/Basic/Attr.td Sun Nov 11 18:44:33 2018
@@ -723,6 +723,7 @@ def Availability : InheritableAttr {
.Case("macos_app_extension", "macOS (App Extension)")
.Case("tvos_app_extension", "tvOS (App Extension)")
.Case("watchos_app_extension", "watchOS (App Extension)")
+ .Case("swift", "Swift")
.Default(llvm::StringRef());
}
static llvm::StringRef getPlatformNameSourceSpelling(llvm::StringRef Platform)
{
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=346633=346632=346633=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Sun Nov 11 18:44:33
2018
@@ -2993,6 +2993,9 @@ def warn_availability_on_static_initiali
InGroup;
def note_overridden_method : Note<
"overridden method is here">;
+def warn_availability_swift_unavailable_deprecated_only : Warning<
+ "only 'unavailable' and 'deprecated' are supported for Swift availability">,
+ InGroup;
def note_protocol_method : Note<
"protocol method is here">;
Modified: cfe/trunk/include/clang/Basic/Features.def
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Features.def?rev=346633=346632=346633=diff
==
--- cfe/trunk/include/clang/Basic/Features.def (original)
+++ cfe/trunk/include/clang/Basic/Features.def Sun Nov 11 18:44:33 2018
@@ -51,6 +51,7 @@ FEATURE(attribute_availability_watchos,
FEATURE(attribute_availability_with_strict, true)
FEATURE(attribute_availability_with_replacement, true)
FEATURE(attribute_availability_in_templates, true)
+FEATURE(attribute_availability_swift, true)
FEATURE(attribute_cf_returns_not_retained, true)
FEATURE(attribute_cf_returns_retained, true)
FEATURE(attribute_cf_returns_on_parameters, true)
Modified: cfe/trunk/lib/Parse/ParseDecl.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDecl.cpp?rev=346633=346632=346633=diff
==
--- cfe/trunk/lib/Parse/ParseDecl.cpp (original)
+++ cfe/trunk/lib/Parse/ParseDecl.cpp Sun Nov 11 18:44:33 2018
@@ -1001,6 +1001,21 @@ void Parser::ParseAvailabilityAttribute(
continue;
}
+if (Keyword == Ident_deprecated && Platform->Ident &&
+Platform->Ident->isStr("swift")) {
+ // For swift, we deprecate for all versions.
+ if (Changes[Deprecated].KeywordLoc.isValid()) {
+Diag(KeywordLoc, diag::err_availability_redundant)
+ << Keyword
+ << SourceRange(Changes[Deprecated].KeywordLoc);
+ }
+
+ Changes[Deprecated].KeywordLoc = KeywordLoc;
+ // Use a fake version here.
+ Changes[Deprecated].Version = VersionTuple(1);
+ continue;
+}
+
if (Tok.isNot(tok::equal)) {
Diag(Tok, diag::err_expected_after) << Keyword << tok::equal;
SkipUntil(tok::r_paren, StopAtSemi);
Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=346633=346632=346633=diff
==
---
[PATCH] D50318: Support Swift in platform availability attribute
michaelwu updated this revision to Diff 173607.
michaelwu added a comment.
Thanks for the review! All requested changes have been made, and I also had to
fix a test due to https://reviews.llvm.org/D50214
https://reviews.llvm.org/D50318
Files:
include/clang/Basic/Attr.td
include/clang/Basic/DiagnosticSemaKinds.td
include/clang/Basic/Features.def
lib/Parse/ParseDecl.cpp
lib/Sema/SemaDeclAttr.cpp
test/Index/availability.c
test/Sema/attr-availability-swift.c
Index: test/Sema/attr-availability-swift.c
===
--- test/Sema/attr-availability-swift.c
+++ test/Sema/attr-availability-swift.c
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin9 -fsyntax-only -fblocks -verify %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin9 -fsyntax-only -ast-dump %s | FileCheck %s
+//
+
+#if !__has_feature(attribute_availability_with_message)
+# error "Missing __has_feature"
+#endif
+
+#if __has_feature(attribute_availability_swift)
+# warning "okay"
+// expected-warning@-1{{okay}}
+#else
+# error "Missing __has_feature"
+#endif
+
+extern int noSwiftGlobal1 __attribute__((availability(swift, unavailable)));
+// CHECK: AvailabilityAttr {{.*}}swift 0 0 0 Unavailable "" ""
+extern int noSwiftGlobal1 __attribute__((availability(macosx, introduced=10.1))); // okay
+// CHECK: AvailabilityAttr {{.*}}Inherited swift 0 0 0 Unavailable "" ""
+// CHECK: AvailabilityAttr {{.*}}macos 10.1 0 0 "" ""
+extern int noSwiftGlobal1 __attribute__((availability(swift, unavailable, message="and this one has a message"))); // okay
+// CHECK: AvailabilityAttr {{.*}}Inherited macos 10.1 0 0 "" ""
+// CHECK: AvailabilityAttr {{.*}}swift 0 0 0 Unavailable "and this one has a message" ""
+extern int noSwiftGlobal2 __attribute__((availability(swift, introduced=5))); // expected-warning{{only 'unavailable' and 'deprecated' are supported for Swift availability}}
+// CHECK: VarDecl
+// CHECK-NOT: AvailabilityAttr
+extern int noSwiftGlobal3 __attribute__((availability(swift, deprecated, message="t")));
+// CHECK: VarDecl
+// CHECK: AvailabilityAttr {{.*}}swift 0 1 0 "t" ""
Index: test/Index/availability.c
===
--- test/Index/availability.c
+++ test/Index/availability.c
@@ -14,9 +14,14 @@
void bar2(void) __attribute__((availability(macosx,introduced=10.4,deprecated=10.5,obsoleted=10.7))) __attribute__((availability(ios,introduced=3.2,deprecated=10.0))) __attribute__((availability(macosx,introduced=10.4,deprecated=10.5,obsoleted=10.7))) __attribute__((availability(ios,introduced=3.2,deprecated=10.0)));
+void foo2(void) __attribute__((availability(swift,unavailable)));
+void foo3(void) __attribute__((availability(swift,deprecated)));
+
// RUN: c-index-test -test-load-source all %s | FileCheck %s
// CHECK: FunctionDecl=foo:3:6{{.*}}(ios, introduced=3.2, deprecated=4.1) (macos, introduced=10.4, deprecated=10.5, obsoleted=10.7)
// CHECK: EnumConstantDecl=old_enum:6:3 (Definition) (deprecated)
// CHECK: EnumConstantDecl=old_enum_plat:10:3 {{.*}} (macos, introduced=10.4, deprecated=10.5, obsoleted=10.7)
// CHECK: FunctionDecl=bar:13:6{{.*}}(ios, introduced=3.2) (macos, introduced=10.4, deprecated=10.5, obsoleted=10.6, message="use foobar")
// CHECK: FunctionDecl=bar2:15:6{{.*}}(ios, introduced=3.2, deprecated=10.0) (macos, introduced=10.4, deprecated=10.5, obsoleted=10.7)
+// CHECK: FunctionDecl=foo2:17:6{{.*}}(swift, unavailable)
+// CHECK: FunctionDecl=foo3:18:6{{.*}}(swift, deprecated=1)
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -2410,6 +2410,15 @@
if (const auto *SE = dyn_cast_or_null(AL.getReplacementExpr()))
Replacement = SE->getString();
+ if (II->isStr("swift")) {
+if (Introduced.isValid() || Obsoleted.isValid() ||
+(!IsUnavailable && !Deprecated.isValid())) {
+ S.Diag(AL.getLoc(),
+ diag::warn_availability_swift_unavailable_deprecated_only);
+ return;
+}
+ }
+
AvailabilityAttr *NewAttr = S.mergeAvailabilityAttr(ND, AL.getRange(), II,
false/*Implicit*/,
Introduced.Version,
Index: lib/Parse/ParseDecl.cpp
===
--- lib/Parse/ParseDecl.cpp
+++ lib/Parse/ParseDecl.cpp
@@ -1001,6 +1001,21 @@
continue;
}
+if (Keyword == Ident_deprecated && Platform->Ident &&
+Platform->Ident->isStr("swift")) {
+ // For swift, we deprecate for all versions.
+ if (Changes[Deprecated].KeywordLoc.isValid()) {
+Diag(KeywordLoc, diag::err_availability_redundant)
+ << Keyword
+ << SourceRange(Changes[Deprecated].KeywordLoc);
+ }
+
+ Changes[Deprecated].KeywordLoc = KeywordLoc;
+ // Use a fake version here.
+
[PATCH] D54349: [clang-tidy] new check 'readability-redundant-preprocessor'
aaron.ballman added inline comments. Comment at: clang-tidy/readability/ReadabilityTidyModule.cpp:84 +CheckFactories.registerCheck( +"readability-redundant-preprocessor"); CheckFactories.registerCheck( Please keep this list sorted alphabetically. Comment at: clang-tidy/readability/RedundantPreprocessorCheck.cpp:56-59 +StringRef SourceText = +Lexer::getSourceText(CharSourceRange::getTokenRange(ConditionRange), + PP.getSourceManager(), PP.getLangOpts()); +std::string Condition = getCondition(SourceText); Szelethus wrote: > vmiklos wrote: > > Szelethus wrote: > > > I'm a little confused. To me, it seems like you acquired the condition > > > already -- doesn't `ConditionRange` actually cover the, well, condition > > > range? This is how I imagined it: > > > > > > ``` > > > #ifdef CUTE_PANDA_CUBS > > >^~~ > > >ConditionRange > > > ``` > > > Why is there a need for `getCondition`? Is there any? If there is (maybe > > > the acquired text contains other things), can you document it? I haven't > > > played with `PPCallbacks` much, so I'm fine with being in the wrong. > > ConditionRange covers more than what you expect: > > > > ``` > > #if FOO == 4 > >^ > > void f(); > > ~ > > #endif > > ~~ > > ``` > > > > to find out if the condition of the `#if` is the same as a previous one, I > > want to extract just `FOO == 4` from that, then deal with that part similar > > to `#ifdef` and `#ifndef`, which are easier as you have a single Token for > > the condition. But you're right, I should add a comment explaining this. > Oh my god. There is no tool or a convenient method for this??? I had the > displeasure of working with the preprocessor in the last couple months, and I > get shocked by things like this almost every day. > > Yea, unfortunately you will have to write excessive amount of comments to > counterweights the shortcomings of `Preprocessor` :/ This is working around a bug, and I think it would be better to fix that bug instead of jump through these hoops here. `Preprocessor::EvaluateDirectiveExpression()` needs to squirrel away the condition range in the `DirectiveEvalResult` it returns. I'll take a stab at it and report back. https://reviews.llvm.org/D54349 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D54344: [Clang][CodeGen][CXX]: Workaround __attribute((no_destroy)) crash/incorrect code generation.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL346628: [CodeGen][CXX]: Fix no_destroy CG bug under specific
circumstances (authored by kristina, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D54344?vs=173602=173605#toc
Repository:
rL LLVM
https://reviews.llvm.org/D54344
Files:
cfe/trunk/lib/CodeGen/CGDeclCXX.cpp
cfe/trunk/test/CodeGenCXX/attr-no-destroy-d54344.cpp
Index: cfe/trunk/lib/CodeGen/CGDeclCXX.cpp
===
--- cfe/trunk/lib/CodeGen/CGDeclCXX.cpp
+++ cfe/trunk/lib/CodeGen/CGDeclCXX.cpp
@@ -64,6 +64,15 @@
/// static storage duration.
static void EmitDeclDestroy(CodeGenFunction , const VarDecl ,
ConstantAddress Addr) {
+ // Honor __attribute__((no_destroy)) and bail instead of attempting
+ // to emit a reference to a possibly nonexistent destructor, which
+ // in turn can cause a crash. This will result in a global constructor
+ // that isn't balanced out by a destructor call as intended by the
+ // attribute. This also checks for -fno-c++-static-destructors and
+ // bails even if the attribute is not present.
+ if (D.isNoDestroy(CGF.getContext()))
+return;
+
CodeGenModule = CGF.CGM;
// FIXME: __attribute__((cleanup)) ?
Index: cfe/trunk/test/CodeGenCXX/attr-no-destroy-d54344.cpp
===
--- cfe/trunk/test/CodeGenCXX/attr-no-destroy-d54344.cpp
+++ cfe/trunk/test/CodeGenCXX/attr-no-destroy-d54344.cpp
@@ -0,0 +1,42 @@
+// RUN: %clang_cc1 -std=c++2a -emit-llvm -O0 -triple x86_64-unknown-linux-gnu
-DNOATTR %s -o - | FileCheck %s
+// RUN: %clang_cc1 -std=c++2a -emit-llvm -O0 -triple x86_64-unknown-linux-gnu
%s -o - | FileCheck %s --check-prefix=CHECK-ATTR
+// RUN: %clang_cc1 -std=c++2a -emit-llvm -O0 -triple x86_64-unknown-linux-gnu
-DNOATTR -fno-c++-static-destructors %s -o - | FileCheck %s
--check-prefix=CHECK-FLAG
+
+// Regression test for D54344. Class with no user-defined destructor
+// that has an inherited member that has a non-trivial destructor
+// and a non-default constructor will attempt to emit a destructor
+// despite being marked as __attribute((no_destroy)) in which case
+// it would trigger an assertion due to an incorrect assumption.
+
+// This test is more reliable with asserts to work as without
+// the crash may (unlikely) could generate working but semantically
+// incorrect code.
+
+class a {
+public:
+ ~a();
+};
+class logger_base {
+ a d;
+};
+class e : logger_base {};
+#ifndef NOATTR
+__attribute((no_destroy))
+#endif
+e g;
+
+// In the absence of the attribute and flag, both ctor and dtor should
+// be emitted, check for that.
+// CHECK: @__cxx_global_var_init
+// CHECK: @__cxa_atexit
+
+// When attribute is enabled, the constructor should not be balanced
+// by a destructor. Make sure we have the ctor but not the dtor
+// registration.
+// CHECK-ATTR: @__cxx_global_var_init
+// CHECK-ATTR-NOT: @__cxa_atexit
+
+// Same scenario except with global flag (-fno-c++-static-destructors)
+// supressing it instead of the attribute.
+// CHECK-FLAG: @__cxx_global_var_init
+// CHECK-FLAG-NOT: @__cxa_atexit
Index: cfe/trunk/lib/CodeGen/CGDeclCXX.cpp
===
--- cfe/trunk/lib/CodeGen/CGDeclCXX.cpp
+++ cfe/trunk/lib/CodeGen/CGDeclCXX.cpp
@@ -64,6 +64,15 @@
/// static storage duration.
static void EmitDeclDestroy(CodeGenFunction , const VarDecl ,
ConstantAddress Addr) {
+ // Honor __attribute__((no_destroy)) and bail instead of attempting
+ // to emit a reference to a possibly nonexistent destructor, which
+ // in turn can cause a crash. This will result in a global constructor
+ // that isn't balanced out by a destructor call as intended by the
+ // attribute. This also checks for -fno-c++-static-destructors and
+ // bails even if the attribute is not present.
+ if (D.isNoDestroy(CGF.getContext()))
+return;
+
CodeGenModule = CGF.CGM;
// FIXME: __attribute__((cleanup)) ?
Index: cfe/trunk/test/CodeGenCXX/attr-no-destroy-d54344.cpp
===
--- cfe/trunk/test/CodeGenCXX/attr-no-destroy-d54344.cpp
+++ cfe/trunk/test/CodeGenCXX/attr-no-destroy-d54344.cpp
@@ -0,0 +1,42 @@
+// RUN: %clang_cc1 -std=c++2a -emit-llvm -O0 -triple x86_64-unknown-linux-gnu -DNOATTR %s -o - | FileCheck %s
+// RUN: %clang_cc1 -std=c++2a -emit-llvm -O0 -triple x86_64-unknown-linux-gnu %s -o - | FileCheck %s --check-prefix=CHECK-ATTR
+// RUN: %clang_cc1 -std=c++2a -emit-llvm -O0 -triple x86_64-unknown-linux-gnu -DNOATTR -fno-c++-static-destructors %s -o - | FileCheck %s --check-prefix=CHECK-FLAG
+
+// Regression test for D54344. Class with no user-defined destructor
+// that has an inherited member that has a non-trivial destructor
+// and a non-default
r346628 - [CodeGen][CXX]: Fix no_destroy CG bug under specific circumstances
Author: kristina
Date: Sun Nov 11 17:19:16 2018
New Revision: 346628
URL: http://llvm.org/viewvc/llvm-project?rev=346628=rev
Log:
[CodeGen][CXX]: Fix no_destroy CG bug under specific circumstances
Summary:
Class with no user-defined destructor that has an inherited member that has a
non-trivial destructor and a non-default constructor will attempt to emit a
destructor despite being marked as __attribute((no_destroy)) in which case it
would trigger an assertion due to an incorrect assumption.
In addition this adds missing test coverage for IR generation for no_destroy.
(Note that here use of no_destroy is synonymous with its global flag
counterpart `-fno-c++-static-destructors` being enabled)
Differential Revision: https://reviews.llvm.org/D54344
Added:
cfe/trunk/test/CodeGenCXX/attr-no-destroy-d54344.cpp
Modified:
cfe/trunk/lib/CodeGen/CGDeclCXX.cpp
Modified: cfe/trunk/lib/CodeGen/CGDeclCXX.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDeclCXX.cpp?rev=346628=346627=346628=diff
==
--- cfe/trunk/lib/CodeGen/CGDeclCXX.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDeclCXX.cpp Sun Nov 11 17:19:16 2018
@@ -64,6 +64,15 @@ static void EmitDeclInit(CodeGenFunction
/// static storage duration.
static void EmitDeclDestroy(CodeGenFunction , const VarDecl ,
ConstantAddress Addr) {
+ // Honor __attribute__((no_destroy)) and bail instead of attempting
+ // to emit a reference to a possibly nonexistent destructor, which
+ // in turn can cause a crash. This will result in a global constructor
+ // that isn't balanced out by a destructor call as intended by the
+ // attribute. This also checks for -fno-c++-static-destructors and
+ // bails even if the attribute is not present.
+ if (D.isNoDestroy(CGF.getContext()))
+return;
+
CodeGenModule = CGF.CGM;
// FIXME: __attribute__((cleanup)) ?
Added: cfe/trunk/test/CodeGenCXX/attr-no-destroy-d54344.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/attr-no-destroy-d54344.cpp?rev=346628=auto
==
--- cfe/trunk/test/CodeGenCXX/attr-no-destroy-d54344.cpp (added)
+++ cfe/trunk/test/CodeGenCXX/attr-no-destroy-d54344.cpp Sun Nov 11 17:19:16
2018
@@ -0,0 +1,42 @@
+// RUN: %clang_cc1 -std=c++2a -emit-llvm -O0 -triple x86_64-unknown-linux-gnu
-DNOATTR %s -o - | FileCheck %s
+// RUN: %clang_cc1 -std=c++2a -emit-llvm -O0 -triple x86_64-unknown-linux-gnu
%s -o - | FileCheck %s --check-prefix=CHECK-ATTR
+// RUN: %clang_cc1 -std=c++2a -emit-llvm -O0 -triple x86_64-unknown-linux-gnu
-DNOATTR -fno-c++-static-destructors %s -o - | FileCheck %s
--check-prefix=CHECK-FLAG
+
+// Regression test for D54344. Class with no user-defined destructor
+// that has an inherited member that has a non-trivial destructor
+// and a non-default constructor will attempt to emit a destructor
+// despite being marked as __attribute((no_destroy)) in which case
+// it would trigger an assertion due to an incorrect assumption.
+
+// This test is more reliable with asserts to work as without
+// the crash may (unlikely) could generate working but semantically
+// incorrect code.
+
+class a {
+public:
+ ~a();
+};
+class logger_base {
+ a d;
+};
+class e : logger_base {};
+#ifndef NOATTR
+__attribute((no_destroy))
+#endif
+e g;
+
+// In the absence of the attribute and flag, both ctor and dtor should
+// be emitted, check for that.
+// CHECK: @__cxx_global_var_init
+// CHECK: @__cxa_atexit
+
+// When attribute is enabled, the constructor should not be balanced
+// by a destructor. Make sure we have the ctor but not the dtor
+// registration.
+// CHECK-ATTR: @__cxx_global_var_init
+// CHECK-ATTR-NOT: @__cxa_atexit
+
+// Same scenario except with global flag (-fno-c++-static-destructors)
+// supressing it instead of the attribute.
+// CHECK-FLAG: @__cxx_global_var_init
+// CHECK-FLAG-NOT: @__cxa_atexit
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D54379: Add Hurd support
sthibaul updated this revision to Diff 173604.
sthibaul added a comment.
Herald added a subscriber: srhines.
This includes version finding the gcc Hurd triplet (i[3456]-gnu) in the Gcc
detector.
Repository:
rC Clang
https://reviews.llvm.org/D54379
Files:
lib/Basic/Targets.cpp
lib/Basic/Targets/OSTargets.h
lib/Driver/CMakeLists.txt
lib/Driver/Driver.cpp
lib/Driver/ToolChains/Clang.cpp
lib/Driver/ToolChains/Gnu.cpp
lib/Driver/ToolChains/Hurd.cpp
lib/Driver/ToolChains/Hurd.h
lib/Frontend/InitHeaderSearch.cpp
Index: lib/Frontend/InitHeaderSearch.cpp
===
--- lib/Frontend/InitHeaderSearch.cpp
+++ lib/Frontend/InitHeaderSearch.cpp
@@ -260,6 +260,7 @@
switch (os) {
case llvm::Triple::Linux:
+ case llvm::Triple::Hurd:
case llvm::Triple::Solaris:
llvm_unreachable("Include management is handled in the driver.");
Index: lib/Driver/ToolChains/Hurd.h
===
--- lib/Driver/ToolChains/Hurd.h
+++ lib/Driver/ToolChains/Hurd.h
@@ -0,0 +1,36 @@
+//===--- Hurd.h - Hurd ToolChain Implementations --*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_Hurd_H
+#define LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_Hurd_H
+
+#include "Gnu.h"
+#include "clang/Driver/ToolChain.h"
+
+namespace clang {
+namespace driver {
+namespace toolchains {
+
+class LLVM_LIBRARY_VISIBILITY Hurd : public Generic_ELF {
+public:
+ Hurd(const Driver , const llvm::Triple ,
+ const llvm::opt::ArgList );
+
+ void
+ AddClangSystemIncludeArgs(const llvm::opt::ArgList ,
+llvm::opt::ArgStringList ) const override;
+
+ virtual std::string computeSysRoot() const;
+};
+
+} // end namespace toolchains
+} // end namespace driver
+} // end namespace clang
+
+#endif // LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_Hurd_H
Index: lib/Driver/ToolChains/Hurd.cpp
===
--- lib/Driver/ToolChains/Hurd.cpp
+++ lib/Driver/ToolChains/Hurd.cpp
@@ -0,0 +1,142 @@
+//===--- Hurd.cpp - Hurd ToolChain Implementations *- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "Hurd.h"
+#include "CommonArgs.h"
+#include "clang/Basic/VirtualFileSystem.h"
+#include "clang/Config/config.h"
+#include "clang/Driver/Driver.h"
+#include "clang/Driver/Options.h"
+#include "llvm/Support/Path.h"
+
+using namespace clang::driver;
+using namespace clang::driver::toolchains;
+using namespace clang;
+using namespace llvm::opt;
+
+using tools::addPathIfExists;
+
+/// Get our best guess at the multiarch triple for a target.
+///
+/// Debian-based systems are starting to use a multiarch setup where they use
+/// a target-triple directory in the library and header search paths.
+/// Unfortunately, this triple does not align with the vanilla target triple,
+/// so we provide a rough mapping here.
+static std::string getMultiarchTriple(const Driver ,
+ const llvm::Triple ,
+ StringRef SysRoot) {
+ // For most architectures, just use whatever we have rather than trying to be
+ // clever.
+ switch (TargetTriple.getArch()) {
+ default:
+break;
+
+ // We use the existence of '/lib/' as a directory to detect some
+ // common hurd triples that don't quite match the Clang triple for both
+ // 32-bit and 64-bit targets. Multiarch fixes its install triples to these
+ // regardless of what the actual target triple is.
+ case llvm::Triple::x86:
+if (D.getVFS().exists(SysRoot + "/lib/i386-gnu"))
+ return "i386-gnu";
+break;
+ }
+
+ return TargetTriple.str();
+}
+
+static StringRef getOSLibDir(const llvm::Triple , const ArgList ) {
+ // It happens that only x86 and PPC use the 'lib32' variant of oslibdir, and
+ // using that variant while targeting other architectures causes problems
+ // because the libraries are laid out in shared system roots that can't cope
+ // with a 'lib32' library search path being considered. So we only enable
+ // them when we know we may need it.
+ //
+ // FIXME: This is a bit of a hack. We should really unify this code for
+ // reasoning about oslibdir spellings with the lib dir spellings in the
+ // GCCInstallationDetector, but that is a more significant refactoring.
+
+ if (Triple.getArch() == llvm::Triple::x86)
+return "lib32";
+
+ return Triple.isArch32Bit() ? "lib" : "lib64";
+}
+
+Hurd::Hurd(const Driver ,
[PATCH] D54344: [Clang][CodeGen][CXX]: Workaround __attribute((no_destroy)) crash/incorrect code generation.
erik.pilkington added a comment.
In https://reviews.llvm.org/D54344#1294960, @kristina wrote:
> In https://reviews.llvm.org/D54344#1294942, @erik.pilkington wrote:
>
> > LGTM too, with one small fix in test. Thanks for working on this!
>
>
> Well, I figured since the assertion being tripped was (before investigation)
> the only reliable way to notice the bug it makes sense for it to stay in,
> main concern being that should anything regress and assertions are off, the
> code generation is essentially undefined. Though a good argument for taking
> it out would be the fact that currently it's the only test that verifies IR
> generated with the attribute (last time I checked), but I would also imagine
> most people running tests would have assertion enabled (or debug) builds.
clang/test/CodeGenCXX/{always,no}_destroy.cpp also test the generated IR. I
would prefer always running it, its an interesting code path that might break
in other ways, so even if we can't test as thoroughly without assertions, its
still worth it to run.
> Though I'm happy to revise the test to remove the assertion requirement,
> deferring to your judgement with regards to above.
>
> Aside from that, are you happy for me to land this after the revision?
Yep, land away! Thanks again for fixing this.
Repository:
rC Clang
https://reviews.llvm.org/D54344
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D54344: [Clang][CodeGen][CXX]: Workaround __attribute((no_destroy)) crash/incorrect code generation.
kristina added a comment. Though on the other hand it makes no sense to skip it with asserts off either, that just clicked in my head, sorry. Repository: rC Clang https://reviews.llvm.org/D54344 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D54344: [Clang][CodeGen][CXX]: Workaround __attribute((no_destroy)) crash/incorrect code generation.
kristina updated this revision to Diff 173602.
kristina added a comment.
Revised to address comments.
Repository:
rC Clang
https://reviews.llvm.org/D54344
Files:
lib/CodeGen/CGDeclCXX.cpp
test/CodeGenCXX/attr-no-destroy-d54344.cpp
Index: test/CodeGenCXX/attr-no-destroy-d54344.cpp
===
--- test/CodeGenCXX/attr-no-destroy-d54344.cpp
+++ test/CodeGenCXX/attr-no-destroy-d54344.cpp
@@ -0,0 +1,40 @@
+// RUN: %clang_cc1 -std=c++2a -emit-llvm -O0 -triple x86_64-unknown-linux-gnu
-DNOATTR %s -o - | FileCheck %s
+// RUN: %clang_cc1 -std=c++2a -emit-llvm -O0 -triple x86_64-unknown-linux-gnu
%s -o - | FileCheck %s --check-prefix=CHECK-ATTR
+// RUN: %clang_cc1 -std=c++2a -emit-llvm -O0 -triple x86_64-unknown-linux-gnu
-DNOATTR -fno-c++-static-destructors %s -o - | FileCheck %s
--check-prefix=CHECK-FLAG
+
+// Regression test for D54344. Class with no user-defined destructor
+// that has an inherited member that has a non-trivial destructor
+// and a non-default constructor will attempt to emit a destructor
+// despite being marked as __attribute((no_destroy)) in which case
+// it would trigger an assertion due to an incorrect assumption.
+// This test requires asserts to reliably work as without the crash
+// it generates working but semantically incorrect code.
+
+class a {
+public:
+ ~a();
+};
+class logger_base {
+ a d;
+};
+class e : logger_base {};
+#ifndef NOATTR
+__attribute((no_destroy))
+#endif
+e g;
+
+// In the absence of the attribute and flag, both ctor and dtor should
+// be emitted, check for that.
+// CHECK: @__cxx_global_var_init
+// CHECK: @__cxa_atexit
+
+// When attribute is enabled, the constructor should not be balanced
+// by a destructor. Make sure we have the ctor but not the dtor
+// registration.
+// CHECK-ATTR: @__cxx_global_var_init
+// CHECK-ATTR-NOT: @__cxa_atexit
+
+// Same scenario except with global flag (-fno-c++-static-destructors)
+// supressing it instead of the attribute.
+// CHECK-FLAG: @__cxx_global_var_init
+// CHECK-FLAG-NOT: @__cxa_atexit
Index: lib/CodeGen/CGDeclCXX.cpp
===
--- lib/CodeGen/CGDeclCXX.cpp
+++ lib/CodeGen/CGDeclCXX.cpp
@@ -64,6 +64,15 @@
/// static storage duration.
static void EmitDeclDestroy(CodeGenFunction , const VarDecl ,
ConstantAddress Addr) {
+ // Honor __attribute__((no_destroy)) and bail instead of attempting
+ // to emit a reference to a possibly nonexistent destructor, which
+ // in turn can cause a crash. This will result in a global constructor
+ // that isn't balanced out by a destructor call as intended by the
+ // attribute. This also checks for -fno-c++-static-destructors and
+ // bails even if the attribute is not present.
+ if (D.isNoDestroy(CGF.getContext()))
+return;
+
CodeGenModule = CGF.CGM;
// FIXME: __attribute__((cleanup)) ?
Index: test/CodeGenCXX/attr-no-destroy-d54344.cpp
===
--- test/CodeGenCXX/attr-no-destroy-d54344.cpp
+++ test/CodeGenCXX/attr-no-destroy-d54344.cpp
@@ -0,0 +1,40 @@
+// RUN: %clang_cc1 -std=c++2a -emit-llvm -O0 -triple x86_64-unknown-linux-gnu -DNOATTR %s -o - | FileCheck %s
+// RUN: %clang_cc1 -std=c++2a -emit-llvm -O0 -triple x86_64-unknown-linux-gnu %s -o - | FileCheck %s --check-prefix=CHECK-ATTR
+// RUN: %clang_cc1 -std=c++2a -emit-llvm -O0 -triple x86_64-unknown-linux-gnu -DNOATTR -fno-c++-static-destructors %s -o - | FileCheck %s --check-prefix=CHECK-FLAG
+
+// Regression test for D54344. Class with no user-defined destructor
+// that has an inherited member that has a non-trivial destructor
+// and a non-default constructor will attempt to emit a destructor
+// despite being marked as __attribute((no_destroy)) in which case
+// it would trigger an assertion due to an incorrect assumption.
+// This test requires asserts to reliably work as without the crash
+// it generates working but semantically incorrect code.
+
+class a {
+public:
+ ~a();
+};
+class logger_base {
+ a d;
+};
+class e : logger_base {};
+#ifndef NOATTR
+__attribute((no_destroy))
+#endif
+e g;
+
+// In the absence of the attribute and flag, both ctor and dtor should
+// be emitted, check for that.
+// CHECK: @__cxx_global_var_init
+// CHECK: @__cxa_atexit
+
+// When attribute is enabled, the constructor should not be balanced
+// by a destructor. Make sure we have the ctor but not the dtor
+// registration.
+// CHECK-ATTR: @__cxx_global_var_init
+// CHECK-ATTR-NOT: @__cxa_atexit
+
+// Same scenario except with global flag (-fno-c++-static-destructors)
+// supressing it instead of the attribute.
+// CHECK-FLAG: @__cxx_global_var_init
+// CHECK-FLAG-NOT: @__cxa_atexit
Index: lib/CodeGen/CGDeclCXX.cpp
===
--- lib/CodeGen/CGDeclCXX.cpp
+++ lib/CodeGen/CGDeclCXX.cpp
@@ -64,6 +64,15 @@
/// static storage duration.
[PATCH] D54344: [Clang][CodeGen][CXX]: Workaround __attribute((no_destroy)) crash/incorrect code generation.
kristina added a comment. In https://reviews.llvm.org/D54344#1294942, @erik.pilkington wrote: > LGTM too, with one small fix in test. Thanks for working on this! Well, I figured since the assertion being tripped was (before investigation) the only reliable way to notice the bug it makes sense for it to stay in, main concern being that should anything regress and assertions are off, the code generation is essentially undefined. Though a good argument for taking it out would be the fact that currently it's the only test that verifies IR generated with the attribute (last time I checked), but I would also imagine most people running tests would have assertion enabled (or debug) builds. Though I'm happy to revise the test to remove the assertion requirement, deferring to your judgement with regards to above. Aside from that, are you happy for me to land this after the revision? Repository: rC Clang https://reviews.llvm.org/D54344 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D54379: Add Hurd support
sthibaul updated this revision to Diff 173601.
Repository:
rC Clang
https://reviews.llvm.org/D54379
Files:
lib/Basic/Targets.cpp
lib/Basic/Targets/OSTargets.h
lib/Driver/CMakeLists.txt
lib/Driver/Driver.cpp
lib/Driver/ToolChains/Clang.cpp
lib/Driver/ToolChains/Gnu.cpp
lib/Driver/ToolChains/Hurd.cpp
lib/Driver/ToolChains/Hurd.h
lib/Frontend/InitHeaderSearch.cpp
Index: lib/Frontend/InitHeaderSearch.cpp
===
--- lib/Frontend/InitHeaderSearch.cpp
+++ lib/Frontend/InitHeaderSearch.cpp
@@ -260,6 +260,7 @@
switch (os) {
case llvm::Triple::Linux:
+ case llvm::Triple::Hurd:
case llvm::Triple::Solaris:
llvm_unreachable("Include management is handled in the driver.");
Index: lib/Driver/ToolChains/Hurd.h
===
--- lib/Driver/ToolChains/Hurd.h
+++ lib/Driver/ToolChains/Hurd.h
@@ -0,0 +1,36 @@
+//===--- Hurd.h - Hurd ToolChain Implementations --*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_Hurd_H
+#define LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_Hurd_H
+
+#include "Gnu.h"
+#include "clang/Driver/ToolChain.h"
+
+namespace clang {
+namespace driver {
+namespace toolchains {
+
+class LLVM_LIBRARY_VISIBILITY Hurd : public Generic_ELF {
+public:
+ Hurd(const Driver , const llvm::Triple ,
+ const llvm::opt::ArgList );
+
+ void
+ AddClangSystemIncludeArgs(const llvm::opt::ArgList ,
+llvm::opt::ArgStringList ) const override;
+
+ virtual std::string computeSysRoot() const;
+};
+
+} // end namespace toolchains
+} // end namespace driver
+} // end namespace clang
+
+#endif // LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_Hurd_H
Index: lib/Driver/ToolChains/Hurd.cpp
===
--- lib/Driver/ToolChains/Hurd.cpp
+++ lib/Driver/ToolChains/Hurd.cpp
@@ -0,0 +1,142 @@
+//===--- Hurd.cpp - Hurd ToolChain Implementations *- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "Hurd.h"
+#include "CommonArgs.h"
+#include "clang/Basic/VirtualFileSystem.h"
+#include "clang/Config/config.h"
+#include "clang/Driver/Driver.h"
+#include "clang/Driver/Options.h"
+#include "llvm/Support/Path.h"
+
+using namespace clang::driver;
+using namespace clang::driver::toolchains;
+using namespace clang;
+using namespace llvm::opt;
+
+using tools::addPathIfExists;
+
+/// Get our best guess at the multiarch triple for a target.
+///
+/// Debian-based systems are starting to use a multiarch setup where they use
+/// a target-triple directory in the library and header search paths.
+/// Unfortunately, this triple does not align with the vanilla target triple,
+/// so we provide a rough mapping here.
+static std::string getMultiarchTriple(const Driver ,
+ const llvm::Triple ,
+ StringRef SysRoot) {
+ // For most architectures, just use whatever we have rather than trying to be
+ // clever.
+ switch (TargetTriple.getArch()) {
+ default:
+break;
+
+ // We use the existence of '/lib/' as a directory to detect some
+ // common hurd triples that don't quite match the Clang triple for both
+ // 32-bit and 64-bit targets. Multiarch fixes its install triples to these
+ // regardless of what the actual target triple is.
+ case llvm::Triple::x86:
+if (D.getVFS().exists(SysRoot + "/lib/i386-gnu"))
+ return "i386-gnu";
+break;
+ }
+
+ return TargetTriple.str();
+}
+
+static StringRef getOSLibDir(const llvm::Triple , const ArgList ) {
+ // It happens that only x86 and PPC use the 'lib32' variant of oslibdir, and
+ // using that variant while targeting other architectures causes problems
+ // because the libraries are laid out in shared system roots that can't cope
+ // with a 'lib32' library search path being considered. So we only enable
+ // them when we know we may need it.
+ //
+ // FIXME: This is a bit of a hack. We should really unify this code for
+ // reasoning about oslibdir spellings with the lib dir spellings in the
+ // GCCInstallationDetector, but that is a more significant refactoring.
+
+ if (Triple.getArch() == llvm::Triple::x86)
+return "lib32";
+
+ return Triple.isArch32Bit() ? "lib" : "lib64";
+}
+
+Hurd::Hurd(const Driver , const llvm::Triple ,
+ const ArgList )
+: Generic_ELF(D, Triple, Args) { }
+
+std::string Hurd::computeSysRoot() const {
+ if
[PATCH] D54379: Add Hurd support
sthibaul updated this revision to Diff 173599.
sthibaul added a comment.
In this version, the Driver introduces the "-hurd-" part
Repository:
rC Clang
https://reviews.llvm.org/D54379
Files:
lib/Basic/Targets.cpp
lib/Basic/Targets/OSTargets.h
lib/Driver/CMakeLists.txt
lib/Driver/Driver.cpp
lib/Driver/ToolChains/Clang.cpp
lib/Driver/ToolChains/Gnu.cpp
lib/Driver/ToolChains/Hurd.cpp
lib/Driver/ToolChains/Hurd.h
lib/Frontend/InitHeaderSearch.cpp
Index: lib/Frontend/InitHeaderSearch.cpp
===
--- lib/Frontend/InitHeaderSearch.cpp
+++ lib/Frontend/InitHeaderSearch.cpp
@@ -260,6 +260,7 @@
switch (os) {
case llvm::Triple::Linux:
+ case llvm::Triple::Hurd:
case llvm::Triple::Solaris:
llvm_unreachable("Include management is handled in the driver.");
Index: lib/Driver/ToolChains/Hurd.h
===
--- lib/Driver/ToolChains/Hurd.h
+++ lib/Driver/ToolChains/Hurd.h
@@ -0,0 +1,36 @@
+//===--- Hurd.h - Hurd ToolChain Implementations --*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_Hurd_H
+#define LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_Hurd_H
+
+#include "Gnu.h"
+#include "clang/Driver/ToolChain.h"
+
+namespace clang {
+namespace driver {
+namespace toolchains {
+
+class LLVM_LIBRARY_VISIBILITY Hurd : public Generic_ELF {
+public:
+ Hurd(const Driver , const llvm::Triple ,
+ const llvm::opt::ArgList );
+
+ void
+ AddClangSystemIncludeArgs(const llvm::opt::ArgList ,
+llvm::opt::ArgStringList ) const override;
+
+ virtual std::string computeSysRoot() const;
+};
+
+} // end namespace toolchains
+} // end namespace driver
+} // end namespace clang
+
+#endif // LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_Hurd_H
Index: lib/Driver/ToolChains/Hurd.cpp
===
--- lib/Driver/ToolChains/Hurd.cpp
+++ lib/Driver/ToolChains/Hurd.cpp
@@ -0,0 +1,142 @@
+//===--- Hurd.cpp - Hurd ToolChain Implementations *- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "Hurd.h"
+#include "CommonArgs.h"
+#include "clang/Basic/VirtualFileSystem.h"
+#include "clang/Config/config.h"
+#include "clang/Driver/Driver.h"
+#include "clang/Driver/Options.h"
+#include "llvm/Support/Path.h"
+
+using namespace clang::driver;
+using namespace clang::driver::toolchains;
+using namespace clang;
+using namespace llvm::opt;
+
+using tools::addPathIfExists;
+
+/// Get our best guess at the multiarch triple for a target.
+///
+/// Debian-based systems are starting to use a multiarch setup where they use
+/// a target-triple directory in the library and header search paths.
+/// Unfortunately, this triple does not align with the vanilla target triple,
+/// so we provide a rough mapping here.
+static std::string getMultiarchTriple(const Driver ,
+ const llvm::Triple ,
+ StringRef SysRoot) {
+ // For most architectures, just use whatever we have rather than trying to be
+ // clever.
+ switch (TargetTriple.getArch()) {
+ default:
+break;
+
+ // We use the existence of '/lib/' as a directory to detect some
+ // common hurd triples that don't quite match the Clang triple for both
+ // 32-bit and 64-bit targets. Multiarch fixes its install triples to these
+ // regardless of what the actual target triple is.
+ case llvm::Triple::x86:
+if (D.getVFS().exists(SysRoot + "/lib/i386-gnu"))
+ return "i386-gnu";
+break;
+ }
+
+ return TargetTriple.str();
+}
+
+static StringRef getOSLibDir(const llvm::Triple , const ArgList ) {
+ // It happens that only x86 and PPC use the 'lib32' variant of oslibdir, and
+ // using that variant while targeting other architectures causes problems
+ // because the libraries are laid out in shared system roots that can't cope
+ // with a 'lib32' library search path being considered. So we only enable
+ // them when we know we may need it.
+ //
+ // FIXME: This is a bit of a hack. We should really unify this code for
+ // reasoning about oslibdir spellings with the lib dir spellings in the
+ // GCCInstallationDetector, but that is a more significant refactoring.
+
+ if (Triple.getArch() == llvm::Triple::x86)
+return "lib32";
+
+ return Triple.isArch32Bit() ? "lib" : "lib64";
+}
+
+Hurd::Hurd(const Driver , const llvm::Triple ,
+ const ArgList )
+:
[PATCH] D54344: [Clang][CodeGen][CXX]: Workaround __attribute((no_destroy)) crash/incorrect code generation.
erik.pilkington accepted this revision. erik.pilkington added a comment. LGTM too, with one small fix in test. Thanks for working on this! Comment at: test/CodeGenCXX/attr-no-destroy-d54344.cpp:4 +// RUN: %clang_cc1 -std=c++2a -emit-llvm -O0 -triple x86_64-unknown-linux-gnu -DNOATTR -fno-c++-static-destructors %s -o - | FileCheck %s --check-prefix=CHECK-FLAG +// REQUIRES: asserts + Why do you require asserts here? If this test still passes regardless of whether clang was compiled with assertions enabled, we should always run it. Repository: rC Clang https://reviews.llvm.org/D54344 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D54407: Record the matcher type when storing a binding
aaron.ballman added a reviewer: sbenza. aaron.ballman added a subscriber: sbenza. aaron.ballman added a comment. Adding @sbenza in case he has opinions on this approach. I think it's reasonable, but I also know that changes to the the AST matcher internals sometimes have unintended side effects with the various instantiations. Comment at: include/clang/ASTMatchers/ASTMatchersInternal.h:189-190 + using IDToNodeMap = + std::map>; Use of a `std::pair` here is unfortunate because it's really hard to keep track of what first and second actually mean. A tiny aggregate with two named members would help. Repository: rC Clang https://reviews.llvm.org/D54407 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D54406: Add matchDynamic convenience functions
aaron.ballman added inline comments.
Comment at: include/clang/ASTMatchers/ASTMatchFinder.h:314
+inline SmallVector
+matchDynamic(internal::DynTypedMatcher Matcher, const
ast_type_traits::DynTypedNode ,
+ ASTContext ) {
80-col issue.
Comment at: include/clang/ASTMatchers/ASTMatchFinder.h:325-326
+SmallVector
+matchDynamic(internal::DynTypedMatcher Matcher, const NodeT , ASTContext
) {
+ return matchDynamic(Matcher, ast_type_traits::DynTypedNode::create(Node),
Context);
+}
Same here; run the patch through clang-format?
Repository:
rC Clang
https://reviews.llvm.org/D54406
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D54405: Record whether a AST Matcher constructs a Node
aaron.ballman added inline comments.
Comment at: lib/ASTMatchers/Dynamic/Registry.cpp:70
+ void registerIfNodeMatcher(MatcherType,
+ internal::MatcherDescriptor *matchDescriptor,
+ StringRef) {}
Fix `matchDescriptor` for coding conventions, but please don't reuse the name
of a type when fixing it. :-)
Comment at: lib/ASTMatchers/Dynamic/Registry.cpp:76
+ ast_matchers::internal::VariadicAllOfMatcher VarFunc,
+ internal::MatcherDescriptor *matchDescriptor, StringRef MatcherName) {
+auto K = ast_type_traits::ASTNodeKind::getFromNodeKind<
Same comment here.
Comment at: lib/ASTMatchers/Dynamic/Registry.cpp:77
+ internal::MatcherDescriptor *matchDescriptor, StringRef MatcherName) {
+auto K = ast_type_traits::ASTNodeKind::getFromNodeKind<
+typename
ast_matchers::internal::VariadicAllOfMatcher::Type>();
Mildly not keen on the use of `auto` here. It's a factory function, so it kind
of names the resulting type, but it also looks like the type will be
`ast_matchers::internal::VariadicAllOfMatcher::Type` from the template
argument, which is incorrect.
Comment at: lib/ASTMatchers/Dynamic/Registry.cpp:79
+typename
ast_matchers::internal::VariadicAllOfMatcher::Type>();
+if (!K.isNone()) {
+ NodeConstructors[K] = std::make_pair(MatcherName, matchDescriptor);
Elide braces.
Comment at: lib/ASTMatchers/Dynamic/Registry.cpp:88
+ VarFunc,
+ internal::MatcherDescriptor *matchDescriptor, StringRef MatcherName) {
+auto K = ast_type_traits::ASTNodeKind::getFromNodeKind();
Same `matchDescriptor` here.
Comment at: lib/ASTMatchers/Dynamic/Registry.cpp:89
+ internal::MatcherDescriptor *matchDescriptor, StringRef MatcherName) {
+auto K = ast_type_traits::ASTNodeKind::getFromNodeKind();
+if (!K.isNone()) {
Similar comment about `auto` here.
Comment at: lib/ASTMatchers/Dynamic/Registry.cpp:90
+auto K = ast_type_traits::ASTNodeKind::getFromNodeKind();
+if (!K.isNone()) {
+ NodeConstructors[K] = std::make_pair(MatcherName, matchDescriptor);
Elide braces.
Repository:
rC Clang
https://reviews.llvm.org/D54405
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D54404: Exclude matchers which can have multiple results
aaron.ballman added inline comments.
Comment at: lib/ASTMatchers/Dynamic/Registry.cpp:604
+ static std::vector excludedMatchers{
+ "allOf",
`excludedMatchers` -> `ExcludedMatchers`
Comment at: lib/ASTMatchers/Dynamic/Registry.cpp:604
+ static std::vector excludedMatchers{
+ "allOf",
aaron.ballman wrote:
> `excludedMatchers` -> `ExcludedMatchers`
Please add comments that explain why these are excluded and under what
circumstances this list should be updated to add or remove items.
Also: how do we ensure this list stays properly updated? We sometimes miss
updating `RegistryMaps()`, so I'm a bit concerned that adding a second list in
a different location will be inviting trouble.
Repository:
rC Clang
https://reviews.llvm.org/D54404
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D54403: Add new API for returning matching matchers
aaron.ballman added inline comments.
Comment at: lib/ASTMatchers/Dynamic/Registry.cpp:604-605
+
+ std::vector AcceptedTypes;
+ AcceptedTypes.push_back(StaticType);
+
`std::vector AcceptedTypes{StaticType};` instead?
Comment at: lib/ASTMatchers/Dynamic/Registry.cpp:608-611
+ AcceptedTypes, [](StringRef Name, const MatcherDescriptor
,
+ std::set ,
+ std::vector> ArgsKinds,
+ unsigned MaxSpecificity) {
Elide the identifiers for arguments that are not named in the lambda.
Comment at: unittests/ASTMatchers/Dynamic/RegistryTest.cpp:563
+
+ auto Contains = [](std::vector const , StringRef Name) {
+return std::find_if(C.begin(), C.end(), [Name](const MatchingMatcher ) {
Move the `const` to the other side of the declaration.
Comment at: unittests/ASTMatchers/Dynamic/RegistryTest.cpp:564
+ auto Contains = [](std::vector const , StringRef Name) {
+return std::find_if(C.begin(), C.end(), [Name](const MatchingMatcher ) {
+ return M.MatcherString == Name;
`std::find_if(C.begin(), C.end(), ...)` -> `llvm::find_if(C, ...)`
Repository:
rC Clang
https://reviews.llvm.org/D54403
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D54402: Extract method to allow re-use
aaron.ballman added a comment.
Generally LGTM but this has no test coverage. What are your plans for how you
want to see this proceed? Do you intend to commit everything in one batch once
all of the reviews are accepted, or do piecemeal commits?
Comment at: lib/ASTMatchers/Dynamic/Registry.cpp:565
+template
+void processAcceptableMatchers(ArrayRef AcceptedTypes, T func) {
I'd prefer `T` to be named `Callable` and `func` to be `Func`. Also, I think it
should be `Callable &`.
Comment at: lib/ASTMatchers/Dynamic/Registry.cpp:595
if (!RetKinds.empty() && MaxSpecificity > 0) {
- std::string Decl;
- llvm::raw_string_ostream OS(Decl);
-
- if (IsPolymorphic) {
-OS << "Matcher " << Name << "(Matcher";
- } else {
-OS << "Matcher<" << RetKinds << "> " << Name << "(";
-for (const std::vector : ArgsKinds) {
- if ( != [0])
-OS << ", ";
-
- bool FirstArgKind = true;
- std::set MatcherKinds;
- // Two steps. First all non-matchers, then matchers only.
- for (const ArgKind : Arg) {
-if (AK.getArgKind() == ArgKind::AK_Matcher) {
- MatcherKinds.insert(AK.getMatcherKind());
-} else {
- if (!FirstArgKind) OS << "|";
- FirstArgKind = false;
- OS << AK.asString();
+ func(Name, Matcher, RetKinds, ArgsKinds, MaxSpecificity);
+}
Then you can use `std::forward(Func)(...);` because we can't use
`std::invoke()` yet.
Repository:
rC Clang
https://reviews.llvm.org/D54402
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51575: [clang-tidy/checks] Implement a clang-tidy check to verify Google Objective-C function naming conventions
aaron.ballman added a comment. Generally LGTM, but I leave it to someone more well-versed in ObjC to sign off that this actually does everything those users would expect. Comment at: docs/clang-tidy/checks/google-objc-function-naming.rst:12 + +All function names should be in upper camel case. Functions whose storage class +is not static should have an appropriate prefix. "Upper camel case" is more commonly known as Pascal case, btw (http://wiki.c2.com/?PascalCase). Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51575 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D54408: Add matchers available through casting to derived
steveire added a comment. These commits are available on github if it's convenient to see it all together there: https://github.com/steveire/clang/commits/matcher-output Here is context for the changes I'm making in case it is useful: https://steveire.wordpress.com/2018/11/11/future-developments-in-clang-query/ Note that the commits pushed for review so far don't implement everything I wrote about there. Repository: rC Clang https://reviews.llvm.org/D54408 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D54246: [clang-tidy] Add the abseil-duration-factory-scale check
aaron.ballman added inline comments.
Comment at: clang-tidy/abseil/DurationFactoryScaleCheck.cpp:39
+static llvm::Optional
+GetScaleForFactory(llvm::StringRef FactoryName) {
+ static const std::unordered_map ScaleMap(
GetScaleForFactory -> getScaleForFactory, per naming conventions. Same for the
other functions.
Comment at: clang-tidy/abseil/DurationFactoryScaleCheck.cpp:48
+
+ const auto ScaleIter = ScaleMap.find(FactoryName);
+ if (ScaleIter == ScaleMap.end())
Drop top-level `const` (this returns an iterator, not a pointer/reference).
Comment at: clang-tidy/abseil/DurationFactoryScaleCheck.cpp:55
+
+// Given either an integer or float literal, return it's value.
+// One and only one of `IntLit` and `FloatLit` should be provided.
it's -> its
Comment at: clang-tidy/abseil/DurationFactoryScaleCheck.cpp:57-58
+// One and only one of `IntLit` and `FloatLit` should be provided.
+static double GetValue(const IntegerLiteral *IntLit,
+ const FloatingLiteral *FloatLit) {
+ if (IntLit) {
I really don't like this interface where you pass two arguments, only one of
which is ever valid. That is pretty confusing. Given that the result of this
function is only ever passed to `GetNewMultScale()`, and that function only
does integral checks, I'd prefer logic more like:
* If the literal is integral, get its value and call `GetNewMultScale()`.
* If the literal is float, convert it to an integral and call
`GetNewMultScale()` only if the conversion is exact (this can be done via
`APFloat::convertToInteger()`).
* `GetNewMultScale()` can now accept an integer value and removes the questions
about inexact equality tests from the function.
With that logic, I don't see a need for `GetValue()` at all, but if a helper
function is useful, I'd probably guess this is a better signature: `int64_t
getIntegralValue(const Expr *Literal, bool );`
Comment at: clang-tidy/abseil/DurationFactoryScaleCheck.cpp:59
+ const FloatingLiteral *FloatLit) {
+ if (IntLit) {
+return IntLit->getValue().getLimitedValue();
Elide these braces.
Comment at: clang-tidy/abseil/DurationFactoryScaleCheck.cpp:63
+ assert(FloatLit != nullptr && "Neither IntLit nor FloatLit set");
+ return FloatLit->getValueAsApproximateDouble();
+}
I believe the approximate results here can lead to bugs where the
floating-point literal is subnormal -- it may return 0.0 for literals that are
not zero.
Comment at: clang-tidy/abseil/DurationFactoryScaleCheck.cpp:72
+ case DurationScale::Hours:
+// We can't scale Hours.
+break;
Hours with a multiplier of `1.0/60.0` would scale to minutes, wouldn't it?
Comment at: clang-tidy/abseil/DurationFactoryScaleCheck.cpp:81-84
+if (Multiplier == 60.0)
+ return DurationScale::Minutes;
+if (Multiplier == 1e-3)
+ return DurationScale::Milliseconds;
What about scaling with a multiplier of 3600 to go from seconds to hours, and
other plausible conversions?
Comment at: clang-tidy/abseil/DurationFactoryScaleCheck.cpp:112
+// `Divisor` would produce a new scale. If so, return it, otherwise `None`.
+static llvm::Optional GetNewDivScale(DurationScale OldScale,
+double Divisor) {
Similar comments here as above. In fact, I feel like these functions should be
combined into `getNewScale()` and have normalized the scaling value in the
caller so that it covers both operations rather than manually spelling out the
conversions in either direction. e.g., convert everything into multiplier form
in the caller by treating divisors as a multiplication by 1/divisor.
Comment at: clang-tidy/abseil/DurationFactoryScaleCheck.cpp:150
+// constructing a `Duration` for that scale.
+static std::string GetFactoryForScale(DurationScale Scale) {
+ switch (Scale) {
This should return a `StringRef`.
Comment at: clang-tidy/abseil/DurationFactoryScaleCheck.cpp:194
+
+ // Don't try and replace things inside of macro definitions.
+ if (Call->getExprLoc().isMacroID())
and -> to
Comment at: clang-tidy/abseil/DurationFactoryScaleCheck.cpp:198
+
+ const Expr *Arg = Call->getArg(0)->IgnoreImpCasts();
+ // Arguments which are macros are ignored.
I suspect you want to ignore paren expressions as well, so
`IgnoreParenImpCasts()`.
Comment at: clang-tidy/abseil/DurationFactoryScaleCheck.cpp:213
+ const auto *CallDecl = Result.Nodes.getNodeAs("call_decl");
+ auto MaybeScale = GetScaleForFactory(CallDecl->getName());
+ if (!MaybeScale)
Do not use `auto` here as the type is not
[PATCH] D54408: Add matchers available through casting to derived
steveire created this revision.
steveire added a reviewer: aaron.ballman.
Herald added a subscriber: cfe-commits.
Repository:
rC Clang
https://reviews.llvm.org/D54408
Files:
lib/ASTMatchers/Dynamic/Registry.cpp
unittests/ASTMatchers/Dynamic/RegistryTest.cpp
Index: unittests/ASTMatchers/Dynamic/RegistryTest.cpp
===
--- unittests/ASTMatchers/Dynamic/RegistryTest.cpp
+++ unittests/ASTMatchers/Dynamic/RegistryTest.cpp
@@ -571,14 +571,14 @@
EXPECT_TRUE(Contains(Matchers, "hasType()"));
EXPECT_TRUE(Contains(Matchers, "hasTypeLoc()"));
EXPECT_TRUE(Contains(Matchers, "parameterCountIs()"));
+ EXPECT_TRUE(Contains(Matchers, "cxxMethodDecl(isOverride())"));
EXPECT_TRUE(!Contains(Matchers, "decl()"));
EXPECT_TRUE(!Contains(Matchers, "namedDecl()"));
EXPECT_TRUE(!Contains(Matchers, "valueDecl()"));
EXPECT_TRUE(!Contains(Matchers, "declaratorDecl()"));
EXPECT_TRUE(!Contains(Matchers, "functionDecl()"));
-
- EXPECT_TRUE(Contains(Matchers, "cxxMethodDecl()"));
+ EXPECT_TRUE(!Contains(Matchers, "cxxMethodDecl()"));
EXPECT_TRUE(!Contains(Matchers, "has()"));
}
Index: lib/ASTMatchers/Dynamic/Registry.cpp
===
--- lib/ASTMatchers/Dynamic/Registry.cpp
+++ lib/ASTMatchers/Dynamic/Registry.cpp
@@ -640,8 +640,26 @@
}
}
+using SR = ast_matchers::dynamic::SourceRange;
+
+llvm::Optional>
+getNodeConstructorType(MatcherCtor targetCtor) {
+ auto const = RegistryData->nodeConstructors();
+
+ for (auto ctor : ctors) {
+if (ctor.second.second == targetCtor)
+ return std::make_pair(ctor.first, ctor.second.first);
+ }
+ return llvm::None;
+}
+
std::vector
-Registry::getMatchingMatchers(ast_type_traits::ASTNodeKind StaticType) {
+getDerivedResults(ast_type_traits::ASTNodeKind StaticType, StringRef Name);
+
+std::vector
+getMatchingMatchersImpl(ast_type_traits::ASTNodeKind StaticType,
+bool ExactOnly = false) {
+
std::vector Result;
static std::vector excludedMatchers{
@@ -684,10 +702,39 @@
AcceptedTypes.push_back(StaticType);
processAcceptableMatchers(
- AcceptedTypes, [](StringRef Name, const MatcherDescriptor ,
- std::set ,
- std::vector> ArgsKinds,
- unsigned MaxSpecificity) {
+ AcceptedTypes, [&](StringRef Name, const MatcherDescriptor ,
+ std::set ,
+ std::vector> ArgsKinds,
+ unsigned MaxSpecificity) {
+{
+ unsigned Specificity;
+ ASTNodeKind LeastDerivedKind;
+ if (ExactOnly) {
+auto isConv = Matcher.isConvertibleTo(StaticType, ,
+ );
+if (isConv && !LeastDerivedKind.isSame(StaticType)) {
+ return;
+}
+ }
+}
+
+{
+ auto TypeForMatcherOpt = getNodeConstructorType();
+ if (TypeForMatcherOpt) {
+// TODO: Should not be needed.
+// Actually, all base types should be part of it.
+// Nah, only all derived types.
+if (TypeForMatcherOpt->first.isBaseOf(StaticType)) {
+ return;
+}
+if (StaticType.isBaseOf(TypeForMatcherOpt->first)) {
+ auto derRes = getDerivedResults(TypeForMatcherOpt->first, Name);
+ Result.insert(Result.end(), derRes.begin(), derRes.end());
+ return;
+}
+ }
+}
+
if (!std::binary_search(excludedMatchers.begin(),
excludedMatchers.end(), Name)) {
Result.emplace_back((Name + "()").str());
@@ -697,6 +744,34 @@
return Result;
}
+std::vector
+getDerivedResults(ast_type_traits::ASTNodeKind StaticType, StringRef Name) {
+ auto NestedResult = getMatchingMatchersImpl(StaticType, true);
+
+ std::vector Result;
+
+ Diagnostics DiagnosticIgnorer;
+
+ for (auto item : NestedResult) {
+
+auto nestedMatcherName = item.MatcherString;
+
+nestedMatcherName = Name.str() + "(" + nestedMatcherName + ")";
+
+Result.emplace_back(nestedMatcherName);
+ }
+
+ return Result;
+}
+
+std::vector
+Registry::getMatchingMatchers(ast_type_traits::ASTNodeKind StaticType) {
+
+ std::vector Result = getMatchingMatchersImpl(StaticType);
+
+ return Result;
+}
+
std::vector
Registry::getMatcherCompletions(ArrayRef AcceptedTypes) {
std::vector Completions;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D54406: Add matchDynamic convenience functions
steveire created this revision.
steveire added a reviewer: aaron.ballman.
Herald added a subscriber: cfe-commits.
These correspond to the existing match() free functions.
Repository:
rC Clang
https://reviews.llvm.org/D54406
Files:
include/clang/ASTMatchers/ASTMatchFinder.h
unittests/ASTMatchers/ASTMatchersNodeTest.cpp
Index: unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -1719,5 +1719,28 @@
EXPECT_FALSE(matchesObjC(ObjCStringNoPool, autoreleasePoolStmt()));
}
+TEST(MatchFinderAPI, matchesDynamic) {
+
+ std::string SourceCode = "struct A { void f() {} };";
+ auto Matcher = functionDecl(isDefinition()).bind("method");
+
+ auto astUnit = tooling::buildASTFromCode(SourceCode);
+
+ auto GlobalBoundNodes = matchDynamic(Matcher, astUnit->getASTContext());
+
+ EXPECT_EQ(GlobalBoundNodes.size(), 1u);
+ EXPECT_EQ(GlobalBoundNodes[0].getMap().size(), 1u);
+
+ auto GlobalMethodNode =
GlobalBoundNodes[0].getNodeAs("method");
+ EXPECT_TRUE(GlobalMethodNode != nullptr);
+
+ auto MethodBoundNodes = matchDynamic(Matcher, *GlobalMethodNode,
astUnit->getASTContext());
+ EXPECT_EQ(MethodBoundNodes.size(), 1u);
+ EXPECT_EQ(MethodBoundNodes[0].getMap().size(), 1u);
+
+ auto MethodNode = MethodBoundNodes[0].getNodeAs("method");
+ EXPECT_EQ(MethodNode, GlobalMethodNode);
+}
+
} // namespace ast_matchers
} // namespace clang
Index: include/clang/ASTMatchers/ASTMatchFinder.h
===
--- include/clang/ASTMatchers/ASTMatchFinder.h
+++ include/clang/ASTMatchers/ASTMatchFinder.h
@@ -310,6 +310,31 @@
return std::move(Callback.Nodes);
}
+inline SmallVector
+matchDynamic(internal::DynTypedMatcher Matcher, const
ast_type_traits::DynTypedNode ,
+ ASTContext ) {
+ internal::CollectMatchesCallback Callback;
+ MatchFinder Finder;
+ Finder.addDynamicMatcher(Matcher, );
+ Finder.match(Node, Context);
+ return std::move(Callback.Nodes);
+}
+
+template
+SmallVector
+matchDynamic(internal::DynTypedMatcher Matcher, const NodeT , ASTContext
) {
+ return matchDynamic(Matcher, ast_type_traits::DynTypedNode::create(Node),
Context);
+}
+
+inline SmallVector
+matchDynamic(internal::DynTypedMatcher Matcher, ASTContext ) {
+ internal::CollectMatchesCallback Callback;
+ MatchFinder Finder;
+ Finder.addDynamicMatcher(Matcher, );
+ Finder.matchAST(Context);
+ return std::move(Callback.Nodes);
+}
+
} // end namespace ast_matchers
} // end namespace clang
Index: unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -1719,5 +1719,28 @@
EXPECT_FALSE(matchesObjC(ObjCStringNoPool, autoreleasePoolStmt()));
}
+TEST(MatchFinderAPI, matchesDynamic) {
+
+ std::string SourceCode = "struct A { void f() {} };";
+ auto Matcher = functionDecl(isDefinition()).bind("method");
+
+ auto astUnit = tooling::buildASTFromCode(SourceCode);
+
+ auto GlobalBoundNodes = matchDynamic(Matcher, astUnit->getASTContext());
+
+ EXPECT_EQ(GlobalBoundNodes.size(), 1u);
+ EXPECT_EQ(GlobalBoundNodes[0].getMap().size(), 1u);
+
+ auto GlobalMethodNode = GlobalBoundNodes[0].getNodeAs("method");
+ EXPECT_TRUE(GlobalMethodNode != nullptr);
+
+ auto MethodBoundNodes = matchDynamic(Matcher, *GlobalMethodNode, astUnit->getASTContext());
+ EXPECT_EQ(MethodBoundNodes.size(), 1u);
+ EXPECT_EQ(MethodBoundNodes[0].getMap().size(), 1u);
+
+ auto MethodNode = MethodBoundNodes[0].getNodeAs("method");
+ EXPECT_EQ(MethodNode, GlobalMethodNode);
+}
+
} // namespace ast_matchers
} // namespace clang
Index: include/clang/ASTMatchers/ASTMatchFinder.h
===
--- include/clang/ASTMatchers/ASTMatchFinder.h
+++ include/clang/ASTMatchers/ASTMatchFinder.h
@@ -310,6 +310,31 @@
return std::move(Callback.Nodes);
}
+inline SmallVector
+matchDynamic(internal::DynTypedMatcher Matcher, const ast_type_traits::DynTypedNode ,
+ ASTContext ) {
+ internal::CollectMatchesCallback Callback;
+ MatchFinder Finder;
+ Finder.addDynamicMatcher(Matcher, );
+ Finder.match(Node, Context);
+ return std::move(Callback.Nodes);
+}
+
+template
+SmallVector
+matchDynamic(internal::DynTypedMatcher Matcher, const NodeT , ASTContext ) {
+ return matchDynamic(Matcher, ast_type_traits::DynTypedNode::create(Node), Context);
+}
+
+inline SmallVector
+matchDynamic(internal::DynTypedMatcher Matcher, ASTContext ) {
+ internal::CollectMatchesCallback Callback;
+ MatchFinder Finder;
+ Finder.addDynamicMatcher(Matcher, );
+ Finder.matchAST(Context);
+ return std::move(Callback.Nodes);
+}
+
} // end namespace ast_matchers
} // end namespace clang
___
cfe-commits
[PATCH] D54407: Record the matcher type when storing a binding
steveire created this revision.
steveire added a reviewer: aaron.ballman.
Herald added a subscriber: cfe-commits.
This is necessary so that when we wish to print the matchers for a
binding of type `CXXMeethodDecl`, but which was matched with a base
matcher such as `functionDecl()` we can inform the user that they can
write `functionDecl(cxxMethodDecl(isOverride()))` etc.
Repository:
rC Clang
https://reviews.llvm.org/D54407
Files:
include/clang/ASTMatchers/ASTMatchersInternal.h
lib/ASTMatchers/ASTMatchersInternal.cpp
lib/Tooling/RefactoringCallbacks.cpp
unittests/ASTMatchers/ASTMatchersNodeTest.cpp
unittests/ASTMatchers/ASTMatchersTest.h
Index: unittests/ASTMatchers/ASTMatchersTest.h
===
--- unittests/ASTMatchers/ASTMatchersTest.h
+++ unittests/ASTMatchers/ASTMatchersTest.h
@@ -349,12 +349,12 @@
BoundNodes::IDToNodeMap::const_iterator I = M.find(Id);
EXPECT_NE(M.end(), I);
if (I != M.end()) {
-EXPECT_EQ(Nodes->getNodeAs(Id), I->second.get());
+EXPECT_EQ(Nodes->getNodeAs(Id), I->second.first.get());
}
return true;
}
EXPECT_TRUE(M.count(Id) == 0 ||
- M.find(Id)->second.template get() == nullptr);
+ M.find(Id)->second.first.get() == nullptr);
return false;
}
Index: unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -1724,19 +1724,30 @@
std::string SourceCode = "struct A { void f() {} };";
auto Matcher = functionDecl(isDefinition()).bind("method");
+ using namespace ast_type_traits;
+
auto astUnit = tooling::buildASTFromCode(SourceCode);
auto GlobalBoundNodes = matchDynamic(Matcher, astUnit->getASTContext());
EXPECT_EQ(GlobalBoundNodes.size(), 1u);
EXPECT_EQ(GlobalBoundNodes[0].getMap().size(), 1u);
+ auto GlobalMapPair = *GlobalBoundNodes[0].getMap().begin();
+ EXPECT_TRUE(GlobalMapPair.second.first.getNodeKind().isSame(ASTNodeKind::getFromNodeKind()));
+ EXPECT_TRUE(GlobalMapPair.second.second.isSame(ASTNodeKind::getFromNodeKind()));
auto GlobalMethodNode = GlobalBoundNodes[0].getNodeAs("method");
EXPECT_TRUE(GlobalMethodNode != nullptr);
auto MethodBoundNodes = matchDynamic(Matcher, *GlobalMethodNode, astUnit->getASTContext());
EXPECT_EQ(MethodBoundNodes.size(), 1u);
EXPECT_EQ(MethodBoundNodes[0].getMap().size(), 1u);
+ auto MethodMapPair = *MethodBoundNodes[0].getMap().begin();
+ EXPECT_TRUE(MethodMapPair.second.first.getNodeKind().isSame(ASTNodeKind::getFromNodeKind()));
+ EXPECT_TRUE(MethodMapPair.second.second.isSame(ASTNodeKind::getFromNodeKind()));
+ EXPECT_EQ(MethodMapPair.second.first, GlobalMapPair.second.first);
+ EXPECT_TRUE(MethodMapPair.second.second.isSame(GlobalMapPair.second.second));
+
auto MethodNode = MethodBoundNodes[0].getNodeAs("method");
EXPECT_EQ(MethodNode, GlobalMethodNode);
Index: lib/Tooling/RefactoringCallbacks.cpp
===
--- lib/Tooling/RefactoringCallbacks.cpp
+++ lib/Tooling/RefactoringCallbacks.cpp
@@ -213,8 +213,8 @@
<< " used in replacement template not bound in Matcher \n";
llvm::report_fatal_error("Unbound node in replacement template.");
}
- CharSourceRange Source =
- CharSourceRange::getTokenRange(NodeIter->second.getSourceRange());
+ CharSourceRange Source = CharSourceRange::getTokenRange(
+ NodeIter->second.first.getSourceRange());
ToText += Lexer::getSourceText(Source, *Result.SourceManager,
Result.Context->getLangOpts());
break;
@@ -227,8 +227,8 @@
llvm::report_fatal_error("FromId node not bound in MatchResult");
}
auto Replacement =
- tooling::Replacement(*Result.SourceManager, (FromId), ToText,
- Result.Context->getLangOpts());
+ tooling::Replacement(*Result.SourceManager, (FromId).first,
+ ToText, Result.Context->getLangOpts());
llvm::Error Err = Replace.add(Replacement);
if (Err) {
llvm::errs() << "Query and replace failed in " << Replacement.getFilePath()
Index: lib/ASTMatchers/ASTMatchersInternal.cpp
===
--- lib/ASTMatchers/ASTMatchersInternal.cpp
+++ lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -52,21 +52,25 @@
bool NotUnaryOperator(const ast_type_traits::DynTypedNode ,
ASTMatchFinder *Finder, BoundNodesTreeBuilder *Builder,
+ ast_type_traits::ASTNodeKind NodeKind,
ArrayRef InnerMatchers);
bool AllOfVariadicOperator(const ast_type_traits::DynTypedNode ,
ASTMatchFinder *Finder,
BoundNodesTreeBuilder *Builder,
+
[PATCH] D54405: Record whether a AST Matcher constructs a Node
steveire created this revision.
steveire added a reviewer: aaron.ballman.
Herald added a subscriber: cfe-commits.
These matchers are bindable. Recording this information will make it
possible to introspect the matchers which can be used inside another
matcher.
Repository:
rC Clang
https://reviews.llvm.org/D54405
Files:
include/clang/ASTMatchers/ASTMatchersInternal.h
lib/ASTMatchers/Dynamic/Registry.cpp
Index: lib/ASTMatchers/Dynamic/Registry.cpp
===
--- lib/ASTMatchers/Dynamic/Registry.cpp
+++ lib/ASTMatchers/Dynamic/Registry.cpp
@@ -44,18 +44,56 @@
using ConstructorMap = llvm::StringMap>;
+using NodeConstructorMap =
+std::map>;
+
class RegistryMaps {
public:
RegistryMaps();
~RegistryMaps();
const ConstructorMap () const { return Constructors; }
+ const NodeConstructorMap () const {
+return NodeConstructors;
+ }
+
+ void registerNodeMatcher(ast_type_traits::ASTNodeKind kind,
+ internal::MatcherDescriptor *Callback);
private:
void registerMatcher(StringRef MatcherName,
std::unique_ptr Callback);
+ template
+ void registerIfNodeMatcher(MatcherType,
+ internal::MatcherDescriptor *matchDescriptor,
+ StringRef) {}
+
+ template
+ void registerIfNodeMatcher(
+ ast_matchers::internal::VariadicAllOfMatcher VarFunc,
+ internal::MatcherDescriptor *matchDescriptor, StringRef MatcherName) {
+auto K = ast_type_traits::ASTNodeKind::getFromNodeKind<
+typename
ast_matchers::internal::VariadicAllOfMatcher::Type>();
+if (!K.isNone()) {
+ NodeConstructors[K] = std::make_pair(MatcherName, matchDescriptor);
+}
+ }
+
+ template
+ void registerIfNodeMatcher(
+ ast_matchers::internal::VariadicDynCastAllOfMatcher
+ VarFunc,
+ internal::MatcherDescriptor *matchDescriptor, StringRef MatcherName) {
+auto K = ast_type_traits::ASTNodeKind::getFromNodeKind();
+if (!K.isNone()) {
+ NodeConstructors[K] = std::make_pair(MatcherName, matchDescriptor);
+}
+ }
+
ConstructorMap Constructors;
+ NodeConstructorMap NodeConstructors;
};
} // namespace
@@ -67,8 +105,13 @@
}
#define REGISTER_MATCHER(name)
\
- registerMatcher(#name, internal::makeMatcherAutoMarshall(
\
- ::clang::ast_matchers::name, #name));
+ {
\
+auto matcherDescriptor =
\
+internal::makeMatcherAutoMarshall(::clang::ast_matchers::name, #name);
\
+registerIfNodeMatcher(::clang::ast_matchers::name,
\
+ matcherDescriptor.get(), #name);
\
+registerMatcher(#name, std::move(matcherDescriptor));
\
+ }
#define REGISTER_MATCHER_OVERLOAD(name)
\
registerMatcher(#name,
\
Index: include/clang/ASTMatchers/ASTMatchersInternal.h
===
--- include/clang/ASTMatchers/ASTMatchersInternal.h
+++ include/clang/ASTMatchers/ASTMatchersInternal.h
@@ -1577,6 +1577,7 @@
: public VariadicFunction, Matcher,
makeAllOfComposite> {
public:
+ using Type = T;
VariadicAllOfMatcher() {}
};
Index: lib/ASTMatchers/Dynamic/Registry.cpp
===
--- lib/ASTMatchers/Dynamic/Registry.cpp
+++ lib/ASTMatchers/Dynamic/Registry.cpp
@@ -44,18 +44,56 @@
using ConstructorMap = llvm::StringMap>;
+using NodeConstructorMap =
+std::map>;
+
class RegistryMaps {
public:
RegistryMaps();
~RegistryMaps();
const ConstructorMap () const { return Constructors; }
+ const NodeConstructorMap () const {
+return NodeConstructors;
+ }
+
+ void registerNodeMatcher(ast_type_traits::ASTNodeKind kind,
+ internal::MatcherDescriptor *Callback);
private:
void registerMatcher(StringRef MatcherName,
std::unique_ptr Callback);
+ template
+ void registerIfNodeMatcher(MatcherType,
+ internal::MatcherDescriptor *matchDescriptor,
+ StringRef) {}
+
+ template
+ void registerIfNodeMatcher(
+ ast_matchers::internal::VariadicAllOfMatcher VarFunc,
+ internal::MatcherDescriptor *matchDescriptor, StringRef MatcherName) {
+auto K = ast_type_traits::ASTNodeKind::getFromNodeKind<
+typename ast_matchers::internal::VariadicAllOfMatcher::Type>();
+if (!K.isNone()) {
+ NodeConstructors[K] = std::make_pair(MatcherName, matchDescriptor);
+}
+ }
+
+ template
+ void registerIfNodeMatcher(
[PATCH] D54404: Exclude matchers which can have multiple results
steveire created this revision.
steveire added a reviewer: aaron.ballman.
Herald added a subscriber: cfe-commits.
Repository:
rC Clang
https://reviews.llvm.org/D54404
Files:
lib/ASTMatchers/Dynamic/Registry.cpp
unittests/ASTMatchers/Dynamic/RegistryTest.cpp
Index: unittests/ASTMatchers/Dynamic/RegistryTest.cpp
===
--- unittests/ASTMatchers/Dynamic/RegistryTest.cpp
+++ unittests/ASTMatchers/Dynamic/RegistryTest.cpp
@@ -579,6 +579,8 @@
EXPECT_TRUE(!Contains(Matchers, "functionDecl()"));
EXPECT_TRUE(Contains(Matchers, "cxxMethodDecl()"));
+
+ EXPECT_TRUE(!Contains(Matchers, "has()"));
}
} // end anonymous namespace
Index: lib/ASTMatchers/Dynamic/Registry.cpp
===
--- lib/ASTMatchers/Dynamic/Registry.cpp
+++ lib/ASTMatchers/Dynamic/Registry.cpp
@@ -601,15 +601,54 @@
Registry::getMatchingMatchers(ast_type_traits::ASTNodeKind StaticType) {
std::vector Result;
+ static std::vector excludedMatchers{
+ "allOf",
+ "anyOf",
+ "anything",
+ "containsDeclaration",
+ "eachOf",
+ "equalsNode",
+ "findAll",
+ "forEach",
+ "forEachConstructorInitializer",
+ "forEachDescendant",
+ "forEachOverridden",
+ "forEachSwitchCase",
+ "has",
+ "hasAncestor",
+ "hasAnyArgument",
+ "hasAnyConstructorInitializer",
+ "hasAnyDeclaration",
+ "hasAnyName",
+ "hasAnyParameter",
+ "hasAnySelector",
+ "hasAnySubstatement",
+ "hasAnyTemplateArgument",
+ "hasAnyUsingShadowDecl",
+ "hasArgumentOfType",
+ "hasDescendant",
+ "hasEitherOperand",
+ "hasMethod",
+ "hasParent",
+ "isExpansionInFileMatching",
+ "isSameOrDerivedFrom",
+ "matchesName",
+ "matchesSelector",
+ "unless"};
+ assert(std::is_sorted(excludedMatchers.begin(), excludedMatchers.end()));
+
std::vector AcceptedTypes;
AcceptedTypes.push_back(StaticType);
processAcceptableMatchers(
AcceptedTypes, [](StringRef Name, const MatcherDescriptor
,
std::set ,
std::vector> ArgsKinds,
unsigned MaxSpecificity) {
-Result.emplace_back((Name + "()").str());
+if (!std::binary_search(excludedMatchers.begin(),
+excludedMatchers.end(), Name)) {
+ Result.emplace_back((Name + "()").str());
+}
});
return Result;
Index: unittests/ASTMatchers/Dynamic/RegistryTest.cpp
===
--- unittests/ASTMatchers/Dynamic/RegistryTest.cpp
+++ unittests/ASTMatchers/Dynamic/RegistryTest.cpp
@@ -579,6 +579,8 @@
EXPECT_TRUE(!Contains(Matchers, "functionDecl()"));
EXPECT_TRUE(Contains(Matchers, "cxxMethodDecl()"));
+
+ EXPECT_TRUE(!Contains(Matchers, "has()"));
}
} // end anonymous namespace
Index: lib/ASTMatchers/Dynamic/Registry.cpp
===
--- lib/ASTMatchers/Dynamic/Registry.cpp
+++ lib/ASTMatchers/Dynamic/Registry.cpp
@@ -601,15 +601,54 @@
Registry::getMatchingMatchers(ast_type_traits::ASTNodeKind StaticType) {
std::vector Result;
+ static std::vector excludedMatchers{
+ "allOf",
+ "anyOf",
+ "anything",
+ "containsDeclaration",
+ "eachOf",
+ "equalsNode",
+ "findAll",
+ "forEach",
+ "forEachConstructorInitializer",
+ "forEachDescendant",
+ "forEachOverridden",
+ "forEachSwitchCase",
+ "has",
+ "hasAncestor",
+ "hasAnyArgument",
+ "hasAnyConstructorInitializer",
+ "hasAnyDeclaration",
+ "hasAnyName",
+ "hasAnyParameter",
+ "hasAnySelector",
+ "hasAnySubstatement",
+ "hasAnyTemplateArgument",
+ "hasAnyUsingShadowDecl",
+ "hasArgumentOfType",
+ "hasDescendant",
+ "hasEitherOperand",
+ "hasMethod",
+ "hasParent",
+ "isExpansionInFileMatching",
+ "isSameOrDerivedFrom",
+ "matchesName",
+ "matchesSelector",
+ "unless"};
+ assert(std::is_sorted(excludedMatchers.begin(), excludedMatchers.end()));
+
std::vector AcceptedTypes;
AcceptedTypes.push_back(StaticType);
processAcceptableMatchers(
AcceptedTypes, [](StringRef Name, const MatcherDescriptor ,
std::set ,
std::vector> ArgsKinds,
unsigned MaxSpecificity) {
-Result.emplace_back((Name + "()").str());
+if (!std::binary_search(excludedMatchers.begin(),
+excludedMatchers.end(), Name)) {
+ Result.emplace_back((Name + "()").str());
+}
});
return Result;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
[PATCH] D54403: Add new API for returning matching matchers
steveire created this revision.
steveire added a reviewer: aaron.ballman.
Herald added a subscriber: cfe-commits.
The MatchingMatcher struct currently contains only a string, but it will
be expanded in the future.
Repository:
rC Clang
https://reviews.llvm.org/D54403
Files:
include/clang/ASTMatchers/Dynamic/Registry.h
lib/ASTMatchers/Dynamic/Registry.cpp
unittests/ASTMatchers/Dynamic/RegistryTest.cpp
Index: unittests/ASTMatchers/Dynamic/RegistryTest.cpp
===
--- unittests/ASTMatchers/Dynamic/RegistryTest.cpp
+++ unittests/ASTMatchers/Dynamic/RegistryTest.cpp
@@ -555,6 +555,32 @@
EXPECT_FALSE(matches("int x = 120;", CharStmt));
}
+TEST_F(RegistryTest, MatchingMatchers) {
+
+ auto Matchers = Registry::getMatchingMatchers(
+ ast_type_traits::ASTNodeKind::getFromNodeKind());
+
+ auto Contains = [](std::vector const , StringRef Name) {
+return std::find_if(C.begin(), C.end(), [Name](const MatchingMatcher ) {
+ return M.MatcherString == Name;
+ }) != C.end();
+ };
+
+ EXPECT_TRUE(Contains(Matchers, "isPublic()"));
+ EXPECT_TRUE(Contains(Matchers, "hasName()"));
+ EXPECT_TRUE(Contains(Matchers, "hasType()"));
+ EXPECT_TRUE(Contains(Matchers, "hasTypeLoc()"));
+ EXPECT_TRUE(Contains(Matchers, "parameterCountIs()"));
+
+ EXPECT_TRUE(!Contains(Matchers, "decl()"));
+ EXPECT_TRUE(!Contains(Matchers, "namedDecl()"));
+ EXPECT_TRUE(!Contains(Matchers, "valueDecl()"));
+ EXPECT_TRUE(!Contains(Matchers, "declaratorDecl()"));
+ EXPECT_TRUE(!Contains(Matchers, "functionDecl()"));
+
+ EXPECT_TRUE(Contains(Matchers, "cxxMethodDecl()"));
+}
+
} // end anonymous namespace
} // end namespace dynamic
} // end namespace ast_matchers
Index: lib/ASTMatchers/Dynamic/Registry.cpp
===
--- lib/ASTMatchers/Dynamic/Registry.cpp
+++ lib/ASTMatchers/Dynamic/Registry.cpp
@@ -597,6 +597,24 @@
}
}
+std::vector
+Registry::getMatchingMatchers(ast_type_traits::ASTNodeKind StaticType) {
+ std::vector Result;
+
+ std::vector AcceptedTypes;
+ AcceptedTypes.push_back(StaticType);
+
+ processAcceptableMatchers(
+ AcceptedTypes, [](StringRef Name, const MatcherDescriptor
,
+ std::set ,
+ std::vector> ArgsKinds,
+ unsigned MaxSpecificity) {
+Result.emplace_back((Name + "()").str());
+ });
+
+ return Result;
+}
+
std::vector
Registry::getMatcherCompletions(ArrayRef AcceptedTypes) {
std::vector Completions;
Index: include/clang/ASTMatchers/Dynamic/Registry.h
===
--- include/clang/ASTMatchers/Dynamic/Registry.h
+++ include/clang/ASTMatchers/Dynamic/Registry.h
@@ -63,6 +63,12 @@
unsigned Specificity;
};
+struct MatchingMatcher {
+ MatchingMatcher(std::string MatcherString)
+ : MatcherString(std::move(MatcherString)) {}
+ std::string MatcherString;
+};
+
class Registry {
public:
Registry() = delete;
@@ -96,6 +102,11 @@
static std::vector
getMatcherCompletions(ArrayRef AcceptedTypes);
+ /// Compute matchers which can be used within a matcher of
+ /// type @p StaticType.
+ static std::vector
+ getMatchingMatchers(ast_type_traits::ASTNodeKind StaticType);
+
/// Construct a matcher from the registry.
///
/// \param Ctor The matcher constructor to instantiate.
Index: unittests/ASTMatchers/Dynamic/RegistryTest.cpp
===
--- unittests/ASTMatchers/Dynamic/RegistryTest.cpp
+++ unittests/ASTMatchers/Dynamic/RegistryTest.cpp
@@ -555,6 +555,32 @@
EXPECT_FALSE(matches("int x = 120;", CharStmt));
}
+TEST_F(RegistryTest, MatchingMatchers) {
+
+ auto Matchers = Registry::getMatchingMatchers(
+ ast_type_traits::ASTNodeKind::getFromNodeKind());
+
+ auto Contains = [](std::vector const , StringRef Name) {
+return std::find_if(C.begin(), C.end(), [Name](const MatchingMatcher ) {
+ return M.MatcherString == Name;
+ }) != C.end();
+ };
+
+ EXPECT_TRUE(Contains(Matchers, "isPublic()"));
+ EXPECT_TRUE(Contains(Matchers, "hasName()"));
+ EXPECT_TRUE(Contains(Matchers, "hasType()"));
+ EXPECT_TRUE(Contains(Matchers, "hasTypeLoc()"));
+ EXPECT_TRUE(Contains(Matchers, "parameterCountIs()"));
+
+ EXPECT_TRUE(!Contains(Matchers, "decl()"));
+ EXPECT_TRUE(!Contains(Matchers, "namedDecl()"));
+ EXPECT_TRUE(!Contains(Matchers, "valueDecl()"));
+ EXPECT_TRUE(!Contains(Matchers, "declaratorDecl()"));
+ EXPECT_TRUE(!Contains(Matchers, "functionDecl()"));
+
+ EXPECT_TRUE(Contains(Matchers, "cxxMethodDecl()"));
+}
+
} // end anonymous namespace
} // end namespace dynamic
} // end namespace ast_matchers
Index: lib/ASTMatchers/Dynamic/Registry.cpp
===
--- lib/ASTMatchers/Dynamic/Registry.cpp
+++
[PATCH] D54402: Extract method to allow re-use
steveire created this revision.
steveire added a reviewer: aaron.ballman.
Herald added a subscriber: cfe-commits.
The list of matchers which can be used with a top matcher can be used in
other contexts than code completion. It can be output as data in
clang-tidy.
Repository:
rC Clang
https://reviews.llvm.org/D54402
Files:
lib/ASTMatchers/Dynamic/Registry.cpp
Index: lib/ASTMatchers/Dynamic/Registry.cpp
===
--- lib/ASTMatchers/Dynamic/Registry.cpp
+++ lib/ASTMatchers/Dynamic/Registry.cpp
@@ -561,9 +561,8 @@
return std::vector(TypeSet.begin(), TypeSet.end());
}
-std::vector
-Registry::getMatcherCompletions(ArrayRef AcceptedTypes) {
- std::vector Completions;
+template
+void processAcceptableMatchers(ArrayRef AcceptedTypes, T func) {
// Search the registry for acceptable matchers.
for (const auto : RegistryData->constructors()) {
@@ -593,49 +592,65 @@
}
if (!RetKinds.empty() && MaxSpecificity > 0) {
- std::string Decl;
- llvm::raw_string_ostream OS(Decl);
-
- if (IsPolymorphic) {
-OS << "Matcher " << Name << "(Matcher";
- } else {
-OS << "Matcher<" << RetKinds << "> " << Name << "(";
-for (const std::vector : ArgsKinds) {
- if ( != [0])
-OS << ", ";
-
- bool FirstArgKind = true;
- std::set MatcherKinds;
- // Two steps. First all non-matchers, then matchers only.
- for (const ArgKind : Arg) {
-if (AK.getArgKind() == ArgKind::AK_Matcher) {
- MatcherKinds.insert(AK.getMatcherKind());
-} else {
- if (!FirstArgKind) OS << "|";
- FirstArgKind = false;
- OS << AK.asString();
+ func(Name, Matcher, RetKinds, ArgsKinds, MaxSpecificity);
+}
+ }
+}
+
+std::vector
+Registry::getMatcherCompletions(ArrayRef AcceptedTypes) {
+ std::vector Completions;
+
+ processAcceptableMatchers(
+ AcceptedTypes,
+ [](StringRef Name, const MatcherDescriptor ,
+ std::set ,
+ std::vector> ArgsKinds,
+ unsigned MaxSpecificity) {
+std::string Decl;
+llvm::raw_string_ostream OS(Decl);
+
+if (Matcher.isPolymorphic()) {
+ OS << "Matcher " << Name << "(Matcher";
+} else {
+ OS << "Matcher<" << RetKinds << "> " << Name << "(";
+ for (const std::vector : ArgsKinds) {
+if ( != [0])
+ OS << ", ";
+
+bool FirstArgKind = true;
+std::set MatcherKinds;
+// Two steps. First all non-matchers, then matchers only.
+for (const ArgKind : Arg) {
+ if (AK.getArgKind() == ArgKind::AK_Matcher) {
+MatcherKinds.insert(AK.getMatcherKind());
+ } else {
+if (!FirstArgKind)
+ OS << "|";
+FirstArgKind = false;
+OS << AK.asString();
+ }
+}
+if (!MatcherKinds.empty()) {
+ if (!FirstArgKind)
+OS << "|";
+ OS << "Matcher<" << MatcherKinds << ">";
}
- }
- if (!MatcherKinds.empty()) {
-if (!FirstArgKind) OS << "|";
-OS << "Matcher<" << MatcherKinds << ">";
}
}
- }
- if (Matcher.isVariadic())
-OS << "...";
- OS << ")";
-
- std::string TypedText = Name;
- TypedText += "(";
- if (ArgsKinds.empty())
-TypedText += ")";
- else if (ArgsKinds[0][0].getArgKind() == ArgKind::AK_String)
-TypedText += "\"";
-
- Completions.emplace_back(TypedText, OS.str(), MaxSpecificity);
-}
- }
+if (Matcher.isVariadic())
+ OS << "...";
+OS << ")";
+
+std::string TypedText = Name;
+TypedText += "(";
+if (ArgsKinds.empty())
+ TypedText += ")";
+else if (ArgsKinds[0][0].getArgKind() == ArgKind::AK_String)
+ TypedText += "\"";
+
+Completions.emplace_back(TypedText, OS.str(), MaxSpecificity);
+ });
return Completions;
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D54399: Move ExprMutationAnalyzer to Tooling/Analysis (1/3)
JonasToth added a comment. Will you update the existing clang-tidy checks as well? Repository: rC Clang https://reviews.llvm.org/D54399 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D54401: [analyzer] Prefer returns values to out-params in CheckerRegistry.cpp
Szelethus updated this revision to Diff 173584.
Szelethus added a comment.
Also, remove `diags::note_suggest_disabling_all_checkers`.
https://reviews.llvm.org/D54401
Files:
include/clang/Basic/DiagnosticFrontendKinds.td
include/clang/StaticAnalyzer/Core/CheckerRegistry.h
include/clang/StaticAnalyzer/Frontend/FrontendActions.h
lib/FrontendTool/ExecuteCompilerInvocation.cpp
lib/StaticAnalyzer/Core/CheckerRegistry.cpp
lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp
test/Analysis/disable-all-checks.c
Index: test/Analysis/disable-all-checks.c
===
--- test/Analysis/disable-all-checks.c
+++ test/Analysis/disable-all-checks.c
@@ -1,10 +1,18 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-store=region -analyzer-disable-all-checks -verify %s
-// RUN: %clang_analyze_cc1 -analyzer-disable-all-checks -analyzer-checker=core -analyzer-store=region -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-store=region \
+// RUN: -analyzer-disable-all-checks -verify %s
+//
+// RUN: %clang_analyze_cc1 -analyzer-disable-all-checks -analyzer-checker=core \
+// RUN: -analyzer-store=region -verify %s
+//
// RUN: %clang_analyze_cc1 -analyzer-disable-all-checks -verify %s
-// RUN: not %clang_analyze_cc1 -analyzer-checker=core -analyzer-store=region -analyzer-disable-checker -verify %s 2>&1 | FileCheck %s
+//
+// RUN: not %clang_analyze_cc1 -analyzer-checker=core -analyzer-store=region \
+// RUN: -analyzer-disable-checker non.existant.Checker -verify %s 2>&1 \
+// RUN: | FileCheck %s
+//
// expected-no-diagnostics
-// CHECK: use -analyzer-disable-all-checks to disable all static analyzer checkers
+// CHECK: no analyzer checkers are associated with 'non.existant.Checker'
int buggy() {
int x = 0;
return 5/x; // no warning
Index: lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp
===
--- lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp
+++ lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp
@@ -130,10 +130,11 @@
void ento::printEnabledCheckerList(raw_ostream ,
ArrayRef plugins,
- const AnalyzerOptions ) {
+ const AnalyzerOptions ,
+ DiagnosticsEngine ) {
out << "OVERVIEW: Clang Static Analyzer Enabled Checkers List\n\n";
- ClangCheckerRegistry(plugins).printList(out, opts);
+ ClangCheckerRegistry(plugins).printList(out, opts, diags);
}
void ento::printAnalyzerConfigList(raw_ostream ) {
Index: lib/StaticAnalyzer/Core/CheckerRegistry.cpp
===
--- lib/StaticAnalyzer/Core/CheckerRegistry.cpp
+++ lib/StaticAnalyzer/Core/CheckerRegistry.cpp
@@ -9,7 +9,6 @@
#include "clang/StaticAnalyzer/Core/CheckerRegistry.h"
#include "clang/Basic/Diagnostic.h"
-#include "clang/Basic/LLVM.h"
#include "clang/Frontend/FrontendDiagnostic.h"
#include "clang/StaticAnalyzer/Core/CheckerManager.h"
#include "clang/StaticAnalyzer/Core/AnalyzerOptions.h"
@@ -19,51 +18,14 @@
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/raw_ostream.h"
#include
-#include
-#include
using namespace clang;
using namespace ento;
-static const char PackageSeparator = '.';
+static constexpr char PackageSeparator = '.';
using CheckerInfoSet = llvm::SetVector;
-namespace {
-/// Represents a request to include or exclude a checker or package from a
-/// specific analysis run.
-///
-/// \sa CheckerRegistry::initializeManager
-class CheckerOptInfo {
- StringRef Name;
- bool Enable;
- bool Claimed;
-
-public:
- CheckerOptInfo(StringRef name, bool enable)
-: Name(name), Enable(enable), Claimed(false) { }
-
- StringRef getName() const { return Name; }
- bool isEnabled() const { return Enable; }
- bool isDisabled() const { return !isEnabled(); }
-
- bool isClaimed() const { return Claimed; }
- bool isUnclaimed() const { return !isClaimed(); }
- void claim() { Claimed = true; }
-};
-
-} // end of anonymous namespace
-
-static SmallVector
-getCheckerOptList(const AnalyzerOptions ) {
- SmallVector checkerOpts;
- for (unsigned i = 0, e = opts.CheckersControlList.size(); i != e; ++i) {
-const std::pair = opts.CheckersControlList[i];
-checkerOpts.push_back(CheckerOptInfo(opt.first, opt.second));
- }
- return checkerOpts;
-}
-
static bool checkerNameLT(const CheckerRegistry::CheckerInfo ,
const CheckerRegistry::CheckerInfo ) {
return a.FullName < b.FullName;
@@ -86,40 +48,50 @@
return false;
}
-/// Collects the checkers for the supplied \p opt option into \p collected.
-static void collectCheckers(const CheckerRegistry::CheckerInfoList ,
-const llvm::StringMap ,
-CheckerOptInfo , CheckerInfoSet ) {
- // Use a binary search to find the
[PATCH] D54401: [analyzer] Prefer returns values to out-params in CheckerRegistry.cpp
Szelethus created this revision.
Szelethus added reviewers: NoQ, george.karpenkov, xazax.hun, MTC.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, mikhail.ramalho,
a.sidorin, rnkovacs, szepet, whisperity.
This bugged me for a long time, so it's time to put an end to it:
`collectCheckers` was cryptic and hard to understand. This is done by
- Renaming `collectCheckers` to `getEnabledCheckers`
- Changing the functionality to acquire //all// enabled checkers, rather then
collect checkers for a specific `CheckerOptInfo` (for example, collecting all
checkers for `{ "core", true }`, which meant enabling all checkers from the
core package, which was an unnecessary complication).
- Removing `CheckerOptInfo`, instead of storing whether the option was claimed
via a field, we handle errors immediately, as `getEnabledCheckers` can now
access a `DiagnosticsEngine`. Realize that the remaining information it stored
is directly accessible through `AnalyzerOptions.CheckerControlList`.
- Removing the suggestion to disable all checkers when a checker option is left
unclaimed. I personally found this super annoying, and I'd be very surprised if
this was a welcome suggestion to anyone.
- Fix a test with `-analyzer-disable-checker -verify` accidentally left in.
Repository:
rC Clang
https://reviews.llvm.org/D54401
Files:
include/clang/StaticAnalyzer/Core/CheckerRegistry.h
include/clang/StaticAnalyzer/Frontend/FrontendActions.h
lib/FrontendTool/ExecuteCompilerInvocation.cpp
lib/StaticAnalyzer/Core/CheckerRegistry.cpp
lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp
test/Analysis/disable-all-checks.c
Index: test/Analysis/disable-all-checks.c
===
--- test/Analysis/disable-all-checks.c
+++ test/Analysis/disable-all-checks.c
@@ -1,10 +1,18 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-store=region -analyzer-disable-all-checks -verify %s
-// RUN: %clang_analyze_cc1 -analyzer-disable-all-checks -analyzer-checker=core -analyzer-store=region -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-store=region \
+// RUN: -analyzer-disable-all-checks -verify %s
+//
+// RUN: %clang_analyze_cc1 -analyzer-disable-all-checks -analyzer-checker=core \
+// RUN: -analyzer-store=region -verify %s
+//
// RUN: %clang_analyze_cc1 -analyzer-disable-all-checks -verify %s
-// RUN: not %clang_analyze_cc1 -analyzer-checker=core -analyzer-store=region -analyzer-disable-checker -verify %s 2>&1 | FileCheck %s
+//
+// RUN: not %clang_analyze_cc1 -analyzer-checker=core -analyzer-store=region \
+// RUN: -analyzer-disable-checker non.existant.Checker -verify %s 2>&1 \
+// RUN: | FileCheck %s
+//
// expected-no-diagnostics
-// CHECK: use -analyzer-disable-all-checks to disable all static analyzer checkers
+// CHECK: no analyzer checkers are associated with 'non.existant.Checker'
int buggy() {
int x = 0;
return 5/x; // no warning
Index: lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp
===
--- lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp
+++ lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp
@@ -130,10 +130,11 @@
void ento::printEnabledCheckerList(raw_ostream ,
ArrayRef plugins,
- const AnalyzerOptions ) {
+ const AnalyzerOptions ,
+ DiagnosticsEngine ) {
out << "OVERVIEW: Clang Static Analyzer Enabled Checkers List\n\n";
- ClangCheckerRegistry(plugins).printList(out, opts);
+ ClangCheckerRegistry(plugins).printList(out, opts, diags);
}
void ento::printAnalyzerConfigList(raw_ostream ) {
Index: lib/StaticAnalyzer/Core/CheckerRegistry.cpp
===
--- lib/StaticAnalyzer/Core/CheckerRegistry.cpp
+++ lib/StaticAnalyzer/Core/CheckerRegistry.cpp
@@ -9,7 +9,6 @@
#include "clang/StaticAnalyzer/Core/CheckerRegistry.h"
#include "clang/Basic/Diagnostic.h"
-#include "clang/Basic/LLVM.h"
#include "clang/Frontend/FrontendDiagnostic.h"
#include "clang/StaticAnalyzer/Core/CheckerManager.h"
#include "clang/StaticAnalyzer/Core/AnalyzerOptions.h"
@@ -19,51 +18,14 @@
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/raw_ostream.h"
#include
-#include
-#include
using namespace clang;
using namespace ento;
-static const char PackageSeparator = '.';
+static constexpr char PackageSeparator = '.';
using CheckerInfoSet = llvm::SetVector;
-namespace {
-/// Represents a request to include or exclude a checker or package from a
-/// specific analysis run.
-///
-/// \sa CheckerRegistry::initializeManager
-class CheckerOptInfo {
- StringRef Name;
- bool Enable;
- bool Claimed;
-
-public:
- CheckerOptInfo(StringRef name, bool enable)
-: Name(name), Enable(enable), Claimed(false) { }
-
- StringRef getName() const { return
[PATCH] D54400: Move ExprMutationAnalyzer to Tooling/Analysis (2/3)
shuaiwang created this revision.
shuaiwang added reviewers: george.karpenkov, rsmith, dblaikie.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus,
a.sidorin, mgorny.
shuaiwang added a dependency: D54399: Move ExprMutationAnalyzer to
Tooling/Analysis (1/3).
Reference the new location of ExprMutationAnalyzer.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D54400
Files:
clang-tidy/performance/CMakeLists.txt
clang-tidy/performance/ForRangeCopyCheck.cpp
clang-tidy/performance/UnnecessaryValueParamCheck.cpp
clang-tidy/performance/UnnecessaryValueParamCheck.h
Index: clang-tidy/performance/UnnecessaryValueParamCheck.h
===
--- clang-tidy/performance/UnnecessaryValueParamCheck.h
+++ clang-tidy/performance/UnnecessaryValueParamCheck.h
@@ -12,7 +12,7 @@
#include "../ClangTidy.h"
#include "../utils/IncludeInserter.h"
-#include "clang/Analysis/Analyses/ExprMutationAnalyzer.h"
+#include "clang/Tooling/Analysis/ExprMutationAnalyzer.h"
namespace clang {
namespace tidy {
@@ -36,7 +36,7 @@
void handleMoveFix(const ParmVarDecl , const DeclRefExpr ,
const ASTContext );
- llvm::DenseMap
+ llvm::DenseMap
MutationAnalyzers;
std::unique_ptr Inserter;
const utils::IncludeSorter::IncludeStyle IncludeStyle;
Index: clang-tidy/performance/UnnecessaryValueParamCheck.cpp
===
--- clang-tidy/performance/UnnecessaryValueParamCheck.cpp
+++ clang-tidy/performance/UnnecessaryValueParamCheck.cpp
@@ -97,7 +97,7 @@
const auto *Param = Result.Nodes.getNodeAs("param");
const auto *Function = Result.Nodes.getNodeAs("functionDecl");
- FunctionParmMutationAnalyzer =
+ tooling::FunctionParmMutationAnalyzer =
MutationAnalyzers.try_emplace(Function, *Function, *Result.Context)
.first->second;
if (Analyzer.isMutated(Param))
Index: clang-tidy/performance/ForRangeCopyCheck.cpp
===
--- clang-tidy/performance/ForRangeCopyCheck.cpp
+++ clang-tidy/performance/ForRangeCopyCheck.cpp
@@ -13,7 +13,7 @@
#include "../utils/Matchers.h"
#include "../utils/OptionsUtils.h"
#include "../utils/TypeTraits.h"
-#include "clang/Analysis/Analyses/ExprMutationAnalyzer.h"
+#include "clang/Tooling/Analysis/ExprMutationAnalyzer.h"
using namespace clang::ast_matchers;
@@ -98,7 +98,8 @@
// Because the fix (changing to `const auto &`) will introduce an unused
// compiler warning which can't be suppressed.
// Since this case is very rare, it is safe to ignore it.
- if (!ExprMutationAnalyzer(*ForRange.getBody(), Context).isMutated()
&&
+ if (!tooling::ExprMutationAnalyzer(*ForRange.getBody(), Context)
+ .isMutated() &&
!utils::decl_ref_expr::allDeclRefExprs(LoopVar, *ForRange.getBody(),
Context)
.empty()) {
Index: clang-tidy/performance/CMakeLists.txt
===
--- clang-tidy/performance/CMakeLists.txt
+++ clang-tidy/performance/CMakeLists.txt
@@ -18,9 +18,9 @@
LINK_LIBS
clangAST
clangASTMatchers
- clangAnalysis
clangBasic
clangLex
clangTidy
clangTidyUtils
+ clangToolingAnalysis
)
Index: clang-tidy/performance/UnnecessaryValueParamCheck.h
===
--- clang-tidy/performance/UnnecessaryValueParamCheck.h
+++ clang-tidy/performance/UnnecessaryValueParamCheck.h
@@ -12,7 +12,7 @@
#include "../ClangTidy.h"
#include "../utils/IncludeInserter.h"
-#include "clang/Analysis/Analyses/ExprMutationAnalyzer.h"
+#include "clang/Tooling/Analysis/ExprMutationAnalyzer.h"
namespace clang {
namespace tidy {
@@ -36,7 +36,7 @@
void handleMoveFix(const ParmVarDecl , const DeclRefExpr ,
const ASTContext );
- llvm::DenseMap
+ llvm::DenseMap
MutationAnalyzers;
std::unique_ptr Inserter;
const utils::IncludeSorter::IncludeStyle IncludeStyle;
Index: clang-tidy/performance/UnnecessaryValueParamCheck.cpp
===
--- clang-tidy/performance/UnnecessaryValueParamCheck.cpp
+++ clang-tidy/performance/UnnecessaryValueParamCheck.cpp
@@ -97,7 +97,7 @@
const auto *Param = Result.Nodes.getNodeAs("param");
const auto *Function = Result.Nodes.getNodeAs("functionDecl");
- FunctionParmMutationAnalyzer =
+ tooling::FunctionParmMutationAnalyzer =
MutationAnalyzers.try_emplace(Function, *Function, *Result.Context)
.first->second;
if (Analyzer.isMutated(Param))
Index: clang-tidy/performance/ForRangeCopyCheck.cpp
===
--- clang-tidy/performance/ForRangeCopyCheck.cpp
+++ clang-tidy/performance/ForRangeCopyCheck.cpp
@@ -13,7 +13,7 @@
#include "../utils/Matchers.h"
#include
[PATCH] D54399: Move ExprMutationAnalyzer to Tooling/Analysis (1/3)
shuaiwang created this revision.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus,
a.sidorin, mgorny.
Herald added a reviewer: george.karpenkov.
This just copies ExprMutationAnalyzer to Tooling/Analysis with minor tweaks
around including path & namespaces.
2/3 will change existing references to old location to new location.
3/3 will delete the old copy.
Repository:
rC Clang
https://reviews.llvm.org/D54399
Files:
include/clang/Tooling/Analysis/ExprMutationAnalyzer.h
lib/Tooling/Analysis/CMakeLists.txt
lib/Tooling/Analysis/ExprMutationAnalyzer.cpp
lib/Tooling/CMakeLists.txt
unittests/Tooling/CMakeLists.txt
unittests/Tooling/ExprMutationAnalyzerTest.cpp
Index: unittests/Tooling/ExprMutationAnalyzerTest.cpp
===
--- /dev/null
+++ unittests/Tooling/ExprMutationAnalyzerTest.cpp
@@ -0,0 +1,1107 @@
+//===-- ExprMutationAnalyzerTest.cpp --===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "clang/Tooling/Analysis/ExprMutationAnalyzer.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Tooling/Tooling.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include
+
+namespace clang {
+namespace {
+
+using namespace ::clang::ast_matchers;
+using ::testing::ElementsAre;
+using ::testing::IsEmpty;
+using ::testing::ResultOf;
+using ::testing::StartsWith;
+using ::testing::Values;
+
+using ExprMatcher = internal::Matcher;
+using StmtMatcher = internal::Matcher;
+
+std::unique_ptr
+buildASTFromCodeWithArgs(const Twine ,
+ const std::vector ) {
+ auto AST = tooling::buildASTFromCodeWithArgs(Code, Args);
+ EXPECT_FALSE(AST->getDiagnostics().hasErrorOccurred());
+ return AST;
+}
+
+std::unique_ptr buildASTFromCode(const Twine ) {
+ return buildASTFromCodeWithArgs(Code, {});
+}
+
+ExprMatcher declRefTo(StringRef Name) {
+ return declRefExpr(to(namedDecl(hasName(Name;
+}
+
+StmtMatcher withEnclosingCompound(ExprMatcher Matcher) {
+ return expr(Matcher, hasAncestor(compoundStmt().bind("stmt"))).bind("expr");
+}
+
+bool isMutated(const SmallVectorImpl , ASTUnit *AST) {
+ const auto *const S = selectFirst("stmt", Results);
+ const auto *const E = selectFirst("expr", Results);
+ return tooling::ExprMutationAnalyzer(*S, AST->getASTContext()).isMutated(E);
+}
+
+SmallVector
+mutatedBy(const SmallVectorImpl , ASTUnit *AST) {
+ const auto *const S = selectFirst("stmt", Results);
+ SmallVector Chain;
+ tooling::ExprMutationAnalyzer Analyzer(*S, AST->getASTContext());
+ for (const auto *E = selectFirst("expr", Results); E != nullptr;) {
+const Stmt *By = Analyzer.findMutation(E);
+std::string buffer;
+llvm::raw_string_ostream stream(buffer);
+By->printPretty(stream, nullptr, AST->getASTContext().getPrintingPolicy());
+Chain.push_back(StringRef(stream.str()).trim().str());
+E = dyn_cast(By);
+ }
+ return Chain;
+}
+
+std::string removeSpace(std::string s) {
+ s.erase(std::remove_if(s.begin(), s.end(),
+ [](char c) { return std::isspace(c); }),
+ s.end());
+ return s;
+}
+
+const std::string StdRemoveReference =
+"namespace std {"
+"template struct remove_reference { typedef T type; };"
+"template struct remove_reference { typedef T type; };"
+"template struct remove_reference { typedef T type; }; }";
+
+const std::string StdMove =
+"namespace std {"
+"template typename remove_reference::type&& "
+"move(T&& t) noexcept {"
+"return static_cast::type&&>(t); } }";
+
+const std::string StdForward =
+"namespace std {"
+"template T&& "
+"forward(typename remove_reference::type& t) noexcept { return t; }"
+"template T&& "
+"forward(typename remove_reference::type&& t) noexcept { return t; } }";
+
+TEST(ExprMutationAnalyzerTest, Trivial) {
+ const auto AST = buildASTFromCode("void f() { int x; x; }");
+ const auto Results =
+ match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+ EXPECT_FALSE(isMutated(Results, AST.get()));
+}
+
+class AssignmentTest : public ::testing::TestWithParam {};
+
+TEST_P(AssignmentTest, AssignmentModifies) {
+ const std::string ModExpr = "x " + GetParam() + " 10";
+ const auto AST = buildASTFromCode("void f() { int x; " + ModExpr + "; }");
+ const auto Results =
+ match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+ EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre(ModExpr));
+}
+
+INSTANTIATE_TEST_CASE_P(AllAssignmentOperators, AssignmentTest,
+Values("=", "+=", "-=", "*=", "/=", "%=", "&=", "|=",
+ "^=", "<<=", ">>="), );
+
+class
[PATCH] D52273: [clangd] Initial implementation of expected types
malaperle added a comment. What is the goal for doing this without the AST? Is the goal to not have to keep the AST and save memory? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52273 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D54349: [clang-tidy] new check 'readability-redundant-preprocessor'
Szelethus added inline comments. Comment at: clang-tidy/readability/RedundantPreprocessorCheck.cpp:56-59 +StringRef SourceText = +Lexer::getSourceText(CharSourceRange::getTokenRange(ConditionRange), + PP.getSourceManager(), PP.getLangOpts()); +std::string Condition = getCondition(SourceText); vmiklos wrote: > Szelethus wrote: > > I'm a little confused. To me, it seems like you acquired the condition > > already -- doesn't `ConditionRange` actually cover the, well, condition > > range? This is how I imagined it: > > > > ``` > > #ifdef CUTE_PANDA_CUBS > >^~~ > >ConditionRange > > ``` > > Why is there a need for `getCondition`? Is there any? If there is (maybe > > the acquired text contains other things), can you document it? I haven't > > played with `PPCallbacks` much, so I'm fine with being in the wrong. > ConditionRange covers more than what you expect: > > ``` > #if FOO == 4 >^ > void f(); > ~ > #endif > ~~ > ``` > > to find out if the condition of the `#if` is the same as a previous one, I > want to extract just `FOO == 4` from that, then deal with that part similar > to `#ifdef` and `#ifndef`, which are easier as you have a single Token for > the condition. But you're right, I should add a comment explaining this. Oh my god. There is no tool or a convenient method for this??? I had the displeasure of working with the preprocessor in the last couple months, and I get shocked by things like this almost every day. Yea, unfortunately you will have to write excessive amount of comments to counterweights the shortcomings of `Preprocessor` :/ https://reviews.llvm.org/D54349 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D54349: [clang-tidy] new check 'readability-redundant-preprocessor'
vmiklos updated this revision to Diff 173575. vmiklos marked an inline comment as done. https://reviews.llvm.org/D54349 Files: clang-tidy/readability/CMakeLists.txt clang-tidy/readability/ReadabilityTidyModule.cpp clang-tidy/readability/RedundantPreprocessorCheck.cpp clang-tidy/readability/RedundantPreprocessorCheck.h docs/ReleaseNotes.rst docs/clang-tidy/checks/list.rst docs/clang-tidy/checks/readability-redundant-preprocessor.rst test/clang-tidy/readability-redundant-preprocessor-ifdef.cpp test/clang-tidy/readability-redundant-preprocessor.cpp test/clang-tidy/readability-redundant-preprocessor.h Index: test/clang-tidy/readability-redundant-preprocessor.h === --- /dev/null +++ test/clang-tidy/readability-redundant-preprocessor.h @@ -0,0 +1,5 @@ +#ifndef FOO +#ifndef FOO // this would warn, but not in a header +void f(); +#endif +#endif Index: test/clang-tidy/readability-redundant-preprocessor.cpp === --- /dev/null +++ test/clang-tidy/readability-redundant-preprocessor.cpp @@ -0,0 +1,76 @@ +// RUN: %check_clang_tidy %s readability-redundant-preprocessor %t -- -- -I %S + +// Positive testing. +#ifndef FOO +// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant ifndef; consider removing it [readability-redundant-preprocessor] +#ifndef FOO +// CHECK-NOTES: [[@LINE-3]]:2: note: previous ifndef was here +void f(); +#endif +#endif + +// Positive testing of inverted condition. +#ifndef FOO +// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant ifdef; consider removing it [readability-redundant-preprocessor] +#ifdef FOO +// CHECK-NOTES: [[@LINE-3]]:2: note: previous ifndef was here +void f2(); +#endif +#endif + +// Negative testing. +#include "readability-redundant-preprocessor.h" + +#ifndef BAR +void g(); +#endif + +#ifndef FOO +#ifndef BAR +void h(); +#endif +#endif + +#ifndef FOO +#ifdef BAR +void i(); +#endif +#endif + +// Positive #if testing. +#define FOO 4 + +#if FOO == 4 +// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant if; consider removing it [readability-redundant-preprocessor] +#if FOO == 4 +// CHECK-NOTES: [[@LINE-3]]:2: note: previous if was here +void j(); +#endif +#endif + +#if FOO == \ +4 +// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant if; consider removing it [readability-redundant-preprocessor] +#if FOO == \ +4 +// CHECK-NOTES: [[@LINE-5]]:2: note: previous if was here +void j(); +#endif +#endif + +// Negative #if testing. +#define BAR 4 + +#if FOO == 4 +#if BAR == 4 +void k(); +#endif +#endif + +#if FOO == \ +4 +#if BAR == \ +5 +void k(); +#endif +#endif Index: test/clang-tidy/readability-redundant-preprocessor-ifdef.cpp === --- /dev/null +++ test/clang-tidy/readability-redundant-preprocessor-ifdef.cpp @@ -0,0 +1,36 @@ +// RUN: %check_clang_tidy %s readability-redundant-preprocessor %t -- -- -DFOO + +// Positive testing. +#ifdef FOO +// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant ifdef; consider removing it [readability-redundant-preprocessor] +#ifdef FOO +// CHECK-NOTES: [[@LINE-3]]:2: note: previous ifdef was here +void f(); +#endif +#endif + +// Positive testing of inverted condition. +#ifdef FOO +// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant ifndef; consider removing it [readability-redundant-preprocessor] +#ifndef FOO +// CHECK-NOTES: [[@LINE-3]]:2: note: previous ifdef was here +void f2(); +#endif +#endif + +// Negative testing. +#ifdef BAR +void g(); +#endif + +#ifdef FOO +#ifdef BAR +void h(); +#endif +#endif + +#ifdef FOO +#ifndef BAR +void i(); +#endif +#endif Index: docs/clang-tidy/checks/readability-redundant-preprocessor.rst === --- /dev/null +++ docs/clang-tidy/checks/readability-redundant-preprocessor.rst @@ -0,0 +1,61 @@ +.. title:: clang-tidy - readability-redundant-preprocessor + +readability-redundant-preprocessor +== + +Finds potentially redundant preprocessor directives. At the moment the +following cases are detected: + +* `#ifdef` .. `#endif` pairs which are nested inside an outer pair with the + same condition. For example: + +.. code-block:: c++ + + #ifdef FOO + #ifdef FOO // inner ifdef is considered redundant + void f(); + #endif + #endif + +* Same for `#ifndef` .. `#endif` pairs. For example: + +.. code-block:: c++ + + #ifndef FOO + #ifndef FOO // inner ifndef is considered redundant + void f(); + #endif + #endif + +* `#ifndef` inside an `#ifdef` with the same condition: + +.. code-block:: c++ + + #ifdef FOO + #ifndef FOO // inner ifndef is considered redundant + void f(); + #endif + #endif + +* `#ifdef` inside an `#ifndef` with the same condition: + +.. code-block:: c++ + + #ifndef FOO + #ifdef FOO // inner ifdef is considered redundant + void f(); + #endif + #endif + +* `#if` ..
[PATCH] D54349: [clang-tidy] new check 'readability-redundant-preprocessor'
vmiklos marked 2 inline comments as done. vmiklos added inline comments. Comment at: clang-tidy/readability/RedundantPreprocessorCheck.cpp:56-59 +StringRef SourceText = +Lexer::getSourceText(CharSourceRange::getTokenRange(ConditionRange), + PP.getSourceManager(), PP.getLangOpts()); +std::string Condition = getCondition(SourceText); Szelethus wrote: > I'm a little confused. To me, it seems like you acquired the condition > already -- doesn't `ConditionRange` actually cover the, well, condition > range? This is how I imagined it: > > ``` > #ifdef CUTE_PANDA_CUBS >^~~ >ConditionRange > ``` > Why is there a need for `getCondition`? Is there any? If there is (maybe the > acquired text contains other things), can you document it? I haven't played > with `PPCallbacks` much, so I'm fine with being in the wrong. ConditionRange covers more than what you expect: ``` #if FOO == 4 ^ void f(); ~ #endif ~~ ``` to find out if the condition of the `#if` is the same as a previous one, I want to extract just `FOO == 4` from that, then deal with that part similar to `#ifdef` and `#ifndef`, which are easier as you have a single Token for the condition. But you're right, I should add a comment explaining this. https://reviews.llvm.org/D54349 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53974: [clang-tidy] new check: bugprone-too-small-loop-variable
ztamas added a comment. > Agree. I think this check is good to go. > > I would commit this check tomorrow if that is ok with you. Of course, It would be great if this check can get in, Thanks! Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53974 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53974: [clang-tidy] new check: bugprone-too-small-loop-variable
ztamas updated this revision to Diff 173573.
ztamas added a comment.
- Add requested test case
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53974
Files:
clang-tidy/bugprone/BugproneTidyModule.cpp
clang-tidy/bugprone/CMakeLists.txt
clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
clang-tidy/bugprone/TooSmallLoopVariableCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/bugprone-too-small-loop-variable.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/bugprone-too-small-loop-variable.cpp
Index: test/clang-tidy/bugprone-too-small-loop-variable.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-too-small-loop-variable.cpp
@@ -0,0 +1,251 @@
+// RUN: %check_clang_tidy %s bugprone-too-small-loop-variable %t
+
+long size() { return 294967296l; }
+
+
+/// Test cases correctly caught by bugprone-too-small-loop-variable.
+
+void voidBadForLoop() {
+ for (int i = 0; i < size(); ++i) {
+// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: loop variable has narrower type 'int' than iteration's upper bound 'long' [bugprone-too-small-loop-variable]
+ }
+}
+
+void voidBadForLoop2() {
+ for (int i = 0; i < size() + 10; ++i) {
+// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: loop variable has narrower type 'int' than iteration's upper bound 'long' [bugprone-too-small-loop-variable]
+ }
+}
+
+void voidBadForLoop3() {
+ for (int i = 0; i <= size() - 1; ++i) {
+// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: loop variable has narrower type 'int' than iteration's upper bound 'long' [bugprone-too-small-loop-variable]
+ }
+}
+
+void voidBadForLoop4() {
+ for (int i = 0; size() > i; ++i) {
+// CHECK-MESSAGES: :[[@LINE-1]]:28: warning: loop variable has narrower type 'int' than iteration's upper bound 'long' [bugprone-too-small-loop-variable]
+ }
+}
+
+void voidBadForLoop5() {
+ for (int i = 0; size() - 1 >= i; ++i) {
+// CHECK-MESSAGES: :[[@LINE-1]]:33: warning: loop variable has narrower type 'int' than iteration's upper bound 'long' [bugprone-too-small-loop-variable]
+ }
+}
+
+void voidBadForLoop6() {
+ int i = 0;
+ for (; i < size(); ++i) {
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: loop variable has narrower type 'int' than iteration's upper bound 'long' [bugprone-too-small-loop-variable]
+ }
+}
+
+void voidForLoopUnsignedBound() {
+ unsigned size = 3147483647;
+ for (int i = 0; i < size; ++i) {
+// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: loop variable has narrower type 'int' than iteration's upper bound 'unsigned int' [bugprone-too-small-loop-variable]
+ }
+}
+
+// The iteration's upper bound has a template dependent value.
+template
+void doSomething() {
+ for (short i = 0; i < size; ++i) {
+// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: loop variable has narrower type 'short' than iteration's upper bound 'long' [bugprone-too-small-loop-variable]
+ }
+}
+
+// The iteration's upper bound has a template dependent type.
+template
+void doSomething() {
+ for (T i = 0; i < size(); ++i) {
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: loop variable has narrower type 'short' than iteration's upper bound 'long' [bugprone-too-small-loop-variable]
+ }
+}
+
+void voidForLoopInstantiation() {
+ // This line does not trigger the warning.
+ doSomething();
+ // This one triggers the warning.
+ doSomething();
+}
+
+// A suspicious function used in a macro.
+#define SUSPICIOUS_SIZE (size())
+void voidBadForLoopWithMacroBound() {
+ for (short i = 0; i < SUSPICIOUS_SIZE; ++i) {
+// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: loop variable has narrower type 'short' than iteration's upper bound 'long' [bugprone-too-small-loop-variable]
+ }
+}
+
+
+/// Correct loops: we should not warn here.
+
+// A simple use case when both expressions have the same type.
+void voidGoodForLoop() {
+ for (long i = 0; i < size(); ++i) { // no warning
+ }
+}
+
+// Other use case where both expressions have the same type,
+// but short expressions are converted to int by the compare operator.
+void voidGoodForLoop2() {
+ short loopCond = 10;
+ for (short i = 0; i < loopCond; ++i) { // no warning
+ }
+}
+
+// Because of the integer literal, the iteration's upper bound is int, but we suppress the warning here.
+void voidForLoopShortPlusLiteral() {
+ short size = 3;
+ for (short i = 0; i <= (size - 1); ++i) { // no warning
+ }
+}
+
+// Addition of two short variables results in an int value, but we suppress this to avoid false positives.
+void voidForLoopShortPlusShort() {
+ short size = 256;
+ short increment = 14;
+ for (short i = 0; i < size + increment; ++i) { // no warning
+ }
+}
+
+// In this test case we have different integer types, but here the loop variable has the bigger type.
+// The iteration's bound is cast implicitly, not
[PATCH] D54349: [clang-tidy] new check 'readability-redundant-preprocessor'
Szelethus added inline comments. Comment at: clang-tidy/readability/RedundantPreprocessorCheck.cpp:56-59 +StringRef SourceText = +Lexer::getSourceText(CharSourceRange::getTokenRange(ConditionRange), + PP.getSourceManager(), PP.getLangOpts()); +std::string Condition = getCondition(SourceText); I'm a little confused. To me, it seems like you acquired the condition already -- doesn't `ConditionRange` actually cover the, well, condition range? This is how I imagined it: ``` #ifdef CUTE_PANDA_CUBS ^~~ ConditionRange ``` Why is there a need for `getCondition`? Is there any? If there is (maybe the acquired text contains other things), can you document it? I haven't played with `PPCallbacks` much, so I'm fine with being in the wrong. https://reviews.llvm.org/D54349 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D54349: [clang-tidy] new check 'readability-redundant-preprocessor'
vmiklos marked an inline comment as done. vmiklos added inline comments. Comment at: docs/clang-tidy/checks/readability-redundant-preprocessor.rst:34 + + #ifdef FOO + #ifndef FOO // inner ifndef is considered redundant JonasToth wrote: > The indendation for these examples (following as well) is not enough. Please > use 2 spaces. Fixed. https://reviews.llvm.org/D54349 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D54349: [clang-tidy] new check 'readability-redundant-preprocessor'
vmiklos updated this revision to Diff 173572. https://reviews.llvm.org/D54349 Files: clang-tidy/readability/CMakeLists.txt clang-tidy/readability/ReadabilityTidyModule.cpp clang-tidy/readability/RedundantPreprocessorCheck.cpp clang-tidy/readability/RedundantPreprocessorCheck.h docs/ReleaseNotes.rst docs/clang-tidy/checks/list.rst docs/clang-tidy/checks/readability-redundant-preprocessor.rst test/clang-tidy/readability-redundant-preprocessor-ifdef.cpp test/clang-tidy/readability-redundant-preprocessor.cpp test/clang-tidy/readability-redundant-preprocessor.h Index: test/clang-tidy/readability-redundant-preprocessor.h === --- /dev/null +++ test/clang-tidy/readability-redundant-preprocessor.h @@ -0,0 +1,5 @@ +#ifndef FOO +#ifndef FOO // this would warn, but not in a header +void f(); +#endif +#endif Index: test/clang-tidy/readability-redundant-preprocessor.cpp === --- /dev/null +++ test/clang-tidy/readability-redundant-preprocessor.cpp @@ -0,0 +1,76 @@ +// RUN: %check_clang_tidy %s readability-redundant-preprocessor %t -- -- -I %S + +// Positive testing. +#ifndef FOO +// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant ifndef; consider removing it [readability-redundant-preprocessor] +#ifndef FOO +// CHECK-NOTES: [[@LINE-3]]:2: note: previous ifndef was here +void f(); +#endif +#endif + +// Positive testing of inverted condition. +#ifndef FOO +// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant ifdef; consider removing it [readability-redundant-preprocessor] +#ifdef FOO +// CHECK-NOTES: [[@LINE-3]]:2: note: previous ifndef was here +void f2(); +#endif +#endif + +// Negative testing. +#include "readability-redundant-preprocessor.h" + +#ifndef BAR +void g(); +#endif + +#ifndef FOO +#ifndef BAR +void h(); +#endif +#endif + +#ifndef FOO +#ifdef BAR +void i(); +#endif +#endif + +// Positive #if testing. +#define FOO 4 + +#if FOO == 4 +// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant if; consider removing it [readability-redundant-preprocessor] +#if FOO == 4 +// CHECK-NOTES: [[@LINE-3]]:2: note: previous if was here +void j(); +#endif +#endif + +#if FOO == \ +4 +// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant if; consider removing it [readability-redundant-preprocessor] +#if FOO == \ +4 +// CHECK-NOTES: [[@LINE-5]]:2: note: previous if was here +void j(); +#endif +#endif + +// Negative #if testing. +#define BAR 4 + +#if FOO == 4 +#if BAR == 4 +void k(); +#endif +#endif + +#if FOO == \ +4 +#if BAR == \ +5 +void k(); +#endif +#endif Index: test/clang-tidy/readability-redundant-preprocessor-ifdef.cpp === --- /dev/null +++ test/clang-tidy/readability-redundant-preprocessor-ifdef.cpp @@ -0,0 +1,36 @@ +// RUN: %check_clang_tidy %s readability-redundant-preprocessor %t -- -- -DFOO + +// Positive testing. +#ifdef FOO +// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant ifdef; consider removing it [readability-redundant-preprocessor] +#ifdef FOO +// CHECK-NOTES: [[@LINE-3]]:2: note: previous ifdef was here +void f(); +#endif +#endif + +// Positive testing of inverted condition. +#ifdef FOO +// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant ifndef; consider removing it [readability-redundant-preprocessor] +#ifndef FOO +// CHECK-NOTES: [[@LINE-3]]:2: note: previous ifdef was here +void f2(); +#endif +#endif + +// Negative testing. +#ifdef BAR +void g(); +#endif + +#ifdef FOO +#ifdef BAR +void h(); +#endif +#endif + +#ifdef FOO +#ifndef BAR +void i(); +#endif +#endif Index: docs/clang-tidy/checks/readability-redundant-preprocessor.rst === --- /dev/null +++ docs/clang-tidy/checks/readability-redundant-preprocessor.rst @@ -0,0 +1,61 @@ +.. title:: clang-tidy - readability-redundant-preprocessor + +readability-redundant-preprocessor +== + +Finds potentially redundant preprocessor directives. At the moment the +following cases are detected: + +* `#ifdef` .. `#endif` pairs which are nested inside an outer pair with the + same condition. For example: + +.. code-block:: c++ + + #ifdef FOO + #ifdef FOO // inner ifdef is considered redundant + void f(); + #endif + #endif + +* Same for `#ifndef` .. `#endif` pairs. For example: + +.. code-block:: c++ + + #ifndef FOO + #ifndef FOO // inner ifndef is considered redundant + void f(); + #endif + #endif + +* `#ifndef` inside an `#ifdef` with the same condition: + +.. code-block:: c++ + + #ifdef FOO + #ifndef FOO // inner ifndef is considered redundant + void f(); + #endif + #endif + +* `#ifdef` inside an `#ifndef` with the same condition: + +.. code-block:: c++ + + #ifndef FOO + #ifdef FOO // inner ifdef is considered redundant + void f(); + #endif + #endif + +* `#if` .. `#endif` pairs which are nested inside an
[PATCH] D54397: [analyzer][NFC] Move CheckerOptInfo to CheckerRegistry.cpp, and make it local
Szelethus created this revision.
Szelethus added reviewers: NoQ, george.karpenkov, xazax.hun, rnkovacs, MTC.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, mikhail.ramalho,
a.sidorin, szepet, whisperity.
TL;DR: `CheckerOptInfo` feels very much out of place in
`CheckerRegistration.cpp`, so I moved it to `CheckerRegistry.h`.
Details:
While on the quest of fixing checker options the same way I cleaned up
non-checker options, although I'm making good progress, I ran into a wall: In
order to emit warnings on incorrect checker options, we need to ensure that
checkers actually acquire their options properly -- but, I unearthed a huge bug
in checker registration: dependencies are currently implemented in a way that
breaks the already very fragile registration infrastructure.
Here is where the problem really originates from: this is a snippet from
`CheckerRegistry::initializeManager`.
// Initialize the CheckerManager with all enabled checkers.
for (const auto *i :enabledCheckers) {
checkerMgr.setCurrentCheckName(CheckName(i->FullName));
i->Initialize(checkerMgr);
}
Note that `Initialize` is a function pointer to `register##CHECKERNAME`, like
`registerInnerPointerChecker`. Since for each register function call the
current checker name is only set once, when `InnerPointerChecker` attempts to
also register it's dependent `MallocChecker`, both it and `MallocChecker` will
receive the `cplusplus.InnerPointer` name. This results in `MallocChecker`
trying to acquire it's `Optimistic` option as
`cplusplus.InnerPointerChecker:Optimistic`.
Clearly, this problem has to be solved at a higher level -- it makes no sense
to be affect other checkers in a registry function. Since I'll already make
changes to how checker registration works, I'll probably tune some things for
the current C++ standard, add much needed documentation, and try to make the
code //a lot less confusing//.
Repository:
rC Clang
https://reviews.llvm.org/D54397
Files:
include/clang/StaticAnalyzer/Core/CheckerOptInfo.h
include/clang/StaticAnalyzer/Core/CheckerRegistry.h
lib/StaticAnalyzer/Core/CheckerRegistry.cpp
lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp
Index: lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp
===
--- lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp
+++ lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp
@@ -17,7 +17,6 @@
#include "clang/StaticAnalyzer/Checkers/ClangCheckers.h"
#include "clang/StaticAnalyzer/Core/AnalyzerOptions.h"
#include "clang/StaticAnalyzer/Core/CheckerManager.h"
-#include "clang/StaticAnalyzer/Core/CheckerOptInfo.h"
#include "clang/StaticAnalyzer/Core/CheckerRegistry.h"
#include "clang/StaticAnalyzer/Frontend/FrontendActions.h"
#include "llvm/ADT/SmallVector.h"
@@ -102,44 +101,23 @@
<< pluginAPIVersion;
}
-static SmallVector
-getCheckerOptList(const AnalyzerOptions ) {
- SmallVector checkerOpts;
- for (unsigned i = 0, e = opts.CheckersControlList.size(); i != e; ++i) {
-const std::pair = opts.CheckersControlList[i];
-checkerOpts.push_back(CheckerOptInfo(opt.first, opt.second));
- }
- return checkerOpts;
-}
-
std::unique_ptr ento::createCheckerManager(
ASTContext ,
AnalyzerOptions ,
ArrayRef plugins,
ArrayRef> checkerRegistrationFns,
DiagnosticsEngine ) {
auto checkerMgr = llvm::make_unique(context, opts);
- SmallVector checkerOpts = getCheckerOptList(opts);
-
ClangCheckerRegistry allCheckers(plugins, );
for (const auto : checkerRegistrationFns)
Fn(allCheckers);
- allCheckers.initializeManager(*checkerMgr, checkerOpts);
+ allCheckers.initializeManager(*checkerMgr, opts, diags);
allCheckers.validateCheckerOptions(opts, diags);
checkerMgr->finishedCheckerRegistration();
- for (unsigned i = 0, e = checkerOpts.size(); i != e; ++i) {
-if (checkerOpts[i].isUnclaimed()) {
- diags.Report(diag::err_unknown_analyzer_checker)
- << checkerOpts[i].getName();
- diags.Report(diag::note_suggest_disabling_all_checkers);
-}
-
- }
-
return checkerMgr;
}
@@ -155,8 +133,7 @@
const AnalyzerOptions ) {
out << "OVERVIEW: Clang Static Analyzer Enabled Checkers List\n\n";
- SmallVector checkerOpts = getCheckerOptList(opts);
- ClangCheckerRegistry(plugins).printList(out, checkerOpts);
+ ClangCheckerRegistry(plugins).printList(out, opts);
}
void ento::printAnalyzerConfigList(raw_ostream ) {
Index: lib/StaticAnalyzer/Core/CheckerRegistry.cpp
===
--- lib/StaticAnalyzer/Core/CheckerRegistry.cpp
+++ lib/StaticAnalyzer/Core/CheckerRegistry.cpp
@@ -12,7 +12,6 @@
#include "clang/Basic/LLVM.h"
#include "clang/Frontend/FrontendDiagnostic.h"
#include "clang/StaticAnalyzer/Core/CheckerManager.h"
-#include "clang/StaticAnalyzer/Core/CheckerOptInfo.h"
#include
[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables
JonasToth created this revision.
JonasToth added reviewers: aaron.ballman, hokein, alexfh, shuaiwang, lebedev.ri.
Herald added subscribers: cfe-commits, xazax.hun, mgorny.
JonasToth added a dependent revision: D45444: [clang-tidy] implement new check
for const-correctness.
This patch extends the already existing facility to add 'const' to variables
to be more flexible and correct. The previous version did not consider pointers
as value AND pointee. For future automatic introduction for const-correctness
this shortcoming needs to be fixed.
It always allows configuration where the 'const' token is inserted, either on
the left side (if possible) or the right side.
It adds many unit-tests to the utility-function that did not exist before, as
the function was implicitly tested through clang-tidy checks. These
tests were not changed, as the API is still compatible.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D54395
Files:
clang-tidy/performance/ForRangeCopyCheck.cpp
clang-tidy/performance/UnnecessaryCopyInitialization.cpp
clang-tidy/performance/UnnecessaryValueParamCheck.cpp
clang-tidy/utils/FixItHintUtils.cpp
clang-tidy/utils/FixItHintUtils.h
clang-tidy/utils/LexerUtils.cpp
clang-tidy/utils/LexerUtils.h
unittests/clang-tidy/AddConstTest.cpp
unittests/clang-tidy/CMakeLists.txt
Index: unittests/clang-tidy/CMakeLists.txt
===
--- unittests/clang-tidy/CMakeLists.txt
+++ unittests/clang-tidy/CMakeLists.txt
@@ -7,6 +7,7 @@
include_directories(${CLANG_LINT_SOURCE_DIR})
add_extra_unittest(ClangTidyTests
+ AddConstTest.cpp
ClangTidyDiagnosticConsumerTest.cpp
ClangTidyOptionsTest.cpp
IncludeInserterTest.cpp
Index: unittests/clang-tidy/AddConstTest.cpp
===
--- /dev/null
+++ unittests/clang-tidy/AddConstTest.cpp
@@ -0,0 +1,826 @@
+#include "../clang-tidy/utils/FixItHintUtils.h"
+#include "ClangTidyTest.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Tooling/Tooling.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace tidy {
+
+namespace {
+using namespace clang::ast_matchers;
+using namespace utils::fixit;
+
+template
+class ConstTransform : public ClangTidyCheck {
+public:
+ ConstTransform(StringRef CheckName, ClangTidyContext *Context)
+ : ClangTidyCheck(CheckName, Context) {}
+
+ void registerMatchers(MatchFinder *Finder) override {
+Finder->addMatcher(varDecl(hasName("target")).bind("var"), this);
+ }
+
+ void check(const MatchFinder::MatchResult ) override {
+const auto *D = Result.Nodes.getNodeAs("var");
+using utils::fixit::changeVarDeclToConst;
+Optional Fix = changeVarDeclToConst(*D, CT, CP, Result.Context);
+auto Diag = diag(D->getBeginLoc(), "doing const transformation");
+if (Fix)
+ Diag << *Fix;
+ }
+};
+} // namespace
+
+namespace test {
+using PointeeLTransform =
+ConstTransform;
+using PointeeRTransform =
+ConstTransform;
+
+using ValueLTransform = ConstTransform;
+using ValueRTransform = ConstTransform;
+
+//
+// Test Value-like types. Everything with indirection is done later.
+//
+
+// TODO: Template-code
+
+TEST(Values, Builtin) {
+ StringRef Snippet = "int target = 0;";
+
+ EXPECT_EQ("const int target = 0;", runCheckOnCode(Snippet));
+ EXPECT_EQ("const int target = 0;",
+runCheckOnCode(Snippet));
+
+ EXPECT_EQ("int const target = 0;", runCheckOnCode(Snippet));
+ EXPECT_EQ("int const target = 0;",
+runCheckOnCode(Snippet));
+}
+TEST(Values, TypedefBuiltin) {
+ StringRef T = "typedef int MyInt;";
+ StringRef S = "MyInt target = 0;";
+ auto Cat = [](StringRef S) { return (T + S).str(); };
+
+ EXPECT_EQ(Cat("const MyInt target = 0;"),
+runCheckOnCode(Cat(S)));
+ EXPECT_EQ(Cat("const MyInt target = 0;"),
+runCheckOnCode(Cat(S)));
+
+ EXPECT_EQ(Cat("MyInt const target = 0;"),
+runCheckOnCode(Cat(S)));
+ EXPECT_EQ(Cat("MyInt const target = 0;"),
+runCheckOnCode(Cat(S)));
+}
+TEST(Values, TypedefBuiltinPointer) {
+ StringRef T = "typedef int* MyInt;";
+ StringRef S = "MyInt target = nullptr;";
+ auto Cat = [](StringRef S) { return (T + S).str(); };
+
+ EXPECT_EQ(Cat("const MyInt target = nullptr;"),
+runCheckOnCode(Cat(S)));
+ EXPECT_EQ(Cat("const MyInt target = nullptr;"),
+runCheckOnCode(Cat(S)));
+
+ EXPECT_EQ(Cat("MyInt const target = nullptr;"),
+runCheckOnCode(Cat(S)));
+ EXPECT_EQ(Cat("MyInt const target = nullptr;"),
+runCheckOnCode(Cat(S)));
+}
+TEST(Values, AutoValue) {
+ StringRef T = "int f() { return 42; }\n";
+ StringRef S = "auto target = f();";
+ auto Cat = [](StringRef S) { return (T + S).str(); };
+
+
[PATCH] D53974: [clang-tidy] new check: bugprone-too-small-loop-variable
JonasToth added inline comments.
Comment at: test/clang-tidy/bugprone-too-small-loop-variable.cpp:138
+ bool cond = false;
+ for (short i = 0; i < (cond ? 0 : size); ++i) {
+// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: loop variable has narrower
type 'short' than iteration's upper bound 'int'
[bugprone-too-small-loop-variable]
Could you please add a test
```
for (short i = 0; i < (!cond ? size : 0); ++i) {
}
```
as well. Just to make sure the analysis does not depent on the type of the LHS.
If it does, add a FIXME for later.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53974
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53974: [clang-tidy] new check: bugprone-too-small-loop-variable
JonasToth added a comment. > Yes, that's right, these are not full false positives, but the check's main > focus is on those loops which are runtime dependent. If a loop's upper bound > can be calculated in compile time then this loop should be caught by a > compiler warning based on the actual value of that constant. See > -Wtautological-constant-out-of-range-compare for example. So I think it's the > best if we can avoid catching these issues using a type based matching. > Anyway, there is not too many of this kind of false positives, so it's not a > big issue. In LLVM code I did not find any similar case. > I can't see full false positives where the check works incorrectly. The > detected type mismatch seems correctly detected in every case. Agree. I think this check is good to go. I would commit this check tomorrow if that is ok with you. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53974 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52418: [driver][mips] Enable integrated assembler for MIPS64 except N32 ABI selected
brad added a comment. Simon? Repository: rC Clang https://reviews.llvm.org/D52418 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53974: [clang-tidy] new check: bugprone-too-small-loop-variable
ztamas added a comment. In https://reviews.llvm.org/D53974#1294659, @JonasToth wrote: > >> I would be very interested why these false positives are created. Is there > >> a way to avoid them and maybe it makes sense to already add `FIXME` at the > >> possible places the check needs improvements. > > > > voidForLoopFalsePositive() and voidForLoopFalsePositive2() test cases > > covers most of the false positives found in LibreOffice. > > I would not consider them as full false positives, the constants that are > created allow to filter more, but type-based the diagnostics are correct. So > I think that is fine. If the constants would be a big value, the check does > find a real problem. Yes, that's right, these are not full false positives, but the check's main focus is on those loops which are runtime dependent. If a loop's upper bound can be calculated in compile time then this loop should be caught by a compiler warning based on the actual value of that constant. See -Wtautological-constant-out-of-range-compare for example. So I think it's the best if we can avoid catching these issues using a type based matching. Anyway, there is not too many of this kind of false positives, so it's not a big issue. In LLVM code I did not find any similar case. I can't see full false positives where the check works incorrectly. The detected type mismatch seems correctly detected in every case. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53974 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53974: [clang-tidy] new check: bugprone-too-small-loop-variable
JonasToth added a comment. >> I would be very interested why these false positives are created. Is there a >> way to avoid them and maybe it makes sense to already add `FIXME` at the >> possible places the check needs improvements. > > voidForLoopFalsePositive() and voidForLoopFalsePositive2() test cases covers > most of the false positives found in LibreOffice. I would not consider them as full false positives, the constants that are created allow to filter more, but type-based the diagnostics are correct. So I think that is fine. If the constants would be a big value, the check does find a real problem. @whisperity > I am fairly concerned the example with unsigned use for container iteration > are not false positives, just examples of bad happenstance code which never > breaks under real life applications due to uint32_t being good enough but is > actually not type-safe. > Those examples that I kept in my quote are especially bad and should be > fixed eventually... clang-tidy is not about finding _only_ bugs, but find patterns that can be problematic but are not in every instance (therefore `bugprone-` and not `bug-`). `uint32_t` does not span the whole memory-space for current hardware and a `std::string` can have more then `uint32_t::max` elements. Diagnosing this is valid. Containers where `uint32_t::max * sizeof(element_type) > size_t::max` could be filtered for normal iteration over the container. I would consider it still a bugprone pattern. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53974 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D54349: [clang-tidy] new check 'readability-redundant-preprocessor'
JonasToth added inline comments. Comment at: docs/clang-tidy/checks/readability-redundant-preprocessor.rst:34 + + #ifdef FOO + #ifndef FOO // inner ifndef is considered redundant The indendation for these examples (following as well) is not enough. Please use 2 spaces. https://reviews.llvm.org/D54349 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53974: [clang-tidy] new check: bugprone-too-small-loop-variable
ztamas updated this revision to Diff 173560.
ztamas added a comment.
- Fix comment
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53974
Files:
clang-tidy/bugprone/BugproneTidyModule.cpp
clang-tidy/bugprone/CMakeLists.txt
clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
clang-tidy/bugprone/TooSmallLoopVariableCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/bugprone-too-small-loop-variable.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/bugprone-too-small-loop-variable.cpp
Index: test/clang-tidy/bugprone-too-small-loop-variable.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-too-small-loop-variable.cpp
@@ -0,0 +1,243 @@
+// RUN: %check_clang_tidy %s bugprone-too-small-loop-variable %t
+
+long size() { return 294967296l; }
+
+
+/// Test cases correctly caught by bugprone-too-small-loop-variable.
+
+void voidBadForLoop() {
+ for (int i = 0; i < size(); ++i) {
+// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: loop variable has narrower type 'int' than iteration's upper bound 'long' [bugprone-too-small-loop-variable]
+ }
+}
+
+void voidBadForLoop2() {
+ for (int i = 0; i < size() + 10; ++i) {
+// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: loop variable has narrower type 'int' than iteration's upper bound 'long' [bugprone-too-small-loop-variable]
+ }
+}
+
+void voidBadForLoop3() {
+ for (int i = 0; i <= size() - 1; ++i) {
+// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: loop variable has narrower type 'int' than iteration's upper bound 'long' [bugprone-too-small-loop-variable]
+ }
+}
+
+void voidBadForLoop4() {
+ for (int i = 0; size() > i; ++i) {
+// CHECK-MESSAGES: :[[@LINE-1]]:28: warning: loop variable has narrower type 'int' than iteration's upper bound 'long' [bugprone-too-small-loop-variable]
+ }
+}
+
+void voidBadForLoop5() {
+ for (int i = 0; size() - 1 >= i; ++i) {
+// CHECK-MESSAGES: :[[@LINE-1]]:33: warning: loop variable has narrower type 'int' than iteration's upper bound 'long' [bugprone-too-small-loop-variable]
+ }
+}
+
+void voidBadForLoop6() {
+ int i = 0;
+ for (; i < size(); ++i) {
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: loop variable has narrower type 'int' than iteration's upper bound 'long' [bugprone-too-small-loop-variable]
+ }
+}
+
+void voidForLoopUnsignedBound() {
+ unsigned size = 3147483647;
+ for (int i = 0; i < size; ++i) {
+// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: loop variable has narrower type 'int' than iteration's upper bound 'unsigned int' [bugprone-too-small-loop-variable]
+ }
+}
+
+// The iteration's upper bound has a template dependent value.
+template
+void doSomething() {
+ for (short i = 0; i < size; ++i) {
+// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: loop variable has narrower type 'short' than iteration's upper bound 'long' [bugprone-too-small-loop-variable]
+ }
+}
+
+// The iteration's upper bound has a template dependent type.
+template
+void doSomething() {
+ for (T i = 0; i < size(); ++i) {
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: loop variable has narrower type 'short' than iteration's upper bound 'long' [bugprone-too-small-loop-variable]
+ }
+}
+
+void voidForLoopInstantiation() {
+ // This line does not trigger the warning.
+ doSomething();
+ // This one triggers the warning.
+ doSomething();
+}
+
+// A suspicious function used in a macro.
+#define SUSPICIOUS_SIZE (size())
+void voidBadForLoopWithMacroBound() {
+ for (short i = 0; i < SUSPICIOUS_SIZE; ++i) {
+// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: loop variable has narrower type 'short' than iteration's upper bound 'long' [bugprone-too-small-loop-variable]
+ }
+}
+
+
+/// Correct loops: we should not warn here.
+
+// A simple use case when both expressions have the same type.
+void voidGoodForLoop() {
+ for (long i = 0; i < size(); ++i) { // no warning
+ }
+}
+
+// Other use case where both expressions have the same type,
+// but short expressions are converted to int by the compare operator.
+void voidGoodForLoop2() {
+ short loopCond = 10;
+ for (short i = 0; i < loopCond; ++i) { // no warning
+ }
+}
+
+// Because of the integer literal, the iteration's upper bound is int, but we suppress the warning here.
+void voidForLoopShortPlusLiteral() {
+ short size = 3;
+ for (short i = 0; i <= (size - 1); ++i) { // no warning
+ }
+}
+
+// Addition of two short variables results in an int value, but we suppress this to avoid false positives.
+void voidForLoopShortPlusShort() {
+ short size = 256;
+ short increment = 14;
+ for (short i = 0; i < size + increment; ++i) { // no warning
+ }
+}
+
+// In this test case we have different integer types, but here the loop variable has the bigger type.
+// The iteration's bound is cast implicitly, not the loop
[PATCH] D53974: [clang-tidy] new check: bugprone-too-small-loop-variable
ztamas added a comment. >> Also found some new false positives related to macros. The number of all >> false positives is around 38, which is still seems good to me. > > I would be very interested why these false positives are created. Is there a > way to avoid them and maybe it makes sense to already add `FIXME` at the > possible places the check needs improvements. voidForLoopFalsePositive() and voidForLoopFalsePositive2() test cases covers most of the false positives found in LibreOffice. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53974 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53974: [clang-tidy] new check: bugprone-too-small-loop-variable
ztamas updated this revision to Diff 173559.
ztamas added a comment.
- Add new test cases based on findings in LibreOffice code base
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53974
Files:
clang-tidy/bugprone/BugproneTidyModule.cpp
clang-tidy/bugprone/CMakeLists.txt
clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
clang-tidy/bugprone/TooSmallLoopVariableCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/bugprone-too-small-loop-variable.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/bugprone-too-small-loop-variable.cpp
Index: test/clang-tidy/bugprone-too-small-loop-variable.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-too-small-loop-variable.cpp
@@ -0,0 +1,243 @@
+// RUN: %check_clang_tidy %s bugprone-too-small-loop-variable %t
+
+long size() { return 294967296l; }
+
+
+/// Test cases correctly caught by bugprone-too-small-loop-variable.
+
+void voidBadForLoop() {
+ for (int i = 0; i < size(); ++i) {
+// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: loop variable has narrower type 'int' than iteration's upper bound 'long' [bugprone-too-small-loop-variable]
+ }
+}
+
+void voidBadForLoop2() {
+ for (int i = 0; i < size() + 10; ++i) {
+// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: loop variable has narrower type 'int' than iteration's upper bound 'long' [bugprone-too-small-loop-variable]
+ }
+}
+
+void voidBadForLoop3() {
+ for (int i = 0; i <= size() - 1; ++i) {
+// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: loop variable has narrower type 'int' than iteration's upper bound 'long' [bugprone-too-small-loop-variable]
+ }
+}
+
+void voidBadForLoop4() {
+ for (int i = 0; size() > i; ++i) {
+// CHECK-MESSAGES: :[[@LINE-1]]:28: warning: loop variable has narrower type 'int' than iteration's upper bound 'long' [bugprone-too-small-loop-variable]
+ }
+}
+
+void voidBadForLoop5() {
+ for (int i = 0; size() - 1 >= i; ++i) {
+// CHECK-MESSAGES: :[[@LINE-1]]:33: warning: loop variable has narrower type 'int' than iteration's upper bound 'long' [bugprone-too-small-loop-variable]
+ }
+}
+
+void voidBadForLoop6() {
+ int i = 0;
+ for (; i < size(); ++i) {
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: loop variable has narrower type 'int' than iteration's upper bound 'long' [bugprone-too-small-loop-variable]
+ }
+}
+
+void voidForLoopUnsignedBound() {
+ unsigned size = 3147483647;
+ for (int i = 0; i < size; ++i) {
+// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: loop variable has narrower type 'int' than iteration's upper bound 'unsigned int' [bugprone-too-small-loop-variable]
+ }
+}
+
+// The iteration's upper bound has a template dependent value.
+template
+void doSomething() {
+ for (short i = 0; i < size; ++i) {
+// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: loop variable has narrower type 'short' than iteration's upper bound 'long' [bugprone-too-small-loop-variable]
+ }
+}
+
+// The iteration's upper bound has a template dependent type.
+template
+void doSomething() {
+ for (T i = 0; i < size(); ++i) {
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: loop variable has narrower type 'short' than iteration's upper bound 'long' [bugprone-too-small-loop-variable]
+ }
+}
+
+void voidForLoopInstantiation() {
+ // This line does not trigger the warning.
+ doSomething();
+ // This one triggers the warning.
+ doSomething();
+}
+
+// Suspicious funtcion defined by macro.
+#define SUSPICIOUS_SIZE (size())
+void voidBadForLoopWithMacroBound() {
+ for (short i = 0; i < SUSPICIOUS_SIZE; ++i) {
+// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: loop variable has narrower type 'short' than iteration's upper bound 'long' [bugprone-too-small-loop-variable]
+ }
+}
+
+
+/// Correct loops: we should not warn here.
+
+// A simple use case when both expressions have the same type.
+void voidGoodForLoop() {
+ for (long i = 0; i < size(); ++i) { // no warning
+ }
+}
+
+// Other use case where both expressions have the same type,
+// but short expressions are converted to int by the compare operator.
+void voidGoodForLoop2() {
+ short loopCond = 10;
+ for (short i = 0; i < loopCond; ++i) { // no warning
+ }
+}
+
+// Because of the integer literal, the iteration's upper bound is int, but we suppress the warning here.
+void voidForLoopShortPlusLiteral() {
+ short size = 3;
+ for (short i = 0; i <= (size - 1); ++i) { // no warning
+ }
+}
+
+// Addition of two short variables results in an int value, but we suppress this to avoid false positives.
+void voidForLoopShortPlusShort() {
+ short size = 256;
+ short increment = 14;
+ for (short i = 0; i < size + increment; ++i) { // no warning
+ }
+}
+
+// In this test case we have different integer types, but here the loop variable has the bigger type.
+// The
[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions
xbolva00 added a comment. Since no futher comments, i think it is ok. After day or two I will commit it. https://reviews.llvm.org/D52835 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D54349: [clang-tidy] new check 'readability-redundant-preprocessor'
vmiklos added a comment. In https://reviews.llvm.org/D54349#1294600, @vmiklos wrote: > Let's see if that works in practice. I've implemented this now, please take a look. (Extended test + docs as well, as usual.) Thanks! https://reviews.llvm.org/D54349 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D54349: [clang-tidy] new check 'readability-redundant-preprocessor'
vmiklos updated this revision to Diff 173556. vmiklos marked an inline comment as done. https://reviews.llvm.org/D54349 Files: clang-tidy/readability/CMakeLists.txt clang-tidy/readability/ReadabilityTidyModule.cpp clang-tidy/readability/RedundantPreprocessorCheck.cpp clang-tidy/readability/RedundantPreprocessorCheck.h docs/ReleaseNotes.rst docs/clang-tidy/checks/list.rst docs/clang-tidy/checks/readability-redundant-preprocessor.rst test/clang-tidy/readability-redundant-preprocessor-ifdef.cpp test/clang-tidy/readability-redundant-preprocessor.cpp test/clang-tidy/readability-redundant-preprocessor.h Index: test/clang-tidy/readability-redundant-preprocessor.h === --- /dev/null +++ test/clang-tidy/readability-redundant-preprocessor.h @@ -0,0 +1,5 @@ +#ifndef FOO +#ifndef FOO // this would warn, but not in a header +void f(); +#endif +#endif Index: test/clang-tidy/readability-redundant-preprocessor.cpp === --- /dev/null +++ test/clang-tidy/readability-redundant-preprocessor.cpp @@ -0,0 +1,76 @@ +// RUN: %check_clang_tidy %s readability-redundant-preprocessor %t -- -- -I %S + +// Positive testing. +#ifndef FOO +// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant ifndef; consider removing it [readability-redundant-preprocessor] +#ifndef FOO +// CHECK-NOTES: [[@LINE-3]]:2: note: previous ifndef was here +void f(); +#endif +#endif + +// Positive testing of inverted condition. +#ifndef FOO +// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant ifdef; consider removing it [readability-redundant-preprocessor] +#ifdef FOO +// CHECK-NOTES: [[@LINE-3]]:2: note: previous ifndef was here +void f2(); +#endif +#endif + +// Negative testing. +#include "readability-redundant-preprocessor.h" + +#ifndef BAR +void g(); +#endif + +#ifndef FOO +#ifndef BAR +void h(); +#endif +#endif + +#ifndef FOO +#ifdef BAR +void i(); +#endif +#endif + +// Positive #if testing. +#define FOO 4 + +#if FOO == 4 +// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant if; consider removing it [readability-redundant-preprocessor] +#if FOO == 4 +// CHECK-NOTES: [[@LINE-3]]:2: note: previous if was here +void j(); +#endif +#endif + +#if FOO == \ +4 +// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant if; consider removing it [readability-redundant-preprocessor] +#if FOO == \ +4 +// CHECK-NOTES: [[@LINE-5]]:2: note: previous if was here +void j(); +#endif +#endif + +// Negative #if testing. +#define BAR 4 + +#if FOO == 4 +#if BAR == 4 +void k(); +#endif +#endif + +#if FOO == \ +4 +#if BAR == \ +5 +void k(); +#endif +#endif Index: test/clang-tidy/readability-redundant-preprocessor-ifdef.cpp === --- /dev/null +++ test/clang-tidy/readability-redundant-preprocessor-ifdef.cpp @@ -0,0 +1,36 @@ +// RUN: %check_clang_tidy %s readability-redundant-preprocessor %t -- -- -DFOO + +// Positive testing. +#ifdef FOO +// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant ifdef; consider removing it [readability-redundant-preprocessor] +#ifdef FOO +// CHECK-NOTES: [[@LINE-3]]:2: note: previous ifdef was here +void f(); +#endif +#endif + +// Positive testing of inverted condition. +#ifdef FOO +// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant ifndef; consider removing it [readability-redundant-preprocessor] +#ifndef FOO +// CHECK-NOTES: [[@LINE-3]]:2: note: previous ifdef was here +void f2(); +#endif +#endif + +// Negative testing. +#ifdef BAR +void g(); +#endif + +#ifdef FOO +#ifdef BAR +void h(); +#endif +#endif + +#ifdef FOO +#ifndef BAR +void i(); +#endif +#endif Index: docs/clang-tidy/checks/readability-redundant-preprocessor.rst === --- /dev/null +++ docs/clang-tidy/checks/readability-redundant-preprocessor.rst @@ -0,0 +1,61 @@ +.. title:: clang-tidy - readability-redundant-preprocessor + +readability-redundant-preprocessor +== + +Finds potentially redundant preprocessor directives. At the moment the +following cases are detected: + +* `#ifdef` .. `#endif` pairs which are nested inside an outer pair with the + same condition. For example: + +.. code-block:: c++ + + #ifdef FOO + #ifdef FOO // inner ifdef is considered redundant + void f(); + #endif + #endif + +* Same for `#ifndef` .. `#endif` pairs. For example: + +.. code-block:: c++ + + #ifndef FOO + #ifndef FOO // inner ifndef is considered redundant + void f(); + #endif + #endif + +* `#ifndef` inside an `#ifdef` with the same condition: + +.. code-block:: c++ + + #ifdef FOO + #ifndef FOO // inner ifndef is considered redundant + void f(); + #endif + #endif + +* `#ifdef` inside an `#ifndef` with the same condition: + +.. code-block:: c++ + + #ifndef FOO + #ifdef FOO // inner ifdef is considered redundant + void f(); + #endif + #endif + +* `#if` .. `#endif`
[PATCH] D54391: Fix compatibility with z3-4.8.1
jankratochvil created this revision.
jankratochvil added a reviewer: clang.
jankratochvil added a project: clang.
With `z3-4.8.1` as found in Fedora Rawhide (future Fedora 30) as
`z3-devel-4.8.1-1.fc30.x86_64`:
../tools/clang/lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp: In function
'void {anonymous}::Z3ErrorHandler(Z3_context, Z3_error_code)':
../tools/clang/lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp:49:40: error:
'Z3_get_error_msg_ex' was not declared in this scope
llvm::Twine(Z3_get_error_msg_ex(Context, Error)));
^~~
../tools/clang/lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp:49:40: note:
suggested alternative: 'Z3_get_error_msg'
llvm::Twine(Z3_get_error_msg_ex(Context, Error)));
^~~
Z3_get_error_msg
clang CMakeLists.txt has:
find_package(Z3 4.7.1 EXACT)
Despite cmake documentation
claims "//If no such version file is available then the configuration file is
assumed to not be compatible with any requested version.//" in reality
`cmake-3.12.2-1.fc30.x86_64` with `z3-devel-4.8.1-1.fc30.x86_64` still does
find it:
-- Found Z3: /usr/lib64/libz3.so (Required is exact version "4.7.1")
clang is using `Z3_get_error_msg_ex()`, already `/usr/include/z3/z3_api.h` of
`z3-devel-4.7.1-1.fc28.x86_64` states for it "`Retained function name for
backwards compatibility within v4.1`" and it is implemented only as:
Z3_API char const * Z3_get_error_msg_ex(Z3_context c, Z3_error_code err) {
return Z3_get_error_msg(c, err);
}
Repository:
rC Clang
https://reviews.llvm.org/D54391
Files:
lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
Index: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
===
--- lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
+++ lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
@@ -46,7 +46,7 @@
// Function used to report errors
void Z3ErrorHandler(Z3_context Context, Z3_error_code Error) {
llvm::report_fatal_error("Z3 error: " +
- llvm::Twine(Z3_get_error_msg_ex(Context, Error)));
+ llvm::Twine(Z3_get_error_msg(Context, Error)));
}
/// Wrapper for Z3 context
Index: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
===
--- lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
+++ lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
@@ -46,7 +46,7 @@
// Function used to report errors
void Z3ErrorHandler(Z3_context Context, Z3_error_code Error) {
llvm::report_fatal_error("Z3 error: " +
- llvm::Twine(Z3_get_error_msg_ex(Context, Error)));
+ llvm::Twine(Z3_get_error_msg(Context, Error)));
}
/// Wrapper for Z3 context
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D54349: [clang-tidy] new check 'readability-redundant-preprocessor'
vmiklos marked an inline comment as done.
vmiklos added a comment.
> As it stands, I feel like this check is a bit too simplistic to have much
> value, but if you covered some of these other cases, it would alleviate that
> worry for me.
I've now added support for detecting inverted conditions when it comes to
`#ifdef` and `#ifndef` (see extended documentation and testcases). I'll also
check what can I do for `#if`. I think it would not be too hard to detect just
repeated conditions. if I have the full range, then I can look for end of the
first line that does not end with `\`, and that would be a poor man's way to
get the `#if` condition. Let's see if that works in practice.
Comment at: clang-tidy/readability/RedundantPreprocessorCheck.cpp:19-22
+struct Entry {
+ SourceLocation Loc;
+ std::string MacroName;
+};
Szelethus wrote:
> vmiklos wrote:
> > Szelethus wrote:
> > > This is a way too general name in my opinion. Either include comments
> > > that describe it, or rename it (preferably both).
> > Renamed to `PreprocessorCondition`, hope it helps. :-)
> I actually meant it for `Entry`, if you hover your mouse over an inline
> comment, you can see which lines it applies to. Sorry for the confusing
> communication :D
Done now. Nah, it's more about I'm not so fluent with phabricator. :-)
https://reviews.llvm.org/D54349
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D54349: [clang-tidy] new check 'readability-redundant-preprocessor'
vmiklos updated this revision to Diff 173554. https://reviews.llvm.org/D54349 Files: clang-tidy/readability/CMakeLists.txt clang-tidy/readability/ReadabilityTidyModule.cpp clang-tidy/readability/RedundantPreprocessorCheck.cpp clang-tidy/readability/RedundantPreprocessorCheck.h docs/ReleaseNotes.rst docs/clang-tidy/checks/list.rst docs/clang-tidy/checks/readability-redundant-preprocessor.rst test/clang-tidy/readability-redundant-preprocessor-ifdef.cpp test/clang-tidy/readability-redundant-preprocessor.cpp test/clang-tidy/readability-redundant-preprocessor.h Index: test/clang-tidy/readability-redundant-preprocessor.h === --- /dev/null +++ test/clang-tidy/readability-redundant-preprocessor.h @@ -0,0 +1,5 @@ +#ifndef FOO +#ifndef FOO // this would warn, but not in a header +void f(); +#endif +#endif Index: test/clang-tidy/readability-redundant-preprocessor.cpp === --- /dev/null +++ test/clang-tidy/readability-redundant-preprocessor.cpp @@ -0,0 +1,38 @@ +// RUN: %check_clang_tidy %s readability-redundant-preprocessor %t -- -- -I %S + +// Positive testing. +#ifndef FOO +// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant ifndef; consider removing it [readability-redundant-preprocessor] +#ifndef FOO +// CHECK-NOTES: [[@LINE-3]]:2: note: previous ifndef was here +void f(); +#endif +#endif + +// Positive testing of inverted condition. +#ifndef FOO +// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant ifdef; consider removing it [readability-redundant-preprocessor] +#ifdef FOO +// CHECK-NOTES: [[@LINE-3]]:2: note: previous ifndef was here +void f2(); +#endif +#endif + +// Negative testing. +#include "readability-redundant-preprocessor.h" + +#ifndef BAR +void g(); +#endif + +#ifndef FOO +#ifndef BAR +void h(); +#endif +#endif + +#ifndef FOO +#ifdef BAR +void i(); +#endif +#endif Index: test/clang-tidy/readability-redundant-preprocessor-ifdef.cpp === --- /dev/null +++ test/clang-tidy/readability-redundant-preprocessor-ifdef.cpp @@ -0,0 +1,36 @@ +// RUN: %check_clang_tidy %s readability-redundant-preprocessor %t -- -- -DFOO + +// Positive testing. +#ifdef FOO +// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant ifdef; consider removing it [readability-redundant-preprocessor] +#ifdef FOO +// CHECK-NOTES: [[@LINE-3]]:2: note: previous ifdef was here +void f(); +#endif +#endif + +// Positive testing of inverted condition. +#ifdef FOO +// CHECK-NOTES: [[@LINE+1]]:2: warning: nested redundant ifndef; consider removing it [readability-redundant-preprocessor] +#ifndef FOO +// CHECK-NOTES: [[@LINE-3]]:2: note: previous ifdef was here +void f2(); +#endif +#endif + +// Negative testing. +#ifdef BAR +void g(); +#endif + +#ifdef FOO +#ifdef BAR +void h(); +#endif +#endif + +#ifdef FOO +#ifndef BAR +void i(); +#endif +#endif Index: docs/clang-tidy/checks/readability-redundant-preprocessor.rst === --- /dev/null +++ docs/clang-tidy/checks/readability-redundant-preprocessor.rst @@ -0,0 +1,49 @@ +.. title:: clang-tidy - readability-redundant-preprocessor + +readability-redundant-preprocessor +== + +Finds potentially redundant preprocessor directives. At the moment the +following cases are detected: + +* `#ifdef` .. `#endif` pairs which are nested inside an outer pair with the + same condition. For example: + +.. code-block:: c++ + + #ifdef FOO + #ifdef FOO // inner ifdef is considered redundant + void f(); + #endif + #endif + +* Same for `#ifndef` .. `#endif` pairs. For example: + +.. code-block:: c++ + + #ifndef FOO + #ifndef FOO // inner ifndef is considered redundant + void f(); + #endif + #endif + +* `#ifndef` inside an `#ifdef` with the same condition: + +.. code-block:: c++ + + #ifdef FOO + #ifndef FOO // inner ifndef is considered redundant + void f(); + #endif + #endif + +* `#ifdef` inside an `#ifndef` with the same condition: + +.. code-block:: c++ + + #ifndef FOO + #ifdef FOO + void f(); + #endif + #endif + Index: docs/clang-tidy/checks/list.rst === --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -244,6 +244,7 @@ readability-redundant-declaration readability-redundant-function-ptr-dereference readability-redundant-member-init + readability-redundant-preprocessor readability-redundant-smartptr-get readability-redundant-string-cstr readability-redundant-string-init Index: docs/ReleaseNotes.rst === --- docs/ReleaseNotes.rst +++ docs/ReleaseNotes.rst @@ -154,6 +154,11 @@ Detects usage of magic numbers, numbers that are used as literals instead of introduced via constants or symbols. +- New
[PATCH] D53263: Fix places where the return type of a FunctionDecl was being used in place of the function type
bobsayshilol added a comment. In https://reviews.llvm.org/D53263#1294477, @kristina wrote: > Huge apologies, it seems I can't get this to patch cleanly against my fork > and therefore can't test it before committing, which is something I generally > always do. I'll leave it to someone else. Again, huge apologies, hopefully > you won't have to wait too long. No worries! Thanks anyway for taking a look. In https://reviews.llvm.org/D53263#1294488, @JDevlieghere wrote: > The patch applies for me but has quite a few style violations. I'll fix those > up before landing it. Also needs a test (I'll add the one from the > description). Ah I was trying to match the code around it and didn't think to run clang-tidy so I'll keep that in mind next time, and I didn't add a new test because just the assert in `Decl.cpp` was enough to flag up misuses with `check-clang` but I'm definitely not against adding more tests. Thanks for landing! Thanks again everyone! Repository: rL LLVM https://reviews.llvm.org/D53263 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r346608 - [clangd] Make ClangdFuzzer compile again.
Author: d0k
Date: Sun Nov 11 03:09:58 2018
New Revision: 346608
URL: http://llvm.org/viewvc/llvm-project?rev=346608=rev
Log:
[clangd] Make ClangdFuzzer compile again.
Modified:
clang-tools-extra/trunk/clangd/fuzzer/ClangdFuzzer.cpp
Modified: clang-tools-extra/trunk/clangd/fuzzer/ClangdFuzzer.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/fuzzer/ClangdFuzzer.cpp?rev=346608=346607=346608=diff
==
--- clang-tools-extra/trunk/clangd/fuzzer/ClangdFuzzer.cpp (original)
+++ clang-tools-extra/trunk/clangd/fuzzer/ClangdFuzzer.cpp Sun Nov 11 03:09:58
2018
@@ -19,20 +19,23 @@
#include
#include
+using namespace clang::clangd;
+
extern "C" int LLVMFuzzerTestOneInput(uint8_t *data, size_t size) {
if (size == 0)
return 0;
- clang::clangd::JSONOutput Out(llvm::nulls(), llvm::nulls(),
-clang::clangd::Logger::Error, nullptr);
- clang::clangd::CodeCompleteOptions CCOpts;
+ // fmemopen isn't portable, but I think we only run the fuzzer on Linux.
+ std::FILE *In = fmemopen(data, size, "r");
+ auto Transport = newJSONTransport(In, llvm::nulls(),
+/*InMirror=*/nullptr, /*Pretty=*/false,
+/*Style=*/JSONStreamStyle::Standard);
+ CodeCompleteOptions CCOpts;
CCOpts.EnableSnippets = false;
- clang::clangd::ClangdServer::Options Opts;
+ ClangdServer::Options Opts;
// Initialize and run ClangdLSPServer.
- clang::clangd::ClangdLSPServer LSPServer(Out, CCOpts, llvm::None, false,
- Opts);
- // fmemopen isn't portable, but I think we only run the fuzzer on Linux.
- LSPServer.run(fmemopen(data, size, "r"));
+ ClangdLSPServer LSPServer(*Transport, CCOpts, llvm::None, false, Opts);
+ LSPServer.run();
return 0;
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53974: [clang-tidy] new check: bugprone-too-small-loop-variable
whisperity added a comment.
In https://reviews.llvm.org/D53974#1294385, @ztamas wrote:
> I also tested on LLVm code.
> The output is here:
> https://gist.github.com/tzolnai/fe4f23031d3f9fdbdbf1ee38abda00a4
>
> I found 362 warnings.
>
> Around 95% of these warnings are similar to the next example:
>
> /home/zolnai/lohome/llvm/lib/Support/Chrono.cpp:64:24: warning: loop
> variable has narrower type 'unsigned int' than iteration's upper bound
> 'size_t' (aka 'unsigned long') [bugprone-too-small-loop-variable]
> for (unsigned I = 0; I < Style.size(); ++I) {
>
>
> Where the loop variable has an `unsigned int` type while in the loop
> condition it is compared with a container size which has `size_t` type. The
> actual size method can be `std::string::length()` or `array_lengthof()` too.
>
> //[snip snip]//
>
> I can't see similar false positives what LibreOffice code produces.
I am fairly concerned the example with unsigned use for container iteration are
not false positives, just examples of bad happenstance code which never breaks
under real life applications due to uint32_t being good enough but is actually
not type-safe.
Those examples that I kept in my quote are especially bad and should be fixed
eventually...
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53974
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
