[PATCH] D47358: : Implement {un, }synchronized_pool_resource.

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

Quuxplusone created this revision.
Quuxplusone added a reviewer: EricWF.
Herald added a subscriber: cfe-commits.

Split out from https://reviews.llvm.org/D47090.
This patch is based on top of (depends on) https://reviews.llvm.org/D47111, but 
I'm posting it now for review.


Repository:
  rCXX libc++

https://reviews.llvm.org/D47358

Files:
  include/experimental/memory_resource
  src/experimental/memory_resource.cpp
  test/std/experimental/memory/memory.resource.pool/synchronized_pool.pass.cpp
  test/std/experimental/memory/memory.resource.pool/unsynchronized_pool.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/unsynchronized_pool.pass.cpp
===
--- /dev/null
+++ test/std/experimental/memory/memory.resource.pool/unsynchronized_pool.pass.cpp
@@ -0,0 +1,203 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// UNSUPPORTED: c++98, c++03
+
+// 
+
+// class unsynchronized_pool_resource
+
+#include 
+#include 
+#include 
+#include 
+
+#include "count_new.hpp"
+
+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); }
+};
+
+static bool is_aligned_to(void *p, size_t alignment)
+{
+void *p2 = p;
+size_t space = 1;
+void *result = std::align(alignment, 1, p2, space);
+return (result == p);
+}
+
+void test_construction_with_default_resource()
+{
+std::experimental::pmr::memory_resource *expected = std::experimental::pmr::null_memory_resource();
+std::experimental::pmr::set_default_resource(expected);
+{
+std::experimental::pmr::pool_options opts { 0, 0 };
+std::experimental::pmr::unsynchronized_pool_resource r1;
+std::experimental::pmr::unsynchronized_pool_resource r2(opts);
+assert(r1.upstream_resource() == expected);
+assert(r2.upstream_resource() == expected);
+}
+
+expected = std::experimental::pmr::new_delete_resource();
+std::experimental::pmr::set_default_resource(expected);
+{
+std::experimental::pmr::pool_options opts { 1024, 2048 };
+std::experimental::pmr::unsynchronized_pool_resource r1;
+std::experimental::pmr::unsynchronized_pool_resource r2(opts);
+assert(r1.upstream_resource() == expected);
+assert(r2.upstream_resource() == expected);
+}
+}
+
+void test_construction_does_not_allocate()
+{
+// Constructing a unsynchronized_pool_resource should not cause allocations
+// by itself; the resource should wait to allocate until an allocation is
+// requested.
+
+globalMemCounter.reset();
+std::experimental::pmr::set_default_resource(std::experimental::pmr::new_delete_resource());
+
+std::experimental::pmr::unsynchronized_pool_resource r1;
+assert(globalMemCounter.checkNewCalledEq(0));
+
+std::experimental::pmr::unsynchronized_pool_resource r2(std::experimental::pmr::pool_options{ 1024, 2048 });
+assert(globalMemCounter.checkNewCalledEq(0));
+
+std::experimental::pmr::unsynchronized_pool_resource r3(std::experimental::pmr::pool_options{ 1024, 2048 }, std::experimental::pmr::new_delete_resource());
+assert(globalMemCounter.checkNewCalledEq(0));
+}
+
+void test_equality()
+{
+// Same object
+{
+std::experimental::pmr::unsynchronized_pool_resource r1;
+std::experimental::pmr::unsynchronized_pool_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::unsynchronized_pool_resource unsync1;
+std::experimental::pmr::memory_resource & r1 = unsync1;
+  

[PATCH] D47111: : Implement monotonic_buffer_resource.

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

Quuxplusone updated this revision to Diff 148542.
Quuxplusone retitled this revision from "Implement monotonic_buffer_resource in 
" to ": Implement 
monotonic_buffer_resource.".
Quuxplusone added 1 blocking reviewer(s): EricWF.
Quuxplusone added a comment.

Fix one visibility macro.


Repository:
  rCXX libc++

https://reviews.llvm.org/D47111

Files:
  include/experimental/memory_resource
  src/experimental/memory_resource.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic_buffer.pass.cpp

Index: test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic_buffer.pass.cpp
===
--- /dev/null
+++ test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic_buffer.pass.cpp
@@ -0,0 +1,408 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// UNSUPPORTED: c++98, c++03
+
+// 
+
+// class monotonic_buffer_resource
+
+#include 
+#include 
+#include 
+#include 
+
+#include "count_new.hpp"
+
+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); }
+};
+
+struct repointable_resource : public std::experimental::pmr::memory_resource
+{
+std::experimental::pmr::memory_resource *which;
+
+explicit repointable_resource(std::experimental::pmr::memory_resource *res) : which(res) {}
+
+protected:
+virtual void *do_allocate(size_t size, size_t align)
+{ return which->allocate(size, align); }
+
+virtual void do_deallocate(void *p, size_t size, size_t align)
+{ return which->deallocate(p, size, align); }
+
+virtual bool do_is_equal(std::experimental::pmr::memory_resource const ) const noexcept
+{ return which->is_equal(rhs); }
+};
+
+void test_construction_with_default_resource()
+{
+std::experimental::pmr::memory_resource *expected = std::experimental::pmr::null_memory_resource();
+std::experimental::pmr::set_default_resource(expected);
+{
+char buffer[16];
+std::experimental::pmr::monotonic_buffer_resource r1;
+std::experimental::pmr::monotonic_buffer_resource r2(16);
+std::experimental::pmr::monotonic_buffer_resource r3(buffer, sizeof buffer);
+assert(r1.upstream_resource() == expected);
+assert(r2.upstream_resource() == expected);
+assert(r3.upstream_resource() == expected);
+}
+
+expected = std::experimental::pmr::new_delete_resource();
+std::experimental::pmr::set_default_resource(expected);
+{
+char buffer[16];
+std::experimental::pmr::monotonic_buffer_resource r1;
+std::experimental::pmr::monotonic_buffer_resource r2(16);
+std::experimental::pmr::monotonic_buffer_resource r3(buffer, sizeof buffer);
+assert(r1.upstream_resource() == expected);
+assert(r2.upstream_resource() == expected);
+assert(r3.upstream_resource() == expected);
+}
+}
+
+void test_construction_without_buffer()
+{
+// Constructing a monotonic_buffer_resource should not cause allocations
+// by itself; the resource should wait to allocate until an allocation is
+// requested.
+
+globalMemCounter.reset();
+std::experimental::pmr::set_default_resource(std::experimental::pmr::new_delete_resource());
+
+std::experimental::pmr::monotonic_buffer_resource r1;
+assert(globalMemCounter.checkNewCalledEq(0));
+
+std::experimental::pmr::monotonic_buffer_resource r2(1024);
+assert(globalMemCounter.checkNewCalledEq(0));
+
+std::experimental::pmr::monotonic_buffer_resource r3(1024, std::experimental::pmr::new_delete_resource());
+assert(globalMemCounter.checkNewCalledEq(0));
+}
+
+void test_equality()
+{
+// 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;
+

[PATCH] D47354: [CodeGen][Darwin] Set the calling-convention of a thread-local variable initialization function to fix calling-convention mismatch

[John McCall via Phabricator via cfe-commits]

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

LGTM.


Repository:
  rC Clang

https://reviews.llvm.org/D47354



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

[PATCH] D47344: LWG 2843 "Unclear behavior of std::pmr::memory_resource::do_allocate()"

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

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

Oops. If we do get an underaligned result, let's not leak it!


Repository:
  rCXX libc++

https://reviews.llvm.org/D47344

Files:
  src/experimental/memory_resource.cpp


Index: src/experimental/memory_resource.cpp
===
--- src/experimental/memory_resource.cpp
+++ src/experimental/memory_resource.cpp
@@ -8,6 +8,7 @@
 
//===--===//
 
 #include "experimental/memory_resource"
+#include "memory"
 
 #ifndef _LIBCPP_HAS_NO_ATOMIC_HEADER
 #include "atomic"
@@ -23,18 +24,37 @@
 
 // new_delete_resource()
 
+#ifdef _LIBCPP_HAS_NO_ALIGNED_ALLOCATION
+static bool is_aligned_to(void *ptr, size_t align)
+{
+void *p2 = ptr;
+size_t space = 1;
+void *result = _VSTD::align(align, 1, p2, space);
+return (result == ptr);
+}
+#endif
+
 class _LIBCPP_TYPE_VIS __new_delete_memory_resource_imp
 : public memory_resource
 {
 public:
 ~__new_delete_memory_resource_imp() = default;
 
 protected:
 virtual void* do_allocate(size_t __size, size_t __align)
-{ return _VSTD::__libcpp_allocate(__size, __align); /* FIXME */}
+{
+void *result = _VSTD::__libcpp_allocate(__size, __align);
+#ifdef _LIBCPP_HAS_NO_ALIGNED_ALLOCATION
+if (__size != 0 && !is_aligned_to(result, __align)) {
+_VSTD::__libcpp_deallocate(result, __align);
+__throw_bad_alloc();
+}
+#endif
+return result;
+}
 
 virtual void do_deallocate(void * __p, size_t, size_t __align)
-{ _VSTD::__libcpp_deallocate(__p, __align); /* FIXME */ }
+{ _VSTD::__libcpp_deallocate(__p, __align); }
 
 virtual bool do_is_equal(memory_resource const & __other) const _NOEXCEPT
 { return &__other == this; }


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,18 +24,37 @@
 
 // new_delete_resource()
 
+#ifdef _LIBCPP_HAS_NO_ALIGNED_ALLOCATION
+static bool is_aligned_to(void *ptr, size_t align)
+{
+void *p2 = ptr;
+size_t space = 1;
+void *result = _VSTD::align(align, 1, p2, space);
+return (result == ptr);
+}
+#endif
+
 class _LIBCPP_TYPE_VIS __new_delete_memory_resource_imp
 : public memory_resource
 {
 public:
 ~__new_delete_memory_resource_imp() = default;
 
 protected:
 virtual void* do_allocate(size_t __size, size_t __align)
-{ return _VSTD::__libcpp_allocate(__size, __align); /* FIXME */}
+{
+void *result = _VSTD::__libcpp_allocate(__size, __align);
+#ifdef _LIBCPP_HAS_NO_ALIGNED_ALLOCATION
+if (__size != 0 && !is_aligned_to(result, __align)) {
+_VSTD::__libcpp_deallocate(result, __align);
+__throw_bad_alloc();
+}
+#endif
+return result;
+}
 
 virtual void do_deallocate(void * __p, size_t, size_t __align)
-{ _VSTD::__libcpp_deallocate(__p, __align); /* FIXME */ }
+{ _VSTD::__libcpp_deallocate(__p, __align); }
 
 virtual bool do_is_equal(memory_resource const & __other) const _NOEXCEPT
 { return &__other == this; }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[libcxx] r333252 - Add one more test for optional

[Marshall Clow via cfe-commits]

Author: marshall
Date: Thu May 24 19:22:54 2018
New Revision: 333252

URL: http://llvm.org/viewvc/llvm-project?rev=333252=rev
Log:
Add one more test for optional

Modified:

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

Modified: 
libcxx/trunk/test/std/utilities/optional/optional.object/optional.object.ctor/deduct.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/utilities/optional/optional.object/optional.object.ctor/deduct.pass.cpp?rev=333252=333251=333252=diff
==
--- 
libcxx/trunk/test/std/utilities/optional/optional.object/optional.object.ctor/deduct.pass.cpp
 (original)
+++ 
libcxx/trunk/test/std/utilities/optional/optional.object/optional.object.ctor/deduct.pass.cpp
 Thu May 24 19:22:54 2018
@@ -25,20 +25,29 @@ int main()
 {  
 //  Test the explicit deduction guides
 {
-// optional(T)
+//  optional(T)
 std::optional opt(5);
 static_assert(std::is_same_v, "");
-   assert(static_cast(opt));
-   assert(*opt == 5);
+assert(static_cast(opt));
+assert(*opt == 5);
 }
 
 {
-// optional(T)
+//  optional(T)
 std::optional opt(A{});
 static_assert(std::is_same_v, "");
-   assert(static_cast(opt));
+assert(static_cast(opt));
 }
 
 //  Test the implicit deduction guides
 
+{
+//  optional(const optional &);
+std::optional source('A');
+std::optional opt(source);
+static_assert(std::is_same_v, "");
+assert(static_cast(opt) == static_cast(source));
+assert(*opt == *source);
+}
+
 }


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

[PATCH] D47357: [Driver] Rename DefaultTargetTriple to TargetTriple

[Petr Hosek via Phabricator via cfe-commits]

phosek created this revision.
phosek added reviewers: hans, rnk, echristo.
Herald added a subscriber: cfe-commits.

While this value is initialized with the DefaultTargetTriple, it
can be later overriden using the -target flag so TargetTriple is
a more accurate name. This change also provides an accessor which
could be accessed from ToolChain implementations.


Repository:
  rC Clang

https://reviews.llvm.org/D47357

Files:
  clang/include/clang/Driver/Driver.h
  clang/lib/Driver/Driver.cpp

Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -84,7 +84,7 @@
 using namespace clang;
 using namespace llvm::opt;
 
-Driver::Driver(StringRef ClangExecutable, StringRef DefaultTargetTriple,
+Driver::Driver(StringRef ClangExecutable, StringRef TargetTriple,
DiagnosticsEngine ,
IntrusiveRefCntPtr VFS)
 : Opts(createDriverOptTable()), Diags(Diags), VFS(std::move(VFS)),
@@ -94,7 +94,7 @@
   CCPrintOptionsFilename(nullptr), CCPrintHeadersFilename(nullptr),
   CCLogDiagnosticsFilename(nullptr), CCCPrintBindings(false),
   CCPrintOptions(false), CCPrintHeaders(false), CCLogDiagnostics(false),
-  CCGenDiagnostics(false), DefaultTargetTriple(DefaultTargetTriple),
+  CCGenDiagnostics(false), TargetTriple(TargetTriple),
   CCCGenericGCCName(""), Saver(Alloc), CheckInputsExist(true),
   CCCUsePCH(true), GenReproducer(false),
   SuppressMissingInputWarning(false) {
@@ -388,14 +388,14 @@
 /// This routine provides the logic to compute a target triple from various
 /// args passed to the driver and the default triple string.
 static llvm::Triple computeTargetTriple(const Driver ,
-StringRef DefaultTargetTriple,
+StringRef TargetTriple,
 const ArgList ,
 StringRef DarwinArchName = "") {
   // FIXME: Already done in Compilation *Driver::BuildCompilation
   if (const Arg *A = Args.getLastArg(options::OPT_target))
-DefaultTargetTriple = A->getValue();
+TargetTriple = A->getValue();
 
-  llvm::Triple Target(llvm::Triple::normalize(DefaultTargetTriple));
+  llvm::Triple Target(llvm::Triple::normalize(TargetTriple));
 
   // Handle Apple-specific options available here.
   if (Target.isOSBinFormatMachO()) {
@@ -941,19 +941,19 @@
   GenReproducer = Args.hasFlag(options::OPT_gen_reproducer,
options::OPT_fno_crash_diagnostics,
!!::getenv("FORCE_CLANG_DIAGNOSTICS_CRASH"));
-  // FIXME: DefaultTargetTriple is used by the target-prefixed calls to as/ld
+  // FIXME: TargetTriple is used by the target-prefixed calls to as/ld
   // and getToolChain is const.
   if (IsCLMode()) {
 // clang-cl targets MSVC-style Win32.
-llvm::Triple T(DefaultTargetTriple);
+llvm::Triple T(TargetTriple);
 T.setOS(llvm::Triple::Win32);
 T.setVendor(llvm::Triple::PC);
 T.setEnvironment(llvm::Triple::MSVC);
 T.setObjectFormat(llvm::Triple::COFF);
-DefaultTargetTriple = T.str();
+TargetTriple = T.str();
   }
   if (const Arg *A = Args.getLastArg(options::OPT_target))
-DefaultTargetTriple = A->getValue();
+TargetTriple = A->getValue();
   if (const Arg *A = Args.getLastArg(options::OPT_ccc_install_dir))
 Dir = InstalledDir = A->getValue();
   for (const Arg *A : Args.filtered(options::OPT_B)) {
@@ -1001,7 +1001,7 @@
 
   // Owned by the host.
   const ToolChain  = getToolChain(
-  *UArgs, computeTargetTriple(*this, DefaultTargetTriple, *UArgs));
+  *UArgs, computeTargetTriple(*this, TargetTriple, *UArgs));
 
   // The compilation takes ownership of Args.
   Compilation *C = new Compilation(*this, TC, UArgs.release(), TranslatedArgs,
@@ -3665,7 +3665,7 @@
 
 if (!ArchName.empty())
   TC = (C.getArgs(),
- computeTargetTriple(*this, DefaultTargetTriple,
+ computeTargetTriple(*this, TargetTriple,
  C.getArgs(), ArchName));
 else
   TC = ();
@@ -3834,7 +3834,7 @@
 }
 
 const char *Driver::getDefaultImageName() const {
-  llvm::Triple Target(llvm::Triple::normalize(DefaultTargetTriple));
+  llvm::Triple Target(llvm::Triple::normalize(TargetTriple));
   return Target.isOSWindows() ? "a.exe" : "a.out";
 }
 
@@ -4073,14 +4073,14 @@
 void Driver::generatePrefixedToolNames(
 StringRef Tool, const ToolChain ,
 SmallVectorImpl ) const {
-  // FIXME: Needs a better variable than DefaultTargetTriple
-  Names.emplace_back((DefaultTargetTriple + "-" + Tool).str());
+  // FIXME: Needs a better variable than TargetTriple
+  Names.emplace_back((TargetTriple + "-" + Tool).str());
   Names.emplace_back(Tool);
 
   // Allow the discovery of tools prefixed with LLVM's default target triple.
-  std::string 

[libcxx] r333251 - Add deduction guides for optional

[Marshall Clow via cfe-commits]

Author: marshall
Date: Thu May 24 19:08:49 2018
New Revision: 333251

URL: http://llvm.org/viewvc/llvm-project?rev=333251=rev
Log:
Add deduction guides for optional

Added:

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

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

Modified: libcxx/trunk/include/optional
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/optional?rev=333251=333250=333251=diff
==
--- libcxx/trunk/include/optional (original)
+++ libcxx/trunk/include/optional Thu May 24 19:08:49 2018
@@ -139,6 +139,10 @@ namespace std {
   private:
 T *val; // exposition only
   };
+
+template
+  optional(T) -> optional;
+
 } // namespace std
 
 */
@@ -1003,6 +1007,11 @@ private:
 }
 };
 
+#ifndef _LIBCPP_HAS_NO_DEDUCTION_GUIDES
+template
+optional(T) -> optional;
+#endif
+
 // Comparisons between optionals
 template 
 _LIBCPP_INLINE_VISIBILITY constexpr

Added: 
libcxx/trunk/test/std/utilities/optional/optional.object/optional.object.ctor/deduct.fail.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/utilities/optional/optional.object/optional.object.ctor/deduct.fail.cpp?rev=333251=auto
==
--- 
libcxx/trunk/test/std/utilities/optional/optional.object/optional.object.ctor/deduct.fail.cpp
 (added)
+++ 
libcxx/trunk/test/std/utilities/optional/optional.object/optional.object.ctor/deduct.fail.cpp
 Thu May 24 19:08:49 2018
@@ -0,0 +1,38 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// 
+// UNSUPPORTED: c++98, c++03, c++11, c++14
+// UNSUPPORTED: libcpp-no-deduction-guides
+
+
+// template
+//   optional(T) -> optional;
+
+
+#include 
+#include 
+
+struct A {};
+
+int main()
+{  
+//  Test the explicit deduction guides
+
+//  Test the implicit deduction guides
+{
+//  optional()
+std::optional opt;   // expected-error {{no viable constructor or 
deduction guide for deduction of template arguments of 'optional'}}
+}
+
+{
+//  optional(nullopt_t)
+std::optional opt(std::nullopt);   // expected-error-re@optional:* 
{{static_assert failed{{.*}} "instantiation of optional with nullopt_t is 
ill-formed"}}
+}
+}

Added: 
libcxx/trunk/test/std/utilities/optional/optional.object/optional.object.ctor/deduct.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/utilities/optional/optional.object/optional.object.ctor/deduct.pass.cpp?rev=333251=auto
==
--- 
libcxx/trunk/test/std/utilities/optional/optional.object/optional.object.ctor/deduct.pass.cpp
 (added)
+++ 
libcxx/trunk/test/std/utilities/optional/optional.object/optional.object.ctor/deduct.pass.cpp
 Thu May 24 19:08:49 2018
@@ -0,0 +1,44 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// 
+// UNSUPPORTED: c++98, c++03, c++11, c++14
+// UNSUPPORTED: libcpp-no-deduction-guides
+
+
+// template
+//   optional(T) -> optional;
+
+
+#include 
+#include 
+
+struct A {};
+
+int main()
+{  
+//  Test the explicit deduction guides
+{
+// optional(T)
+std::optional opt(5);
+static_assert(std::is_same_v, "");
+   assert(static_cast(opt));
+   assert(*opt == 5);
+}
+
+{
+// optional(T)
+std::optional opt(A{});
+static_assert(std::is_same_v, "");
+   assert(static_cast(opt));
+}
+
+//  Test the implicit deduction guides
+
+}


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

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

[Shiva Chen via Phabricator via cfe-commits]

shiva0217 added a comment.

In https://reviews.llvm.org/D44888#1109361, @asb wrote:

> This is looking good to me, just needs an update to address this 
>  request for a test in 
> riscv-features.c that demonstrates the default +relax/-relax setting.


Hi Alex. I added the testing line on https://reviews.llvm.org/D47127 which is 
the patch we turn on relaxation as default. Do you think it's ok?


Repository:
  rL LLVM

https://reviews.llvm.org/D44888



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

[PATCH] D47355: [CMake] Allow specifying extra dependencies of bootstrap stage

[Petr Hosek via Phabricator via cfe-commits]

phosek added inline comments.



Comment at: clang/CMakeLists.txt:586
 
+  if(CLANG_BOOTSTRAP_EXTRA_COMPONENTS)
+add_dependencies(clang-bootstrap-deps ${CLANG_BOOTSTRAP_EXTRA_COMPONENTS})

I don't know if `COMPONENTS` is the best term, this could also possibly be 
`DEPS`.


Repository:
  rC Clang

https://reviews.llvm.org/D47355



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

[PATCH] D47356: [CMake] Use libc++ and compiler-rt for bootstrap Fuchsia Clang

[Petr Hosek via Phabricator via cfe-commits]

phosek created this revision.
phosek added reviewers: beanz, jakehehrlich, mcgrathr.
Herald added subscribers: cfe-commits, mgorny, dberris.
Herald added a reviewer: EricWF.

We want to build the second stage compiler with libc++ and compiler-rt,
also include builtins and runtimes into extra bootstrap components to
ensure these get built.


Repository:
  rC Clang

https://reviews.llvm.org/D47356

Files:
  clang/cmake/caches/Fuchsia.cmake


Index: clang/cmake/caches/Fuchsia.cmake
===
--- clang/cmake/caches/Fuchsia.cmake
+++ clang/cmake/caches/Fuchsia.cmake
@@ -21,6 +21,9 @@
   set(BOOTSTRAP_LLVM_ENABLE_LLD ON CACHE BOOL "")
 endif()
 
+set(CLANG_DEFAULT_CXX_STDLIB libc++ CACHE STRING "")
+set(CLANG_DEFAULT_RTLIB compiler-rt CACHE STRING "")
+
 if(APPLE)
   set(COMPILER_RT_ENABLE_IOS OFF CACHE BOOL "")
   set(COMPILER_RT_ENABLE_TVOS OFF CACHE BOOL "")
@@ -50,6 +53,10 @@
 
 # Setup the bootstrap build.
 set(CLANG_ENABLE_BOOTSTRAP ON CACHE BOOL "")
+set(CLANG_BOOTSTRAP_EXTRA_COMPONENTS
+  builtins
+  runtimes
+  CACHE STRING "")
 set(CLANG_BOOTSTRAP_CMAKE_ARGS
   ${EXTRA_ARGS}
   -C ${CMAKE_CURRENT_LIST_DIR}/Fuchsia-stage2.cmake


Index: clang/cmake/caches/Fuchsia.cmake
===
--- clang/cmake/caches/Fuchsia.cmake
+++ clang/cmake/caches/Fuchsia.cmake
@@ -21,6 +21,9 @@
   set(BOOTSTRAP_LLVM_ENABLE_LLD ON CACHE BOOL "")
 endif()
 
+set(CLANG_DEFAULT_CXX_STDLIB libc++ CACHE STRING "")
+set(CLANG_DEFAULT_RTLIB compiler-rt CACHE STRING "")
+
 if(APPLE)
   set(COMPILER_RT_ENABLE_IOS OFF CACHE BOOL "")
   set(COMPILER_RT_ENABLE_TVOS OFF CACHE BOOL "")
@@ -50,6 +53,10 @@
 
 # Setup the bootstrap build.
 set(CLANG_ENABLE_BOOTSTRAP ON CACHE BOOL "")
+set(CLANG_BOOTSTRAP_EXTRA_COMPONENTS
+  builtins
+  runtimes
+  CACHE STRING "")
 set(CLANG_BOOTSTRAP_CMAKE_ARGS
   ${EXTRA_ARGS}
   -C ${CMAKE_CURRENT_LIST_DIR}/Fuchsia-stage2.cmake
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47355: [CMake] Allow specifying extra dependencies of bootstrap stage

[Petr Hosek via Phabricator via cfe-commits]

phosek created this revision.
phosek added a reviewer: beanz.
Herald added subscribers: cfe-commits, mgorny.

This allows adding additional bootstrap dependencies to the bootstrap
compiler that may be needed by later stages.


Repository:
  rC Clang

https://reviews.llvm.org/D47355

Files:
  clang/CMakeLists.txt


Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -583,6 +583,10 @@
 endif()
   endif()
 
+  if(CLANG_BOOTSTRAP_EXTRA_COMPONENTS)
+add_dependencies(clang-bootstrap-deps ${CLANG_BOOTSTRAP_EXTRA_COMPONENTS})
+  endif()
+
   add_custom_target(${NEXT_CLANG_STAGE}-clear
 DEPENDS ${CMAKE_CURRENT_BINARY_DIR}/${NEXT_CLANG_STAGE}-cleared
 )


Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -583,6 +583,10 @@
 endif()
   endif()
 
+  if(CLANG_BOOTSTRAP_EXTRA_COMPONENTS)
+add_dependencies(clang-bootstrap-deps ${CLANG_BOOTSTRAP_EXTRA_COMPONENTS})
+  endif()
+
   add_custom_target(${NEXT_CLANG_STAGE}-clear
 DEPENDS ${CMAKE_CURRENT_BINARY_DIR}/${NEXT_CLANG_STAGE}-cleared
 )
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47354: [CodeGen][Darwin] Set the calling-convention of a thread-local variable initialization function to fix calling-convention mismatch

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak created this revision.
ahatanak added reviewers: rjmccall, t.p.northover.

There is a bug in IRGen where the calling convention of initialization 
functions for thread-local static members of c++ template classes isn't set. 
This caused InstCombine to remove a call to an initialization function because 
of the mismatch in the calling conventions between the initialization function 
and the call.

For example, when the following piece of code (this is in 
test/CodeGenCXX/cxx11-thread-local.cpp) is compiled,

  int g();
  template struct V { static thread_local int m; };
  template thread_local int V::m = g();
  int e = V::m;

IRGen generates the following IR:

  @_ZTHN1VIiE1mE = linkonce_odr alias void (), void ()* @__cxx_global_var_init.9
  
  define weak_odr hidden cxx_fast_tlscc i32* @_ZTWN1VIiE1mE() #2 {
call cxx_fast_tlscc void @_ZTHN1VIiE1mE() ; this calls 
@__cxx_global_var_init.9
ret i32* @_ZN1VIiE1mE
  }
  
  ; this function is missing the calling convention "cxx_fast_tlscc".
  
  define internal void @__cxx_global_var_init.9() #0 section 
"__TEXT,__StaticInit,regular,pure_instructions" {
...
  }

To fix the bug, this patch sets the calling convention of the initialization 
functions to 'cxx_fast_tlscc'. Alternatively, I could remove 'cxx_fast_tlscc' 
from the call instruction, but I suppose we don't want to do so for performance 
reasons.

rdar://problem/40447463


Repository:
  rC Clang

https://reviews.llvm.org/D47354

Files:
  lib/CodeGen/ItaniumCXXABI.cpp
  test/CodeGenCXX/cxx11-thread-local.cpp


Index: test/CodeGenCXX/cxx11-thread-local.cpp
===
--- test/CodeGenCXX/cxx11-thread-local.cpp
+++ test/CodeGenCXX/cxx11-thread-local.cpp
@@ -166,7 +166,8 @@
 // DARWIN: call cxx_fast_tlscc void @_ZTHN1XIiE1mE()
 // CHECK: ret {{.*}}* @_ZN1XIiE1mE
 
-// CHECK: define internal {{.*}} @[[VF_M_INIT]]()
+// LINUX: define internal void @[[VF_M_INIT]]()
+// DARWIN: define internal cxx_fast_tlscc void @[[VF_M_INIT]]()
 // LINUX-SAME: comdat($_ZN1VIfE1mE)
 // DARWIN-NOT: comdat
 // CHECK: load i8, i8* bitcast (i64* @_ZGVN1VIfE1mE to i8*)
@@ -178,7 +179,8 @@
 // CHECK: store i64 1, i64* @_ZGVN1VIfE1mE
 // CHECK: br label
 
-// CHECK: define internal {{.*}} @[[XF_M_INIT]]()
+// LINUX: define internal void @[[XF_M_INIT]]()
+// DARWIN: define internal cxx_fast_tlscc void @[[XF_M_INIT]]()
 // LINUX-SAME: comdat($_ZN1XIfE1mE)
 // DARWIN-NOT: comdat
 // CHECK: load i8, i8* bitcast (i64* @_ZGVN1XIfE1mE to i8*)
@@ -268,7 +270,8 @@
 // LINUX-LABEL: define internal i32* @_ZTWN12_GLOBAL__N_16anon_iE()
 // DARWIN-LABEL: define internal cxx_fast_tlscc i32* 
@_ZTWN12_GLOBAL__N_16anon_iE()
 
-// CHECK: define internal {{.*}} @[[V_M_INIT]]()
+// LINUX: define internal void @[[V_M_INIT]]()
+// DARWIN: define internal cxx_fast_tlscc void @[[V_M_INIT]]()
 // LINUX-SAME: comdat($_ZN1VIiE1mE)
 // DARWIN-NOT: comdat
 // CHECK: load i8, i8* bitcast (i64* @_ZGVN1VIiE1mE to i8*)
@@ -280,7 +283,8 @@
 // CHECK: store i64 1, i64* @_ZGVN1VIiE1mE
 // CHECK: br label
 
-// CHECK: define internal {{.*}} @[[X_M_INIT]]()
+// LINUX: define internal void @[[X_M_INIT]]()
+// DARWIN: define internal cxx_fast_tlscc void @[[X_M_INIT]]()
 // LINUX-SAME: comdat($_ZN1XIiE1mE)
 // DARWIN-NOT: comdat
 // CHECK: load i8, i8* bitcast (i64* @_ZGVN1XIiE1mE to i8*)
Index: lib/CodeGen/ItaniumCXXABI.cpp
===
--- lib/CodeGen/ItaniumCXXABI.cpp
+++ lib/CodeGen/ItaniumCXXABI.cpp
@@ -2450,8 +2450,12 @@
 if (InitIsInitFunc) {
   if (Init) {
 llvm::CallInst *CallVal = Builder.CreateCall(Init);
-if (isThreadWrapperReplaceable(VD, CGM))
+if (isThreadWrapperReplaceable(VD, CGM)) {
   CallVal->setCallingConv(llvm::CallingConv::CXX_FAST_TLS);
+  llvm::Function *Fn =
+  
cast(cast(Init)->getAliasee());
+  Fn->setCallingConv(llvm::CallingConv::CXX_FAST_TLS);
+}
   }
 } else {
   // Don't know whether we have an init function. Call it if it exists.


Index: test/CodeGenCXX/cxx11-thread-local.cpp
===
--- test/CodeGenCXX/cxx11-thread-local.cpp
+++ test/CodeGenCXX/cxx11-thread-local.cpp
@@ -166,7 +166,8 @@
 // DARWIN: call cxx_fast_tlscc void @_ZTHN1XIiE1mE()
 // CHECK: ret {{.*}}* @_ZN1XIiE1mE
 
-// CHECK: define internal {{.*}} @[[VF_M_INIT]]()
+// LINUX: define internal void @[[VF_M_INIT]]()
+// DARWIN: define internal cxx_fast_tlscc void @[[VF_M_INIT]]()
 // LINUX-SAME: comdat($_ZN1VIfE1mE)
 // DARWIN-NOT: comdat
 // CHECK: load i8, i8* bitcast (i64* @_ZGVN1VIfE1mE to i8*)
@@ -178,7 +179,8 @@
 // CHECK: store i64 1, i64* @_ZGVN1VIfE1mE
 // CHECK: br label
 
-// CHECK: define internal {{.*}} @[[XF_M_INIT]]()
+// LINUX: define internal void @[[XF_M_INIT]]()
+// DARWIN: define internal cxx_fast_tlscc void @[[XF_M_INIT]]()
 // LINUX-SAME: comdat($_ZN1XIfE1mE)
 // DARWIN-NOT: comdat
 

[PATCH] D47340: [ClangDiagnostics] Silence warning about fallthrough after PrintFatalError

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 updated this revision to Diff 148523.
xbolva00 added a comment.

Add correct NORETURN attribute


https://reviews.llvm.org/D47340

Files:
  utils/TableGen/ClangDiagnosticsEmitter.cpp


Index: utils/TableGen/ClangDiagnosticsEmitter.cpp
===
--- utils/TableGen/ClangDiagnosticsEmitter.cpp
+++ utils/TableGen/ClangDiagnosticsEmitter.cpp
@@ -633,7 +633,7 @@
 return It->second.Root;
   }
 
-  void PrintFatalError(llvm::Twine const ) const {
+  LLVM_ATTRIBUTE_NORETURN void PrintFatalError(llvm::Twine const ) const {
 assert(EvaluatingRecord && "not evaluating a record?");
 llvm::PrintFatalError(EvaluatingRecord->getLoc(), Msg);
   }


Index: utils/TableGen/ClangDiagnosticsEmitter.cpp
===
--- utils/TableGen/ClangDiagnosticsEmitter.cpp
+++ utils/TableGen/ClangDiagnosticsEmitter.cpp
@@ -633,7 +633,7 @@
 return It->second.Root;
   }
 
-  void PrintFatalError(llvm::Twine const ) const {
+  LLVM_ATTRIBUTE_NORETURN void PrintFatalError(llvm::Twine const ) const {
 assert(EvaluatingRecord && "not evaluating a record?");
 llvm::PrintFatalError(EvaluatingRecord->getLoc(), Msg);
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47340: [ClangDiagnostics] Silence warning about fallthrough after PrintFatalError

[Richard Trieu via Phabricator via cfe-commits]

rtrieu added a comment.

Instead of adding a break, you should add a noreturn attribute to the 
PrintFatalError function.  LLVM has the macro LLVM_ATTRIBUTE_NORETURN to do 
this.  You can confirm that PrintFatalError is a noreturn function by seeing 
that it unconditionally calls llvm::PrintFatalError which itself is attributed 
with LLVM_ATTRIBUTE_NORETURN.


Repository:
  rC Clang

https://reviews.llvm.org/D47340



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

[PATCH] D47225: Add nonnull; use it for atomics

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

Ping! clang side landed in https://reviews.llvm.org/rL333246


Repository:
  rCXX libc++

https://reviews.llvm.org/D47225



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

[PATCH] D47229: Make atomic non-member functions as nonnull

[JF Bastien via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL333246: Make atomic non-member functions as nonnull 
(authored by jfb, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D47229

Files:
  cfe/trunk/lib/Sema/SemaChecking.cpp
  cfe/trunk/test/Sema/atomic-ops.c

Index: cfe/trunk/test/Sema/atomic-ops.c
===
--- cfe/trunk/test/Sema/atomic-ops.c
+++ cfe/trunk/test/Sema/atomic-ops.c
@@ -531,8 +531,80 @@
 }
 
 void nullPointerWarning(_Atomic(int) *Ap, int *p, int val) {
-  // The 'expected' pointer shouldn't be NULL.
-  (void)__c11_atomic_compare_exchange_strong(Ap, NULL, val, memory_order_relaxed, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
-  (void)atomic_compare_exchange_weak(Ap, ((void*)0), val); // expected-warning {{null passed to a callee that requires a non-null argument}}
-  (void)__atomic_compare_exchange_n(p, NULL, val, 0, memory_order_relaxed, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  volatile _Atomic(int) vai;
+  _Atomic(int) ai;
+  volatile int vi = 42;
+  int i = 42;
+
+  __c11_atomic_init((volatile _Atomic(int)*)0, 42); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  __c11_atomic_init((_Atomic(int)*)0, 42); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  __c11_atomic_store((volatile _Atomic(int)*)0, 42, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  __c11_atomic_store((_Atomic(int)*)0, 42, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  (void)__c11_atomic_load((volatile _Atomic(int)*)0, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  (void)__c11_atomic_load((_Atomic(int)*)0, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  (void)__c11_atomic_exchange((volatile _Atomic(int)*)0, 42, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  (void)__c11_atomic_exchange((_Atomic(int)*)0, 42, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  (void)__c11_atomic_compare_exchange_weak((volatile _Atomic(int)*)0, , 42, memory_order_relaxed, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  (void)__c11_atomic_compare_exchange_weak((_Atomic(int)*)0, , 42, memory_order_relaxed, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  (void)__c11_atomic_compare_exchange_weak(, (int*)0, 42, memory_order_relaxed, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  (void)__c11_atomic_compare_exchange_weak(, (int*)0, 42, memory_order_relaxed, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  (void)__c11_atomic_compare_exchange_strong((volatile _Atomic(int)*)0, , 42, memory_order_relaxed, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  (void)__c11_atomic_compare_exchange_strong((_Atomic(int)*)0, , 42, memory_order_relaxed, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  (void)__c11_atomic_compare_exchange_strong(, (int*)0, 42, memory_order_relaxed, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  (void)__c11_atomic_compare_exchange_strong(, (int*)0, 42, memory_order_relaxed, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  (void)__c11_atomic_fetch_add((volatile _Atomic(int)*)0, 42, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  (void)__c11_atomic_fetch_add((_Atomic(int)*)0, 42, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  (void)__c11_atomic_fetch_sub((volatile _Atomic(int)*)0, 42, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  (void)__c11_atomic_fetch_sub((_Atomic(int)*)0, 42, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  (void)__c11_atomic_fetch_and((volatile _Atomic(int)*)0, 42, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  (void)__c11_atomic_fetch_and((_Atomic(int)*)0, 42, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a 

r333246 - Make atomic non-member functions as nonnull

[JF Bastien via cfe-commits]

Author: jfb
Date: Thu May 24 17:07:09 2018
New Revision: 333246

URL: http://llvm.org/viewvc/llvm-project?rev=333246=rev
Log:
Make atomic non-member functions as nonnull

Summary:
As a companion to libc++ patch https://reviews.llvm.org/D47225, mark builtin 
atomic non-member functions which accept pointers as nonnull.

The atomic non-member functions accept pointers to std::atomic / 
std::atomic_flag as well as to the non-atomic value. These are all dereferenced 
unconditionally when lowered, and therefore will fault if null. It's a tiny 
gotcha for new users, especially when they pass in NULL as expected value 
(instead of passing a pointer to a NULL value).



Reviewers: arphaman

Subscribers: aheejin, cfe-commits

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

Modified:
cfe/trunk/lib/Sema/SemaChecking.cpp
cfe/trunk/test/Sema/atomic-ops.c

Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=333246=333245=333246=diff
==
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Thu May 24 17:07:09 2018
@@ -3451,9 +3451,10 @@ ExprResult Sema::SemaAtomicOpsOverloaded
 return ExprError();
   }
 
-  // atomic_fetch_or takes a pointer to a volatile 'A'.  We shouldn't let the
-  // volatile-ness of the pointee-type inject itself into the result or the
-  // other operands. Similarly atomic_load can take a pointer to a const 'A'.
+  // All atomic operations have an overload which takes a pointer to a volatile
+  // 'A'.  We shouldn't let the volatile-ness of the pointee-type inject itself
+  // into the result or the other operands. Similarly atomic_load takes a
+  // pointer to a const 'A'.
   ValType.removeLocalVolatile();
   ValType.removeLocalConst();
   QualType ResultType = ValType;
@@ -3466,16 +3467,27 @@ ExprResult Sema::SemaAtomicOpsOverloaded
   // The type of a parameter passed 'by value'. In the GNU atomics, such
   // arguments are actually passed as pointers.
   QualType ByValType = ValType; // 'CP'
-  if (!IsC11 && !IsN)
+  bool IsPassedByAddress = false;
+  if (!IsC11 && !IsN) {
 ByValType = Ptr->getType();
+IsPassedByAddress = true;
+  }
 
-  // The first argument --- the pointer --- has a fixed type; we
-  // deduce the types of the rest of the arguments accordingly.  Walk
-  // the remaining arguments, converting them to the deduced value type.
-  for (unsigned i = 1; i != TheCall->getNumArgs(); ++i) {
+  // The first argument's non-CV pointer type is used to deduce the type of
+  // subsequent arguments, except for:
+  //  - weak flag (always converted to bool)
+  //  - memory order (always converted to int)
+  //  - scope  (always converted to int)
+  for (unsigned i = 0; i != TheCall->getNumArgs(); ++i) {
 QualType Ty;
 if (i < NumVals[Form] + 1) {
   switch (i) {
+  case 0:
+// The first argument is always a pointer. It has a fixed type.
+// It is always dereferenced, a nullptr is undefined.
+CheckNonNullArgument(*this, TheCall->getArg(i), DRE->getLocStart());
+// Nothing else to do: we already know all we want about this pointer.
+continue;
   case 1:
 // The second argument is the non-atomic operand. For arithmetic, this
 // is always passed by value, and for a compare_exchange it is always
@@ -3484,14 +3496,16 @@ ExprResult Sema::SemaAtomicOpsOverloaded
 assert(Form != Load);
 if (Form == Init || (Form == Arithmetic && ValType->isIntegerType()))
   Ty = ValType;
-else if (Form == Copy || Form == Xchg)
+else if (Form == Copy || Form == Xchg) {
+  if (IsPassedByAddress)
+// The value pointer is always dereferenced, a nullptr is 
undefined.
+CheckNonNullArgument(*this, TheCall->getArg(i), 
DRE->getLocStart());
   Ty = ByValType;
-else if (Form == Arithmetic)
+} else if (Form == Arithmetic)
   Ty = Context.getPointerDiffType();
 else {
   Expr *ValArg = TheCall->getArg(i);
-  // Treat this argument as _Nonnull as we want to show a warning if
-  // NULL is passed into it.
+  // The value pointer is always dereferenced, a nullptr is undefined.
   CheckNonNullArgument(*this, ValArg, DRE->getLocStart());
   LangAS AS = LangAS::Default;
   // Keep address space of non-atomic pointer type.
@@ -3504,8 +3518,10 @@ ExprResult Sema::SemaAtomicOpsOverloaded
 }
 break;
   case 2:
-// The third argument to compare_exchange / GNU exchange is a
-// (pointer to a) desired value.
+// The third argument to compare_exchange / GNU exchange is the desired
+// value, either by-value (for the *_n variant) or as a pointer.
+if (!IsN)
+  CheckNonNullArgument(*this, TheCall->getArg(i), 

Advice for a Newcoming Subscriber

[Tanveer Salim via cfe-commits]

Dear All LLVM list members,

Hello to all of you. As I am a brand new subscriber, it is easy to get lost
in the plethora of open source projects I may get started in!

For now, I am mostly interested in optimizing Clang's performance.

I just have three questions:

1. What have been the toughest, most recurring problems of the Clang
Project for the past year of 2018?

2. What book resources on ANSI C and LLVM Assembly Language would you
recommend for a novice open source contributor to the Clang and LLVM
projects?

3. And most importantly, what personal advice did you wish you were aware
of before attempting to complete LLVM Open Source Projects like Clang?

For now, my skills are as far as the following:

1. More than one year's experience in Java

2. Beginner in C: Currently reading through the third chapter of K, 2nd
edition.

3. Knowledge of introductory C++: Taking an introductory course at
university).
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added inline comments.



Comment at: include/clang/Basic/TargetInfo.h:382
+// enough bits to fit the minumum.
+if (getIntWidth() < MinSignedAccumDataBits)
+  return getLongWidth();

ebevhan wrote:
> I'm not sure I agree with this interpretation. It's simply not correct to 
> consider 'short' the 'underlying type' of 'short _Accum', 'int' the 
> 'underlying type' of '_Accum', etc. They are wholly independent and should 
> have separate settings altogether.
> 
> Asserting/ensuring that a target has set an invalid width for its types 
> should be done separately. This currently feels a bit like the TargetInfo is 
> guessing.
> 
> (For the record, the reason I'm requesting this change is because this 
> implementation does not work for us downstream.)
You're right. They should be different types. I was stuck in the mindset that 
`short _Accum` should be tied to `short`, `_Accum` be tied to `int`, etc.


Repository:
  rC Clang

https://reviews.llvm.org/D46084



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

[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type

[Leonard Chan via Phabricator via cfe-commits]

leonardchan updated this revision to Diff 148506.
leonardchan added a comment.

Re-added individual getters/members for _Accum types


Repository:
  rC Clang

https://reviews.llvm.org/D46084

Files:
  include/clang-c/Index.h
  include/clang/AST/ASTContext.h
  include/clang/AST/BuiltinTypes.def
  include/clang/Basic/DiagnosticCommonKinds.td
  include/clang/Basic/LangOptions.def
  include/clang/Basic/Specifiers.h
  include/clang/Basic/TargetInfo.h
  include/clang/Basic/TokenKinds.def
  include/clang/Driver/Options.td
  include/clang/Sema/DeclSpec.h
  include/clang/Serialization/ASTBitCodes.h
  lib/AST/ASTContext.cpp
  lib/AST/ExprConstant.cpp
  lib/AST/ItaniumMangle.cpp
  lib/AST/MicrosoftMangle.cpp
  lib/AST/NSAPI.cpp
  lib/AST/Type.cpp
  lib/AST/TypeLoc.cpp
  lib/Analysis/PrintfFormatString.cpp
  lib/Basic/TargetInfo.cpp
  lib/CodeGen/CGDebugInfo.cpp
  lib/CodeGen/CodeGenTypes.cpp
  lib/CodeGen/ItaniumCXXABI.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Index/USRGeneration.cpp
  lib/Parse/ParseDecl.cpp
  lib/Sema/DeclSpec.cpp
  lib/Sema/SemaTemplateVariadic.cpp
  lib/Sema/SemaType.cpp
  lib/Serialization/ASTCommon.cpp
  lib/Serialization/ASTReader.cpp
  test/Frontend/fixed_point.c
  test/Frontend/fixed_point_bit_widths.c
  test/Frontend/fixed_point_errors.c
  test/Frontend/fixed_point_errors.cpp
  test/Frontend/fixed_point_not_enabled.c
  tools/libclang/CXType.cpp

Index: tools/libclang/CXType.cpp
===
--- tools/libclang/CXType.cpp
+++ tools/libclang/CXType.cpp
@@ -53,6 +53,12 @@
 BTCASE(Float);
 BTCASE(Double);
 BTCASE(LongDouble);
+BTCASE(ShortAccum);
+BTCASE(Accum);
+BTCASE(LongAccum);
+BTCASE(UShortAccum);
+BTCASE(UAccum);
+BTCASE(ULongAccum);
 BTCASE(Float16);
 BTCASE(Float128);
 BTCASE(NullPtr);
@@ -546,6 +552,12 @@
 TKIND(Float);
 TKIND(Double);
 TKIND(LongDouble);
+TKIND(ShortAccum);
+TKIND(Accum);
+TKIND(LongAccum);
+TKIND(UShortAccum);
+TKIND(UAccum);
+TKIND(ULongAccum);
 TKIND(Float16);
 TKIND(Float128);
 TKIND(NullPtr);
Index: test/Frontend/fixed_point_not_enabled.c
===
--- /dev/null
+++ test/Frontend/fixed_point_not_enabled.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -x c -verify %s
+
+// Primary fixed point types
+signed short _Accum s_short_accum;// expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+signed _Accum s_accum;// expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+signed long _Accum s_long_accum;  // expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+unsigned short _Accum u_short_accum;  // expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+unsigned _Accum u_accum;  // expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+unsigned long _Accum u_long_accum;// expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+
+// Aliased fixed point types
+short _Accum short_accum; // expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+_Accum accum; // expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+  // expected-warning@-1{{type specifier missing, defaults to 'int'}}
+long _Accum long_accum;   // expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
Index: test/Frontend/fixed_point_errors.cpp
===
--- /dev/null
+++ test/Frontend/fixed_point_errors.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -x c++ -enable-fixed-point %s -verify
+
+// Name namgling is not provided for fixed point types in c++
+
+signed short _Accum s_short_accum;  // expected-error{{fixed point types are only allowed in C}}
+signed _Accum s_accum;  // expected-error{{fixed point types are only allowed in C}}
+signed long _Accum s_long_accum;// expected-error{{fixed point types are only allowed in C}}
+unsigned short _Accum u_short_accum;// expected-error{{fixed point types are only allowed in C}}
+unsigned _Accum u_accum;// expected-error{{fixed point types are only allowed in C}}
+unsigned long _Accum u_long_accum;  // expected-error{{fixed point types are only allowed in C}}
+
+short _Accum short_accum;   // expected-error{{fixed point types are only allowed in C}}
+_Accum accum;   // expected-error{{fixed point types are only allowed in C}}
+// expected-error@-1{{C++ requires a type specifier for all declarations}}
+long _Accum long_accum; // expected-error{{fixed point types are only 

[PATCH] D47351: [analyzer] Re-enable C++17-specific variable and member variable construction contexts.

[Artem Dergachev via Phabricator via cfe-commits]

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

The recent refactoring allows us to easily support C++17 "copy-elided" 
constructor syntax for variables and constructor-initializers. Similar 
constructors for return values are still to be supported.

In particular, the change of using path-sensitive state traits made our 
liveness problems go away. The `elision_on_ternary_op_branches` test was 
failing without that.

https://reviews.llvm.org/D47350 allowed us to support the C++17 destructor in 
`testCtorInitializer()`.


Repository:
  rC Clang

https://reviews.llvm.org/D47351

Files:
  lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  test/Analysis/cxx17-mandatory-elision.cpp

Index: test/Analysis/cxx17-mandatory-elision.cpp
===
--- test/Analysis/cxx17-mandatory-elision.cpp
+++ test/Analysis/cxx17-mandatory-elision.cpp
@@ -49,9 +49,64 @@
   }
 };
 
+
+struct A {
+  int x;
+  A(): x(0) {}
+  ~A() {}
+};
+
+struct B {
+  A a;
+  B() : a(A()) {}
+};
+
+void foo() {
+  B b;
+  clang_analyzer_eval(b.a.x == 0); // expected-warning{{TRUE}}
+}
+
 } // namespace ctor_initializer
 
 
+namespace elision_on_ternary_op_branches {
+class C1 {
+  int x;
+public:
+  C1(int x): x(x) {}
+  int getX() const { return x; }
+  ~C1();
+};
+
+class C2 {
+  int x;
+  int y;
+public:
+  C2(int x, int y): x(x), y(y) {}
+  int getX() const { return x; }
+  int getY() const { return y; }
+  ~C2();
+};
+
+void foo(int coin) {
+  C1 c1 = coin ? C1(1) : C1(2);
+  if (coin) {
+clang_analyzer_eval(c1.getX() == 1); // expected-warning{{TRUE}}
+  } else {
+clang_analyzer_eval(c1.getX() == 2); // expected-warning{{TRUE}}
+  }
+  C2 c2 = coin ? C2(3, 4) : C2(5, 6);
+  if (coin) {
+clang_analyzer_eval(c2.getX() == 3); // expected-warning{{TRUE}}
+clang_analyzer_eval(c2.getY() == 4); // expected-warning{{TRUE}}
+  } else {
+clang_analyzer_eval(c2.getX() == 5); // expected-warning{{TRUE}}
+clang_analyzer_eval(c2.getY() == 6); // expected-warning{{TRUE}}
+  }
+}
+} // namespace elision_on_ternary_op_branches
+
+
 namespace address_vector_tests {
 
 template  struct AddressVector {
@@ -108,4 +163,68 @@
 #endif
 }
 
+class ClassWithDestructor {
+  AddressVector 
+
+public:
+  ClassWithDestructor(AddressVector ) : v(v) {
+v.push(this);
+  }
+
+  ClassWithDestructor(ClassWithDestructor &) : v(c.v) { v.push(this); }
+  ClassWithDestructor(const ClassWithDestructor ) : v(c.v) {
+v.push(this);
+  }
+
+  ~ClassWithDestructor() { v.push(this); }
+};
+
+void testVariable() {
+  AddressVector v;
+  {
+ClassWithDestructor c = ClassWithDestructor(v);
+  }
+#if __cplusplus >= 201703L
+  // 0. Construct the variable.
+  // 1. Destroy the variable.
+  clang_analyzer_eval(v.len == 2); // expected-warning{{TRUE}}
+  clang_analyzer_eval(v.buf[0] == v.buf[1]); // expected-warning{{TRUE}}
+#else
+  // 0. Construct the temporary.
+  // 1. Construct the variable.
+  // 2. Destroy the temporary.
+  // 3. Destroy the variable.
+  clang_analyzer_eval(v.len == 4); // expected-warning{{TRUE}}
+  clang_analyzer_eval(v.buf[0] == v.buf[2]); // expected-warning{{TRUE}}
+  clang_analyzer_eval(v.buf[1] == v.buf[3]); // expected-warning{{TRUE}}
+#endif
+}
+
+struct TestCtorInitializer {
+  ClassWithDestructor c;
+  TestCtorInitializer(AddressVector )
+: c(ClassWithDestructor(v)) {}
+};
+
+void testCtorInitializer() {
+  AddressVector v;
+  {
+TestCtorInitializer t(v);
+  }
+#if __cplusplus >= 201703L
+  // 0. Construct the member variable.
+  // 1. Destroy the member variable.
+  clang_analyzer_eval(v.len == 2); // expected-warning{{TRUE}}
+  clang_analyzer_eval(v.buf[0] == v.buf[1]); // expected-warning{{TRUE}}
+#else
+  // 0. Construct the temporary.
+  // 1. Construct the member variable.
+  // 2. Destroy the temporary.
+  // 3. Destroy the member variable.
+  clang_analyzer_eval(v.len == 4); // expected-warning{{TRUE}}
+  clang_analyzer_eval(v.buf[0] == v.buf[2]); // expected-warning{{TRUE}}
+  clang_analyzer_eval(v.buf[1] == v.buf[3]); // expected-warning{{TRUE}}
+#endif
+}
+
 } // namespace address_vector_tests
Index: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -119,8 +119,9 @@
   // current construction context.
   if (CC) {
 switch (CC->getKind()) {
+case ConstructionContext::CXX17ElidedCopyVariableKind:
 case ConstructionContext::SimpleVariableKind: {
-  const auto *DSCC = cast(CC);
+  const auto *DSCC = cast(CC);
   const auto *DS = DSCC->getDeclStmt();
   const auto *Var = cast(DS->getSingleDecl());
   SVal LValue = State->getLValue(Var, LCtx);
@@ -131,6 +132,7 @@
   addObjectUnderConstruction(State, DSCC->getDeclStmt(), LCtx, LValue);
   

[PATCH] D47229: Make atomic non-member functions as nonnull

[JF Bastien via Phabricator via cfe-commits]

jfb added inline comments.



Comment at: lib/Sema/SemaChecking.cpp:3497
+else if (Form == Copy || Form == Xchg) {
+  if (!IsC11 && !IsN)
+// The value pointer is always dereferenced, a nullptr is 
undefined.

rsmith wrote:
> arphaman wrote:
> > Nit: might make more sense to check if `ByValType` is a pointer here 
> > instead of duplicating the `if` condition from above.
> I think the suggestion was to check `ByValType->isPointerType()` here. But... 
> that doesn't work, because we could have a pointer value being passed by 
> value (where a null is allowed), rather than a value being passed by address 
> (where a null is disallowed). I think this comparison is actually less clear 
> than the `!IsC11 && !IsN` check; factoring out a `bool IsPassedByAddress` or 
> similar instead would aid readability here.
I went with the `bool` as you suggested.


Repository:
  rC Clang

https://reviews.llvm.org/D47229



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

[PATCH] D47229: Make atomic non-member functions as nonnull

[JF Bastien via Phabricator via cfe-commits]

jfb updated this revision to Diff 148504.
jfb marked an inline comment as done.
jfb added a comment.

- Address nit.
- Change suggested by Richard


Repository:
  rC Clang

https://reviews.llvm.org/D47229

Files:
  lib/Sema/SemaChecking.cpp
  test/Sema/atomic-ops.c

Index: test/Sema/atomic-ops.c
===
--- test/Sema/atomic-ops.c
+++ test/Sema/atomic-ops.c
@@ -531,8 +531,80 @@
 }
 
 void nullPointerWarning(_Atomic(int) *Ap, int *p, int val) {
-  // The 'expected' pointer shouldn't be NULL.
-  (void)__c11_atomic_compare_exchange_strong(Ap, NULL, val, memory_order_relaxed, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
-  (void)atomic_compare_exchange_weak(Ap, ((void*)0), val); // expected-warning {{null passed to a callee that requires a non-null argument}}
-  (void)__atomic_compare_exchange_n(p, NULL, val, 0, memory_order_relaxed, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  volatile _Atomic(int) vai;
+  _Atomic(int) ai;
+  volatile int vi = 42;
+  int i = 42;
+
+  __c11_atomic_init((volatile _Atomic(int)*)0, 42); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  __c11_atomic_init((_Atomic(int)*)0, 42); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  __c11_atomic_store((volatile _Atomic(int)*)0, 42, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  __c11_atomic_store((_Atomic(int)*)0, 42, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  (void)__c11_atomic_load((volatile _Atomic(int)*)0, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  (void)__c11_atomic_load((_Atomic(int)*)0, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  (void)__c11_atomic_exchange((volatile _Atomic(int)*)0, 42, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  (void)__c11_atomic_exchange((_Atomic(int)*)0, 42, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  (void)__c11_atomic_compare_exchange_weak((volatile _Atomic(int)*)0, , 42, memory_order_relaxed, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  (void)__c11_atomic_compare_exchange_weak((_Atomic(int)*)0, , 42, memory_order_relaxed, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  (void)__c11_atomic_compare_exchange_weak(, (int*)0, 42, memory_order_relaxed, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  (void)__c11_atomic_compare_exchange_weak(, (int*)0, 42, memory_order_relaxed, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  (void)__c11_atomic_compare_exchange_strong((volatile _Atomic(int)*)0, , 42, memory_order_relaxed, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  (void)__c11_atomic_compare_exchange_strong((_Atomic(int)*)0, , 42, memory_order_relaxed, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  (void)__c11_atomic_compare_exchange_strong(, (int*)0, 42, memory_order_relaxed, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  (void)__c11_atomic_compare_exchange_strong(, (int*)0, 42, memory_order_relaxed, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  (void)__c11_atomic_fetch_add((volatile _Atomic(int)*)0, 42, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  (void)__c11_atomic_fetch_add((_Atomic(int)*)0, 42, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  (void)__c11_atomic_fetch_sub((volatile _Atomic(int)*)0, 42, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  (void)__c11_atomic_fetch_sub((_Atomic(int)*)0, 42, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  (void)__c11_atomic_fetch_and((volatile _Atomic(int)*)0, 42, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  (void)__c11_atomic_fetch_and((_Atomic(int)*)0, 42, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  (void)__c11_atomic_fetch_or((volatile _Atomic(int)*)0, 42, memory_order_relaxed); // expected-warning 

[PATCH] D47350: [analyzer] Track class member initializer constructors path-sensitively within their construction context.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ updated this revision to Diff 148502.
NoQ added a comment.

Add a forgotten FIXME to the test.


https://reviews.llvm.org/D47350

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  test/Analysis/cxx17-mandatory-elision.cpp

Index: test/Analysis/cxx17-mandatory-elision.cpp
===
--- test/Analysis/cxx17-mandatory-elision.cpp
+++ test/Analysis/cxx17-mandatory-elision.cpp
@@ -21,6 +21,37 @@
 } // namespace variable_functional_cast_crash
 
 
+namespace ctor_initializer {
+
+struct S {
+  int x, y, z;
+};
+
+struct T {
+  S s;
+  int w;
+  T(int w): s(), w(w) {}
+};
+
+class C {
+  T t;
+public:
+  C() : t(T(4)) {
+S s = {1, 2, 3};
+t.s = s;
+// FIXME: Should be TRUE in C++11 as well.
+clang_analyzer_eval(t.w == 4);
+#if __cplusplus >= 201703L
+// expected-warning@-2{{TRUE}}
+#else
+// expected-warning@-4{{UNKNOWN}}
+#endif
+  }
+};
+
+} // namespace ctor_initializer
+
+
 namespace address_vector_tests {
 
 template  struct AddressVector {
Index: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -153,6 +153,7 @@
   QualType Ty = Field->getType();
   FieldVal = makeZeroElementRegion(State, FieldVal, Ty,
CallOpts.IsArrayCtorOrDtor);
+  State = addObjectUnderConstruction(State, Init, LCtx, FieldVal);
   return std::make_pair(State, FieldVal);
 }
 case ConstructionContext::NewAllocatedObjectKind: {
@@ -272,35 +273,6 @@
   State, loc::MemRegionVal(MRMgr.getCXXTempObjectRegion(E, LCtx)));
 }
 
-const CXXConstructExpr *
-ExprEngine::findDirectConstructorForCurrentCFGElement() {
-  // Go backward in the CFG to see if the previous element (ignoring
-  // destructors) was a CXXConstructExpr. If so, that constructor
-  // was constructed directly into an existing region.
-  // This process is essentially the inverse of that performed in
-  // findElementDirectlyInitializedByCurrentConstructor().
-  if (currStmtIdx == 0)
-return nullptr;
-
-  const CFGBlock *B = getBuilderContext().getBlock();
-
-  unsigned int PreviousStmtIdx = currStmtIdx - 1;
-  CFGElement Previous = (*B)[PreviousStmtIdx];
-
-  while (Previous.getAs() && PreviousStmtIdx > 0) {
---PreviousStmtIdx;
-Previous = (*B)[PreviousStmtIdx];
-  }
-
-  if (Optional PrevStmtElem = Previous.getAs()) {
-if (auto *CtorExpr = dyn_cast(PrevStmtElem->getStmt())) {
-  return CtorExpr;
-}
-  }
-
-  return nullptr;
-}
-
 void ExprEngine::VisitCXXConstructExpr(const CXXConstructExpr *CE,
ExplodedNode *Pred,
ExplodedNodeSet ) {
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -100,7 +100,68 @@
 
 // Keeps track of whether objects corresponding to the statement have been
 // fully constructed.
-typedef std::pair ConstructedObjectKey;
+
+/// ConstructedObjectKey is used for being able to find the path-sensitive
+/// memory region of a freshly constructed object while modeling the AST node
+/// that syntactically represents the object that is being constructed.
+/// Semantics of such nodes may sometimes require access to the region that's
+/// not otherwise present in the program state, or to the very fact that
+/// the construction context was present and contained references to these
+/// AST nodes.
+class ConstructedObjectKey {
+  llvm::PointerUnion P;
+  const LocationContext *LC;
+
+  const void *getAnyASTNodePtr() const {
+if (const Stmt *S = getStmt())
+  return S;
+else
+  return getCXXCtorInitializer();
+  }
+
+public:
+  ConstructedObjectKey(
+  llvm::PointerUnion P,
+  const LocationContext *LC)
+  : P(P), LC(LC) {
+// This is the full list of statements that require additional actions when
+// encountered. This list may be expanded when new actions are implemented.
+assert(getCXXCtorInitializer() || isa(getStmt()) ||
+   isa(getStmt()) || isa(getStmt()) ||
+   isa(getStmt()));
+  }
+
+  const Stmt *getStmt() const {
+return P.dyn_cast();
+  }
+  const CXXCtorInitializer *getCXXCtorInitializer() const {
+return P.dyn_cast();
+  }
+  const LocationContext *getLocationContext() const {
+return LC;
+  }
+
+  void print(llvm::raw_ostream , PrinterHelper *Helper, PrintingPolicy ) {
+OS << '(' << LC << ',' << getAnyASTNodePtr() << ") ";
+if (const Stmt *S = P.dyn_cast()) {
+  S->printPretty(OS, Helper, PP);
+} else {
+  const CXXCtorInitializer *I = P.get();
+  OS << I->getAnyMember()->getNameAsString();
+}
+  }
+  

[PATCH] D47350: [analyzer] Track class member initializer constructors path-sensitively within their construction context.

[Artem Dergachev via Phabricator via cfe-commits]

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

Same as https://reviews.llvm.org/D47305 but for `CXXCtorInitializer`-based 
constructors, based on the discussion in 
http://lists.llvm.org/pipermail/cfe-dev/2018-May/058055.html

Because these don't suffer from liveness bugs (member variables are born much 
earlier than their constructors are called and live much longer than stack 
locals), this is mostly a refactoring pass. It has minor observable side 
effects, but it will have way more visible effects when we enable C++17 
construction contexts because finding the direct constructor would be much 
harder.

Currently the observable effect is that we can preserve direct bindings to the 
object more often and we need to fall back to binding the lazy compound value 
of the initializer expression less often. Direct bindings are modeled better by 
the store. In the provided test case the default binding produced by 
trivial-copying `s` to `t.s` would overwrite the existing default binding to 
`t`. But if we instead preserve direct bindings, only bindings that correspond 
to `t.s` will be overwritten and the binding for `t.w` will remain untouched. 
This only works for C++17 because in C++11 the member variable is still modeled 
as a trivial copy from the temporary object, because there semantically *is* a 
temporary object, while in C++17 it is elided.


Repository:
  rC Clang

https://reviews.llvm.org/D47350

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  test/Analysis/cxx17-mandatory-elision.cpp

Index: test/Analysis/cxx17-mandatory-elision.cpp
===
--- test/Analysis/cxx17-mandatory-elision.cpp
+++ test/Analysis/cxx17-mandatory-elision.cpp
@@ -21,6 +21,36 @@
 } // namespace variable_functional_cast_crash
 
 
+namespace ctor_initializer {
+
+struct S {
+  int x, y, z;
+};
+
+struct T {
+  S s;
+  int w;
+  T(int w): s(), w(w) {}
+};
+
+class C {
+  T t;
+public:
+  C() : t(T(4)) {
+S s = {1, 2, 3};
+t.s = s;
+clang_analyzer_eval(t.w == 4);
+#if __cplusplus >= 201703L
+// expected-warning@-2{{TRUE}}
+#else
+// expected-warning@-4{{UNKNOWN}}
+#endif
+  }
+};
+
+} // namespace ctor_initializer
+
+
 namespace address_vector_tests {
 
 template  struct AddressVector {
Index: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -153,6 +153,7 @@
   QualType Ty = Field->getType();
   FieldVal = makeZeroElementRegion(State, FieldVal, Ty,
CallOpts.IsArrayCtorOrDtor);
+  State = addObjectUnderConstruction(State, Init, LCtx, FieldVal);
   return std::make_pair(State, FieldVal);
 }
 case ConstructionContext::NewAllocatedObjectKind: {
@@ -272,35 +273,6 @@
   State, loc::MemRegionVal(MRMgr.getCXXTempObjectRegion(E, LCtx)));
 }
 
-const CXXConstructExpr *
-ExprEngine::findDirectConstructorForCurrentCFGElement() {
-  // Go backward in the CFG to see if the previous element (ignoring
-  // destructors) was a CXXConstructExpr. If so, that constructor
-  // was constructed directly into an existing region.
-  // This process is essentially the inverse of that performed in
-  // findElementDirectlyInitializedByCurrentConstructor().
-  if (currStmtIdx == 0)
-return nullptr;
-
-  const CFGBlock *B = getBuilderContext().getBlock();
-
-  unsigned int PreviousStmtIdx = currStmtIdx - 1;
-  CFGElement Previous = (*B)[PreviousStmtIdx];
-
-  while (Previous.getAs() && PreviousStmtIdx > 0) {
---PreviousStmtIdx;
-Previous = (*B)[PreviousStmtIdx];
-  }
-
-  if (Optional PrevStmtElem = Previous.getAs()) {
-if (auto *CtorExpr = dyn_cast(PrevStmtElem->getStmt())) {
-  return CtorExpr;
-}
-  }
-
-  return nullptr;
-}
-
 void ExprEngine::VisitCXXConstructExpr(const CXXConstructExpr *CE,
ExplodedNode *Pred,
ExplodedNodeSet ) {
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -100,7 +100,68 @@
 
 // Keeps track of whether objects corresponding to the statement have been
 // fully constructed.
-typedef std::pair ConstructedObjectKey;
+
+/// ConstructedObjectKey is used for being able to find the path-sensitive
+/// memory region of a freshly constructed object while modeling the AST node
+/// that syntactically represents the object that is being constructed.
+/// Semantics of such nodes may sometimes require access to the region that's
+/// not otherwise present 

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

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

Rather than disabling typo correction in `TransformTypos`, it would be 
preferable to attempt to immediately correct them. (That should allow the 
`variableX.getX()` case to still work.)


https://reviews.llvm.org/D47341



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

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

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added inline comments.



Comment at: clang/test/SemaCXX/typo-correction-delayed.cpp:146
+  // expected-error@+1 {{use of undeclared identifier 'variableX'}}
   int x = variableX.getX();
 }

Loosing typo correction for 'getX' is fine, however, I think we still want to 
typo-correct 'variableX' to 'variable' here.


https://reviews.llvm.org/D47341



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

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

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added inline comments.



Comment at: lib/Frontend/DependencyFile.cpp:182-185
 for (const auto  : Opts.ExtraDeps) {
   AddFilename(ExtraDep);
+  ++InputFileIndex;
 }

This is incorrect if there are duplicates in `Opts.ExtraDeps`. Also please 
update the test to have duplicate extra dependencies.



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

I think it would be better to have a check

// CHECK:   dependency-gen-extradeps-phony.c

Because you expect it to be there and it's not that simple to notice the colon 
in `.c:`, so it's not immediately clear how CHECK-NOT is applied here.



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

For these repeated CHECK-NOT please consider using `FileCheck 
--implicit-check-not`. In this case it's not that important as the test is 
small but it can still help to capture your intention more clearly.


Repository:
  rC Clang

https://reviews.llvm.org/D44568



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

[PATCH] D47229: Make atomic non-member functions as nonnull

[Richard Smith - zygoloid via Phabricator via cfe-commits]

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

Either the previous version of this patch or a version with a `bool` factored 
out seem OK to me.




Comment at: lib/Sema/SemaChecking.cpp:3497
+else if (Form == Copy || Form == Xchg) {
+  if (!IsC11 && !IsN)
+// The value pointer is always dereferenced, a nullptr is 
undefined.

arphaman wrote:
> Nit: might make more sense to check if `ByValType` is a pointer here instead 
> of duplicating the `if` condition from above.
I think the suggestion was to check `ByValType->isPointerType()` here. But... 
that doesn't work, because we could have a pointer value being passed by value 
(where a null is allowed), rather than a value being passed by address (where a 
null is disallowed). I think this comparison is actually less clear than the 
`!IsC11 && !IsN` check; factoring out a `bool IsPassedByAddress` or similar 
instead would aid readability here.


Repository:
  rC Clang

https://reviews.llvm.org/D47229



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

r333234 - Improve diagonstic for braced-init-list as operand to ?: expression.

[Richard Smith via cfe-commits]

Author: rsmith
Date: Thu May 24 15:02:52 2018
New Revision: 333234

URL: http://llvm.org/viewvc/llvm-project?rev=333234=rev
Log:
Improve diagonstic for braced-init-list as operand to ?: expression.

Modified:
cfe/trunk/lib/Parse/ParseExpr.cpp
cfe/trunk/test/Parser/cxx11-brace-initializers.cpp

Modified: cfe/trunk/lib/Parse/ParseExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseExpr.cpp?rev=333234=333233=333234=diff
==
--- cfe/trunk/lib/Parse/ParseExpr.cpp (original)
+++ cfe/trunk/lib/Parse/ParseExpr.cpp Thu May 24 15:02:52 2018
@@ -336,7 +336,17 @@ Parser::ParseRHSOfBinaryExpression(ExprR
 // Special case handling for the ternary operator.
 ExprResult TernaryMiddle(true);
 if (NextTokPrec == prec::Conditional) {
-  if (Tok.isNot(tok::colon)) {
+  if (getLangOpts().CPlusPlus11 && Tok.is(tok::l_brace)) {
+// Parse a braced-init-list here for error recovery purposes.
+SourceLocation BraceLoc = Tok.getLocation();
+TernaryMiddle = ParseBraceInitializer();
+if (!TernaryMiddle.isInvalid()) {
+  Diag(BraceLoc, diag::err_init_list_bin_op)
+  << /*RHS*/ 1 << PP.getSpelling(OpToken)
+  << Actions.getExprRange(TernaryMiddle.get());
+  TernaryMiddle = ExprError();
+}
+  } else if (Tok.isNot(tok::colon)) {
 // Don't parse FOO:BAR as if it were a typo for FOO::BAR.
 ColonProtectionRAIIObject X(*this);
 
@@ -345,11 +355,6 @@ Parser::ParseRHSOfBinaryExpression(ExprR
 // In particular, the RHS of the '?' is 'expression', not
 // 'logical-OR-expression' as we might expect.
 TernaryMiddle = ParseExpression();
-if (TernaryMiddle.isInvalid()) {
-  Actions.CorrectDelayedTyposInExpr(LHS);
-  LHS = ExprError();
-  TernaryMiddle = nullptr;
-}
   } else {
 // Special case handling of "X ? Y : Z" where Y is empty:
 //   logical-OR-expression '?' ':' conditional-expression   [GNU]
@@ -357,6 +362,12 @@ Parser::ParseRHSOfBinaryExpression(ExprR
 Diag(Tok, diag::ext_gnu_conditional_expr);
   }
 
+  if (TernaryMiddle.isInvalid()) {
+Actions.CorrectDelayedTyposInExpr(LHS);
+LHS = ExprError();
+TernaryMiddle = nullptr;
+  }
+
   if (!TryConsumeToken(tok::colon, ColonLoc)) {
 // Otherwise, we're missing a ':'.  Assume that this was a typo that
 // the user forgot. If we're not in a macro expansion, we can suggest
@@ -469,6 +480,11 @@ Parser::ParseRHSOfBinaryExpression(ExprR
   if (ThisPrec == prec::Assignment) {
 Diag(OpToken, diag::warn_cxx98_compat_generalized_initializer_lists)
   << Actions.getExprRange(RHS.get());
+  } else if (ColonLoc.isValid()) {
+Diag(ColonLoc, diag::err_init_list_bin_op)
+  << /*RHS*/1 << ":"
+  << Actions.getExprRange(RHS.get());
+LHS = ExprError();
   } else {
 Diag(OpToken, diag::err_init_list_bin_op)
   << /*RHS*/1 << PP.getSpelling(OpToken)

Modified: cfe/trunk/test/Parser/cxx11-brace-initializers.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/cxx11-brace-initializers.cpp?rev=333234=333233=333234=diff
==
--- cfe/trunk/test/Parser/cxx11-brace-initializers.cpp (original)
+++ cfe/trunk/test/Parser/cxx11-brace-initializers.cpp Thu May 24 15:02:52 2018
@@ -1,5 +1,4 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++11
-// expected-no-diagnostics
 
 struct S {
   S(int, int) {}
@@ -25,3 +24,6 @@ namespace PR14948 {
 
   template T Q::x {};
 }
+
+int conditional1 = 1 ? {} : 0; // expected-error {{initializer list cannot be 
used on the right hand side of operator '?'}}
+int conditional2 = 1 ? 0 : {}; // expected-error {{initializer list cannot be 
used on the right hand side of operator ':'}}


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

[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type

[Bevin Hansson via Phabricator via cfe-commits]

ebevhan added inline comments.



Comment at: include/clang/Basic/TargetInfo.h:382
+// enough bits to fit the minumum.
+if (getIntWidth() < MinSignedAccumDataBits)
+  return getLongWidth();

I'm not sure I agree with this interpretation. It's simply not correct to 
consider 'short' the 'underlying type' of 'short _Accum', 'int' the 'underlying 
type' of '_Accum', etc. They are wholly independent and should have separate 
settings altogether.

Asserting/ensuring that a target has set an invalid width for its types should 
be done separately. This currently feels a bit like the TargetInfo is guessing.

(For the record, the reason I'm requesting this change is because this 
implementation does not work for us downstream.)


Repository:
  rC Clang

https://reviews.llvm.org/D46084



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

r333233 - Switch a couple of users of LangOpts::GNUMode to the more appropriate LangOpts::GNUKeywords.

[Richard Smith via cfe-commits]

Author: rsmith
Date: Thu May 24 14:51:52 2018
New Revision: 333233

URL: http://llvm.org/viewvc/llvm-project?rev=333233=rev
Log:
Switch a couple of users of LangOpts::GNUMode to the more appropriate 
LangOpts::GNUKeywords.

Modified:
cfe/trunk/lib/Sema/SemaCodeComplete.cpp
cfe/trunk/lib/Sema/SemaLookup.cpp

Modified: cfe/trunk/lib/Sema/SemaCodeComplete.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCodeComplete.cpp?rev=333233=333232=333233=diff
==
--- cfe/trunk/lib/Sema/SemaCodeComplete.cpp (original)
+++ cfe/trunk/lib/Sema/SemaCodeComplete.cpp Thu May 24 14:51:52 2018
@@ -1371,8 +1371,8 @@ static void AddTypeSpecifierResults(cons
   } else
 Results.AddResult(Result("__auto_type", CCP_Type));
 
-  // GNU extensions
-  if (LangOpts.GNUMode) {
+  // GNU keywords
+  if (LangOpts.GNUKeywords) {
 // FIXME: Enable when we actually support decimal floating point.
 //Results.AddResult(Result("_Decimal32"));
 //Results.AddResult(Result("_Decimal64"));

Modified: cfe/trunk/lib/Sema/SemaLookup.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaLookup.cpp?rev=333233=333232=333233=diff
==
--- cfe/trunk/lib/Sema/SemaLookup.cpp (original)
+++ cfe/trunk/lib/Sema/SemaLookup.cpp Thu May 24 14:51:52 2018
@@ -4459,7 +4459,7 @@ static void AddKeywordsToConsumer(Sema &
   }
 }
 
-if (SemaRef.getLangOpts().GNUMode)
+if (SemaRef.getLangOpts().GNUKeywords)
   Consumer.addKeywordResult("typeof");
   } else if (CCC.WantFunctionLikeCasts) {
 static const char *const CastableTypeSpecs[] = {


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

[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

In https://reviews.llvm.org/D46084#374, @jfb wrote:

> Can you also add a test for `_Bool _Accum`.
>
> Also, `-enable-fixed-point -x c++` failing.


.
Done. Also the failing c++ case is under `test/Frontend/fixed_point_errors.cpp`


Repository:
  rC Clang

https://reviews.llvm.org/D46084



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

[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type

[Leonard Chan via Phabricator via cfe-commits]

leonardchan updated this revision to Diff 148481.
leonardchan marked 2 inline comments as done.
leonardchan added a comment.

- Added test case for `_Bool _Accum`
- Getters for the `_Accum` bit widths return values for their corresponding 
integral types (ie. `sizeof(short _Accum) == sizeof(short)`). The only case 
where this may not happen is if the target architecture uses 16 bits for an 
int. N1169 requires that a `signed/unsigned _Accum` hold at least 15 fractional 
bits and 4 integral bits. To be able to fit these bits, the size is upgraded to 
that of a long which is guaranteed to be large enough to hold them.


Repository:
  rC Clang

https://reviews.llvm.org/D46084

Files:
  include/clang-c/Index.h
  include/clang/AST/ASTContext.h
  include/clang/AST/BuiltinTypes.def
  include/clang/Basic/DiagnosticCommonKinds.td
  include/clang/Basic/LangOptions.def
  include/clang/Basic/Specifiers.h
  include/clang/Basic/TargetInfo.h
  include/clang/Basic/TokenKinds.def
  include/clang/Driver/Options.td
  include/clang/Sema/DeclSpec.h
  include/clang/Serialization/ASTBitCodes.h
  lib/AST/ASTContext.cpp
  lib/AST/ExprConstant.cpp
  lib/AST/ItaniumMangle.cpp
  lib/AST/MicrosoftMangle.cpp
  lib/AST/NSAPI.cpp
  lib/AST/Type.cpp
  lib/AST/TypeLoc.cpp
  lib/Analysis/PrintfFormatString.cpp
  lib/CodeGen/CGDebugInfo.cpp
  lib/CodeGen/CodeGenTypes.cpp
  lib/CodeGen/ItaniumCXXABI.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Index/USRGeneration.cpp
  lib/Parse/ParseDecl.cpp
  lib/Sema/DeclSpec.cpp
  lib/Sema/SemaTemplateVariadic.cpp
  lib/Sema/SemaType.cpp
  lib/Serialization/ASTCommon.cpp
  lib/Serialization/ASTReader.cpp
  test/Frontend/fixed_point.c
  test/Frontend/fixed_point_bit_widths.c
  test/Frontend/fixed_point_errors.c
  test/Frontend/fixed_point_errors.cpp
  test/Frontend/fixed_point_not_enabled.c
  tools/libclang/CXType.cpp

Index: tools/libclang/CXType.cpp
===
--- tools/libclang/CXType.cpp
+++ tools/libclang/CXType.cpp
@@ -53,6 +53,12 @@
 BTCASE(Float);
 BTCASE(Double);
 BTCASE(LongDouble);
+BTCASE(ShortAccum);
+BTCASE(Accum);
+BTCASE(LongAccum);
+BTCASE(UShortAccum);
+BTCASE(UAccum);
+BTCASE(ULongAccum);
 BTCASE(Float16);
 BTCASE(Float128);
 BTCASE(NullPtr);
@@ -546,6 +552,12 @@
 TKIND(Float);
 TKIND(Double);
 TKIND(LongDouble);
+TKIND(ShortAccum);
+TKIND(Accum);
+TKIND(LongAccum);
+TKIND(UShortAccum);
+TKIND(UAccum);
+TKIND(ULongAccum);
 TKIND(Float16);
 TKIND(Float128);
 TKIND(NullPtr);
Index: test/Frontend/fixed_point_not_enabled.c
===
--- /dev/null
+++ test/Frontend/fixed_point_not_enabled.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -x c -verify %s
+
+// Primary fixed point types
+signed short _Accum s_short_accum;// expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+signed _Accum s_accum;// expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+signed long _Accum s_long_accum;  // expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+unsigned short _Accum u_short_accum;  // expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+unsigned _Accum u_accum;  // expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+unsigned long _Accum u_long_accum;// expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+
+// Aliased fixed point types
+short _Accum short_accum; // expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+_Accum accum; // expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+  // expected-warning@-1{{type specifier missing, defaults to 'int'}}
+long _Accum long_accum;   // expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
Index: test/Frontend/fixed_point_errors.cpp
===
--- /dev/null
+++ test/Frontend/fixed_point_errors.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -x c++ -enable-fixed-point %s -verify
+
+// Name namgling is not provided for fixed point types in c++
+
+signed short _Accum s_short_accum;  // expected-error{{fixed point types are only allowed in C}}
+signed _Accum s_accum;  // expected-error{{fixed point types are only allowed in C}}
+signed long _Accum s_long_accum;// expected-error{{fixed point types are only allowed in C}}
+unsigned short _Accum u_short_accum;// expected-error{{fixed point types are only allowed in C}}
+unsigned _Accum u_accum;// expected-error{{fixed point types are only allowed in C}}
+unsigned long _Accum u_long_accum;  // 

[PATCH] D47344: LWG 2843 "Unclear behavior of std::pmr::memory_resource::do_allocate()"

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

Quuxplusone created this revision.
Quuxplusone added a reviewer: EricWF.
Herald added subscribers: cfe-commits, JDevlieghere.

https://github.com/cplusplus/draft/commit/6216651aada9bc2f9cefe90edbde4ea9e32251ab

`new_delete_resource().allocate(n, a)` has basically three permissible results:

- Return an appropriately sized and aligned block.
- Throw bad_alloc.
- If `n == 0`, return an unspecified result.

Before this patch, libc++'s `new_delete_resource` would do a fourth and 
impermissible thing, which was to return an appropriately sized but 
inappropriately under-aligned block. This is now fixed.

(This came up while I was stress-testing `unsynchronized_pool_resource` on my 
MacBook. If we can't trust the default resource to return appropriately aligned 
blocks, pretty much everything breaks. For similar reasons, I would strongly 
support just patching `__libcpp_allocate` directly, but I don't care to die on 
that hill, so I made this patch as a ``-specific workaround.)


Repository:
  rCXX libc++

https://reviews.llvm.org/D47344

Files:
  src/experimental/memory_resource.cpp


Index: src/experimental/memory_resource.cpp
===
--- src/experimental/memory_resource.cpp
+++ src/experimental/memory_resource.cpp
@@ -8,6 +8,7 @@
 
//===--===//
 
 #include "experimental/memory_resource"
+#include "memory"
 
 #ifndef _LIBCPP_HAS_NO_ATOMIC_HEADER
 #include "atomic"
@@ -23,18 +24,36 @@
 
 // new_delete_resource()
 
+#ifdef _LIBCPP_HAS_NO_ALIGNED_ALLOCATION
+static bool is_aligned_to(void *ptr, size_t align)
+{
+void *p2 = ptr;
+size_t space = 1;
+void *result = _VSTD::align(align, 1, p2, space);
+return (result == ptr);
+}
+#endif
+
 class _LIBCPP_TYPE_VIS __new_delete_memory_resource_imp
 : public memory_resource
 {
 public:
 ~__new_delete_memory_resource_imp() = default;
 
 protected:
 virtual void* do_allocate(size_t __size, size_t __align)
-{ return _VSTD::__libcpp_allocate(__size, __align); /* FIXME */}
+{
+void *result = _VSTD::__libcpp_allocate(__size, __align);
+#ifdef _LIBCPP_HAS_NO_ALIGNED_ALLOCATION
+if (__size != 0 && !is_aligned_to(result, __align)) {
+__throw_bad_alloc();
+}
+#endif
+return result;
+}
 
 virtual void do_deallocate(void * __p, size_t, size_t __align)
-{ _VSTD::__libcpp_deallocate(__p, __align); /* FIXME */ }
+{ _VSTD::__libcpp_deallocate(__p, __align); }
 
 virtual bool do_is_equal(memory_resource const & __other) const _NOEXCEPT
 { return &__other == this; }


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,18 +24,36 @@
 
 // new_delete_resource()
 
+#ifdef _LIBCPP_HAS_NO_ALIGNED_ALLOCATION
+static bool is_aligned_to(void *ptr, size_t align)
+{
+void *p2 = ptr;
+size_t space = 1;
+void *result = _VSTD::align(align, 1, p2, space);
+return (result == ptr);
+}
+#endif
+
 class _LIBCPP_TYPE_VIS __new_delete_memory_resource_imp
 : public memory_resource
 {
 public:
 ~__new_delete_memory_resource_imp() = default;
 
 protected:
 virtual void* do_allocate(size_t __size, size_t __align)
-{ return _VSTD::__libcpp_allocate(__size, __align); /* FIXME */}
+{
+void *result = _VSTD::__libcpp_allocate(__size, __align);
+#ifdef _LIBCPP_HAS_NO_ALIGNED_ALLOCATION
+if (__size != 0 && !is_aligned_to(result, __align)) {
+__throw_bad_alloc();
+}
+#endif
+return result;
+}
 
 virtual void do_deallocate(void * __p, size_t, size_t __align)
-{ _VSTD::__libcpp_deallocate(__p, __align); /* FIXME */ }
+{ _VSTD::__libcpp_deallocate(__p, __align); }
 
 virtual bool do_is_equal(memory_resource const & __other) const _NOEXCEPT
 { return &__other == this; }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47290: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %zu/%zi on Darwin

[JF Bastien via Phabricator via cfe-commits]

jfb marked an inline comment as done.
jfb added inline comments.



Comment at: test/SemaObjC/format-size-spec-nsinteger.m:18
+  NSUInteger j = 0;
+  NSLog(@"max NSInteger = %zi", i);  // CHECK: values of type 'NSInteger' 
should not be used as format arguments; add an explicit cast to 'long' instead
+  NSLog(@"max NSUinteger = %zu", j); // CHECK: values of type 'NSUInteger' 
should not be used as format arguments; add an explicit cast to 'unsigned long' 
instead

ahatanak wrote:
> This test looks identical to 
> test/SemaObjC/format-size-spec-nsinteger-nopedantic.m except for the CHECK 
> lines. You can probably merge the two files if you separate the CHECK lines 
> from the lines that call NSLog (see test/Sema/format-strings-darwin.c for 
> example).
Cute, I didn't know about this pattern.


Repository:
  rC Clang

https://reviews.llvm.org/D47290



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

[PATCH] D47290: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %zu/%zi on Darwin

[JF Bastien via Phabricator via cfe-commits]

jfb updated this revision to Diff 148467.
jfb marked an inline comment as done.
jfb added a comment.

- Merge format-size-spec-nsinteger


Repository:
  rC Clang

https://reviews.llvm.org/D47290

Files:
  include/clang/Analysis/Analyses/FormatString.h
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Analysis/PrintfFormatString.cpp
  lib/Sema/SemaChecking.cpp
  test/FixIt/fixit-format-ios-nopedantic.m
  test/FixIt/fixit-format-ios.m
  test/SemaObjC/format-size-spec-nsinteger.m

Index: test/SemaObjC/format-size-spec-nsinteger.m
===
--- /dev/null
+++ test/SemaObjC/format-size-spec-nsinteger.m
@@ -0,0 +1,30 @@
+// RUN: %clang_cc1 -triple thumbv7-apple-ios -Wno-objc-root-class -fsyntax-only -verify -Wformat %s
+// RUN: %clang_cc1 -triple thumbv7-apple-ios -Wno-objc-root-class -fsyntax-only -verify -Wformat-pedantic -DPEDANTIC %s
+
+#if !defined(PEDANTIC)
+// expected-no-diagnostics
+#endif
+
+#if __LP64__
+typedef unsigned long NSUInteger;
+typedef long NSInteger;
+#else
+typedef unsigned int NSUInteger;
+typedef int NSInteger;
+#endif
+
+@class NSString;
+
+extern void NSLog(NSString *format, ...);
+
+void testSizeSpecifier() {
+  NSInteger i = 0;
+  NSUInteger j = 0;
+  NSLog(@"max NSInteger = %zi", i);
+  NSLog(@"max NSUinteger = %zu", j);
+
+#if defined(PEDANTIC)
+  // expected-warning@-4 {{values of type 'NSInteger' should not be used as format arguments; add an explicit cast to 'long' instead}}
+  // expected-warning@-4 {{values of type 'NSUInteger' should not be used as format arguments; add an explicit cast to 'unsigned long' instead}}
+#endif
+}
Index: test/FixIt/fixit-format-ios.m
===
--- test/FixIt/fixit-format-ios.m
+++ test/FixIt/fixit-format-ios.m
@@ -1,5 +1,5 @@
 // RUN: cp %s %t
-// RUN: %clang_cc1 -triple thumbv7-apple-ios8.0.0 -fsyntax-only -Wformat -fixit %t
+// RUN: %clang_cc1 -triple thumbv7-apple-ios8.0.0 -fsyntax-only -Wformat-pedantic -fixit %t
 // RUN: grep -v CHECK %t | FileCheck %s
 
 int printf(const char * restrict, ...);
Index: test/FixIt/fixit-format-ios-nopedantic.m
===
--- /dev/null
+++ test/FixIt/fixit-format-ios-nopedantic.m
@@ -0,0 +1,21 @@
+// RUN: cp %s %t
+// RUN: %clang_cc1 -triple thumbv7-apple-ios8.0.0 -fsyntax-only -Wformat -Werror -fixit %t
+
+int printf(const char *restrict, ...);
+typedef unsigned int NSUInteger;
+typedef int NSInteger;
+NSUInteger getNSUInteger();
+NSInteger getNSInteger();
+
+void test() {
+  // For thumbv7-apple-ios8.0.0 the underlying type of ssize_t is long
+  // and the underlying type of size_t is unsigned long.
+
+  printf("test 1: %zu", getNSUInteger());
+
+  printf("test 2: %zu %zu", getNSUInteger(), getNSUInteger());
+
+  printf("test 3: %zd", getNSInteger());
+
+  printf("test 4: %zd %zd", getNSInteger(), getNSInteger());
+}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -6594,11 +6594,11 @@
 ExprTy = TET->getUnderlyingExpr()->getType();
   }
 
-  analyze_printf::ArgType::MatchKind match = AT.matchesType(S.Context, ExprTy);
-
-  if (match == analyze_printf::ArgType::Match) {
+  const analyze_printf::ArgType::MatchKind match =
+  AT.matchesType(S.Context, ExprTy);
+  bool pedantic = match == analyze_printf::ArgType::NoMatchPedantic;
+  if (match == analyze_printf::ArgType::Match)
 return true;
-  }
 
   // Look through argument promotions for our error message's reported type.
   // This includes the integral and floating promotions, but excludes array
@@ -6674,6 +6674,11 @@
 QualType CastTy;
 std::tie(CastTy, CastTyName) = shouldNotPrintDirectly(S.Context, IntendedTy, E);
 if (!CastTy.isNull()) {
+  // %zi/%zu are OK to use for NSInteger/NSUInteger of type int
+  // (long in ASTContext). Only complain to pedants.
+  if ((CastTyName == "NSInteger" || CastTyName == "NSUInteger") &&
+  AT.isSizeT() && AT.matchesType(S.Context, CastTy))
+pedantic = true;
   IntendedTy = CastTy;
   ShouldNotPrintDirectly = true;
 }
@@ -6693,10 +6698,10 @@
 CharSourceRange SpecRange = getSpecifierRange(StartSpecifier, SpecifierLen);
 
 if (IntendedTy == ExprTy && !ShouldNotPrintDirectly) {
-  unsigned diag = diag::warn_format_conversion_argument_type_mismatch;
-  if (match == analyze_format_string::ArgType::NoMatchPedantic) {
-diag = diag::warn_format_conversion_argument_type_mismatch_pedantic;
-  }
+  unsigned diag =
+  pedantic
+  ? diag::warn_format_conversion_argument_type_mismatch_pedantic
+  : diag::warn_format_conversion_argument_type_mismatch;
   // In this case, the specifier is wrong and should be changed to match
   // the argument.
   EmitFormatDiagnostic(S.PDiag(diag)
@@ -6752,9 +6757,11 @@
   

r333220 - Improve diagnostics for config mismatches with -fmodule-file.

[Richard Smith via cfe-commits]

Author: rsmith
Date: Thu May 24 13:03:51 2018
New Revision: 333220

URL: http://llvm.org/viewvc/llvm-project?rev=333220=rev
Log:
Improve diagnostics for config mismatches with -fmodule-file.

Unless the user uses -Wno-module-file-config-mismatch (or -Wno-error=...),
allow the AST reader to produce errors describing the nature of the config
mismatch.

Modified:
cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td
cfe/trunk/lib/Frontend/CompilerInstance.cpp
cfe/trunk/test/Modules/check-for-sanitizer-feature.cpp
cfe/trunk/test/Modules/merge-target-features.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td?rev=333220=333219=333220=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td Thu May 24 
13:03:51 2018
@@ -38,8 +38,8 @@ def err_pch_targetopt_mismatch : Error<
 "PCH file was compiled for the %0 '%1' but the current translation "
 "unit is being compiled for target '%2'">;
 def err_pch_targetopt_feature_mismatch : Error<
-"%select{AST file|current translation unit}0 was compiled with the target "
-"feature'%1' but the %select{current translation unit is|AST file was}0 "
+"%select{AST file was|current translation unit is}0 compiled with the 
target "
+"feature '%1' but the %select{current translation unit is|AST file was}0 "
 "not">;
 def err_pch_langopt_mismatch : Error<"%0 was %select{disabled|enabled}1 in "
 "PCH file but is currently %select{disabled|enabled}2">;

Modified: cfe/trunk/lib/Frontend/CompilerInstance.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInstance.cpp?rev=333220=333219=333220=diff
==
--- cfe/trunk/lib/Frontend/CompilerInstance.cpp (original)
+++ cfe/trunk/lib/Frontend/CompilerInstance.cpp Thu May 24 13:03:51 2018
@@ -1602,15 +1602,22 @@ bool CompilerInstance::loadModuleFile(St
   if (!ModuleManager)
 createModuleManager();
 
+  // If -Wmodule-file-config-mismatch is mapped as an error or worse, allow the
+  // ASTReader to diagnose it, since it can produce better errors that we can.
+  bool ConfigMismatchIsRecoverable =
+  getDiagnostics().getDiagnosticLevel(diag::warn_module_config_mismatch,
+  SourceLocation())
+<= DiagnosticsEngine::Warning;
+
   auto Listener = llvm::make_unique(*this);
   auto  = *Listener;
   ASTReader::ListenerScope ReadModuleNamesListener(*ModuleManager,
std::move(Listener));
 
   // Try to load the module file.
-  switch (ModuleManager->ReadAST(FileName, serialization::MK_ExplicitModule,
- SourceLocation(),
- ASTReader::ARR_ConfigurationMismatch)) {
+  switch (ModuleManager->ReadAST(
+  FileName, serialization::MK_ExplicitModule, SourceLocation(),
+  ConfigMismatchIsRecoverable ? ASTReader::ARR_ConfigurationMismatch : 0)) 
{
   case ASTReader::Success:
 // We successfully loaded the module file; remember the set of provided
 // modules so that we don't try to load implicit modules for them.

Modified: cfe/trunk/test/Modules/check-for-sanitizer-feature.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/check-for-sanitizer-feature.cpp?rev=333220=333219=333220=diff
==
--- cfe/trunk/test/Modules/check-for-sanitizer-feature.cpp (original)
+++ cfe/trunk/test/Modules/check-for-sanitizer-feature.cpp Thu May 24 13:03:51 
2018
@@ -43,7 +43,7 @@
 //
 // Import the PCH without ASan enabled (we expect an error).
 // RUN: not %clang_cc1 -x c -include-pch %t.asan_pch %s -verify 2>&1 | 
FileCheck %s --check-prefix=PCH_MISMATCH
-// PCH_MISMATCH: AST file was compiled with the target 
feature'-fsanitize=address' but the current translation unit is not
+// PCH_MISMATCH: AST file was compiled with the target feature 
'-fsanitize=address' but the current translation unit is not
 //
 // Emit a PCH with UBSan enabled.
 // RUN: %clang_cc1 -x c -fsanitize=null 
%S/Inputs/check-for-sanitizer-feature/check.h -emit-pch -o %t.ubsan_pch

Modified: cfe/trunk/test/Modules/merge-target-features.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/merge-target-features.cpp?rev=333220=333219=333220=diff
==
--- cfe/trunk/test/Modules/merge-target-features.cpp (original)
+++ cfe/trunk/test/Modules/merge-target-features.cpp Thu May 24 13:03:51 2018
@@ -19,10 +19,9 @@
 // RUN:   -triple i386-unknown-unknown \
 // RUN:   -target-cpu i386 \
 // RUN:   -fsyntax-only 

[PATCH] D47142: [x86] invpcid intrinsic

[Craig Topper via Phabricator via cfe-commits]

craig.topper added a comment.

LGTM, if you fix the ordering in cpuid.h.


https://reviews.llvm.org/D47142



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

[PATCH] D47301: Warning for framework include violation from Headers to PrivateHeaders

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added inline comments.



Comment at: include/clang/Basic/DiagnosticGroups.td:34-35
 def AutoImport : DiagGroup<"auto-import">;
 def FrameworkHdrQuotedInclude : 
DiagGroup<"quoted-include-in-framework-header">;
+def FrameworkIncludePrivateFromPublic : 
DiagGroup<"framework-include-private-from-public">;
 def CXX14BinaryLiteral : DiagGroup<"c++14-binary-literal">;

It might be convenient for users to have a warning group that will cover 
different framework warnings, something like -Wframework-hygiene. But it's out 
of scope for this review, more as an idea for future improvements.



Comment at: lib/Lex/HeaderSearch.cpp:625
+static bool isFrameworkStylePath(StringRef Path, bool ,
+ SmallVectorImpl ) {
   using namespace llvm::sys;

In this function we accept some paths that aren't valid framework paths. Need 
to think more if it is OK or if we want to be stricter.



Comment at: lib/Lex/HeaderSearch.cpp:893
+  // from Foo.framework/PrivateHeaders, since this violates public/private
+  // api boundaries and can cause modular dependency cycles.
+  SmallString<128> ToFramework;

s/api/API/



Comment at: lib/Lex/HeaderSearch.cpp:895
+  SmallString<128> ToFramework;
+  bool IncludeIsFrameworkPrivateHeader = false;
+  if (IsIncluderFromFramework && !IsFrameworkPrivateHeader &&

My opinion on readability is personal, so take it with a grain of salt. What if 
you make variable names more consistent, like
* IsIncluderPrivateHeader
* IsIncluderFromFramework
* IsIncludeePrivateHeader



Comment at: test/Modules/framework-public-includes-private.m:33
+
+int bar() { return foo(); }
+

I'm not entirely sure it's not covered by existing test. It might be worth 
testing private header including public header and private header including 
another private header.


https://reviews.llvm.org/D47301



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

[PATCH] D46024: [clang-format] Add SpaceBeforeCpp11BracedList option.

[Ross Kirsling via Phabricator via cfe-commits]

rkirsling added a comment.

@klimek In our IRC discussion yesterday, I know you expressed disapproval of 
WebKit's choice, but given its reality, am I correct in concluding that this 
can be landed?


Repository:
  rC Clang

https://reviews.llvm.org/D46024



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

[PATCH] D47067: Update NRVO logic to support early return

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

Some minor nits. I would also like to see a test for the unoptimized (-O0) IR 
generated by Clang for the case where there are two NRVO variables in the same 
function. Otherwise, this looks good to go!




Comment at: lib/Sema/Scope.cpp:122-123
+void Scope::setNRVOCandidate(VarDecl *Candidate) {
+  for (auto* D : DeclsInScope) {
+VarDecl* VD = dyn_cast_or_null(D);
+if (VD && VD != Candidate && VD->isNRVOCandidate())

`*` on the right, please. Also `auto` -> `Decl` would be clearer and no longer. 
Is `dyn_cast_or_null` really necessary? (Can `DeclsInScope` contain `nullptr`?) 
I would expect that just `dyn_cast` would suffice.



Comment at: lib/Sema/SemaDecl.cpp:12619-12620
 void Sema::computeNRVO(Stmt *Body, FunctionScopeInfo *Scope) {
-  ReturnStmt **Returns = Scope->Returns.data();
+  for (ReturnStmt* Return : Scope->Returns) {
+const VarDecl* Candidate = Return->getNRVOCandidate();
+if (!Candidate)

`*` on the right, please.


Repository:
  rC Clang

https://reviews.llvm.org/D47067



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

[PATCH] D47229: Make atomic non-member functions as nonnull

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

Addressed nit.


Repository:
  rC Clang

https://reviews.llvm.org/D47229



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

[PATCH] D47229: Make atomic non-member functions as nonnull

[JF Bastien via Phabricator via cfe-commits]

jfb updated this revision to Diff 148458.
jfb marked an inline comment as done.
jfb added a comment.

- Address nit.


Repository:
  rC Clang

https://reviews.llvm.org/D47229

Files:
  lib/Sema/SemaChecking.cpp
  test/Sema/atomic-ops.c

Index: test/Sema/atomic-ops.c
===
--- test/Sema/atomic-ops.c
+++ test/Sema/atomic-ops.c
@@ -531,8 +531,80 @@
 }
 
 void nullPointerWarning(_Atomic(int) *Ap, int *p, int val) {
-  // The 'expected' pointer shouldn't be NULL.
-  (void)__c11_atomic_compare_exchange_strong(Ap, NULL, val, memory_order_relaxed, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
-  (void)atomic_compare_exchange_weak(Ap, ((void*)0), val); // expected-warning {{null passed to a callee that requires a non-null argument}}
-  (void)__atomic_compare_exchange_n(p, NULL, val, 0, memory_order_relaxed, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  volatile _Atomic(int) vai;
+  _Atomic(int) ai;
+  volatile int vi = 42;
+  int i = 42;
+
+  __c11_atomic_init((volatile _Atomic(int)*)0, 42); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  __c11_atomic_init((_Atomic(int)*)0, 42); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  __c11_atomic_store((volatile _Atomic(int)*)0, 42, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  __c11_atomic_store((_Atomic(int)*)0, 42, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  (void)__c11_atomic_load((volatile _Atomic(int)*)0, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  (void)__c11_atomic_load((_Atomic(int)*)0, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  (void)__c11_atomic_exchange((volatile _Atomic(int)*)0, 42, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  (void)__c11_atomic_exchange((_Atomic(int)*)0, 42, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  (void)__c11_atomic_compare_exchange_weak((volatile _Atomic(int)*)0, , 42, memory_order_relaxed, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  (void)__c11_atomic_compare_exchange_weak((_Atomic(int)*)0, , 42, memory_order_relaxed, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  (void)__c11_atomic_compare_exchange_weak(, (int*)0, 42, memory_order_relaxed, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  (void)__c11_atomic_compare_exchange_weak(, (int*)0, 42, memory_order_relaxed, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  (void)__c11_atomic_compare_exchange_strong((volatile _Atomic(int)*)0, , 42, memory_order_relaxed, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  (void)__c11_atomic_compare_exchange_strong((_Atomic(int)*)0, , 42, memory_order_relaxed, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  (void)__c11_atomic_compare_exchange_strong(, (int*)0, 42, memory_order_relaxed, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  (void)__c11_atomic_compare_exchange_strong(, (int*)0, 42, memory_order_relaxed, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  (void)__c11_atomic_fetch_add((volatile _Atomic(int)*)0, 42, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  (void)__c11_atomic_fetch_add((_Atomic(int)*)0, 42, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  (void)__c11_atomic_fetch_sub((volatile _Atomic(int)*)0, 42, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  (void)__c11_atomic_fetch_sub((_Atomic(int)*)0, 42, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  (void)__c11_atomic_fetch_and((volatile _Atomic(int)*)0, 42, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  (void)__c11_atomic_fetch_and((_Atomic(int)*)0, 42, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+  (void)__c11_atomic_fetch_or((volatile _Atomic(int)*)0, 42, memory_order_relaxed); // expected-warning {{null passed to a callee that 

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

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

Quuxplusone added a comment.

> Sounds good to me. I don't have commit privs so could you land it for me?

@EricWF ping!


Repository:
  rCXX libc++

https://reviews.llvm.org/D47109



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

[PATCH] D46052: GNUstep Objective-C ABI version 2

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added inline comments.



Comment at: lib/CodeGen/CGObjCGNU.cpp:512
   /// used to return an untyped selector (with the types field set to NULL).
-  llvm::Value *GetSelector(CodeGenFunction , Selector Sel,
+  virtual llvm::Value *GetSelector(CodeGenFunction , Selector Sel,
const std::string );

This causes 
CGObjCGNU.cpp:589:16: warning: ‘virtual llvm::Value* 
{anonymous}::CGObjCGNU::GetSelector(clang::CodeGen::CodeGenFunction&, const 
clang::ObjCMethodDecl*)’ was hidden [-Woverloaded-virtual]
   llvm::Value *GetSelector(CodeGenFunction ,



Repository:
  rC Clang

https://reviews.llvm.org/D46052



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

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

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai created this revision.
vsapsai added reviewers: arphaman, majnemer.

NumTypos guard value ~0U doesn't prevent from creating new delayed
typos. When you create new delayed typos during typo correction, value
~0U wraps around to 0. This state is inconsistent and depending on total
number of typos you can hit the assertion

> Assertion failed: (DelayedTypos.empty() && "Uncorrected typos!"), function 
> ~Sema, file clang/lib/Sema/Sema.cpp, line 366.

or have infinite typo correction loop.

Fix by disabling typo correction during performing typo correcting
transform. It disables the feature of having typo corrections on top of
other typo corrections because that feature is unreliable.

rdar://problem/38642201


https://reviews.llvm.org/D47341

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/Sema/typo-correction.c
  clang/test/SemaCXX/typo-correction-delayed.cpp

Index: clang/test/SemaCXX/typo-correction-delayed.cpp
===
--- clang/test/SemaCXX/typo-correction-delayed.cpp
+++ clang/test/SemaCXX/typo-correction-delayed.cpp
@@ -137,13 +137,12 @@
 
 namespace PR21925 {
 struct X {
-  int get() { return 7; }  // expected-note {{'get' declared here}}
+  int get() { return 7; }
 };
 void test() {
-  X variable;  // expected-note {{'variable' declared here}}
+  X variable;
 
-  // expected-error@+2 {{use of undeclared identifier 'variableX'; did you mean 'variable'?}}
-  // expected-error@+1 {{no member named 'getX' in 'PR21925::X'; did you mean 'get'?}}
+  // expected-error@+1 {{use of undeclared identifier 'variableX'}}
   int x = variableX.getX();
 }
 }
Index: clang/test/Sema/typo-correction.c
===
--- clang/test/Sema/typo-correction.c
+++ clang/test/Sema/typo-correction.c
@@ -87,3 +87,16 @@
 void overloadable_callexpr(int arg) {
 	func_overloadable(ar); //expected-error{{use of undeclared identifier}}
 }
+
+// rdar://problem/38642201
+struct rdar38642201 {
+  int fieldName;
+};
+
+void rdar38642201_callee(int x, int y);
+void rdar38642201_caller() {
+  struct rdar38642201 structVar;
+  rdar38642201_callee(
+  structVarTypo.fieldNameTypo, //expected-error{{use of undeclared identifier 'structVarTypo'}}
+  structVarTypo2.fieldNameTypo2); //expected-error{{use of undeclared identifier 'structVarTypo2'}}
+}
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -7588,7 +7588,7 @@
 // handling potentially ambiguous typo corrections as any new TypoExprs will
 // have been introduced by the application of one of the correction
 // candidates and add little to no value if corrected.
-SemaRef.DisableTypoCorrection = true;
+Sema::DisableTypoCorrectionScope Scope(SemaRef);
 while (!AmbiguousTypoExprs.empty()) {
   auto TE  = AmbiguousTypoExprs.back();
   auto Cached = TransformCache[TE];
@@ -7605,7 +7605,6 @@
   State.Consumer->restoreSavedPosition();
   TransformCache[TE] = Cached;
 }
-SemaRef.DisableTypoCorrection = false;
 
 // Ensure that all of the TypoExprs within the current Expr have been found.
 if (!Res.isUsable())
@@ -7668,11 +7667,14 @@
   if (E && !ExprEvalContexts.empty() && ExprEvalContexts.back().NumTypos &&
   (E->isTypeDependent() || E->isValueDependent() ||
E->isInstantiationDependent())) {
+DisableTypoCorrectionScope DisableRecurrentCorrectionScope(*this);
 auto TyposInContext = ExprEvalContexts.back().NumTypos;
 assert(TyposInContext < ~0U && "Recursive call of CorrectDelayedTyposInExpr");
 ExprEvalContexts.back().NumTypos = ~0U;
 auto TyposResolved = DelayedTypos.size();
 auto Result = TransformTypos(*this, InitDecl, Filter).Transform(E);
+assert(ExprEvalContexts.back().NumTypos == ~0U &&
+   "Unexpected NumTypos modification");
 ExprEvalContexts.back().NumTypos = TyposInContext;
 TyposResolved -= DelayedTypos.size();
 if (Result.isInvalid() || Result.get() != E) {
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -7561,6 +7561,22 @@
 }
   };
 
+  /// RAII class used to disable typo correction temporarily.
+  class DisableTypoCorrectionScope {
+Sema 
+bool PrevDisableTypoCorrection;
+
+  public:
+explicit DisableTypoCorrectionScope(Sema )
+: SemaRef(SemaRef),
+  PrevDisableTypoCorrection(SemaRef.DisableTypoCorrection) {
+  SemaRef.DisableTypoCorrection = true;
+}
+~DisableTypoCorrectionScope() {
+  SemaRef.DisableTypoCorrection = PrevDisableTypoCorrection;
+}
+  };
+
   /// The current instantiation scope used to store local
   /// variables.
   LocalInstantiationScope *CurrentInstantiationScope;

[PATCH] D47340: [ClangDiagnostics] Silence warning about fallthrough after PrintFatalError

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 created this revision.
Herald added subscribers: cfe-commits, jkorous, MaskRay, ioeric, ilya-biryukov.

ClangDiagnosticsEmitter.cpp:1047:57: warning: this statement may fall through 
[-Wimplicit-fallthrough=]

  Builder.PrintFatalError("Unknown modifier type: " + Modifier);
  ~~^~

ClangDiagnosticsEmitter.cpp:1048:5: note: here

  case MT_Select: {
^


Repository:
  rC Clang

https://reviews.llvm.org/D47340

Files:
  utils/TableGen/ClangDiagnosticsEmitter.cpp


Index: utils/TableGen/ClangDiagnosticsEmitter.cpp
===
--- utils/TableGen/ClangDiagnosticsEmitter.cpp
+++ utils/TableGen/ClangDiagnosticsEmitter.cpp
@@ -1045,6 +1045,7 @@
 switch (ModType) {
 case MT_Unknown:
   Builder.PrintFatalError("Unknown modifier type: " + Modifier);
+  break;
 case MT_Select: {
   SelectPiece *Select = New(MT_Select);
   do {


Index: utils/TableGen/ClangDiagnosticsEmitter.cpp
===
--- utils/TableGen/ClangDiagnosticsEmitter.cpp
+++ utils/TableGen/ClangDiagnosticsEmitter.cpp
@@ -1045,6 +1045,7 @@
 switch (ModType) {
 case MT_Unknown:
   Builder.PrintFatalError("Unknown modifier type: " + Modifier);
+  break;
 case MT_Select: {
   SelectPiece *Select = New(MT_Select);
   do {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47097: [DebugInfo] Preserve scope in auto generated StoreInst

[Vedant Kumar via Phabricator via cfe-commits]

vsk added a comment.

In https://reviews.llvm.org/D47097#248, @aprantl wrote:

> In https://reviews.llvm.org/D47097#223, @gramanas wrote:
>
> > In https://reviews.llvm.org/D47097#149, @probinson wrote:
> >
> > > Can we step back a second and better explain what the problem is? With 
> > > current Clang the debug info for this function looks okay to me.
> > >  The store that is "missing" a debug location is homing the formal 
> > > parameter to its local stack location; this is part of prolog setup, not 
> > > "real" code.
> >
>


The problem @gramanas is trying to address appears after SROA. SROA invokes 
mem2reg, which tries to assign scope information to the phis it creates (see 
https://reviews.llvm.org/D45397). Subsequent passes which touch these phis can 
use these debug locations. This makes it easier for the debugger to find the 
right scope when handling a machine exception.

Store instructions in the prolog without scope information cause mem2reg to 
create phis without scope info. E.g:

  // foo.c
  extern int map[];
  void f(int a, int n) {
for (int i = 0; i < n; ++i)
  a = map[a];
  }
  
  $ clang foo.c -S -emit-llvm -o - -g -O1 -mllvm -opt-bisect-limit=2
  ..
  %a.addr.0 = phi i32 [ %a, %entry ], [ %0, %for.body ]

(Side note: @gramanas, it would help to have the full context/motivation for 
changes in the patch summary.)

>> Isn't this the reason the artificial debug loc exists? The store in the 
>> prolog might not be real code but it should still have the scope that the 
>> rest of the function has.
> 
> Instructions that are part of the function prologue are supposed to have no 
> debug location, not an artificial one. The funciton prologue ends at the 
> first instruction with a nonempty location.

I've been reading through PEI but I'm having a hard time verifying this. How 
does llvm determine where the function prologue ends when there isn't any debug 
info? What would go wrong if llvm started to behave as if the prologue ended 
sooner than it should? Also, why isn't this a problem for the swift compiler, 
which appears to always assign debug locations to each instruction?


Repository:
  rC Clang

https://reviews.llvm.org/D47097



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

[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

Can you also add a test for `_Bool _Accum`.

Also, `-enable-fixed-point -x c++` failing.




Comment at: lib/AST/ExprConstant.cpp:7361
+case BuiltinType::ULongAccum:
+  // GCC does not cover FIXED_POINT_TYPE in it's switch stmt and defaults 
to
+  // no_type_class

"its"



Comment at: lib/Basic/TargetInfo.cpp:45
+  AccumWidth = AccumAlign = 32;
+  LongAccumWidth = LongAccumAlign = 64;
   SuitableAlign = 64;

This seems weird because Targets which don't have these values for the 
non-Accum versions will have .e.g. `sizeof(short) != sizeof(short _Accum)`. Is 
there a point in ever having `_Accum` differ in size, width, and alignment from 
the underlying type? If not, can you set these values after the sub-target has 
specified its preferences?


Repository:
  rC Clang

https://reviews.llvm.org/D46084



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

[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type

[Leonard Chan via Phabricator via cfe-commits]

leonardchan updated this revision to Diff 148452.

Repository:
  rC Clang

https://reviews.llvm.org/D46084

Files:
  include/clang-c/Index.h
  include/clang/AST/ASTContext.h
  include/clang/AST/BuiltinTypes.def
  include/clang/Basic/DiagnosticCommonKinds.td
  include/clang/Basic/LangOptions.def
  include/clang/Basic/Specifiers.h
  include/clang/Basic/TargetInfo.h
  include/clang/Basic/TokenKinds.def
  include/clang/Driver/Options.td
  include/clang/Sema/DeclSpec.h
  include/clang/Serialization/ASTBitCodes.h
  lib/AST/ASTContext.cpp
  lib/AST/ExprConstant.cpp
  lib/AST/ItaniumMangle.cpp
  lib/AST/MicrosoftMangle.cpp
  lib/AST/NSAPI.cpp
  lib/AST/Type.cpp
  lib/AST/TypeLoc.cpp
  lib/Analysis/PrintfFormatString.cpp
  lib/Basic/TargetInfo.cpp
  lib/CodeGen/CGDebugInfo.cpp
  lib/CodeGen/CodeGenTypes.cpp
  lib/CodeGen/ItaniumCXXABI.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Index/USRGeneration.cpp
  lib/Parse/ParseDecl.cpp
  lib/Sema/DeclSpec.cpp
  lib/Sema/SemaTemplateVariadic.cpp
  lib/Sema/SemaType.cpp
  lib/Serialization/ASTCommon.cpp
  lib/Serialization/ASTReader.cpp
  test/Frontend/fixed_point.c
  test/Frontend/fixed_point_errors.c
  test/Frontend/fixed_point_errors.cpp
  test/Frontend/fixed_point_not_enabled.c
  tools/libclang/CXType.cpp

Index: tools/libclang/CXType.cpp
===
--- tools/libclang/CXType.cpp
+++ tools/libclang/CXType.cpp
@@ -53,6 +53,12 @@
 BTCASE(Float);
 BTCASE(Double);
 BTCASE(LongDouble);
+BTCASE(ShortAccum);
+BTCASE(Accum);
+BTCASE(LongAccum);
+BTCASE(UShortAccum);
+BTCASE(UAccum);
+BTCASE(ULongAccum);
 BTCASE(Float16);
 BTCASE(Float128);
 BTCASE(NullPtr);
@@ -546,6 +552,12 @@
 TKIND(Float);
 TKIND(Double);
 TKIND(LongDouble);
+TKIND(ShortAccum);
+TKIND(Accum);
+TKIND(LongAccum);
+TKIND(UShortAccum);
+TKIND(UAccum);
+TKIND(ULongAccum);
 TKIND(Float16);
 TKIND(Float128);
 TKIND(NullPtr);
Index: test/Frontend/fixed_point_not_enabled.c
===
--- /dev/null
+++ test/Frontend/fixed_point_not_enabled.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -x c -verify %s
+
+// Primary fixed point types
+signed short _Accum s_short_accum;// expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+signed _Accum s_accum;// expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+signed long _Accum s_long_accum;  // expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+unsigned short _Accum u_short_accum;  // expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+unsigned _Accum u_accum;  // expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+unsigned long _Accum u_long_accum;// expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+
+// Aliased fixed point types
+short _Accum short_accum; // expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+_Accum accum; // expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+  // expected-warning@-1{{type specifier missing, defaults to 'int'}}
+long _Accum long_accum;   // expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
Index: test/Frontend/fixed_point_errors.cpp
===
--- /dev/null
+++ test/Frontend/fixed_point_errors.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -x c++ -enable-fixed-point %s -verify
+
+// Name namgling is not provided for fixed point types in c++
+
+signed short _Accum s_short_accum;  // expected-error{{fixed point types are only allowed in C}}
+signed _Accum s_accum;  // expected-error{{fixed point types are only allowed in C}}
+signed long _Accum s_long_accum;// expected-error{{fixed point types are only allowed in C}}
+unsigned short _Accum u_short_accum;// expected-error{{fixed point types are only allowed in C}}
+unsigned _Accum u_accum;// expected-error{{fixed point types are only allowed in C}}
+unsigned long _Accum u_long_accum;  // expected-error{{fixed point types are only allowed in C}}
+
+short _Accum short_accum;   // expected-error{{fixed point types are only allowed in C}}
+_Accum accum;   // expected-error{{fixed point types are only allowed in C}}
+// expected-error@-1{{C++ requires a type specifier for all declarations}}
+long _Accum long_accum; // expected-error{{fixed point types are only allowed in C}}
Index: test/Frontend/fixed_point_errors.c
===
--- 

[PATCH] D46950: [ASTImporter] Fix duplicate class template definitions problem

[Aleksei Sidorin via Phabricator via cfe-commits]

a.sidorin added a comment.

Hm. Should we test `-fms-compatibility` in addition to 
`-fdelayed-template-parsing`?


Repository:
  rC Clang

https://reviews.llvm.org/D46950



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

[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type

[Leonard Chan via Phabricator via cfe-commits]

leonardchan marked 6 inline comments as done.
leonardchan added inline comments.



Comment at: lib/CodeGen/ItaniumCXXABI.cpp:2684
 
   // Types added here must also be added to EmitFundamentalRTTIDescriptors.
   switch (Ty->getKind()) {

rsmith wrote:
> Note this comment :)
Returned false for these types now. Not sure if these types should also be 
added to EmitFundamentalRTTIDescriptors since the OCL types do not appear under 
there.


Repository:
  rC Clang

https://reviews.llvm.org/D46084



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

[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type

[Leonard Chan via Phabricator via cfe-commits]

leonardchan updated this revision to Diff 148445.
leonardchan added a comment.

- Reverted changes involving name mangling since we will only support c++ for 
now. Will concurrently raise an issue on 
https://github.com/itanium-cxx-abi/cxx-abi/ to get characters for name mangling.
- Added a flag that needs to be provided to enable usage of fixed point types. 
Not including this flag and using fixed point types throws an error. Currently, 
this patch allows for these types to be used in all versions of C, but this can 
be narrowed down to specific versions of C.
- An error is thrown when using fixed point types in C++.
- Fixed point types are ignored during USRGeneration since the type only gets 
mangled in C++.
- Fixed point types their own width and alignment accessors/variables in 
TargetInfo.
- Updated debug info to use `DW_ATE_signed_fixed` and `DW_ATE_unsigned_fixed`.
- Added tests mixing _Accum with other type specifiers


Repository:
  rC Clang

https://reviews.llvm.org/D46084

Files:
  include/clang-c/Index.h
  include/clang/AST/ASTContext.h
  include/clang/AST/BuiltinTypes.def
  include/clang/Basic/DiagnosticCommonKinds.td
  include/clang/Basic/LangOptions.def
  include/clang/Basic/Specifiers.h
  include/clang/Basic/TargetInfo.h
  include/clang/Basic/TokenKinds.def
  include/clang/Driver/Options.td
  include/clang/Sema/DeclSpec.h
  include/clang/Serialization/ASTBitCodes.h
  lib/AST/ASTContext.cpp
  lib/AST/ExprConstant.cpp
  lib/AST/ItaniumMangle.cpp
  lib/AST/MicrosoftMangle.cpp
  lib/AST/NSAPI.cpp
  lib/AST/Type.cpp
  lib/AST/TypeLoc.cpp
  lib/Analysis/PrintfFormatString.cpp
  lib/Basic/TargetInfo.cpp
  lib/CodeGen/CGDebugInfo.cpp
  lib/CodeGen/CodeGenTypes.cpp
  lib/CodeGen/ItaniumCXXABI.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Index/USRGeneration.cpp
  lib/Parse/ParseDecl.cpp
  lib/Sema/DeclSpec.cpp
  lib/Sema/SemaTemplateVariadic.cpp
  lib/Sema/SemaType.cpp
  lib/Serialization/ASTCommon.cpp
  lib/Serialization/ASTReader.cpp
  test/Frontend/fixed_point.c
  test/Frontend/fixed_point_errors.c
  test/Frontend/fixed_point_errors.cpp
  test/Frontend/fixed_point_not_enabled.c
  tools/libclang/CXType.cpp

Index: tools/libclang/CXType.cpp
===
--- tools/libclang/CXType.cpp
+++ tools/libclang/CXType.cpp
@@ -53,6 +53,12 @@
 BTCASE(Float);
 BTCASE(Double);
 BTCASE(LongDouble);
+BTCASE(ShortAccum);
+BTCASE(Accum);
+BTCASE(LongAccum);
+BTCASE(UShortAccum);
+BTCASE(UAccum);
+BTCASE(ULongAccum);
 BTCASE(Float16);
 BTCASE(Float128);
 BTCASE(NullPtr);
@@ -546,6 +552,12 @@
 TKIND(Float);
 TKIND(Double);
 TKIND(LongDouble);
+TKIND(ShortAccum);
+TKIND(Accum);
+TKIND(LongAccum);
+TKIND(UShortAccum);
+TKIND(UAccum);
+TKIND(ULongAccum);
 TKIND(Float16);
 TKIND(Float128);
 TKIND(NullPtr);
Index: test/Frontend/fixed_point_not_enabled.c
===
--- /dev/null
+++ test/Frontend/fixed_point_not_enabled.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -x c -verify %s
+
+// Primary fixed point types
+signed short _Accum s_short_accum;// expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+signed _Accum s_accum;// expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+signed long _Accum s_long_accum;  // expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+unsigned short _Accum u_short_accum;  // expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+unsigned _Accum u_accum;  // expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+unsigned long _Accum u_long_accum;// expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+
+// Aliased fixed point types
+short _Accum short_accum; // expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+_Accum accum; // expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+  // expected-warning@-1{{type specifier missing, defaults to 'int'}}
+long _Accum long_accum;   // expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
Index: test/Frontend/fixed_point_errors.cpp
===
--- /dev/null
+++ test/Frontend/fixed_point_errors.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -x c++ -enable-fixed-point %s -verify
+
+// Name namgling is not provided for fixed point types in c++
+
+signed short _Accum s_short_accum;  // expected-error{{fixed point types are only allowed in C}}
+signed _Accum s_accum;  // expected-error{{fixed point types are only allowed in C}}
+signed long _Accum s_long_accum;// 

[PATCH] D47260: Testcase for dwarf 5 crash/assert when calculating a checksum for an expansion

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

I see that the per-file info does track the presence of `# [line] N` but it's 
not immediately obvious how to query "has any file seen one" which is really 
what I'd want to know.  The file information has various levels of 
indirection...


Repository:
  rC Clang

https://reviews.llvm.org/D47260



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

[PATCH] D47058: [ASTImporter] Fix ClassTemplateSpecialization in wrong DC

[Aleksei Sidorin via Phabricator via cfe-commits]

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

Ok, I got it, thank you!


Repository:
  rC Clang

https://reviews.llvm.org/D47058



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

r333211 - [X86] Fix a bad cast in _mm512_mask_abs_epi32 and _mm512_maskz_abs_epi32.

[Craig Topper via cfe-commits]

Author: ctopper
Date: Thu May 24 10:32:49 2018
New Revision: 333211

URL: http://llvm.org/viewvc/llvm-project?rev=333211=rev
Log:
[X86] Fix a bad cast in _mm512_mask_abs_epi32 and _mm512_maskz_abs_epi32.

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

Modified: cfe/trunk/lib/Headers/avx512fintrin.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/avx512fintrin.h?rev=333211=333210=333211=diff
==
--- cfe/trunk/lib/Headers/avx512fintrin.h (original)
+++ cfe/trunk/lib/Headers/avx512fintrin.h Thu May 24 10:32:49 2018
@@ -1948,7 +1948,7 @@ _mm512_abs_epi32(__m512i __A)
 static __inline__ __m512i __DEFAULT_FN_ATTRS
 _mm512_mask_abs_epi32 (__m512i __W, __mmask16 __U, __m512i __A)
 {
-  return (__m512i)__builtin_ia32_selectd_512((__mmask8)__U,
+  return (__m512i)__builtin_ia32_selectd_512(__U,
  (__v16si)_mm512_abs_epi32(__A),
  (__v16si)__W);
 }
@@ -1956,7 +1956,7 @@ _mm512_mask_abs_epi32 (__m512i __W, __mm
 static __inline__ __m512i __DEFAULT_FN_ATTRS
 _mm512_maskz_abs_epi32 (__mmask16 __U, __m512i __A)
 {
-  return (__m512i)__builtin_ia32_selectd_512((__mmask8)__U,
+  return (__m512i)__builtin_ia32_selectd_512(__U,
  (__v16si)_mm512_abs_epi32(__A),
  (__v16si)_mm512_setzero_si512());
 }


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

[PATCH] D47097: [DebugInfo] Preserve scope in auto generated StoreInst

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

In https://reviews.llvm.org/D47097#223, @gramanas wrote:

> In https://reviews.llvm.org/D47097#149, @probinson wrote:
>
> > Can we step back a second and better explain what the problem is? With 
> > current Clang the debug info for this function looks okay to me.
> >  The store that is "missing" a debug location is homing the formal 
> > parameter to its local stack location; this is part of prolog setup, not 
> > "real" code.
>
>
> Isn't this the reason the artificial debug loc exists? The store in the 
> prolog might not be real code but it should still have the scope that the 
> rest of the function has.


Instructions that are part of the function prologue are supposed to have no 
debug location, not an artificial one. The funciton prologue ends at the first 
instruction with a nonempty location.


Repository:
  rC Clang

https://reviews.llvm.org/D47097



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

[PATCH] D47267: [UnrollAndJam] Add unroll_and_jam pragma handling

[Michael Kruse via Phabricator via cfe-commits]

Meinersbur added a comment.

Yes, this is what I had in mind. Thank you.

I am preparing an RFC on what I am trying to do. This should clarify our goals 
and to what extend `#pragma clang loop` conflicts with it.


https://reviews.llvm.org/D47267



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

[PATCH] D47290: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %zu/%zi on Darwin

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added inline comments.



Comment at: test/SemaObjC/format-size-spec-nsinteger.m:18
+  NSUInteger j = 0;
+  NSLog(@"max NSInteger = %zi", i);  // CHECK: values of type 'NSInteger' 
should not be used as format arguments; add an explicit cast to 'long' instead
+  NSLog(@"max NSUinteger = %zu", j); // CHECK: values of type 'NSUInteger' 
should not be used as format arguments; add an explicit cast to 'unsigned long' 
instead

This test looks identical to 
test/SemaObjC/format-size-spec-nsinteger-nopedantic.m except for the CHECK 
lines. You can probably merge the two files if you separate the CHECK lines 
from the lines that call NSLog (see test/Sema/format-strings-darwin.c for 
example).


Repository:
  rC Clang

https://reviews.llvm.org/D47290



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

[PATCH] D47097: [DebugInfo] Preserve scope in auto generated StoreInst

[Anastasis via Phabricator via cfe-commits]

gramanas added a comment.

In https://reviews.llvm.org/D47097#149, @probinson wrote:

> Can we step back a second and better explain what the problem is? With 
> current Clang the debug info for this function looks okay to me.
>  The store that is "missing" a debug location is homing the formal parameter 
> to its local stack location; this is part of prolog setup, not "real" code.


Isn't this the reason the artificial debug loc exists? The store in the prolog 
might not be real code but it should still have the scope that the rest of the 
function has.


Repository:
  rC Clang

https://reviews.llvm.org/D47097



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

[PATCH] D47273: [bash-completion] Fix tab separation on macOS

[Ben Langmuir via Phabricator via cfe-commits]

benlangmuir closed this revision.
benlangmuir added a comment.

r333202


https://reviews.llvm.org/D47273



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

r333202 - [bash-completion] Fix tab separation on macOS

[Ben Langmuir via cfe-commits]

Author: benlangmuir
Date: Thu May 24 09:25:40 2018
New Revision: 333202

URL: http://llvm.org/viewvc/llvm-project?rev=333202=rev
Log:
[bash-completion] Fix tab separation on macOS

We have a regex that needs to match a tab character in the command
output, but on macOS sed doesn't support '\t', causing it to split on
the 't' character instead. Fix by having bash expand the \t first.

Modified:
cfe/trunk/utils/bash-autocomplete.sh

Modified: cfe/trunk/utils/bash-autocomplete.sh
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/bash-autocomplete.sh?rev=333202=333201=333202=diff
==
--- cfe/trunk/utils/bash-autocomplete.sh (original)
+++ cfe/trunk/utils/bash-autocomplete.sh Thu May 24 09:25:40 2018
@@ -38,7 +38,8 @@ _clang()
 
   # expand ~ to $HOME
   eval local path=${COMP_WORDS[0]}
-  flags=$( "$path" --autocomplete="$arg" 2>/dev/null | sed -e 's/\t.*//' )
+  # Use $'\t' so that bash expands the \t for older versions of sed.
+  flags=$( "$path" --autocomplete="$arg" 2>/dev/null | sed -e $'s/\t.*//' )
   # If clang is old that it does not support --autocomplete,
   # fall back to the filename completion.
   if [[ "$?" != 0 ]]; then


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

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

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.
ilya-biryukov added reviewers: ioeric, sammccall.
Herald added subscribers: jkorous, MaskRay, mehdi_amini, klimek.

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

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


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D47331

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

Index: unittests/clangd/TestTU.cpp
===
--- unittests/clangd/TestTU.cpp
+++ unittests/clangd/TestTU.cpp
@@ -74,7 +74,7 @@
 
 const NamedDecl (ParsedAST , llvm::StringRef QName) {
   const NamedDecl *Result = nullptr;
-  for (const Decl *D : AST.getTopLevelDecls()) {
+  for (const Decl *D : AST.getLocalTopLevelDecls()) {
 auto *ND = dyn_cast(D);
 if (!ND || ND->getNameAsString() != QName)
   continue;
Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -168,7 +168,7 @@
   IndexOpts.SystemSymbolFilter =
   index::IndexingOptions::SystemSymbolFilterKind::All;
   IndexOpts.IndexFunctionLocals = true;
-  indexTopLevelDecls(AST.getASTContext(), AST.getTopLevelDecls(),
+  indexTopLevelDecls(AST.getASTContext(), AST.getLocalTopLevelDecls(),
  DeclMacrosFinder, IndexOpts);
 
   return {DeclMacrosFinder.takeDecls(), DeclMacrosFinder.takeMacroInfos()};
@@ -418,7 +418,7 @@
   IndexOpts.SystemSymbolFilter =
   index::IndexingOptions::SystemSymbolFilterKind::All;
   IndexOpts.IndexFunctionLocals = true;
-  indexTopLevelDecls(AST.getASTContext(), AST.getTopLevelDecls(),
+  indexTopLevelDecls(AST.getASTContext(), AST.getLocalTopLevelDecls(),
  DocHighlightsFinder, IndexOpts);
 
   return DocHighlightsFinder.takeHighlights();
Index: clangd/ClangdUnit.h
===
--- clangd/ClangdUnit.h
+++ clangd/ClangdUnit.h
@@ -44,12 +44,10 @@
 
 // Stores Preamble and associated data.
 struct PreambleData {
-  PreambleData(PrecompiledPreamble Preamble,
-   std::vector TopLevelDeclIDs,
-   std::vector Diags, std::vector Inclusions);
+  PreambleData(PrecompiledPreamble Preamble, std::vector Diags,
+   std::vector Inclusions);
 
   PrecompiledPreamble Preamble;
-  std::vector TopLevelDeclIDs;
   std::vector Diags;
   // Processes like code completions and go-to-definitions will need #include
   // information, and their compile action skips preamble range.
@@ -87,10 +85,9 @@
   std::shared_ptr getPreprocessorPtr();
   const Preprocessor () const;
 
-  /// This function returns all top-level decls, including those that come
-  /// from Preamble. Decls, coming from Preamble, have to be deserialized, so
-  /// this call might be expensive.
-  ArrayRef getTopLevelDecls();
+  /// This function returns top-level decls, present in the main file of the
+  /// AST. The result does not include the decls that come from the preamble.
+  ArrayRef getLocalTopLevelDecls();
 
   const std::vector () const;
 
@@ -106,9 +103,6 @@
 std::vector TopLevelDecls, std::vector Diags,
 std::vector Inclusions);
 
-private:
-  void ensurePreambleDeclsDeserialized();
-
   // In-memory preambles must outlive the AST, it is important that this member
   // goes before Clang and Action.
   std::shared_ptr Preamble;
@@ -123,7 +117,6 @@
   // Data, stored after parsing.
   std::vector Diags;
   std::vector TopLevelDecls;
-  bool PreambleDeclsDeserialized;
   std::vector Inclusions;
 };
 
Index: clangd/ClangdUnit.cpp
===
--- clangd/ClangdUnit.cpp
+++ clangd/ClangdUnit.cpp
@@ -90,37 +90,15 @@
   CppFilePreambleCallbacks(PathRef File, PreambleParsedCallback ParsedCallback)
   : File(File), ParsedCallback(ParsedCallback) {}
 
-  std::vector takeTopLevelDeclIDs() {
-return std::move(TopLevelDeclIDs);
-  }
-
   std::vector takeInclusions() { return std::move(Inclusions); }
 
-  void AfterPCHEmitted(ASTWriter ) override {
-TopLevelDeclIDs.reserve(TopLevelDecls.size());
-for (Decl *D : TopLevelDecls) {
-  // Invalid top-level decls may not have been serialized.
-  if (D->isInvalidDecl())
-continue;
-  TopLevelDeclIDs.push_back(Writer.getDeclID(D));
-}
-  }
-
   void AfterExecute(CompilerInstance ) override {
 if (!ParsedCallback)
   return;
 trace::Span Tracer("Running PreambleCallback");
 ParsedCallback(File, CI.getASTContext(), CI.getPreprocessorPtr());
   }
 
-  void HandleTopLevelDecl(DeclGroupRef DG) override {
-for (Decl *D : DG) {
-  if 

[PATCH] D46943: [clangd] Boost scores for decls from current file in completion

[Sam McCall via Phabricator via cfe-commits]

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

Nice, simple and will admit refinements later.

Just test nits and a trivial organizational thing.




Comment at: clangd/Quality.cpp:22
 
+namespace {
+bool hasDeclInMainFile(const Decl ) {

nit: per coding style use static for functions
(I'm not sure it's a great rule, but since the ns only has this function...)



Comment at: clangd/Quality.h:52
   unsigned References = 0;
+  float ProximityScore = 0.0; /// Proximity score, in a [0,1] interval.
 

this belongs in `SymbolRelevanceSignals` rather than this struct. In principle, 
SymbolQualitySignals should all be stuff we can store in a global index (which 
is the point of the split). I should probably add a comment to that effect :-)

You could be a little more specific on the semantics, e.g. "Proximity between 
the best declaration and the query location. [0-1] score where 1 is closest."



Comment at: unittests/clangd/QualityTests.cpp:96
 
 TEST(QualityTests, SymbolRelevanceSignalsSanity) {
   SymbolRelevanceSignals Default;

please add a test for proximity here



Comment at: unittests/clangd/QualityTests.cpp:121
 
+TEST(QualityTests, BoostCurrentFileDecls) {
+  TestTU Test;

consider merging into SymbolRelevanceSignalsExtraction test, which tests the 
same entrypoint.

If not, move up next to that one.



Comment at: unittests/clangd/QualityTests.cpp:129
+  Test.Code = R"cpp(
+#include "foo.h"
+int ::test_func_in_header_and_cpp() {

this is not needed, the `#include` is implicit in TestTU

(and so you don't need to specify HeaderFilename either)



Comment at: unittests/clangd/QualityTests.cpp:155
+
+  /// Check the boost in proximity translates into a better score.
+  EXPECT_LE(FuncInHeader.evaluate(), FuncInCpp.evaluate());

this tests end-to-end, but the other tests verify input -> signals and signal 
-> score separately.

I'd prefer to keep (only) doing that, for consistency and because it's 
important we know/assert precisely what each half does so we can actually debug.



Comment at: unittests/clangd/TestTU.cpp:80
   continue;
-if (Result) {
+if (Result && ND->getCanonicalDecl() != Result) {
   ADD_FAILURE() << "Multiple Decls named " << QName;

well, I definitely wanted to flag this as an error (for the tests where this 
function was introduced).

Actually I think this is wrong for your test anyway - don't you want to test 
that no matter which decl the code completion result refers to, you get the 
same boost?

I'd suggest adding a `findDecls()` function that returns a vector, 
and implementing `findDecl()` in terms of it. In the 
`test_func_in_header_and_cpp` test, you could loop over `findDecls()` and run 
the assertions each time.

(I guess findDecls() should assert that the returned vector is non-empty? 
Slightly weird but might catch accidentally vacuous tests)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D46943



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

[clang-tools-extra] r333197 - [clangd] Add clangDriver dependency after r333188

[Eric Liu via cfe-commits]

Author: ioeric
Date: Thu May 24 08:54:32 2018
New Revision: 333197

URL: http://llvm.org/viewvc/llvm-project?rev=333197=rev
Log:
[clangd] Add clangDriver dependency after r333188

Modified:
clang-tools-extra/trunk/clangd/CMakeLists.txt

Modified: clang-tools-extra/trunk/clangd/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CMakeLists.txt?rev=333197=333196=333197=diff
==
--- clang-tools-extra/trunk/clangd/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clangd/CMakeLists.txt Thu May 24 08:54:32 2018
@@ -47,6 +47,7 @@ add_clang_library(clangDaemon
   clangAST
   clangASTMatchers
   clangBasic
+  clangDriver
   clangFormat
   clangFrontend
   clangIndex


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

[PATCH] D47187: [clangd] Skip .inc headers when canonicalizing header #include.

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

malaperle added subscribers: ioeric, sammccall, malaperle.
malaperle added a comment.

I think this might have broken the "shared lib" build? Could you double check 
that you don't need to add "clangDriver" ?

../tools/clang/tools/extra/clangd/index/CanonicalIncludes.cpp:61: error: 
undefined reference to 
'clang::driver::types::lookupTypeForExtension(llvm::StringRef)'
../tools/clang/tools/extra/clangd/index/CanonicalIncludes.cpp:63: error: 
undefined reference to 
'clang::driver::types::onlyPrecompileType(clang::driver::types::ID)'

Thanks!



From: cfe-commits  on behalf of Eric Liu 
via Phabricator via cfe-commits 
Sent: Thursday, May 24, 2018 10:44:26 AM
To: ioe...@google.com; sammcc...@google.com
Cc: mask...@google.com; llvm-comm...@lists.llvm.org; cfe-commits@lists.llvm.org
Subject: [PATCH] https://reviews.llvm.org/D47187: [clangd] Skip .inc headers 
when canonicalizing header #include.

This revision was automatically updated to reflect the committed changes.
Closed by commit https://reviews.llvm.org/rL333188: [clangd] Skip .inc headers 
when canonicalizing header #include. (authored by ioeric, committed by ).
Herald added a subscriber: llvm-commits.

Repository:

  rL LLVM

https://reviews.llvm.org/D47187

Files:

  clang-tools-extra/trunk/clangd/index/CanonicalIncludes.cpp
  clang-tools-extra/trunk/clangd/index/CanonicalIncludes.h
  clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
  clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp


Repository:
  rL LLVM

https://reviews.llvm.org/D47187



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

[PATCH] D47272: [clangd] Build index on preamble changes instead of the AST changes

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE333196: [clangd] Build index on preamble changes instead 
of the AST changes (authored by ibiryukov, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D47272?vs=148421=148424#toc

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D47272

Files:
  clangd/ClangdServer.cpp
  clangd/ClangdUnit.cpp
  clangd/ClangdUnit.h
  clangd/TUScheduler.cpp
  clangd/TUScheduler.h
  clangd/index/FileIndex.cpp
  clangd/index/FileIndex.h
  unittests/clangd/FileIndexTests.cpp
  unittests/clangd/TUSchedulerTests.cpp
  unittests/clangd/TestTU.cpp

Index: unittests/clangd/FileIndexTests.cpp
===
--- unittests/clangd/FileIndexTests.cpp
+++ unittests/clangd/FileIndexTests.cpp
@@ -7,8 +7,13 @@
 //
 //===--===//
 
+#include "ClangdUnit.h"
+#include "TestFS.h"
 #include "TestTU.h"
 #include "index/FileIndex.h"
+#include "clang/Frontend/PCHContainerOperations.h"
+#include "clang/Lex/Preprocessor.h"
+#include "clang/Tooling/CompilationDatabase.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
@@ -87,7 +92,7 @@
   File.HeaderFilename = (Basename + ".h").str();
   File.HeaderCode = Code;
   auto AST = File.build();
-  M.update(File.Filename, );
+  M.update(File.Filename, (), AST.getPreprocessorPtr());
 }
 
 TEST(FileIndexTest, IndexAST) {
@@ -129,13 +134,13 @@
   Req.Scopes = {"ns::"};
   EXPECT_THAT(match(M, Req), UnorderedElementsAre("ns::f", "ns::X"));
 
-  M.update("f1.cpp", nullptr);
+  M.update("f1.cpp", nullptr, nullptr);
   EXPECT_THAT(match(M, Req), UnorderedElementsAre());
 }
 
 TEST(FileIndexTest, RemoveNonExisting) {
   FileIndex M;
-  M.update("no.cpp", nullptr);
+  M.update("no.cpp", nullptr, nullptr);
   EXPECT_THAT(match(M, FuzzyFindRequest()), UnorderedElementsAre());
 }
 
@@ -200,6 +205,58 @@
   EXPECT_TRUE(SeenMakeVector);
 }
 
+TEST(FileIndexTest, RebuildWithPreamble) {
+  auto FooCpp = testPath("foo.cpp");
+  auto FooH = testPath("foo.h");
+  FileIndex Index;
+  bool IndexUpdated = false;
+  CppFile File("foo.cpp", /*StorePreambleInMemory=*/true,
+   std::make_shared(),
+   [, ](PathRef FilePath, ASTContext ,
+   std::shared_ptr PP) {
+ EXPECT_FALSE(IndexUpdated)
+ << "Expected only a single index update";
+ IndexUpdated = true;
+ Index.update(FilePath, , std::move(PP));
+   });
+
+  // Preparse ParseInputs.
+  ParseInputs PI;
+  PI.CompileCommand.Directory = testRoot();
+  PI.CompileCommand.Filename = FooCpp;
+  PI.CompileCommand.CommandLine = {"clang", "-xc++", FooCpp};
+
+  llvm::StringMap Files;
+  Files[FooCpp] = "";
+  Files[FooH] = R"cpp(
+namespace ns_in_header {
+  int func_in_header();
+}
+  )cpp";
+  PI.FS = buildTestFS(std::move(Files));
+
+  PI.Contents = R"cpp(
+#include "foo.h"
+namespace ns_in_source {
+  int func_in_source();
+}
+  )cpp";
+
+  // Rebuild the file.
+  File.rebuild(std::move(PI));
+  ASSERT_TRUE(IndexUpdated);
+
+  // Check the index contains symbols from the preamble, but not from the main
+  // file.
+  FuzzyFindRequest Req;
+  Req.Query = "";
+  Req.Scopes = {"", "ns_in_header::"};
+
+  EXPECT_THAT(
+  match(Index, Req),
+  UnorderedElementsAre("ns_in_header", "ns_in_header::func_in_header"));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: unittests/clangd/TestTU.cpp
===
--- unittests/clangd/TestTU.cpp
+++ unittests/clangd/TestTU.cpp
@@ -43,7 +43,7 @@
 
 SymbolSlab TestTU::headerSymbols() const {
   auto AST = build();
-  return indexAST();
+  return indexAST(AST.getASTContext(), AST.getPreprocessorPtr());
 }
 
 std::unique_ptr TestTU::index() const {
Index: unittests/clangd/TUSchedulerTests.cpp
===
--- unittests/clangd/TUSchedulerTests.cpp
+++ unittests/clangd/TUSchedulerTests.cpp
@@ -42,7 +42,7 @@
 TEST_F(TUSchedulerTests, MissingFiles) {
   TUScheduler S(getDefaultAsyncThreadsCount(),
 /*StorePreamblesInMemory=*/true,
-/*ASTParsedCallback=*/nullptr,
+/*PreambleParsedCallback=*/nullptr,
 /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero());
 
   auto Added = testPath("added.cpp");
@@ -98,7 +98,7 @@
 TUScheduler S(
 getDefaultAsyncThreadsCount(),
 /*StorePreamblesInMemory=*/true,
-/*ASTParsedCallback=*/nullptr,
+/*PreambleParsedCallback=*/nullptr,
 /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero());
 auto Path = testPath("foo.cpp");
 S.update(Path, getInputs(Path, ""), WantDiagnostics::Yes,
@@ -126,7 +126,7 @@
   {
 TUScheduler 

[clang-tools-extra] r333196 - [clangd] Build index on preamble changes instead of the AST changes

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Thu May 24 08:50:15 2018
New Revision: 333196

URL: http://llvm.org/viewvc/llvm-project?rev=333196=rev
Log:
[clangd] Build index on preamble changes instead of the AST changes

Summary:
This is more efficient and avoids data races when reading files that
come from the preamble. The staleness can occur when reading a file
from disk that changed after the preamble was built. This can lead to
crashes, e.g. when parsing comments.

We do not to rely on symbols from the main file anyway, since any info
that those provide can always be taken from the AST.

Reviewers: ioeric, sammccall

Reviewed By: ioeric

Subscribers: malaperle, klimek, javed.absar, MaskRay, jkorous, cfe-commits

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

Modified:
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/ClangdUnit.cpp
clang-tools-extra/trunk/clangd/ClangdUnit.h
clang-tools-extra/trunk/clangd/TUScheduler.cpp
clang-tools-extra/trunk/clangd/TUScheduler.h
clang-tools-extra/trunk/clangd/index/FileIndex.cpp
clang-tools-extra/trunk/clangd/index/FileIndex.h
clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp
clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp
clang-tools-extra/trunk/unittests/clangd/TestTU.cpp

Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=333196=333195=333196=diff
==
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Thu May 24 08:50:15 2018
@@ -17,6 +17,7 @@
 #include "clang/Format/Format.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/CompilerInvocation.h"
+#include "clang/Lex/Preprocessor.h"
 #include "clang/Tooling/CompilationDatabase.h"
 #include "clang/Tooling/Refactoring/RefactoringResultConsumer.h"
 #include "clang/Tooling/Refactoring/Rename/RenamingAction.h"
@@ -92,12 +93,14 @@ ClangdServer::ClangdServer(GlobalCompila
   // is parsed.
   // FIXME(ioeric): this can be slow and we may be able to index on less
   // critical paths.
-  WorkScheduler(Opts.AsyncThreadsCount, Opts.StorePreamblesInMemory,
-FileIdx
-? [this](PathRef Path,
- ParsedAST *AST) { FileIdx->update(Path, AST); 
}
-: ASTParsedCallback(),
-Opts.UpdateDebounce) {
+  WorkScheduler(
+  Opts.AsyncThreadsCount, Opts.StorePreamblesInMemory,
+  FileIdx
+  ? [this](PathRef Path, ASTContext ,
+   std::shared_ptr
+   PP) { FileIdx->update(Path, , std::move(PP)); }
+  : PreambleParsedCallback(),
+  Opts.UpdateDebounce) {
   if (FileIdx && Opts.StaticIndex) {
 MergedIndex = mergeIndex(FileIdx.get(), Opts.StaticIndex);
 Index = MergedIndex.get();

Modified: clang-tools-extra/trunk/clangd/ClangdUnit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnit.cpp?rev=333196=333195=333196=diff
==
--- clang-tools-extra/trunk/clangd/ClangdUnit.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp Thu May 24 08:50:15 2018
@@ -13,6 +13,8 @@
 #include "Logger.h"
 #include "SourceCode.h"
 #include "Trace.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/Basic/LangOptions.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Frontend/FrontendActions.h"
@@ -25,7 +27,6 @@
 #include "clang/Sema/Sema.h"
 #include "clang/Serialization/ASTWriter.h"
 #include "clang/Tooling/CompilationDatabase.h"
-#include "clang/Basic/LangOptions.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/Support/CrashRecoveryContext.h"
@@ -86,6 +87,9 @@ private:
 
 class CppFilePreambleCallbacks : public PreambleCallbacks {
 public:
+  CppFilePreambleCallbacks(PathRef File, PreambleParsedCallback ParsedCallback)
+  : File(File), ParsedCallback(ParsedCallback) {}
+
   std::vector takeTopLevelDeclIDs() {
 return std::move(TopLevelDeclIDs);
   }
@@ -102,6 +106,13 @@ public:
 }
   }
 
+  void AfterExecute(CompilerInstance ) override {
+if (!ParsedCallback)
+  return;
+trace::Span Tracer("Running PreambleCallback");
+ParsedCallback(File, CI.getASTContext(), CI.getPreprocessorPtr());
+  }
+
   void HandleTopLevelDecl(DeclGroupRef DG) override {
 for (Decl *D : DG) {
   if (isa(D))
@@ -122,6 +133,8 @@ public:
   }
 
 private:
+  PathRef File;
+  PreambleParsedCallback ParsedCallback;
   std::vector TopLevelDecls;
   std::vector TopLevelDeclIDs;
   std::vector Inclusions;
@@ -277,9 +290,9 @@ ParsedAST::ParsedAST(std::shared_ptr PCHs,
-  

[PATCH] D47272: [clangd] Build index on preamble changes instead of the AST changes

[Eric Liu via Phabricator via cfe-commits]

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

lg. Thanks for adding the test!




Comment at: clangd/index/FileIndex.cpp:37
+  std::vector TopLevelDecls(
+  AST.getTranslationUnitDecl()->decls().begin(),
+  AST.getTranslationUnitDecl()->decls().end());

ilya-biryukov wrote:
> ioeric wrote:
> > ilya-biryukov wrote:
> > > ioeric wrote:
> > > > Would this give us decls in namespaces? It seems that 
> > > > `AST.getTranslationUnitDecl()->decls()` are only decls that are 
> > > > immediately inside the TU context. 
> > > I'll double check and add a test for that, but I think the indexer visits 
> > > the namespaces that we provide, I'm not sure if we have tests though.
> > Oh, right! In that case, it seems that a single TU decl which contains all 
> > interesting decls would be sufficient? Is it possible that preamble doesn't 
> > have all available top level decls in TU decl? This seems to be possible 
> > for ParsedAST, which is why we used `getTopLevelDecls`, I think?
> > In that case, it seems that a single TU decl which contains all interesting 
> > decls would be sufficient?
> I've tried doing the TU decl only and it does not work, because 
> `indexTopLevelDecl` checks that location of decl that you pass to it is valid 
> xD. Obviously, that's not the case for the TU decl. But it does visit the 
> namespaces recursively, new test also checks for that, so we should be fine 
> with the current code.
> 
> > This seems to be possible for ParsedAST, which is why we used 
> > getTopLevelDecls, I think?
> `getTopLevelDecls` was copied from the `ASTUnit` without much thought.
> IIRC, it allows less deserialization from preamble when traversing the AST 
> for things like "go to def", etc. This is definitely not something we're 
> optimizing for here, since we run on the preamble AST itself.
> Now that I think about it, I don't think we need to store top-level decls of 
> preamble at all for any of our features...
Sounds good. Thanks for the clarification!


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D47272



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

[PATCH] D47097: [DebugInfo] Preserve scope in auto generated StoreInst

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

Can we step back a second and better explain what the problem is?  With current 
Clang the debug info for this function looks okay to me.
The store that is "missing" a debug location is homing the formal parameter to 
its local stack location; this is part of prolog setup, not "real" code.


Repository:
  rC Clang

https://reviews.llvm.org/D47097



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

[PATCH] D47272: [clangd] Build index on preamble changes instead of the AST changes

[Ilya Biryukov via Phabricator via cfe-commits]

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

- Added a test for preamble rebuilds.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D47272

Files:
  clangd/ClangdServer.cpp
  clangd/ClangdUnit.cpp
  clangd/ClangdUnit.h
  clangd/TUScheduler.cpp
  clangd/TUScheduler.h
  clangd/index/FileIndex.cpp
  clangd/index/FileIndex.h
  unittests/clangd/FileIndexTests.cpp
  unittests/clangd/TUSchedulerTests.cpp
  unittests/clangd/TestTU.cpp

Index: unittests/clangd/TestTU.cpp
===
--- unittests/clangd/TestTU.cpp
+++ unittests/clangd/TestTU.cpp
@@ -43,7 +43,7 @@
 
 SymbolSlab TestTU::headerSymbols() const {
   auto AST = build();
-  return indexAST();
+  return indexAST(AST.getASTContext(), AST.getPreprocessorPtr());
 }
 
 std::unique_ptr TestTU::index() const {
Index: unittests/clangd/TUSchedulerTests.cpp
===
--- unittests/clangd/TUSchedulerTests.cpp
+++ unittests/clangd/TUSchedulerTests.cpp
@@ -42,7 +42,7 @@
 TEST_F(TUSchedulerTests, MissingFiles) {
   TUScheduler S(getDefaultAsyncThreadsCount(),
 /*StorePreamblesInMemory=*/true,
-/*ASTParsedCallback=*/nullptr,
+/*PreambleParsedCallback=*/nullptr,
 /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero());
 
   auto Added = testPath("added.cpp");
@@ -98,7 +98,7 @@
 TUScheduler S(
 getDefaultAsyncThreadsCount(),
 /*StorePreamblesInMemory=*/true,
-/*ASTParsedCallback=*/nullptr,
+/*PreambleParsedCallback=*/nullptr,
 /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero());
 auto Path = testPath("foo.cpp");
 S.update(Path, getInputs(Path, ""), WantDiagnostics::Yes,
@@ -126,7 +126,7 @@
   {
 TUScheduler S(getDefaultAsyncThreadsCount(),
   /*StorePreamblesInMemory=*/true,
-  /*ASTParsedCallback=*/nullptr,
+  /*PreambleParsedCallback=*/nullptr,
   /*UpdateDebounce=*/std::chrono::seconds(1));
 // FIXME: we could probably use timeouts lower than 1 second here.
 auto Path = testPath("foo.cpp");
@@ -157,7 +157,7 @@
   {
 TUScheduler S(getDefaultAsyncThreadsCount(),
   /*StorePreamblesInMemory=*/true,
-  /*ASTParsedCallback=*/nullptr,
+  /*PreambleParsedCallback=*/nullptr,
   /*UpdateDebounce=*/std::chrono::milliseconds(50));
 
 std::vector Files;
Index: unittests/clangd/FileIndexTests.cpp
===
--- unittests/clangd/FileIndexTests.cpp
+++ unittests/clangd/FileIndexTests.cpp
@@ -7,8 +7,13 @@
 //
 //===--===//
 
+#include "ClangdUnit.h"
+#include "TestFS.h"
 #include "TestTU.h"
 #include "index/FileIndex.h"
+#include "clang/Frontend/PCHContainerOperations.h"
+#include "clang/Lex/Preprocessor.h"
+#include "clang/Tooling/CompilationDatabase.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
@@ -87,7 +92,7 @@
   File.HeaderFilename = (Basename + ".h").str();
   File.HeaderCode = Code;
   auto AST = File.build();
-  M.update(File.Filename, );
+  M.update(File.Filename, (), AST.getPreprocessorPtr());
 }
 
 TEST(FileIndexTest, IndexAST) {
@@ -129,13 +134,13 @@
   Req.Scopes = {"ns::"};
   EXPECT_THAT(match(M, Req), UnorderedElementsAre("ns::f", "ns::X"));
 
-  M.update("f1.cpp", nullptr);
+  M.update("f1.cpp", nullptr, nullptr);
   EXPECT_THAT(match(M, Req), UnorderedElementsAre());
 }
 
 TEST(FileIndexTest, RemoveNonExisting) {
   FileIndex M;
-  M.update("no.cpp", nullptr);
+  M.update("no.cpp", nullptr, nullptr);
   EXPECT_THAT(match(M, FuzzyFindRequest()), UnorderedElementsAre());
 }
 
@@ -200,6 +205,58 @@
   EXPECT_TRUE(SeenMakeVector);
 }
 
+TEST(FileIndexTest, RebuildWithPreamble) {
+  auto FooCpp = testPath("foo.cpp");
+  auto FooH = testPath("foo.h");
+  FileIndex Index;
+  bool IndexUpdated = false;
+  CppFile File("foo.cpp", /*StorePreambleInMemory=*/true,
+   std::make_shared(),
+   [, ](PathRef FilePath, ASTContext ,
+   std::shared_ptr PP) {
+ EXPECT_FALSE(IndexUpdated)
+ << "Expected only a single index update";
+ IndexUpdated = true;
+ Index.update(FilePath, , std::move(PP));
+   });
+
+  // Preparse ParseInputs.
+  ParseInputs PI;
+  PI.CompileCommand.Directory = testRoot();
+  PI.CompileCommand.Filename = FooCpp;
+  PI.CompileCommand.CommandLine = {"clang", "-xc++", FooCpp};
+
+  llvm::StringMap Files;
+  Files[FooCpp] = "";
+  Files[FooH] = R"cpp(
+namespace ns_in_header {
+  int func_in_header();
+}
+  )cpp";
+  PI.FS = buildTestFS(std::move(Files));
+
+  PI.Contents = R"cpp(
+#include "foo.h"
+namespace ns_in_source {
+  

[PATCH] D47272: [clangd] Build index on preamble changes instead of the AST changes

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Added the test.




Comment at: clangd/index/FileIndex.cpp:37
+  std::vector TopLevelDecls(
+  AST.getTranslationUnitDecl()->decls().begin(),
+  AST.getTranslationUnitDecl()->decls().end());

ioeric wrote:
> ilya-biryukov wrote:
> > ioeric wrote:
> > > Would this give us decls in namespaces? It seems that 
> > > `AST.getTranslationUnitDecl()->decls()` are only decls that are 
> > > immediately inside the TU context. 
> > I'll double check and add a test for that, but I think the indexer visits 
> > the namespaces that we provide, I'm not sure if we have tests though.
> Oh, right! In that case, it seems that a single TU decl which contains all 
> interesting decls would be sufficient? Is it possible that preamble doesn't 
> have all available top level decls in TU decl? This seems to be possible for 
> ParsedAST, which is why we used `getTopLevelDecls`, I think?
> In that case, it seems that a single TU decl which contains all interesting 
> decls would be sufficient?
I've tried doing the TU decl only and it does not work, because 
`indexTopLevelDecl` checks that location of decl that you pass to it is valid 
xD. Obviously, that's not the case for the TU decl. But it does visit the 
namespaces recursively, new test also checks for that, so we should be fine 
with the current code.

> This seems to be possible for ParsedAST, which is why we used 
> getTopLevelDecls, I think?
`getTopLevelDecls` was copied from the `ASTUnit` without much thought.
IIRC, it allows less deserialization from preamble when traversing the AST for 
things like "go to def", etc. This is definitely not something we're optimizing 
for here, since we run on the preamble AST itself.
Now that I think about it, I don't think we need to store top-level decls of 
preamble at all for any of our features...


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D47272



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

[PATCH] D47067: Update NRVO logic to support early return

[Taiju Tsuiki via Phabricator via cfe-commits]

tzik added inline comments.



Comment at: lib/Sema/SemaDecl.cpp:12760
   // to deduce an implicit return type.
-  if (FD->getReturnType()->isRecordType() &&
-  (!getLangOpts().CPlusPlus || !FD->isDependentContext()))
+  if (!FD->getReturnType()->isScalarType())
 computeNRVO(Body, getCurFunction());

Quuxplusone wrote:
> rsmith wrote:
> > tzik wrote:
> > > Quuxplusone wrote:
> > > > What is the purpose of this change?
> > > > If it's "no functional change" it should be done separately IMHO; if it 
> > > > is supposed to change codegen, then it needs some codegen tests. (From 
> > > > looking at the code: maybe this affects codegen on functions that 
> > > > return member function pointers by value?)
> > > I think the previous implementation was incorrect.
> > > Though computeNRVO clears ReturnStmt::NRVOCandidate when the 
> > > corresponding variable is not NRVO variable, CGStmt checks both of 
> > > ReturnStmt::NRVOCandidate and VarDecl::NRVOVariable anyway.
> > > So, computeNRVO took no effect in the previous code, and the absence of 
> > > computeNRVO here on function templates did not matter.
> > > Note that there was no chance to call computeNRVO on function template 
> > > elsewhere too.
> > > 
> > > OTOH in the new implementation, computeNRVO is necessary, and its absence 
> > > on function templates matters.
> > > 
> > > We can remove computeNRVO here separately as a NFC patch and readd the 
> > > new impl to the same place, but it's probably less readable, IMO.
> > What happens if we can't tell whether we have an NRVO candidate or not 
> > until instantiation:
> > 
> > ```
> > template X f() {
> >   T v;
> >   return v; // is an NRVO candidate if T is X, otherwise is not
> > }
> > ```
> > 
> > (I think this is not hard to handle: the dependent construct here can only 
> > affect whether you get NRVO at all, not which variable you perform NRVO on, 
> > but I want to check that it is handled properly.)
> Ah, so if I understand correctly — which I probably don't —...
> the parts of this diff related to `isRecordType()` make NRVO more reliable on 
> template functions? Because the existing code has lots of checks for 
> `isRecordType()` which means that they don't run for dependent 
> (as-yet-unknown) types, even if those types are going to turn out to be 
> record types occasionally?
> 
> I see that line 12838 runs `computeNRVO` unconditionally for *all* 
> ObjCMethodDecls, //even// ones with scalar return types. So maybe running 
> `computeNRVO` unconditionally right here would also be correct?
> Datapoint: I made a patch to do nothing but replace this condition with `if 
> (Body)` and to remove the similar condition guarding `computeNRVO` in 
> `Sema::ActOnBlockStmtExpr`. It passed the Clang test suite with flying 
> colors. I don't know if this means that the test suite is missing some tests, 
> or if it means that the conditions are unnecessary.
> 
> IMHO if you're changing this anyway, it would be nice to put the remaining 
> condition/optimization `if (getReturnType().isScalarType()) return;` inside 
> `computeNRVO` itself, instead of repeating that condition at every call site.
`v` here is marked as an NRVO variable in its function template AST, and that 
propagates to the instantiation if the return type is compatible.
https://github.com/llvm-mirror/clang/blob/9c82d4ff6015e4477e924c8a495a834c2fced29e/lib/Sema/SemaTemplateInstantiateDecl.cpp#L743



Comment at: lib/Sema/SemaDecl.cpp:12760
   // to deduce an implicit return type.
-  if (FD->getReturnType()->isRecordType() &&
-  (!getLangOpts().CPlusPlus || !FD->isDependentContext()))
+  if (!FD->getReturnType()->isScalarType())
 computeNRVO(Body, getCurFunction());

tzik wrote:
> Quuxplusone wrote:
> > rsmith wrote:
> > > tzik wrote:
> > > > Quuxplusone wrote:
> > > > > What is the purpose of this change?
> > > > > If it's "no functional change" it should be done separately IMHO; if 
> > > > > it is supposed to change codegen, then it needs some codegen tests. 
> > > > > (From looking at the code: maybe this affects codegen on functions 
> > > > > that return member function pointers by value?)
> > > > I think the previous implementation was incorrect.
> > > > Though computeNRVO clears ReturnStmt::NRVOCandidate when the 
> > > > corresponding variable is not NRVO variable, CGStmt checks both of 
> > > > ReturnStmt::NRVOCandidate and VarDecl::NRVOVariable anyway.
> > > > So, computeNRVO took no effect in the previous code, and the absence of 
> > > > computeNRVO here on function templates did not matter.
> > > > Note that there was no chance to call computeNRVO on function template 
> > > > elsewhere too.
> > > > 
> > > > OTOH in the new implementation, computeNRVO is necessary, and its 
> > > > absence on function templates matters.
> > > > 
> > > > We can remove computeNRVO here separately as a NFC patch and readd the 
> > > 

[PATCH] D47067: Update NRVO logic to support early return

[Taiju Tsuiki via Phabricator via cfe-commits]

tzik updated this revision to Diff 148417.
tzik added a comment.

Add AST test. computeNRVO unconditionally.


Repository:
  rC Clang

https://reviews.llvm.org/D47067

Files:
  include/clang/AST/Decl.h
  include/clang/Sema/Scope.h
  lib/Sema/Scope.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaStmt.cpp
  lib/Serialization/ASTReaderDecl.cpp
  lib/Serialization/ASTWriterDecl.cpp
  test/CodeGenCXX/nrvo.cpp
  test/SemaCXX/nrvo-ast.cpp

Index: test/SemaCXX/nrvo-ast.cpp
===
--- /dev/null
+++ test/SemaCXX/nrvo-ast.cpp
@@ -0,0 +1,139 @@
+// RUN: %clang_cc1 -fcxx-exceptions -fsyntax-only -ast-dump -o - %s | FileCheck %s
+
+struct X {
+  X();
+  X(const X&);
+  X(X&&);
+};
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_00
+X test_00() {
+  // CHECK: VarDecl {{.*}} x {{.*}} nrvo
+  X x;
+  return x;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_01
+X test_01(bool b) {
+  // CHECK: VarDecl {{.*}} x {{.*}} nrvo
+  X x;
+  if (b)
+return x;
+  return x;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_02
+X test_02(bool b) {
+  // CHECK-NOT: VarDecl {{.*}} x {{.*}} nrvo
+  X x;
+  // CHECK-NOT: VarDecl {{.*}} y {{.*}} nrvo
+  X y;
+  if (b)
+return y;
+  return x;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_03
+X test_03(bool b) {
+  if (b) {
+// CHECK: VarDecl {{.*}} y {{.*}} nrvo
+X y;
+return y;
+  }
+  // CHECK: VarDecl {{.*}} x {{.*}} nrvo
+  X x;
+  return x;
+}
+
+extern "C" _Noreturn void exit(int) throw();
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_04
+X test_04(bool b) {
+  {
+// CHECK: VarDecl {{.*}} x {{.*}} nrvo
+X x;
+if (b)
+  return x;
+  }
+  exit(1);
+}
+
+void may_throw();
+// CHECK-LABEL: FunctionDecl {{.*}} test_05
+X test_05() {
+  try {
+may_throw();
+return X();
+  } catch (X x) {
+// CHECK-NOT: VarDecl {{.*}} x {{.*}} nrvo
+return x;
+  }
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_06
+X test_06() {
+  // CHECK-NOT: VarDecl {{.*}} x {{.*}} nrvo
+  X x __attribute__((aligned(8)));
+  return x;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_07
+X test_07(bool b) {
+  if (b) {
+// CHECK: VarDecl {{.*}} x {{.*}} nrvo
+X x;
+return x;
+  }
+  return X();
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_08
+X test_08(bool b) {
+  if (b) {
+// CHECK: VarDecl {{.*}} x {{.*}} nrvo
+X x;
+return x;
+  } else {
+// CHECK: VarDecl {{.*}} y {{.*}} nrvo
+X y;
+return y;
+  }
+}
+
+template 
+struct Y {
+  Y();
+  // CHECK-LABEL: CXXMethodDecl {{.*}} test_09 'Y ()'
+  // CHECK: VarDecl {{.*}} y 'Y' nrvo
+
+  // CHECK-LABEL: CXXMethodDecl {{.*}} test_09 'Y ()'
+  // CHECK: VarDecl {{.*}} y 'Y' nrvo
+  static Y test_09() {
+Y y;
+return y;
+  }
+};
+
+struct Z {
+  Z(const X&);
+};
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_10 'A ()'
+// CHECK: VarDecl {{.*}} b 'B' nrvo
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_10 'X ()'
+// CHECK: VarDecl {{.*}} b {{.*}} nrvo
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_10 'Z ()'
+// CHECK-NOT: VarDecl {{.*}} b {{.*}} nrvo
+template 
+A test_10() {
+  B b;
+  return b;
+}
+
+void instantiate() {
+  Y::test_09();
+  test_10();
+  test_10();
+}
Index: test/CodeGenCXX/nrvo.cpp
===
--- test/CodeGenCXX/nrvo.cpp
+++ test/CodeGenCXX/nrvo.cpp
@@ -130,17 +130,13 @@
 }
 
 // CHECK-LABEL: define void @_Z5test3b
-X test3(bool B) {
+X test3(bool B, X x) {
   // CHECK: tail call {{.*}} @_ZN1XC1Ev
-  // CHECK-NOT: call {{.*}} @_ZN1XC1ERKS_
-  // CHECK: call {{.*}} @_ZN1XC1Ev
-  // CHECK: call {{.*}} @_ZN1XC1ERKS_
   if (B) {
 X y;
 return y;
   }
-  // FIXME: we should NRVO this variable too.
-  X x;
+  // CHECK: tail call {{.*}} @_ZN1XC1ERKS_
   return x;
 }
 
@@ -191,21 +187,29 @@
 }
 
 // CHECK-LABEL: define void @_Z5test7b
+// CHECK-EH-LABEL: define void @_Z5test7b
 X test7(bool b) {
   // CHECK: tail call {{.*}} @_ZN1XC1Ev
   // CHECK-NEXT: ret
+
+  // CHECK-EH: tail call {{.*}} @_ZN1XC1Ev
+  // CHECK-EH-NEXT: ret
   if (b) {
 X x;
 return x;
   }
   return X();
 }
 
 // CHECK-LABEL: define void @_Z5test8b
+// CHECK-EH-LABEL: define void @_Z5test8b
 X test8(bool b) {
   // CHECK: tail call {{.*}} @_ZN1XC1Ev
   // CHECK-NEXT: ret
-  if (b) {
+
+  // CHECK-EH: tail call {{.*}} @_ZN1XC1Ev
+  // CHECK-EH-NEXT: ret
+if (b) {
 X x;
 return x;
   } else {
@@ -221,4 +225,37 @@
 // CHECK-LABEL: define linkonce_odr void @_ZN1YIiE1fEv
 // CHECK: tail call {{.*}} @_ZN1YIiEC1Ev
 
+// CHECK-LABEL: define void @_Z6test10b
+X test10(bool B, X x) {
+  if (B) {
+// CHECK: tail call {{.*}} @_ZN1XC1ERKS_
+// CHECK-EH: tail call {{.*}} @_ZN1XC1ERKS_
+return x;
+  }
+  // CHECK: tail call {{.*}} @_ZN1XC1Ev
+  // CHECK-NOT: call {{.*}} @_ZN1XC1ERKS_
+
+  // CHECK-EH: tail call {{.*}} @_ZN1XC1Ev
+  // CHECK-EH-NOT: call {{.*}} @_ZN1XC1ERKS_
+  X y;
+  return y;
+}
+
+// CHECK-LABEL: define {{.*}} void @_Z6test11I1XET_v

r333179 - [analyzer] Move RangeSet related declarations into the RangedConstraintManager header.

[Mikhail R. Gadelha via cfe-commits]

Author: mramalho
Date: Thu May 24 05:16:35 2018
New Revision: 333179

URL: http://llvm.org/viewvc/llvm-project?rev=333179=rev
Log:
[analyzer] Move RangeSet related declarations into the RangedConstraintManager 
header.

Summary: I could also move `RangedConstraintManager.h` under `include/` if you 
agree as it seems slightly out of place under `lib/`.

Patch by Réka Kovács

Reviewers: NoQ, george.karpenkov, dcoughlin, rnkovacs

Reviewed By: NoQ

Subscribers: mikhail.ramalho, whisperity, xazax.hun, baloghadamsoftware, 
szepet, a.sidorin, dkrupp, cfe-commits

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

Modified:
cfe/trunk/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
cfe/trunk/lib/StaticAnalyzer/Core/RangedConstraintManager.h

Modified: cfe/trunk/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp?rev=333179=333178=333179=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp Thu May 24 
05:16:35 2018
@@ -23,263 +23,171 @@
 using namespace clang;
 using namespace ento;
 
-/// A Range represents the closed range [from, to].  The caller must
-/// guarantee that from <= to.  Note that Range is immutable, so as not
-/// to subvert RangeSet's immutability.
-namespace {
-class Range : public std::pair {
-public:
-  Range(const llvm::APSInt , const llvm::APSInt )
-  : std::pair(, ) {
-assert(from <= to);
-  }
-  bool Includes(const llvm::APSInt ) const {
-return *first <= v && v <= *second;
-  }
-  const llvm::APSInt () const { return *first; }
-  const llvm::APSInt () const { return *second; }
-  const llvm::APSInt *getConcreteValue() const {
-return () == () ? () : nullptr;
-  }
-
-  void Profile(llvm::FoldingSetNodeID ) const {
-ID.AddPointer(());
-ID.AddPointer(());
-  }
-};
-
-class RangeTrait : public llvm::ImutContainerInfo {
-public:
-  // When comparing if one Range is less than another, we should compare
-  // the actual APSInt values instead of their pointers.  This keeps the order
-  // consistent (instead of comparing by pointer values) and can potentially
-  // be used to speed up some of the operations in RangeSet.
-  static inline bool isLess(key_type_ref lhs, key_type_ref rhs) {
-return *lhs.first < *rhs.first ||
-   (!(*rhs.first < *lhs.first) && *lhs.second < *rhs.second);
-  }
-};
-
-/// RangeSet contains a set of ranges. If the set is empty, then
-///  there the value of a symbol is overly constrained and there are no
-///  possible values for that symbol.
-class RangeSet {
-  typedef llvm::ImmutableSet PrimRangeSet;
-  PrimRangeSet ranges; // no need to make const, since it is an
-   // ImmutableSet - this allows default operator=
-   // to work.
-public:
-  typedef PrimRangeSet::Factory Factory;
-  typedef PrimRangeSet::iterator iterator;
-
-  RangeSet(PrimRangeSet RS) : ranges(RS) {}
-
-  /// Create a new set with all ranges of this set and RS.
-  /// Possible intersections are not checked here.
-  RangeSet addRange(Factory , const RangeSet ) {
-PrimRangeSet Ranges(RS.ranges);
-for (const auto  : ranges)
-  Ranges = F.add(Ranges, range);
-return RangeSet(Ranges);
-  }
-
-  iterator begin() const { return ranges.begin(); }
-  iterator end() const { return ranges.end(); }
-
-  bool isEmpty() const { return ranges.isEmpty(); }
-
-  /// Construct a new RangeSet representing '{ [from, to] }'.
-  RangeSet(Factory , const llvm::APSInt , const llvm::APSInt )
-  : ranges(F.add(F.getEmptySet(), Range(from, to))) {}
-
-  /// Profile - Generates a hash profile of this RangeSet for use
-  ///  by FoldingSet.
-  void Profile(llvm::FoldingSetNodeID ) const { ranges.Profile(ID); }
-
-  /// getConcreteValue - If a symbol is contrained to equal a specific integer
-  ///  constant then this method returns that value.  Otherwise, it returns
-  ///  NULL.
-  const llvm::APSInt *getConcreteValue() const {
-return ranges.isSingleton() ? ranges.begin()->getConcreteValue() : nullptr;
-  }
+void RangeSet::IntersectInRange(BasicValueFactory , Factory ,
+  const llvm::APSInt , const llvm::APSInt ,
+  PrimRangeSet , PrimRangeSet::iterator ,
+  PrimRangeSet::iterator ) const {
+  // There are six cases for each range R in the set:
+  //   1. R is entirely before the intersection range.
+  //   2. R is entirely after the intersection range.
+  //   3. R contains the entire intersection range.
+  //   4. R starts before the intersection range and ends in the middle.
+  //   5. R starts in the middle of the intersection range and ends after it.
+  //   6. R is entirely contained in the intersection range.
+  // These correspond to each of the conditions 

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

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Thanks, this looks a lot better!
There do seem to be a lot of little classes that exist exactly one-per-TU 
(ASTWorker, ASTBuilder, CachedAST, to a lesser extent ParsedAST) and I'm not 
sure the balance of responsibilities is quite right. Some comments below.




Comment at: clangd/ClangdUnit.h:132
 
-/// Manages resources, required by clangd. Allows to rebuild file with new
-/// contents, and provides AST and Preamble for it.
-class CppFile {
+/// A helper class that handles building preambles and ASTs for a file. Also
+/// adds some logging.

ilya-biryukov wrote:
> sammccall wrote:
> > This may be change aversion, but I'm not sure this class does enough after 
> > this change - it doesn't store the inputs or the outputs/cache, so it kind 
> > of seems like it wants to be a function.
> > 
> > I guess the motivation here is that storing the outputs means dealing with 
> > the cache, and the cache is local to TUScheduler.
> > But `CppFile` is only used in TUScheduler, so we could move this there too? 
> > It feels like expanding the scope more than I'd like.
> > 
> > The upside is that I think it's a more natural division of responsibility: 
> > `CppFile` could continue to be the "main" holder of the 
> > `shared_ptr` (which we don't limit, but share), and instead of 
> > `Optional` it'd have a `weak_ptr` which is controlled 
> > and can be refreshed through the cache.
> > 
> > As discussed offline, the cache could look something like:
> > ```
> > class Cache {
> >shared_ptr put(ParsedAST);
> >void hintUsed(ParsedAST*); // optional, bumps LRU when client reads
> >void hintUnused(ParsedAST*); // optional, releases when client abandons
> > }
> > 
> > shared_ptr CppFile::getAST() {
> >   shared_ptr AST = WeakAST.lock();
> >   if (AST)
> > Cache.hintUsed(AST.get());
> >   else
> > WeakAST = AST = Cache.put(build...);
> >   return AST;
> > }
> > ```
> I've reimplemented the cache with weak_ptr/shared_ptr. 
> With a slightly different interface to hide more stuff in the cache API. 
> Please take a look and let me know what you think.
> 
> Nevertheless, I still find `CppFile` refactoring useful. It unties preambles 
> from the ASTs and I believe this is a good thing, given that their lifetimes 
> are different.
I'm not sure how much they were tangled before, they were computed in the same 
place, but accessed separately, and it's not sure you ever *want* to compute 
them separately? (possibly in unit tests?)

I do think making ASTWorker maintain the old preamble and pass it in is 
confusing. The remaining members are trivial and unrelated enough that I do 
think if the references to the preamble/ast are removed, then moving the 
remaining members to ASTWorker or to a dumb struct, and making these free 
functions would make it easier to navigate the class structure.



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

I think I understand this more as "updates the value" but the value is lazy...

I find this API somewhat hard to follow, maybe just because it's unfamiliar.
I've mostly seen cache APIs look like one of:
1. `Cache(function Compute)`, `Value Cache::get(Input)`
2. `void Cache::put(Key, Value)`, `Optional Cache::get(Key)`
3. `Handle Cache::put(Value)`, `Optional Handle::get()`

This one is `Slot Cache::create()`, `void Slot::update(function 
LazyV)`, `Value Slot::get()`.

It's similar-ish to 3, but has 3 nontrivial operations instead of 2, and the 
slot object is externally mutable instead of immutable, so it seems more 
complex. What does it buy us in exchange?

(1 & 2 work well with a natural key or inputs that are easy to compare, which 
we don't particularly have)



Comment at: clangd/TUScheduler.cpp:91
+/// Provides an LRU cache of ASTs.
+class TUScheduler::IdleASTs {
+  friend class CachedAST;

naming: `IdleASTs` doesn't mention the function of this class, which is a cache.
I'd suggest swapping the names: call the class `ASTCache` and the *instance* 
`IdleASTs` as that reflects its role within the TUScheduler.

For `CachedAST`, I think the relationship would be well-exposed by nesting it 
as `ASTCache::Entry`. This also gives you the `friend` for free, which seems 
like a hint that it's an appropriate structure.
(Though I'm not sure CachedAST is that useful)



Comment at: clangd/TUScheduler.cpp:153
+  /// one.
+  std::vector>>
+  LRU; /* GUARDED_BY(Mut) */

as discussed offline, using a CachedAST* as a key shouldn't be necessary, the 
ParsedAST* should be enough I think.



Comment at: clangd/TUScheduler.cpp:157
+
+CachedAST::~CachedAST() { Owner.remove(*this); }
+

document why



Comment at: clangd/TUScheduler.h:48
+/// kept in memory.

[PATCH] D47274: [clangd] Serve comments for headers decls from dynamic index only

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL333189: [clangd] Serve comments for headers decls from 
dynamic index only (authored by ibiryukov, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D47274

Files:
  clang-tools-extra/trunk/clangd/CodeComplete.cpp
  clang-tools-extra/trunk/clangd/CodeCompletionStrings.cpp
  clang-tools-extra/trunk/clangd/CodeCompletionStrings.h
  clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
  clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp

Index: clang-tools-extra/trunk/clangd/CodeComplete.cpp
===
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp
@@ -586,9 +586,11 @@
   const auto *CCS = Candidate.CreateSignatureString(
   CurrentArg, S, *Allocator, CCTUInfo, true);
   assert(CCS && "Expected the CodeCompletionString to be non-null");
+  // FIXME: for headers, we need to get a comment from the index.
   SigHelp.signatures.push_back(ProcessOverloadCandidate(
   Candidate, *CCS,
-  getParameterDocComment(S.getASTContext(), Candidate, CurrentArg)));
+  getParameterDocComment(S.getASTContext(), Candidate, CurrentArg,
+ /*CommentsFromHeader=*/false)));
 }
   }
 
@@ -1030,7 +1032,8 @@
   SemaCCS = Recorder->codeCompletionString(*SR);
   if (Opts.IncludeComments) {
 assert(Recorder->CCSema);
-DocComment = getDocComment(Recorder->CCSema->getASTContext(), *SR);
+DocComment = getDocComment(Recorder->CCSema->getASTContext(), *SR,
+   /*CommentsFromHeader=*/false);
   }
 }
 return Candidate.build(FileName, Scores, Opts, SemaCCS, Includes.get(), DocComment);
Index: clang-tools-extra/trunk/clangd/CodeCompletionStrings.cpp
===
--- clang-tools-extra/trunk/clangd/CodeCompletionStrings.cpp
+++ clang-tools-extra/trunk/clangd/CodeCompletionStrings.cpp
@@ -10,6 +10,7 @@
 #include "CodeCompletionStrings.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/RawCommentList.h"
+#include "clang/Basic/SourceManager.h"
 #include 
 
 namespace clang {
@@ -122,10 +123,23 @@
   }
 }
 
+std::string getFormattedComment(const ASTContext , const RawComment ,
+bool CommentsFromHeaders) {
+  auto  = Ctx.getSourceManager();
+  // Parsing comments from invalid preamble can lead to crashes. So we only
+  // return comments from the main file when doing code completion. For
+  // indexing, we still read all the comments.
+  // FIXME: find a better fix, e.g. store file contents in the preamble or get
+  // doc comments from the index.
+  if (!CommentsFromHeaders && !SourceMgr.isWrittenInMainFile(RC.getLocStart()))
+return "";
+  return RC.getFormattedText(Ctx.getSourceManager(), Ctx.getDiagnostics());
+}
 } // namespace
 
 std::string getDocComment(const ASTContext ,
-  const CodeCompletionResult ) {
+  const CodeCompletionResult ,
+  bool CommentsFromHeaders) {
   // FIXME: clang's completion also returns documentation for RK_Pattern if they
   // contain a pattern for ObjC properties. Unfortunately, there is no API to
   // get this declaration, so we don't show documentation in that case.
@@ -137,20 +151,20 @@
   const RawComment *RC = getCompletionComment(Ctx, Decl);
   if (!RC)
 return "";
-  return RC->getFormattedText(Ctx.getSourceManager(), Ctx.getDiagnostics());
+  return getFormattedComment(Ctx, *RC, CommentsFromHeaders);
 }
 
 std::string
 getParameterDocComment(const ASTContext ,
const CodeCompleteConsumer::OverloadCandidate ,
-   unsigned ArgIndex) {
+   unsigned ArgIndex, bool CommentsFromHeaders) {
   auto Func = Result.getFunction();
   if (!Func)
 return "";
   const RawComment *RC = getParameterComment(Ctx, Result, ArgIndex);
   if (!RC)
 return "";
-  return RC->getFormattedText(Ctx.getSourceManager(), Ctx.getDiagnostics());
+  return getFormattedComment(Ctx, *RC, CommentsFromHeaders);
 }
 
 void getLabelAndInsertText(const CodeCompletionString , std::string *Label,
Index: clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
===
--- clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
@@ -389,7 +389,8 @@
 /*EnableSnippets=*/false);
   std::string FilterText = getFilterText(*CCS);
   std::string Documentation =
-  formatDocumentation(*CCS, getDocComment(Ctx, SymbolCompletion));
+  formatDocumentation(*CCS, getDocComment(Ctx, SymbolCompletion,
+  

[clang-tools-extra] r333189 - [clangd] Serve comments for headers decls from dynamic index only

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Thu May 24 07:49:23 2018
New Revision: 333189

URL: http://llvm.org/viewvc/llvm-project?rev=333189=rev
Log:
[clangd] Serve comments for headers decls from dynamic index only

Summary:
To fix a crash in code completion that occurrs when reading doc
comments from files that were updated after the preamble was
computed. In that case, the files on disk could've been changed and we
can't rely on finding the comment text with the same range anymore.

The current workaround is to not provide comments from the headers at
all and rely on the dynamic index instead.

A more principled solution would be to store contents of the files
read inside the preamble, but it is way harder to implement properly,
given that it would definitely increase the sizes of the preamble.

Together with D47272, this should fix all preamble-related crashes
we're aware of.

Reviewers: sammccall

Reviewed By: sammccall

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

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

Modified:
clang-tools-extra/trunk/clangd/CodeComplete.cpp
clang-tools-extra/trunk/clangd/CodeCompletionStrings.cpp
clang-tools-extra/trunk/clangd/CodeCompletionStrings.h
clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp

Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=333189=333188=333189=diff
==
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Thu May 24 07:49:23 2018
@@ -586,9 +586,11 @@ public:
   const auto *CCS = Candidate.CreateSignatureString(
   CurrentArg, S, *Allocator, CCTUInfo, true);
   assert(CCS && "Expected the CodeCompletionString to be non-null");
+  // FIXME: for headers, we need to get a comment from the index.
   SigHelp.signatures.push_back(ProcessOverloadCandidate(
   Candidate, *CCS,
-  getParameterDocComment(S.getASTContext(), Candidate, CurrentArg)));
+  getParameterDocComment(S.getASTContext(), Candidate, CurrentArg,
+ /*CommentsFromHeader=*/false)));
 }
   }
 
@@ -1030,7 +1032,8 @@ private:
   SemaCCS = Recorder->codeCompletionString(*SR);
   if (Opts.IncludeComments) {
 assert(Recorder->CCSema);
-DocComment = getDocComment(Recorder->CCSema->getASTContext(), *SR);
+DocComment = getDocComment(Recorder->CCSema->getASTContext(), *SR,
+   /*CommentsFromHeader=*/false);
   }
 }
 return Candidate.build(FileName, Scores, Opts, SemaCCS, Includes.get(), 
DocComment);

Modified: clang-tools-extra/trunk/clangd/CodeCompletionStrings.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeCompletionStrings.cpp?rev=333189=333188=333189=diff
==
--- clang-tools-extra/trunk/clangd/CodeCompletionStrings.cpp (original)
+++ clang-tools-extra/trunk/clangd/CodeCompletionStrings.cpp Thu May 24 
07:49:23 2018
@@ -10,6 +10,7 @@
 #include "CodeCompletionStrings.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/RawCommentList.h"
+#include "clang/Basic/SourceManager.h"
 #include 
 
 namespace clang {
@@ -122,10 +123,23 @@ void processSnippetChunks(const CodeComp
   }
 }
 
+std::string getFormattedComment(const ASTContext , const RawComment ,
+bool CommentsFromHeaders) {
+  auto  = Ctx.getSourceManager();
+  // Parsing comments from invalid preamble can lead to crashes. So we only
+  // return comments from the main file when doing code completion. For
+  // indexing, we still read all the comments.
+  // FIXME: find a better fix, e.g. store file contents in the preamble or get
+  // doc comments from the index.
+  if (!CommentsFromHeaders && !SourceMgr.isWrittenInMainFile(RC.getLocStart()))
+return "";
+  return RC.getFormattedText(Ctx.getSourceManager(), Ctx.getDiagnostics());
+}
 } // namespace
 
 std::string getDocComment(const ASTContext ,
-  const CodeCompletionResult ) {
+  const CodeCompletionResult ,
+  bool CommentsFromHeaders) {
   // FIXME: clang's completion also returns documentation for RK_Pattern if 
they
   // contain a pattern for ObjC properties. Unfortunately, there is no API to
   // get this declaration, so we don't show documentation in that case.
@@ -137,20 +151,20 @@ std::string getDocComment(const ASTConte
   const RawComment *RC = getCompletionComment(Ctx, Decl);
   if (!RC)
 return "";
-  return RC->getFormattedText(Ctx.getSourceManager(), Ctx.getDiagnostics());
+  return getFormattedComment(Ctx, *RC, CommentsFromHeaders);
 }
 
 std::string
 

[PATCH] D47097: [DebugInfo] Preserve scope in auto generated StoreInst

[Anastasis via Phabricator via cfe-commits]

gramanas updated this revision to Diff 148414.
gramanas added a comment.

Adress review comments.
Limit changes to the storeInst


Repository:
  rC Clang

https://reviews.llvm.org/D47097

Files:
  lib/CodeGen/CGDecl.cpp
  test/CodeGen/debug-info-preserve-scope.c


Index: test/CodeGen/debug-info-preserve-scope.c
===
--- /dev/null
+++ test/CodeGen/debug-info-preserve-scope.c
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -debug-info-kind=limited -emit-llvm -o - %s | FileCheck %s
+
+static int a;
+
+// CHECK-LABEL: define void @f
+void f(int b) {
+  a = b;
+}
+
+// CHECK: store i32 {{.*}} !dbg ![[artificialDbgLoc:[0-9]+]]
+
+// CHECK: ![[artificialDbgLoc]] = !DILocation(line: 0
Index: lib/CodeGen/CGDecl.cpp
===
--- lib/CodeGen/CGDecl.cpp
+++ lib/CodeGen/CGDecl.cpp
@@ -1946,6 +1946,9 @@
 }
   }
 
+  // Set artificial debug location in order to preserve the scope
+  auto DL = ApplyDebugLocation::CreateArtificial(*this);
+
   Address DeclPtr = Address::invalid();
   bool DoStore = false;
   bool IsScalar = hasScalarEvaluationKind(Ty);


Index: test/CodeGen/debug-info-preserve-scope.c
===
--- /dev/null
+++ test/CodeGen/debug-info-preserve-scope.c
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -debug-info-kind=limited -emit-llvm -o - %s | FileCheck %s
+
+static int a;
+
+// CHECK-LABEL: define void @f
+void f(int b) {
+  a = b;
+}
+
+// CHECK: store i32 {{.*}} !dbg ![[artificialDbgLoc:[0-9]+]]
+
+// CHECK: ![[artificialDbgLoc]] = !DILocation(line: 0
Index: lib/CodeGen/CGDecl.cpp
===
--- lib/CodeGen/CGDecl.cpp
+++ lib/CodeGen/CGDecl.cpp
@@ -1946,6 +1946,9 @@
 }
   }
 
+  // Set artificial debug location in order to preserve the scope
+  auto DL = ApplyDebugLocation::CreateArtificial(*this);
+
   Address DeclPtr = Address::invalid();
   bool DoStore = false;
   bool IsScalar = hasScalarEvaluationKind(Ty);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47274: [clangd] Serve comments for headers decls from dynamic index only

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clangd/CodeCompletionStrings.h:46
const CodeCompleteConsumer::OverloadCandidate ,
-   unsigned ArgIndex);
+   unsigned ArgIndex, bool NoCommentsFromHeaders = true);
 

sammccall wrote:
> this invites double negation.
> Also it doesn't seem like a great default, violates principle of least 
> surprise.
> 
> prefer bool CommentsFromHeaders = true, or with no default.
> (or even consider an enum)
Thanks for spotting this!
I opted for a positive bool flag with no defaults.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D47274



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

[PATCH] D47274: [clangd] Serve comments for headers decls from dynamic index only

[Ilya Biryukov via Phabricator via cfe-commits]

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

- Invert flag, remove the default.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D47274

Files:
  clangd/CodeComplete.cpp
  clangd/CodeCompletionStrings.cpp
  clangd/CodeCompletionStrings.h
  clangd/index/SymbolCollector.cpp
  unittests/clangd/CodeCompleteTests.cpp

Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -17,6 +17,7 @@
 #include "SyncAPI.h"
 #include "TestFS.h"
 #include "index/MemIndex.h"
+#include "llvm/Support/Error.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
@@ -998,6 +999,46 @@
   }
 }
 
+TEST(CompletionTest, DocumentationFromChangedFileCrash) {
+  MockFSProvider FS;
+  auto FooH = testPath("foo.h");
+  auto FooCpp = testPath("foo.cpp");
+  FS.Files[FooH] = R"cpp(
+// this is my documentation comment.
+int func();
+  )cpp";
+  FS.Files[FooCpp] = "";
+
+  MockCompilationDatabase CDB;
+  IgnoreDiagnostics DiagConsumer;
+  ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
+
+  Annotations Source(R"cpp(
+#include "foo.h"
+int func() {
+  // This makes sure we have func from header in the AST.
+}
+int a = fun^
+  )cpp");
+  Server.addDocument(FooCpp, Source.code(), WantDiagnostics::Yes);
+  // We need to wait for preamble to build.
+  ASSERT_TRUE(Server.blockUntilIdleForTest());
+
+  // Change the header file. Completion will reuse the old preamble!
+  FS.Files[FooH] = R"cpp(
+int func();
+  )cpp";
+
+  clangd::CodeCompleteOptions Opts;
+  Opts.IncludeComments = true;
+  CompletionList Completions =
+  cantFail(runCodeComplete(Server, FooCpp, Source.point(), Opts));
+  // We shouldn't crash. Unfortunately, current workaround is to not produce
+  // comments for symbols from headers.
+  EXPECT_THAT(Completions.items,
+  Contains(AllOf(Not(IsDocumented()), Named("func";
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/index/SymbolCollector.cpp
===
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -381,7 +381,8 @@
 /*EnableSnippets=*/false);
   std::string FilterText = getFilterText(*CCS);
   std::string Documentation =
-  formatDocumentation(*CCS, getDocComment(Ctx, SymbolCompletion));
+  formatDocumentation(*CCS, getDocComment(Ctx, SymbolCompletion,
+  /*CommentsFromHeaders=*/true));
   std::string CompletionDetail = getDetail(*CCS);
 
   std::string Include;
Index: clangd/CodeCompletionStrings.h
===
--- clangd/CodeCompletionStrings.h
+++ clangd/CodeCompletionStrings.h
@@ -25,18 +25,25 @@
 /// markers stripped. See clang::RawComment::getFormattedText() for the detailed
 /// explanation of how the comment text is transformed.
 /// Returns empty string when no comment is available.
+/// If \p CommentsFromHeaders parameter is set, only comments from the main
+/// file will be returned. It is used to workaround crashes when parsing
+/// comments in the stale headers, coming from completion preamble.
 std::string getDocComment(const ASTContext ,
-  const CodeCompletionResult );
+  const CodeCompletionResult ,
+  bool CommentsFromHeaders);
 
 /// Gets a minimally formatted documentation for parameter of \p Result,
 /// corresponding to argument number \p ArgIndex.
 /// This currently looks for comments attached to the parameter itself, and
 /// doesn't extract them from function documentation.
 /// Returns empty string when no comment is available.
+/// If \p CommentsFromHeaders parameter is set, only comments from the main
+/// file will be returned. It is used to workaround crashes when parsing
+/// comments in the stale headers, coming from completion preamble.
 std::string
 getParameterDocComment(const ASTContext ,
const CodeCompleteConsumer::OverloadCandidate ,
-   unsigned ArgIndex);
+   unsigned ArgIndex, bool CommentsFromHeaders);
 
 /// Gets label and insert text for a completion item. For example, for function
 /// `Foo`, this returns <"Foo(int x, int y)", "Foo"> without snippts enabled.
Index: clangd/CodeCompletionStrings.cpp
===
--- clangd/CodeCompletionStrings.cpp
+++ clangd/CodeCompletionStrings.cpp
@@ -10,6 +10,7 @@
 #include "CodeCompletionStrings.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/RawCommentList.h"
+#include "clang/Basic/SourceManager.h"
 #include 
 
 namespace clang {
@@ -122,10 +123,23 @@
   }
 }
 
+std::string 

[clang-tools-extra] r333188 - [clangd] Skip .inc headers when canonicalizing header #include.

[Eric Liu via cfe-commits]

Author: ioeric
Date: Thu May 24 07:40:24 2018
New Revision: 333188

URL: http://llvm.org/viewvc/llvm-project?rev=333188=rev
Log:
[clangd] Skip .inc headers when canonicalizing header #include.

Summary:
This assumes that .inc files are supposed to be included via headers
that include them.

Reviewers: sammccall

Reviewed By: sammccall

Subscribers: klimek, ilya-biryukov, MaskRay, jkorous, cfe-commits

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

Modified:
clang-tools-extra/trunk/clangd/index/CanonicalIncludes.cpp
clang-tools-extra/trunk/clangd/index/CanonicalIncludes.h
clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp

Modified: clang-tools-extra/trunk/clangd/index/CanonicalIncludes.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/CanonicalIncludes.cpp?rev=333188=333187=333188=diff
==
--- clang-tools-extra/trunk/clangd/index/CanonicalIncludes.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/CanonicalIncludes.cpp Thu May 24 
07:40:24 2018
@@ -8,6 +8,8 @@
 
//===--===//
 
 #include "CanonicalIncludes.h"
+#include "../Headers.h"
+#include "clang/Driver/Types.h"
 #include "llvm/Support/Regex.h"
 
 namespace clang {
@@ -33,12 +35,33 @@ void CanonicalIncludes::addSymbolMapping
 }
 
 llvm::StringRef
-CanonicalIncludes::mapHeader(llvm::StringRef Header,
+CanonicalIncludes::mapHeader(llvm::ArrayRef Headers,
  llvm::StringRef QualifiedName) const {
+  assert(!Headers.empty());
   auto SE = SymbolMapping.find(QualifiedName);
   if (SE != SymbolMapping.end())
 return SE->second;
   std::lock_guard Lock(RegexMutex);
+  // Find the first header such that the extension is not '.inc', and isn't a
+  // recognized non-header file
+  auto I =
+  std::find_if(Headers.begin(), Headers.end(), [](llvm::StringRef Include) 
{
+// Skip .inc file whose including header file should
+// be #included instead.
+return !Include.endswith(".inc");
+  });
+  if (I == Headers.end())
+return Headers[0]; // Fallback to the declaring header.
+  StringRef Header = *I;
+  // If Header is not expected be included (e.g. .cc file), we fall back to
+  // the declaring header.
+  StringRef Ext = llvm::sys::path::extension(Header).trim('.');
+  // Include-able headers must have precompile type. Treat files with
+  // non-recognized extenstions (TY_INVALID) as headers.
+  auto ExtType = driver::types::lookupTypeForExtension(Ext);
+  if ((ExtType != driver::types::TY_INVALID) &&
+  !driver::types::onlyPrecompileType(ExtType))
+return Headers[0];
   for (auto  : RegexHeaderMappingTable) {
 #ifndef NDEBUG
 std::string Dummy;
@@ -65,9 +88,8 @@ collectIWYUHeaderMaps(CanonicalIncludes
   // FIXME(ioeric): resolve the header and store actual file path. For now,
   // we simply assume the written header is suitable to be #included.
   Includes->addMapping(PP.getSourceManager().getFilename(Range.getBegin()),
-   (Text.startswith("<") || Text.startswith("\""))
-   ? Text.str()
-   : ("\"" + Text + "\"").str());
+   isLiteralInclude(Text) ? Text.str()
+  : ("\"" + Text + 
"\"").str());
   return false;
 }
 

Modified: clang-tools-extra/trunk/clangd/index/CanonicalIncludes.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/CanonicalIncludes.h?rev=333188=333187=333188=diff
==
--- clang-tools-extra/trunk/clangd/index/CanonicalIncludes.h (original)
+++ clang-tools-extra/trunk/clangd/index/CanonicalIncludes.h Thu May 24 
07:40:24 2018
@@ -48,9 +48,10 @@ public:
   void addSymbolMapping(llvm::StringRef QualifiedName,
 llvm::StringRef CanonicalPath);
 
-  /// Returns the canonical include for symbol with \p QualifiedName, which is
-  /// declared in \p Header
-  llvm::StringRef mapHeader(llvm::StringRef Header,
+  /// Returns the canonical include for symbol with \p QualifiedName.
+  /// \p Headers is the include stack: Headers.front() is the file declaring 
the
+  /// symbol, and Headers.back() is the main file.
+  llvm::StringRef mapHeader(llvm::ArrayRef Headers,
 llvm::StringRef QualifiedName) const;
 
 private:

Modified: clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp?rev=333188=333187=333188=diff
==
--- clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp (original)
+++ 

[PATCH] D47187: [clangd] Skip .inc headers when canonicalizing header #include.

[Eric Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL333188: [clangd] Skip .inc headers when canonicalizing 
header #include. (authored by ioeric, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D47187

Files:
  clang-tools-extra/trunk/clangd/index/CanonicalIncludes.cpp
  clang-tools-extra/trunk/clangd/index/CanonicalIncludes.h
  clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
  clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp

Index: clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
@@ -131,9 +131,9 @@
 auto Factory = llvm::make_unique(
 CollectorOpts, PragmaHandler.get());
 
-std::vector Args = {"symbol_collector", "-fsyntax-only",
- "-std=c++11",   "-include",
- TestHeaderName, TestFileName};
+std::vector Args = {
+"symbol_collector", "-fsyntax-only", "-xc++", "-std=c++11",
+"-include", TestHeaderName,  TestFileName};
 Args.insert(Args.end(), ExtraArgs.begin(), ExtraArgs.end());
 tooling::ToolInvocation Invocation(
 Args,
@@ -666,6 +666,70 @@
  IncludeHeader("\"the/good/header.h\"";
 }
 
+TEST_F(SymbolCollectorTest, SkipIncFileWhenCanonicalizeHeaders) {
+  CollectorOpts.CollectIncludePath = true;
+  CanonicalIncludes Includes;
+  Includes.addMapping(TestHeaderName, "");
+  CollectorOpts.Includes = 
+  auto IncFile = testPath("test.inc");
+  auto IncURI = URI::createFile(IncFile).toString();
+  InMemoryFileSystem->addFile(IncFile, 0,
+  llvm::MemoryBuffer::getMemBuffer("class X {};"));
+  runSymbolCollector("#include \"test.inc\"\nclass Y {};", /*Main=*/"",
+ /*ExtraArgs=*/{"-I", testRoot()});
+  EXPECT_THAT(Symbols,
+  UnorderedElementsAre(AllOf(QName("X"), DeclURI(IncURI),
+ IncludeHeader("")),
+   AllOf(QName("Y"), DeclURI(TestHeaderURI),
+ IncludeHeader("";
+}
+
+TEST_F(SymbolCollectorTest, MainFileIsHeaderWhenSkipIncFile) {
+  CollectorOpts.CollectIncludePath = true;
+  CanonicalIncludes Includes;
+  CollectorOpts.Includes = 
+  TestFileName = testPath("main.h");
+  TestFileURI = URI::createFile(TestFileName).toString();
+  auto IncFile = testPath("test.inc");
+  auto IncURI = URI::createFile(IncFile).toString();
+  InMemoryFileSystem->addFile(IncFile, 0,
+  llvm::MemoryBuffer::getMemBuffer("class X {};"));
+  runSymbolCollector("", /*Main=*/"#include \"test.inc\"",
+ /*ExtraArgs=*/{"-I", testRoot()});
+  EXPECT_THAT(Symbols, UnorderedElementsAre(AllOf(QName("X"), DeclURI(IncURI),
+  IncludeHeader(TestFileURI;
+}
+
+TEST_F(SymbolCollectorTest, MainFileIsHeaderWithoutExtensionWhenSkipIncFile) {
+  CollectorOpts.CollectIncludePath = true;
+  CanonicalIncludes Includes;
+  CollectorOpts.Includes = 
+  TestFileName = testPath("no_ext_main");
+  TestFileURI = URI::createFile(TestFileName).toString();
+  auto IncFile = testPath("test.inc");
+  auto IncURI = URI::createFile(IncFile).toString();
+  InMemoryFileSystem->addFile(IncFile, 0,
+  llvm::MemoryBuffer::getMemBuffer("class X {};"));
+  runSymbolCollector("", /*Main=*/"#include \"test.inc\"",
+ /*ExtraArgs=*/{"-I", testRoot()});
+  EXPECT_THAT(Symbols, UnorderedElementsAre(AllOf(QName("X"), DeclURI(IncURI),
+  IncludeHeader(TestFileURI;
+}
+
+TEST_F(SymbolCollectorTest, FallbackToIncFileWhenIncludingFileIsCC) {
+  CollectorOpts.CollectIncludePath = true;
+  CanonicalIncludes Includes;
+  CollectorOpts.Includes = 
+  auto IncFile = testPath("test.inc");
+  auto IncURI = URI::createFile(IncFile).toString();
+  InMemoryFileSystem->addFile(IncFile, 0,
+  llvm::MemoryBuffer::getMemBuffer("class X {};"));
+  runSymbolCollector("", /*Main=*/"#include \"test.inc\"",
+ /*ExtraArgs=*/{"-I", testRoot()});
+  EXPECT_THAT(Symbols, UnorderedElementsAre(AllOf(QName("X"), DeclURI(IncURI),
+  IncludeHeader(IncURI;
+}
+
 TEST_F(SymbolCollectorTest, AvoidUsingFwdDeclsAsCanonicalDecls) {
   CollectorOpts.CollectIncludePath = true;
   Annotations Header(R"(
Index: clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
===
--- clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
+++ 

[PATCH] D47272: [clangd] Build index on preamble changes instead of the AST changes

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: clangd/index/FileIndex.cpp:37
+  std::vector TopLevelDecls(
+  AST.getTranslationUnitDecl()->decls().begin(),
+  AST.getTranslationUnitDecl()->decls().end());

ilya-biryukov wrote:
> ioeric wrote:
> > Would this give us decls in namespaces? It seems that 
> > `AST.getTranslationUnitDecl()->decls()` are only decls that are immediately 
> > inside the TU context. 
> I'll double check and add a test for that, but I think the indexer visits the 
> namespaces that we provide, I'm not sure if we have tests though.
Oh, right! In that case, it seems that a single TU decl which contains all 
interesting decls would be sufficient? Is it possible that preamble doesn't 
have all available top level decls in TU decl? This seems to be possible for 
ParsedAST, which is why we used `getTopLevelDecls`, I think?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D47272



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

Re: [PATCH] D46964: Implement class deduction guides for `std::array`

[Yvan Roux via cfe-commits]

Hi Marshall,

ARM and AArch64 libcxx buildbots are broken since that commits, logs
are available at:

http://lab.llvm.org:8011/builders/libcxx-libcxxabi-libunwind-armv7-linux/builds/107/steps/test.libcxx/logs/FAIL%3A%20libc%2B%2B%3A%3Adeduct.pass.cpp

Thanks
Yvan


On 18 May 2018 at 23:05, Marshall Clow via Phabricator via cfe-commits
 wrote:
> mclow.lists closed this revision.
> mclow.lists added a comment.
>
> Committed as revision 332768
>
>
> https://reviews.llvm.org/D46964
>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47272: [clangd] Build index on preamble changes instead of the AST changes

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

In https://reviews.llvm.org/D47272#1110958, @ioeric wrote:

> Would it be possible to add some integration tests for file index plus 
> preamble?


Sure, will do




Comment at: clangd/index/FileIndex.cpp:37
+  std::vector TopLevelDecls(
+  AST.getTranslationUnitDecl()->decls().begin(),
+  AST.getTranslationUnitDecl()->decls().end());

ioeric wrote:
> Would this give us decls in namespaces? It seems that 
> `AST.getTranslationUnitDecl()->decls()` are only decls that are immediately 
> inside the TU context. 
I'll double check and add a test for that, but I think the indexer visits the 
namespaces that we provide, I'm not sure if we have tests though.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D47272



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

r333186 - Disable an in-memory vfs file path test on windows.

[Eric Liu via cfe-commits]

Author: ioeric
Date: Thu May 24 06:52:48 2018
New Revision: 333186

URL: http://llvm.org/viewvc/llvm-project?rev=333186=rev
Log:
Disable an in-memory vfs file path test on windows.

The test uses unix paths and doesn't make sense to run on windows.

Fix bot failure caused by r333172:
 http://lab.llvm.org:8011/builders/clang-x64-ninja-win7/builds/10799

Modified:
cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp

Modified: cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp?rev=333186=333185=333186=diff
==
--- cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp (original)
+++ cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp Thu May 24 06:52:48 2018
@@ -813,6 +813,7 @@ TEST_F(InMemoryFileSystemTest, WorkingDi
   NormalizedFS.getCurrentWorkingDirectory().get()));
 }
 
+#if !defined(_WIN32)
 TEST_F(InMemoryFileSystemTest, GetRealPath) {
   SmallString<16> Path;
   EXPECT_EQ(FS.getRealPath("b", Path), errc::operation_not_permitted);
@@ -834,6 +835,7 @@ TEST_F(InMemoryFileSystemTest, GetRealPa
   EXPECT_EQ(GetRealPath("../b"), "/b");
   EXPECT_EQ(GetRealPath("b/./c"), "/a/b/c");
 }
+#endif // _WIN32
 
 TEST_F(InMemoryFileSystemTest, AddFileWithUser) {
   FS.addFile("/a/b/c", 0, MemoryBuffer::getMemBuffer("abc"), 0xFEEDFACE);


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

[PATCH] D41537: Optionally add code completion results for arrow instead of dot

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

I have only a few nits left, otherwise looks good. Thanks for making this 
change! Really excited for it to get in finally!




Comment at: include/clang/Driver/CC1Options.td:449
+def code_completion_with_fixits : Flag<["-"], "code-completion-with-fixits">,
+  HelpText<"Include code completion results which require small fix-its .">;
 def disable_free : Flag<["-"], "disable-free">,

NIT: there's a redundant space before the closing `.`



Comment at: include/clang/Sema/CodeCompleteConsumer.h:786
 
+  /// \brief FixIts that *must* be applied before inserting the text for the
+  /// corresponding completion item.

NIT: remove \brief from the start of the comment. Those recently got removed 
from all of llvm



Comment at: include/clang/Sema/CodeCompleteConsumer.h:1059
 
+  /// \brief Whether to include completion items with small fix-its, e.g. 
change
+  /// '.' to '->' on member access, etc.

NIT: another redundant `\brief`



Comment at: include/clang/Sema/CodeCompleteOptions.h:42
 
+  /// Show also results after corrections (small fix-its), e.g. change '.' to
+  /// '->' on member access, etc.

NIT: Maybe `s/Show also results/Include results`?



Comment at: lib/Sema/SemaCodeComplete.cpp:4148
+
+  CompletionSucceded |= DoCompletion(Base, IsArrow, None);
+

NIT: maybe swap the two cases to do the non-fixit ones first? It is the most 
common case, so it should probably go first.


https://reviews.llvm.org/D41537



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

[PATCH] D47272: [clangd] Build index on preamble changes instead of the AST changes

[Eric Liu via Phabricator via cfe-commits]

ioeric added a comment.

Would it be possible to add some integration tests for file index plus preamble?




Comment at: clangd/index/FileIndex.cpp:37
+  std::vector TopLevelDecls(
+  AST.getTranslationUnitDecl()->decls().begin(),
+  AST.getTranslationUnitDecl()->decls().end());

Would this give us decls in namespaces? It seems that 
`AST.getTranslationUnitDecl()->decls()` are only decls that are immediately 
inside the TU context. 


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D47272



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

[PATCH] D47274: [clangd] Serve comments for headers decls from dynamic index only

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.



Comment at: clangd/CodeCompletionStrings.h:46
const CodeCompleteConsumer::OverloadCandidate ,
-   unsigned ArgIndex);
+   unsigned ArgIndex, bool NoCommentsFromHeaders = true);
 

this invites double negation.
Also it doesn't seem like a great default, violates principle of least surprise.

prefer bool CommentsFromHeaders = true, or with no default.
(or even consider an enum)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D47274



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

[PATCH] D47267: [UnrollAndJam] Add unroll_and_jam pragma handling

[Dave Green via Phabricator via cfe-commits]

dmgreen updated this revision to Diff 148393.
dmgreen added a comment.

This splits out the pragma clang loop unroll_and_jam handling into 
https://reviews.llvm.org/D47320, for if/when we need it. Which I believe is 
what you wanted, correct me if I'm wrong.


https://reviews.llvm.org/D47267

Files:
  include/clang/Basic/Attr.td
  include/clang/Parse/Parser.h
  lib/CodeGen/CGLoopInfo.cpp
  lib/CodeGen/CGLoopInfo.h
  lib/Parse/ParsePragma.cpp
  lib/Sema/SemaStmtAttr.cpp
  test/CodeGenCXX/pragma-unroll-and-jam.cpp
  test/Parser/pragma-unroll-and-jam.cpp

Index: test/Parser/pragma-unroll-and-jam.cpp
===
--- /dev/null
+++ test/Parser/pragma-unroll-and-jam.cpp
@@ -0,0 +1,78 @@
+// RUN: %clang_cc1 -std=c++11 -verify %s
+
+// Note that this puts the expected lines before the directives to work around
+// limitations in the -verify mode.
+
+void test(int *List, int Length, int Value) {
+  int i = 0;
+
+#pragma unroll_and_jam
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  List[i * Length + j] = Value;
+}
+  }
+
+#pragma nounroll_and_jam
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  List[i * Length + j] = Value;
+}
+  }
+
+#pragma unroll_and_jam 4
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  List[i * Length + j] = Value;
+}
+  }
+
+/* expected-error {{expected ')'}} */ #pragma unroll_and_jam(4
+/* expected-error {{missing argument; expected an integer value}} */ #pragma unroll_and_jam()
+/* expected-warning {{extra tokens at end of '#pragma unroll_and_jam'}} */ #pragma unroll_and_jam 1 2
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  List[i * Length + j] = Value;
+}
+  }
+
+/* expected-warning {{extra tokens at end of '#pragma nounroll_and_jam'}} */ #pragma nounroll_and_jam 1
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  List[i * Length + j] = Value;
+}
+  }
+
+#pragma unroll_and_jam
+/* expected-error {{expected a for, while, or do-while loop to follow '#pragma unroll_and_jam'}} */ int j = Length;
+#pragma unroll_and_jam 4
+/* expected-error {{expected a for, while, or do-while loop to follow '#pragma unroll_and_jam'}} */ int k = Length;
+#pragma nounroll_and_jam
+/* expected-error {{expected a for, while, or do-while loop to follow '#pragma nounroll_and_jam'}} */ int l = Length;
+
+/* expected-error {{incompatible directives '#pragma nounroll_and_jam' and '#pragma unroll_and_jam(4)'}} */ #pragma unroll_and_jam 4
+#pragma nounroll_and_jam
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  List[i * Length + j] = Value;
+}
+  }
+
+#pragma nounroll_and_jam
+#pragma unroll(4)
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  List[i * Length + j] = Value;
+}
+  }
+
+// pragma clang unroll_and_jam is disabled for the moment
+/* expected-error {{invalid option 'unroll_and_jam'; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, or distribute}} */ #pragma clang loop unroll_and_jam(4)
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  List[i * Length + j] = Value;
+}
+  }
+
+#pragma unroll_and_jam
+/* expected-error {{expected statement}} */ }
Index: test/CodeGenCXX/pragma-unroll-and-jam.cpp
===
--- /dev/null
+++ test/CodeGenCXX/pragma-unroll-and-jam.cpp
@@ -0,0 +1,55 @@
+// RUN: %clang_cc1 -triple arm-none-eabi -std=c++11 -emit-llvm -o - %s | FileCheck %s
+
+void unroll_and_jam(int *List, int Length, int Value) {
+  // CHECK: define {{.*}} @_Z14unroll_and_jam
+#pragma unroll_and_jam
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  // CHECK: br label {{.*}}, !llvm.loop ![[LOOP_1:.*]]
+  List[i * Length + j] = Value;
+}
+  }
+}
+
+void unroll_and_jam_count(int *List, int Length, int Value) {
+  // CHECK: define {{.*}} @_Z20unroll_and_jam_count
+#pragma unroll_and_jam(4)
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  // CHECK: br label {{.*}}, !llvm.loop ![[LOOP_2:.*]]
+  List[i * Length + j] = Value;
+}
+  }
+}
+
+void nounroll_and_jam(int *List, int Length, int Value) {
+  // CHECK: define {{.*}} @_Z16nounroll_and_jam
+#pragma nounroll_and_jam
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  // CHECK: br label {{.*}}, !llvm.loop ![[LOOP_3:.*]]
+  List[i * Length + j] = Value;
+}
+  }
+}
+
+void clang_unroll_plus_nounroll_and_jam(int *List, int Length, int Value) {
+  // CHECK: define {{.*}} @_Z34clang_unroll_plus_nounroll_and_jam
+#pragma nounroll_and_jam
+#pragma unroll(4)
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  // CHECK: br label {{.*}}, !llvm.loop 

[PATCH] D47196: [Time-report ](2): Recursive timers in Clang

[Andrew V. Tischenko via Phabricator via cfe-commits]

avt77 updated this revision to Diff 148395.
avt77 added a comment.

The sources were re-based to fit in LLVM_DEBUG rename.
One more counter was added.
Debug logging was improved again.


https://reviews.llvm.org/D47196

Files:
  include/clang/Frontend/Utils.h
  lib/CodeGen/BackendUtil.cpp
  lib/CodeGen/CodeGenAction.cpp
  lib/CodeGen/CodeGenFunction.cpp
  lib/CodeGen/CodeGenModule.cpp
  lib/Frontend/FrontendTiming.cpp
  lib/Parse/CMakeLists.txt
  lib/Parse/ParseTemplate.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaLambda.cpp
  lib/Sema/SemaTemplateInstantiateDecl.cpp
  lib/Sema/TreeTransform.h
  test/Frontend/ftime-report-template-decl.cpp
  test/Headers/opencl-c-header.cl

Index: test/Headers/opencl-c-header.cl
===
--- test/Headers/opencl-c-header.cl
+++ test/Headers/opencl-c-header.cl
@@ -71,4 +71,5 @@
 }
 #endif //__OPENCL_C_VERSION__
 
-// CHECK-MOD: Reading modules
+// CHECK-DAG-MOD: Clang Timers: CodeGen Functions
+// CHECK-DAG-MOD: Reading modules
Index: test/Frontend/ftime-report-template-decl.cpp
===
--- test/Frontend/ftime-report-template-decl.cpp
+++ test/Frontend/ftime-report-template-decl.cpp
@@ -3,9 +3,15 @@
 
 // Template function declarations
 template 
-void foo();
+T foo(T bar) {
+  T Result = bar * bar + bar / 1.2 + bar;
+  return Result;
+};
 template 
-void foo();
+T foo(T bar, U bar2) {
+  T Result = bar2 * bar + bar / 1.2 + bar2;
+  return Result;
+};
 
 // Template function definitions.
 template 
@@ -130,9 +136,15 @@
 template 
 oneT L<0>::O::Fun(U) { return one; }
 
-void Instantiate() {
+double Instantiate() {
   sassert(sizeof(L<0>::O::Fun(0)) == sizeof(one));
   sassert(sizeof(L<0>::O::Fun(0)) == sizeof(one));
+  int R1 = foo(123);
+  char R2 = foo('d', 1234);
+  int R3 = foo(1.2);
+  double R4 = foo(34.56, 1234);
+  double R5 = R1 + R2 * R3 - R4 + one[0] - two[1];
+  return R5 * R1 + R4 / R3 + R2;
 }
 }
 
@@ -150,7 +162,8 @@
 };
 _Wrap_alloc::rebind w;
 
-// CHECK: Miscellaneous Ungrouped Timers
+// CHECK: Clang Timers: CodeGen Functions
+// CHECK-DAG: Miscellaneous Ungrouped Timers
 // CHECK-DAG: LLVM IR Generation Time
 // CHECK-DAG: Code Generation Time
 // CHECK: Total
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -27,6 +27,7 @@
 #include "clang/AST/StmtCXX.h"
 #include "clang/AST/StmtObjC.h"
 #include "clang/AST/StmtOpenMP.h"
+#include "clang/Frontend/Utils.h"
 #include "clang/Sema/Designator.h"
 #include "clang/Sema/Lookup.h"
 #include "clang/Sema/Ownership.h"
@@ -10945,6 +10946,11 @@
 
   LSI->CallOperator = NewCallOperator;
 
+  if (FrontendTimesIsEnabled) {
+getFrontendFunctionTimeCtx()->startFrontendTimer(
+{NewCallOperator, 0.0});
+  }
+
   for (unsigned I = 0, NumParams = NewCallOperator->getNumParams();
I != NumParams; ++I) {
 auto *P = NewCallOperator->getParamDecl(I);
Index: lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -1,4 +1,5 @@
-//===--- SemaTemplateInstantiateDecl.cpp - C++ Template Decl Instantiation ===/
+//===--- SemaTemplateInstantiateDecl.cpp - C++ T--emplate Decl Instantiation
+//===/
 //
 // The LLVM Compiler Infrastructure
 //
@@ -9,7 +10,6 @@
 //  This file implements C++ template instantiation for declarations.
 //
 //===--===/
-#include "clang/Sema/SemaInternal.h"
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTMutationListener.h"
@@ -20,11 +20,14 @@
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/PrettyDeclStackTrace.h"
 #include "clang/AST/TypeLoc.h"
+#include "clang/Frontend/Utils.h"
 #include "clang/Sema/Initialization.h"
 #include "clang/Sema/Lookup.h"
+#include "clang/Sema/SemaInternal.h"
 #include "clang/Sema/Template.h"
 #include "clang/Sema/TemplateInstCallback.h"
 
+#define DEBUG_TYPE "templateinst"
 using namespace clang;
 
 static bool isDeclWithinFunction(const Decl *D) {
Index: lib/Sema/SemaLambda.cpp
===
--- lib/Sema/SemaLambda.cpp
+++ lib/Sema/SemaLambda.cpp
@@ -10,20 +10,23 @@
 //  This file implements semantic analysis for C++ lambda expressions.
 //
 //===--===//
-#include "clang/Sema/DeclSpec.h"
+#include "clang/Sema/SemaLambda.h"
 #include "TypeLocBuilder.h"
 #include "clang/AST/ASTLambda.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/Basic/TargetInfo.h"
+#include "clang/Frontend/Utils.h"
+#include "clang/Sema/DeclSpec.h"
 #include "clang/Sema/Initialization.h"
 #include "clang/Sema/Lookup.h"
 #include 

[PATCH] D47320: [UnrollAndJam] Add pragma clang loop unroll_and_jam handling.

[Dave Green via Phabricator via cfe-commits]

dmgreen created this revision.
Herald added a subscriber: zzheng.

Split out from https://reviews.llvm.org/D47267 when we need it.


https://reviews.llvm.org/D47320

Files:
  include/clang/Basic/DiagnosticParseKinds.td
  lib/Parse/ParsePragma.cpp
  lib/Sema/SemaStmtAttr.cpp
  test/CodeGenCXX/pragma-unroll-and-jam.cpp
  test/Parser/pragma-loop.cpp
  test/Parser/pragma-unroll-and-jam.cpp

Index: test/Parser/pragma-unroll-and-jam.cpp
===
--- test/Parser/pragma-unroll-and-jam.cpp
+++ test/Parser/pragma-unroll-and-jam.cpp
@@ -50,24 +50,24 @@
 #pragma nounroll_and_jam
 /* expected-error {{expected a for, while, or do-while loop to follow '#pragma nounroll_and_jam'}} */ int l = Length;
 
-/* expected-error {{incompatible directives '#pragma nounroll_and_jam' and '#pragma unroll_and_jam(4)'}} */ #pragma unroll_and_jam 4
-#pragma nounroll_and_jam
+/* expected-error {{incompatible directives 'unroll_and_jam(disable)' and '#pragma unroll_and_jam(4)'}} */ #pragma unroll_and_jam 4
+#pragma clang loop unroll_and_jam(disable)
   for (int i = 0; i < Length; i++) {
 for (int j = 0; j < Length; j++) {
   List[i * Length + j] = Value;
 }
   }
 
-#pragma nounroll_and_jam
-#pragma unroll(4)
+/* expected-error {{incompatible directives 'unroll_and_jam(full)' and '#pragma unroll_and_jam(4)'}} */ #pragma unroll_and_jam(4)
+#pragma clang loop unroll_and_jam(full)
   for (int i = 0; i < Length; i++) {
 for (int j = 0; j < Length; j++) {
   List[i * Length + j] = Value;
 }
   }
 
-// pragma clang unroll_and_jam is disabled for the moment
-/* expected-error {{invalid option 'unroll_and_jam'; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, or distribute}} */ #pragma clang loop unroll_and_jam(4)
+#pragma nounroll_and_jam
+#pragma unroll(4)
   for (int i = 0; i < Length; i++) {
 for (int j = 0; j < Length; j++) {
   List[i * Length + j] = Value;
Index: test/Parser/pragma-loop.cpp
===
--- test/Parser/pragma-loop.cpp
+++ test/Parser/pragma-loop.cpp
@@ -147,7 +147,7 @@
 /* expected-error {{missing argument; expected 'enable', 'full' or 'disable'}} */ #pragma clang loop unroll()
 /* expected-error {{missing argument; expected 'enable' or 'disable'}} */ #pragma clang loop distribute()
 
-/* expected-error {{missing option; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, or distribute}} */ #pragma clang loop
+/* expected-error {{missing option; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, unroll_and_jam, unroll_and_jam_count, or distribute}} */ #pragma clang loop
 /* expected-error {{invalid option 'badkeyword'}} */ #pragma clang loop badkeyword
 /* expected-error {{invalid option 'badkeyword'}} */ #pragma clang loop badkeyword(enable)
 /* expected-error {{invalid option 'badkeyword'}} */ #pragma clang loop vectorize(enable) badkeyword(4)
Index: test/CodeGenCXX/pragma-unroll-and-jam.cpp
===
--- test/CodeGenCXX/pragma-unroll-and-jam.cpp
+++ test/CodeGenCXX/pragma-unroll-and-jam.cpp
@@ -33,6 +33,39 @@
   }
 }
 
+void clang_unroll_and_jam(int *List, int Length, int Value) {
+  // CHECK: define {{.*}} @_Z20clang_unroll_and_jam
+#pragma clang loop unroll_and_jam(enable)
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  // CHECK: br label {{.*}}, !llvm.loop ![[LOOP_4:.*]]
+  List[i * Length + j] = Value;
+}
+  }
+}
+
+void clang_unroll_and_jam_count(int *List, int Length, int Value) {
+  // CHECK: define {{.*}} @_Z26clang_unroll_and_jam_count
+#pragma clang loop unroll_and_jam_count(4)
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  // CHECK: br label {{.*}}, !llvm.loop ![[LOOP_5:.*]]
+  List[i * Length + j] = Value;
+}
+  }
+}
+
+void clang_nounroll_and_jam(int *List, int Length, int Value) {
+  // CHECK: define {{.*}} @_Z22clang_nounroll_and_jam
+#pragma clang loop unroll_and_jam(disable)
+  for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+  // CHECK: br label {{.*}}, !llvm.loop ![[LOOP_6:.*]]
+  List[i * Length + j] = Value;
+}
+  }
+}
+
 void clang_unroll_plus_nounroll_and_jam(int *List, int Length, int Value) {
   // CHECK: define {{.*}} @_Z34clang_unroll_plus_nounroll_and_jam
 #pragma nounroll_and_jam
@@ -51,5 +84,8 @@
 // CHECK: ![[UNJ_4]] = !{!"llvm.loop.unroll_and_jam.count", i32 4}
 // CHECK: ![[LOOP_3]] = distinct !{![[LOOP_3]], ![[UNJ_DISABLE:.*]]}
 // CHECK: ![[UNJ_DISABLE]] = !{!"llvm.loop.unroll_and_jam.disable"}
+// CHECK: ![[LOOP_4]] = distinct !{![[LOOP_4]], ![[UNJ_ENABLE:.*]]}
+// CHECK: ![[LOOP_5]] = distinct !{![[LOOP_5]], ![[UNJ_4:.*]]}
+// CHECK: ![[LOOP_6]] = distinct !{![[LOOP_6]], ![[UNJ_DISABLE:.*]]}
 // CHECK: ![[LOOP_7]] = distinct !{![[LOOP_7]], 

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

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

> But I feel it's a bit odd that completion and workspace symbols would be 
> inconsistent.

It does not seem that odd to me. Completion is something that should follow the 
language rules more closely, i.e. we don't want to deviate from sema 
completions too much. While workspaceSymbol is a user-facing feature and we 
probably want users to type as little as possible there.
E.g. not showing `namespace_name::A` in completion seems bad to me, since 
people are used to referring to the enumerators this way and sema completion 
gives you those results.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D47223



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

[PATCH] D45920: [analyzer] Move RangeSet related declarations into the RangedConstraintManager header.

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC333179: [analyzer] Move RangeSet related declarations into 
the RangedConstraintManager… (authored by mramalho, committed by ).

Repository:
  rC Clang

https://reviews.llvm.org/D45920

Files:
  lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  lib/StaticAnalyzer/Core/RangedConstraintManager.h

Index: lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===
--- lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -23,263 +23,171 @@
 using namespace clang;
 using namespace ento;
 
-/// A Range represents the closed range [from, to].  The caller must
-/// guarantee that from <= to.  Note that Range is immutable, so as not
-/// to subvert RangeSet's immutability.
-namespace {
-class Range : public std::pair {
-public:
-  Range(const llvm::APSInt , const llvm::APSInt )
-  : std::pair(, ) {
-assert(from <= to);
-  }
-  bool Includes(const llvm::APSInt ) const {
-return *first <= v && v <= *second;
-  }
-  const llvm::APSInt () const { return *first; }
-  const llvm::APSInt () const { return *second; }
-  const llvm::APSInt *getConcreteValue() const {
-return () == () ? () : nullptr;
-  }
-
-  void Profile(llvm::FoldingSetNodeID ) const {
-ID.AddPointer(());
-ID.AddPointer(());
-  }
-};
-
-class RangeTrait : public llvm::ImutContainerInfo {
-public:
-  // When comparing if one Range is less than another, we should compare
-  // the actual APSInt values instead of their pointers.  This keeps the order
-  // consistent (instead of comparing by pointer values) and can potentially
-  // be used to speed up some of the operations in RangeSet.
-  static inline bool isLess(key_type_ref lhs, key_type_ref rhs) {
-return *lhs.first < *rhs.first ||
-   (!(*rhs.first < *lhs.first) && *lhs.second < *rhs.second);
-  }
-};
-
-/// RangeSet contains a set of ranges. If the set is empty, then
-///  there the value of a symbol is overly constrained and there are no
-///  possible values for that symbol.
-class RangeSet {
-  typedef llvm::ImmutableSet PrimRangeSet;
-  PrimRangeSet ranges; // no need to make const, since it is an
-   // ImmutableSet - this allows default operator=
-   // to work.
-public:
-  typedef PrimRangeSet::Factory Factory;
-  typedef PrimRangeSet::iterator iterator;
-
-  RangeSet(PrimRangeSet RS) : ranges(RS) {}
-
-  /// Create a new set with all ranges of this set and RS.
-  /// Possible intersections are not checked here.
-  RangeSet addRange(Factory , const RangeSet ) {
-PrimRangeSet Ranges(RS.ranges);
-for (const auto  : ranges)
-  Ranges = F.add(Ranges, range);
-return RangeSet(Ranges);
-  }
-
-  iterator begin() const { return ranges.begin(); }
-  iterator end() const { return ranges.end(); }
-
-  bool isEmpty() const { return ranges.isEmpty(); }
-
-  /// Construct a new RangeSet representing '{ [from, to] }'.
-  RangeSet(Factory , const llvm::APSInt , const llvm::APSInt )
-  : ranges(F.add(F.getEmptySet(), Range(from, to))) {}
-
-  /// Profile - Generates a hash profile of this RangeSet for use
-  ///  by FoldingSet.
-  void Profile(llvm::FoldingSetNodeID ) const { ranges.Profile(ID); }
-
-  /// getConcreteValue - If a symbol is contrained to equal a specific integer
-  ///  constant then this method returns that value.  Otherwise, it returns
-  ///  NULL.
-  const llvm::APSInt *getConcreteValue() const {
-return ranges.isSingleton() ? ranges.begin()->getConcreteValue() : nullptr;
-  }
+void RangeSet::IntersectInRange(BasicValueFactory , Factory ,
+  const llvm::APSInt , const llvm::APSInt ,
+  PrimRangeSet , PrimRangeSet::iterator ,
+  PrimRangeSet::iterator ) const {
+  // There are six cases for each range R in the set:
+  //   1. R is entirely before the intersection range.
+  //   2. R is entirely after the intersection range.
+  //   3. R contains the entire intersection range.
+  //   4. R starts before the intersection range and ends in the middle.
+  //   5. R starts in the middle of the intersection range and ends after it.
+  //   6. R is entirely contained in the intersection range.
+  // These correspond to each of the conditions below.
+  for (/* i = begin(), e = end() */; i != e; ++i) {
+if (i->To() < Lower) {
+  continue;
+}
+if (i->From() > Upper) {
+  break;
+}
 
-private:
-  void IntersectInRange(BasicValueFactory , Factory ,
-const llvm::APSInt , const llvm::APSInt ,
-PrimRangeSet , PrimRangeSet::iterator ,
-PrimRangeSet::iterator ) const {
-// There are six cases for each range R in the set:
-//   1. R is entirely before the intersection range.
-//   2. R is entirely after the intersection 

r333157 - [Sparc] Use the leon arch for Leon3's when using an external assembler

[Daniel Cederman via cfe-commits]

Author: dcederman
Date: Wed May 23 23:16:02 2018
New Revision: 333157

URL: http://llvm.org/viewvc/llvm-project?rev=333157=rev
Log:
 [Sparc] Use the leon arch for Leon3's when using an external assembler

Summary: This allows the use of the casa instruction available in most Leon3's.

Reviewers: jyknight

Reviewed By: jyknight

Subscribers: joerg, fedor.sergeev, jrtc27, cfe-commits

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

Modified:
cfe/trunk/lib/Driver/ToolChains/Arch/Sparc.cpp
cfe/trunk/test/Driver/sparc-as.c

Modified: cfe/trunk/lib/Driver/ToolChains/Arch/Sparc.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Arch/Sparc.cpp?rev=333157=333156=333157=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Arch/Sparc.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Arch/Sparc.cpp Wed May 23 23:16:02 2018
@@ -45,14 +45,29 @@ const char *sparc::getSparcAsmModeForCPU
 .Case("niagara2", "-Av8plusb")
 .Case("niagara3", "-Av8plusd")
 .Case("niagara4", "-Av8plusd")
+.Case("ma2100", "-Aleon")
+.Case("ma2150", "-Aleon")
+.Case("ma2155", "-Aleon")
+.Case("ma2450", "-Aleon")
+.Case("ma2455", "-Aleon")
+.Case("ma2x5x", "-Aleon")
+.Case("ma2080", "-Aleon")
+.Case("ma2085", "-Aleon")
+.Case("ma2480", "-Aleon")
+.Case("ma2485", "-Aleon")
+.Case("ma2x8x", "-Aleon")
+.Case("myriad2", "-Aleon")
+.Case("myriad2.1", "-Aleon")
+.Case("myriad2.2", "-Aleon")
+.Case("myriad2.3", "-Aleon")
 .Case("leon2", "-Av8")
 .Case("at697e", "-Av8")
 .Case("at697f", "-Av8")
-.Case("leon3", "-Av8")
+.Case("leon3", "-Aleon")
 .Case("ut699", "-Av8")
-.Case("gr712rc", "-Av8")
-.Case("leon4", "-Av8")
-.Case("gr740", "-Av8")
+.Case("gr712rc", "-Aleon")
+.Case("leon4", "-Aleon")
+.Case("gr740", "-Aleon")
 .Default("-Av8");
   }
 }

Modified: cfe/trunk/test/Driver/sparc-as.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/sparc-as.c?rev=333157=333156=333157=diff
==
--- cfe/trunk/test/Driver/sparc-as.c (original)
+++ cfe/trunk/test/Driver/sparc-as.c Wed May 23 23:16:02 2018
@@ -76,6 +76,66 @@
 // RUN: -no-integrated-as --sysroot=%S/Inputs/basic_netbsd_tree %s -### 2>&1 \
 // RUN: | FileCheck -check-prefix=SPARC-V8PLUSD %s
 
+// RUN: %clang -mcpu=ma2100 -no-canonical-prefixes -target sparc \
+// RUN: -no-integrated-as --sysroot=%S/Inputs/basic_netbsd_tree %s -### 2>&1 \
+// RUN: | FileCheck -check-prefix=SPARC-LEON %s
+
+// RUN: %clang -mcpu=ma2150 -no-canonical-prefixes -target sparc \
+// RUN: -no-integrated-as --sysroot=%S/Inputs/basic_netbsd_tree %s -### 2>&1 \
+// RUN: | FileCheck -check-prefix=SPARC-LEON %s
+
+// RUN: %clang -mcpu=ma2155 -no-canonical-prefixes -target sparc \
+// RUN: -no-integrated-as --sysroot=%S/Inputs/basic_netbsd_tree %s -### 2>&1 \
+// RUN: | FileCheck -check-prefix=SPARC-LEON %s
+
+// RUN: %clang -mcpu=ma2450 -no-canonical-prefixes -target sparc \
+// RUN: -no-integrated-as --sysroot=%S/Inputs/basic_netbsd_tree %s -### 2>&1 \
+// RUN: | FileCheck -check-prefix=SPARC-LEON %s
+
+// RUN: %clang -mcpu=ma2455 -no-canonical-prefixes -target sparc \
+// RUN: -no-integrated-as --sysroot=%S/Inputs/basic_netbsd_tree %s -### 2>&1 \
+// RUN: | FileCheck -check-prefix=SPARC-LEON %s
+
+// RUN: %clang -mcpu=ma2x5x -no-canonical-prefixes -target sparc \
+// RUN: -no-integrated-as --sysroot=%S/Inputs/basic_netbsd_tree %s -### 2>&1 \
+// RUN: | FileCheck -check-prefix=SPARC-LEON %s
+
+// RUN: %clang -mcpu=ma2080 -no-canonical-prefixes -target sparc \
+// RUN: -no-integrated-as --sysroot=%S/Inputs/basic_netbsd_tree %s -### 2>&1 \
+// RUN: | FileCheck -check-prefix=SPARC-LEON %s
+
+// RUN: %clang -mcpu=ma2085 -no-canonical-prefixes -target sparc \
+// RUN: -no-integrated-as --sysroot=%S/Inputs/basic_netbsd_tree %s -### 2>&1 \
+// RUN: | FileCheck -check-prefix=SPARC-LEON %s
+
+// RUN: %clang -mcpu=ma2480 -no-canonical-prefixes -target sparc \
+// RUN: -no-integrated-as --sysroot=%S/Inputs/basic_netbsd_tree %s -### 2>&1 \
+// RUN: | FileCheck -check-prefix=SPARC-LEON %s
+
+// RUN: %clang -mcpu=ma2485 -no-canonical-prefixes -target sparc \
+// RUN: -no-integrated-as --sysroot=%S/Inputs/basic_netbsd_tree %s -### 2>&1 \
+// RUN: | FileCheck -check-prefix=SPARC-LEON %s
+
+// RUN: %clang -mcpu=ma2x8x -no-canonical-prefixes -target sparc \
+// RUN: -no-integrated-as --sysroot=%S/Inputs/basic_netbsd_tree %s -### 2>&1 \
+// RUN: | FileCheck -check-prefix=SPARC-LEON %s
+
+// RUN: %clang -mcpu=myriad2 -no-canonical-prefixes -target sparc \
+// RUN: -no-integrated-as --sysroot=%S/Inputs/basic_netbsd_tree %s -### 2>&1 \
+// RUN: | FileCheck -check-prefix=SPARC-LEON %s
+
+// RUN: %clang -mcpu=myriad2.1 

[PATCH] D46943: [clangd] Boost scores for decls from current file in completion

[Ilya Biryukov via Phabricator via cfe-commits]

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

- Simplify the change by boosting any Decls that have a redecl in the current 
file


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D46943

Files:
  clangd/Quality.cpp
  clangd/Quality.h
  unittests/clangd/QualityTests.cpp
  unittests/clangd/TestTU.cpp

Index: unittests/clangd/TestTU.cpp
===
--- unittests/clangd/TestTU.cpp
+++ unittests/clangd/TestTU.cpp
@@ -1,5 +1,4 @@
-//===--- TestTU.cpp - Scratch source files for testing *-
-//C++-*-===//
+//===--- TestTU.cpp - Scratch source files for testing ---===//
 //
 // The LLVM Compiler Infrastructure
 //
@@ -78,11 +77,11 @@
 auto *ND = dyn_cast(D);
 if (!ND || ND->getNameAsString() != QName)
   continue;
-if (Result) {
+if (Result && ND->getCanonicalDecl() != Result) {
   ADD_FAILURE() << "Multiple Decls named " << QName;
   assert(false && "QName is not unique");
 }
-Result = ND;
+Result = cast(ND->getCanonicalDecl());
   }
   if (!Result) {
 ADD_FAILURE() << "No Decl named " << QName << " in AST";
Index: unittests/clangd/QualityTests.cpp
===
--- unittests/clangd/QualityTests.cpp
+++ unittests/clangd/QualityTests.cpp
@@ -118,6 +118,45 @@
   EXPECT_LT(sortText(0, "a"), sortText(0, "z"));
 }
 
+TEST(QualityTests, BoostCurrentFileDecls) {
+  TestTU Test;
+  Test.HeaderFilename = "foo.h";
+  Test.HeaderCode = R"cpp(
+int test_func_in_header();
+int test_func_in_header_and_cpp();
+)cpp";
+  Test.Code = R"cpp(
+#include "foo.h"
+int ::test_func_in_header_and_cpp() {
+}
+int test_func_in_cpp();
+  )cpp";
+
+  ParsedAST AST = Test.build();
+
+  SymbolQualitySignals FuncInCpp;
+  FuncInCpp.merge(CodeCompletionResult((AST, "test_func_in_cpp"),
+   CCP_Declaration));
+  /// Decls in the current file should get a proximity score of 1.0.
+  EXPECT_FLOAT_EQ(FuncInCpp.ProximityScore, 1.0);
+
+  SymbolQualitySignals FuncInHeader;
+  FuncInHeader.merge(CodeCompletionResult((AST, "test_func_in_header"),
+  CCP_Declaration));
+  /// Decls outside current file currently don't get a proximity score boost.
+  EXPECT_FLOAT_EQ(FuncInHeader.ProximityScore, 0.0);
+
+  SymbolQualitySignals FuncInHeaderAndCpp;
+  FuncInHeaderAndCpp.merge(CodeCompletionResult(
+  (AST, "test_func_in_header_and_cpp"), CCP_Declaration));
+  /// Decls in both header **and** the main file get the same boost.
+  EXPECT_FLOAT_EQ(FuncInHeaderAndCpp.ProximityScore, 1.0);
+
+  /// Check the boost in proximity translates into a better score.
+  EXPECT_LE(FuncInHeader.evaluate(), FuncInCpp.evaluate());
+  EXPECT_FLOAT_EQ(FuncInHeaderAndCpp.evaluate(), FuncInCpp.evaluate());
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/Quality.h
===
--- clangd/Quality.h
+++ clangd/Quality.h
@@ -49,6 +49,7 @@
//quality and relevance. Untangle this.
   bool Deprecated = false;
   unsigned References = 0;
+  float ProximityScore = 0.0; /// Proximity score, in a [0,1] interval.
 
   void merge(const CodeCompletionResult );
   void merge(const Symbol );
Index: clangd/Quality.cpp
===
--- clangd/Quality.cpp
+++ clangd/Quality.cpp
@@ -8,6 +8,8 @@
 //===-===//
 #include "Quality.h"
 #include "index/Index.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/Basic/SourceManager.h"
 #include "clang/Sema/CodeCompleteConsumer.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/MathExtras.h"
@@ -17,9 +19,27 @@
 namespace clangd {
 using namespace llvm;
 
+namespace {
+bool hasDeclInMainFile(const Decl ) {
+  auto  = D.getASTContext().getSourceManager();
+  for (auto *Redecl : D.redecls()) {
+auto Loc = SourceMgr.getSpellingLoc(Redecl->getLocation());
+if (SourceMgr.isWrittenInMainFile(Loc))
+  return true;
+  }
+  return false;
+}
+} // namespace
+
 void SymbolQualitySignals::merge(const CodeCompletionResult ) {
   SemaCCPriority = SemaCCResult.Priority;
-
+  if (SemaCCResult.Declaration) {
+// We boost things that have decls in the main file.
+// The real proximity scores would be more general when we have them.
+float DeclProximity =
+hasDeclInMainFile(*SemaCCResult.Declaration) ? 1.0 : 0.0;
+ProximityScore = std::max(DeclProximity, ProximityScore);
+  }
   if (SemaCCResult.Availability == CXAvailability_Deprecated)
 Deprecated = true;
 }
@@ -41,6 +61,10 @@
 // Priority 80 is a really bad score.
 Score *= 2 - std::min(80, SemaCCPriority) / 40;
 
+  // Proximity scores are [0,1] and we 

Re: r333082 - Fix duplicate class template definitions problem

[Gábor Márton via cfe-commits]

Thanks a lot!

Gabor

On Thu, May 24, 2018 at 12:54 PM Hans Wennborg  wrote:

> On Wed, May 23, 2018 at 3:53 PM, Gabor Marton via cfe-commits
>  wrote:
> > Author: martong
> > Date: Wed May 23 06:53:36 2018
> > New Revision: 333082
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=333082=rev
> > Log:
> > Fix duplicate class template definitions problem
> >
> > Summary:
> > We fail to import a `ClassTemplateDecl` if the "To" context already
> > contains a definition and then a forward decl.  This is because
> > `localUncachedLookup` does not find the definition.  This is not a
> > lookup error, the parser behaves differently than assumed in the
> > importer code.  A `DeclContext` contains one DenseMap (`LookupPtr`)
> > which maps names to lists.  The list is a special list `StoredDeclsList`
> > which is optimized to have one element.  During building the initial
> > AST, the parser first adds the definition to the `DeclContext`.  Then
> > during parsing the second declaration (the forward decl) the parser
> > again calls `DeclContext::addDecl` but that will not add a new element
> > to the `StoredDeclsList` rarther it simply overwrites the old element
> > with the most recent one.  This patch fixes the error by finding the
> > definition in the redecl chain.  Added tests for the same issue with
> > `CXXRecordDecl` and with `ClassTemplateSpecializationDecl`.  These tests
> > pass and they pass because in `VisitRecordDecl` and in
> > `VisitClassTemplateSpecializationDecl` we already use
> > `D->getDefinition()` after the lookup.
> >
> > Reviewers: a.sidorin, xazax.hun, szepet
> >
> > Subscribers: rnkovacs, dkrupp, cfe-commits
> >
> > Differential Revision: https://reviews.llvm.org/D46950
>
> [..]
>
> > +TEST_P(ASTImporterTestBase,
> > +   ImportDefinitionOfClassIfThereIsAnExistingFwdDeclAndDefinition) {
> > +  Decl *ToTU = getToTuDecl(
> > +  R"(
> > +  struct B {
> > +void f();
> > +  };
> > +
> > +  struct B;
> > +  )",
> > +  Lang_CXX);
> > +  ASSERT_EQ(2u, DeclCounter().match(
> > +ToTU,
> cxxRecordDecl(hasParent(translationUnitDecl();
>
> This doesn't hold when targeting Windows, as Sema will add an implicit
> declaration of type_info, causing the matcher to return 3 instead of
> 2.
>
> I've committed r333170 to fix.
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r333174 - [clangd] Fix code completion in MACROs with stringification.

[Eric Liu via cfe-commits]

Author: ioeric
Date: Thu May 24 04:20:19 2018
New Revision: 333174

URL: http://llvm.org/viewvc/llvm-project?rev=333174=rev
Log:
[clangd] Fix code completion in MACROs with stringification.

Summary:
Currently, we only handle the first callback from sema code completion
and ignore results from potential following callbacks. This causes
causes loss of completion results when multiple contexts are tried by Sema.

For example, we wouldn't get any completion result in the following completion
as the first attemped context is natural language which has no
candidate. The parser would backtrack and tried a completion with AST
semantic, which would find candidate "::x".

```
void f(const char*, int);
#define F(x) f(#x, x)
int x;
void main() {
F(::^);
}
```

To fix this, we only process a sema callback when it gives completion results or
the context supports index-based completion.

Reviewers: ilya-biryukov

Reviewed By: ilya-biryukov

Subscribers: klimek, MaskRay, jkorous, cfe-commits

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

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

Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=333174=333173=333174=diff
==
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Thu May 24 04:20:19 2018
@@ -406,6 +406,50 @@ std::vector getQueryScopes(
   return Info.scopesForIndexQuery();
 }
 
+// Should we perform index-based completion in a context of the specified kind?
+// FIXME: consider allowing completion, but restricting the result types.
+bool contextAllowsIndex(enum CodeCompletionContext::Kind K) {
+  switch (K) {
+  case CodeCompletionContext::CCC_TopLevel:
+  case CodeCompletionContext::CCC_ObjCInterface:
+  case CodeCompletionContext::CCC_ObjCImplementation:
+  case CodeCompletionContext::CCC_ObjCIvarList:
+  case CodeCompletionContext::CCC_ClassStructUnion:
+  case CodeCompletionContext::CCC_Statement:
+  case CodeCompletionContext::CCC_Expression:
+  case CodeCompletionContext::CCC_ObjCMessageReceiver:
+  case CodeCompletionContext::CCC_EnumTag:
+  case CodeCompletionContext::CCC_UnionTag:
+  case CodeCompletionContext::CCC_ClassOrStructTag:
+  case CodeCompletionContext::CCC_ObjCProtocolName:
+  case CodeCompletionContext::CCC_Namespace:
+  case CodeCompletionContext::CCC_Type:
+  case CodeCompletionContext::CCC_Name: // FIXME: why does ns::^ give this?
+  case CodeCompletionContext::CCC_PotentiallyQualifiedName:
+  case CodeCompletionContext::CCC_ParenthesizedExpression:
+  case CodeCompletionContext::CCC_ObjCInterfaceName:
+  case CodeCompletionContext::CCC_ObjCCategoryName:
+return true;
+  case CodeCompletionContext::CCC_Other: // Be conservative.
+  case CodeCompletionContext::CCC_OtherWithMacros:
+  case CodeCompletionContext::CCC_DotMemberAccess:
+  case CodeCompletionContext::CCC_ArrowMemberAccess:
+  case CodeCompletionContext::CCC_ObjCPropertyAccess:
+  case CodeCompletionContext::CCC_MacroName:
+  case CodeCompletionContext::CCC_MacroNameUse:
+  case CodeCompletionContext::CCC_PreprocessorExpression:
+  case CodeCompletionContext::CCC_PreprocessorDirective:
+  case CodeCompletionContext::CCC_NaturalLanguage:
+  case CodeCompletionContext::CCC_SelectorName:
+  case CodeCompletionContext::CCC_TypeQualifiers:
+  case CodeCompletionContext::CCC_ObjCInstanceMessage:
+  case CodeCompletionContext::CCC_ObjCClassMessage:
+  case CodeCompletionContext::CCC_Recovery:
+return false;
+  }
+  llvm_unreachable("unknown code completion context");
+}
+
 // The CompletionRecorder captures Sema code-complete output, including 
context.
 // It filters out ignored results (but doesn't apply fuzzy-filtering yet).
 // It doesn't do scoring or conversion to CompletionItem yet, as we want to
@@ -431,12 +475,17 @@ struct CompletionRecorder : public CodeC
   void ProcessCodeCompleteResults(class Sema , CodeCompletionContext Context,
   CodeCompletionResult *InResults,
   unsigned NumResults) override final {
+// If a callback is called without any sema result and the context does not
+// support index-based completion, we simply skip it to give way to
+// potential future callbacks with results.
+if (NumResults == 0 && !contextAllowsIndex(Context.getKind()))
+  return;
 if (CCSema) {
   log(llvm::formatv(
   "Multiple code complete callbacks (parser backtracked?). "
   "Dropping results from context {0}, keeping results from {1}.",
-  getCompletionKindString(this->CCContext.getKind()),
-  getCompletionKindString(Context.getKind(;
+  getCompletionKindString(Context.getKind()),
+