[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 156210.
Quuxplusone edited the summary of this revision.
Quuxplusone added a comment.

Remove some incorrect `noexcept` from ``.

I noticed this because the calls to `__clang_call_terminate` showed up on 
Godbolt. But the pessimization is still there even without these `noexcept`! 
Basically the flow is this:

  std::pmr::vector::~vector (noexcept(true))
  - polymorphic_allocator::destroy (noexcept(X) but inlineable)
- ~T (noexcept(true))
  - polymorphic_allocator::deallocate (noexcept(X) but inlineable)
- memory_resource::deallocate (noexcept(false) and not inlineable)

where "X" is "true" before this patch and "false" afterward.

Even for `monotonic_buffer_resource`, we can't conclude that deallocation will 
never throw, because deallocation *might* involve the upstream resource, which 
we don't know anything about.


Repository:
  rCXX libc++

https://reviews.llvm.org/D47344

Files:
  include/experimental/memory_resource
  src/experimental/memory_resource.cpp

Index: src/experimental/memory_resource.cpp
===
--- src/experimental/memory_resource.cpp
+++ src/experimental/memory_resource.cpp
@@ -8,6 +8,7 @@
 //===--===//
 
 #include "experimental/memory_resource"
+#include "memory"
 
 #ifndef _LIBCPP_HAS_NO_ATOMIC_HEADER
 #include "atomic"
@@ -23,38 +24,47 @@
 
 // new_delete_resource()
 
+#ifdef _LIBCPP_HAS_NO_ALIGNED_ALLOCATION
+static bool is_aligned_to(void *ptr, size_t align)
+{
+void *p2 = ptr;
+size_t space = 1;
+void *result = _VSTD::align(align, 1, p2, space);
+return (result == ptr);
+}
+#endif
+
 class _LIBCPP_TYPE_VIS __new_delete_memory_resource_imp
 : public memory_resource
 {
-public:
-~__new_delete_memory_resource_imp() = default;
-
-protected:
-virtual void* do_allocate(size_t __size, size_t __align)
-{ return _VSTD::__libcpp_allocate(__size, __align); /* FIXME */}
+void *do_allocate(size_t bytes, size_t align) override
+{
+void *result = _VSTD::__libcpp_allocate(bytes, align);
+#ifdef _LIBCPP_HAS_NO_ALIGNED_ALLOCATION
+if (!is_aligned_to(result, align)) {
+_VSTD::__libcpp_deallocate(result, align);
+__throw_bad_alloc();
+}
+#endif
+return result;
+}
 
-virtual void do_deallocate(void * __p, size_t, size_t __align)
-{ _VSTD::__libcpp_deallocate(__p, __align); /* FIXME */ }
+void do_deallocate(void *p, size_t, size_t align) override
+{ _VSTD::__libcpp_deallocate(p, align); }
 
-virtual bool do_is_equal(memory_resource const & __other) const _NOEXCEPT
-{ return &__other == this; }
+bool do_is_equal(const memory_resource& other) const _NOEXCEPT override
+{ return  == this; }
 };
 
 // null_memory_resource()
 
 class _LIBCPP_TYPE_VIS __null_memory_resource_imp
 : public memory_resource
 {
-public:
-~__null_memory_resource_imp() = default;
-
-protected:
-virtual void* do_allocate(size_t, size_t) {
-__throw_bad_alloc();
-}
-virtual void do_deallocate(void *, size_t, size_t) {}
-virtual bool do_is_equal(memory_resource const & __other) const _NOEXCEPT
-{ return &__other == this; }
+void *do_allocate(size_t, size_t) override { __throw_bad_alloc(); }
+void do_deallocate(void *, size_t, size_t) override {}
+bool do_is_equal(const memory_resource& other) const _NOEXCEPT override
+{ return  == this; }
 };
 
 namespace {
Index: include/experimental/memory_resource
===
--- include/experimental/memory_resource
+++ include/experimental/memory_resource
@@ -195,7 +195,7 @@
 }
 
 _LIBCPP_INLINE_VISIBILITY
-void deallocate(_ValueType * __p, size_t __n) _NOEXCEPT {
+void deallocate(_ValueType * __p, size_t __n) {
 _LIBCPP_ASSERT(__n <= __max_size(),
"deallocate called for size which exceeds max_size()");
 __res_->deallocate(__p, __n * sizeof(_ValueType), alignof(_ValueType));
@@ -265,7 +265,7 @@
 
 template 
 _LIBCPP_INLINE_VISIBILITY
-void destroy(_Tp * __p) _NOEXCEPT
+void destroy(_Tp * __p)
 { __p->~_Tp(); }
 
 _LIBCPP_INLINE_VISIBILITY
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49508: [CodeGen] VisitMaterializeTemporaryExpr(): don't skip NoOp Casts.

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

I think it would be reasonable to set a flag on `ImplicitCastExpr`s that are 
actually semantically part of an explicit cast.  I don't think that would be 
hard to get Sema to do, either by passing a flag down to the code that builds 
those casts or just by retroactively setting that flag on all the ICE 
sub-expressions of an explicit cast when "capping" it with the 
`ExplicitCastExpr`.


Repository:
  rC Clang

https://reviews.llvm.org/D49508



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

[PATCH] D48661: [Fixed Point Arithmetic] Fixed Point Constant

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: include/clang/Basic/FixedPoint.h:64
+
+class APFixedPoint {
+ public:

rjmccall wrote:
> This should get a documentation comment describing the class.  You should 
> mention that, like `APSInt`, it carries all of the semantic information about 
> the value's type (width, signed-ness, saturating-ness, etc.), but by design 
> it does not carry the original C type — among other reasons, so that it can 
> more easily be moved to LLVM should fixed-point types gain native IR support.
Documentation comments start with ///.

Otherwise, these changes look good; LGTM, but of course you should settle 
Bevin's review comments before committing.


Repository:
  rC Clang

https://reviews.llvm.org/D48661



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

[PATCH] D49083: [HIP] Register/unregister device fat binary only once

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Thanks for the comment.




Comment at: lib/CodeGen/CGCUDANV.cpp:444
+auto HandleValue =
+CtorBuilder.CreateAlignedLoad(GpuBinaryHandle, CGM.getPointerAlign());
+llvm::Constant *Zero = 
llvm::Constant::getNullValue(HandleValue->getType());

yaxunl wrote:
> rjmccall wrote:
> > yaxunl wrote:
> > > rjmccall wrote:
> > > > Do you not need to worry about concurrency here?
> > > The ctor functions are executed by the dynamic loader before the program 
> > > gains the control. The dynamic loader cannot excute the ctor functions 
> > > concurrently since doing that would not gurantee thread safety of the 
> > > loaded program. Therefore we can assume sequential execution of ctor 
> > > functions here.
> > Okay.  That's worth a comment.
> > 
> > Is the name here specified by some ABI document, or is it just a 
> > conventional name that we're picking now?
> Will add a comment for that.
> 
> You mean `__hip_gpubin_handle`? It is an implementation detail. It is not 
> defined by ABI or other documentation. Since it is only used internally by 
> ctor functions, it is not a visible elf symbol. Its name is by convention 
> since the cuda corresponding one was named __cuda_gpubin_handle.
Well, it *is* ABI, right?  It's necessary for all translation units to agree to 
use the same symbol here or else the registration will happen multiple times.


https://reviews.llvm.org/D49083



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

[PATCH] D49523: [clangd] Add support for per-file override compilation command

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

malaperle added a comment.

Interesting! We also have a need for passing compilation commands in a context 
where there is no compile_commands.json, but we were thinking of putting this 
in a "didChangeConfiguration" message so that all the commands would be 
available even before files are opened. This would be allow Clangd to have the 
necessary information for background indexing which would include unopened 
files. Subsequent changes to compilation commands would probably go through a 
similar didChangeConfiguration and the appropriate (opened) files would get 
reparsed (not unlike https://reviews.llvm.org/D49267). I'm making a few guesses 
here: I assume that in the context of XCode, you would not do background 
indexing in Clangd but let XCode do it as it can also coordinate (and not 
overlap) with build tasks. Is that correct? In any case, I think the approach 
in the patch is not incompatible with what we had in mind, i.e. we could also 
reuse "overrideCompilationCommandForFile" for each file specified in 
didChangeConfiguration. I'm point this out because if you *do* end up needing 
all the compilation commands beforehand like I mentioned, then maybe we can 
skip the approach of specifying them with didOpen and send them all with 
didChangeConfiguration from start.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D49523



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

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

[Florin Iucha via Phabricator via cfe-commits]

0x8000- marked an inline comment as done.
0x8000- added inline comments.



Comment at: test/clang-tidy/readability-magic-numbers.cpp:16
+void BuggyFunction() {
+  int BadLocalInt = 6;
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: magic number integer literal 6 
[readability-magic-numbers]

0x8000- wrote:
> Quuxplusone wrote:
> > Please add test cases immediately following this one, for
> > 
> > const int BadLocalConstInt = 6;
> > constexpr int BadLocalConstexprInt = 6;
> > static const int BadLocalStaticConstInt = 6;
> > static constexpr int BadLocalStaticConstexprInt = 6;
> > 
> > (except of course changing "Bad" to "Good" in any cases where 6 is 
> > acceptable as an initializer).
> > 
> > Also
> > 
> > std::vector BadLocalVector(6);
> > const std::vector BadLocalConstVector(6);
> > 
> > etc. etc.
> Again... all the "const .* (\d)+" patterns should be acceptable. We're 
> initializing a constant. Would you prefer an explicit option?
I have  template and constructor arguments already in the test. I have tried 
including  but somehow it is not found and the std::vector is reported 
as an error in itself.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D49114



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

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

[Florin Iucha via Phabricator via cfe-commits]

0x8000- added a comment.

Any plans for fix-its that will add the suggested 'const' qualifiers?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45444



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

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

[Florin Iucha via Phabricator via cfe-commits]

0x8000- updated this revision to Diff 156203.
0x8000- added a comment.
Herald added subscribers: kbarton, nemanjai.

Significant refactoring to address review comments - mainly to reduce 
duplication and implement in functional style.

cppcoreguidelines-avoid-magic-numbers is documented as an alias to 
readability-magic-numbers check.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D49114

Files:
  clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/MagicNumbersCheck.cpp
  clang-tidy/readability/MagicNumbersCheck.h
  clang-tidy/readability/ReadabilityTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/cppcoreguidelines-avoid-magic-numbers.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-magic-numbers.rst
  test/clang-tidy/readability-magic-numbers.cpp

Index: test/clang-tidy/readability-magic-numbers.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-magic-numbers.cpp
@@ -0,0 +1,151 @@
+// RUN: %check_clang_tidy %s readability-magic-numbers %t
+
+template 
+struct ValueBucket {
+  T value[V];
+};
+
+int BadGlobalInt = 5;
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: 5 is a magic number; consider replacing it with a named constant [readability-magic-numbers]
+
+int IntSquarer(int param) {
+  return param * param;
+}
+
+void BuggyFunction() {
+  int BadLocalInt = 6;
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 6 is a magic number; consider replacing it with a named constant [readability-magic-numbers]
+
+  (void)IntSquarer(7);
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: 7 is a magic number; consider replacing it with a named constant [readability-magic-numbers]
+
+  int LocalArray[8];
+  // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: 8 is a magic number; consider replacing it with a named constant [readability-magic-numbers]
+
+  for (int ii = 0; ii < 8; ++ii)
+  // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: 8 is a magic number; consider replacing it with a named constant [readability-magic-numbers]
+  {
+LocalArray[ii] = 3 * ii;
+// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: 3 is a magic number; consider replacing it with a named constant [readability-magic-numbers]
+  }
+
+  ValueBucket Bucket;
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: 4 is a magic number; consider replacing it with a named constant [readability-magic-numbers]
+}
+
+class TwoIntContainer {
+public:
+  TwoIntContainer(int val) : anotherMember(val * val), yetAnotherMember(6), anotherConstant(val + val) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:73: warning: 6 is a magic number; consider replacing it with a named constant [readability-magic-numbers]
+
+  int getValue() const;
+
+private:
+  int oneMember = 9;
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: 9 is a magic number; consider replacing it with a named constant [readability-magic-numbers]
+
+  int anotherMember;
+
+  int yetAnotherMember;
+
+  const int oneConstant = 2;
+
+  const int anotherConstant;
+};
+
+int ValueArray[] = {3, 5};
+// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 3 is a magic number; consider replacing it with a named constant [readability-magic-numbers]
+// CHECK-MESSAGES: :[[@LINE-2]]:24: warning: 5 is a magic number; consider replacing it with a named constant [readability-magic-numbers]
+
+float FloatPiVariable = 3.1415926535f;
+// CHECK-MESSAGES: :[[@LINE-1]]:25: warning: 3.141592741 is a magic number; consider replacing it with a named constant [readability-magic-numbers]
+double DoublePiVariable = 6.283185307;
+// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: 6.283185307 is a magic number; consider replacing it with a named constant [readability-magic-numbers]
+
+int getAnswer() {
+  if (ValueArray[0] < ValueArray[1])
+return ValueArray[1];
+
+  return -3; // FILENOTFOUND
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: 3 is a magic number; consider replacing it with a named constant [readability-magic-numbers]
+}
+
+/*
+ * Clean code
+ */
+
+#define INT_MACRO 5
+
+const int GoodGlobalIntConstant = 42;
+
+constexpr int AlsoGoodGlobalIntConstant = 42;
+
+void SolidFunction() {
+  const int GoodLocalIntConstant = 43;
+
+  (void)IntSquarer(GoodLocalIntConstant);
+
+  int LocalArray[INT_MACRO];
+
+  ValueBucket Bucket;
+}
+
+const int ConstValueArray[] = {7, 9};
+
+const int ConstValueArray2D[2][2] = {{7, 9}, {13, 15}};
+
+/*
+ * no warnings for grandfathered values
+ */
+int GrandfatheredValues[] = {0, 1, 2, 10, 100, -1, -10, -100};
+
+/*
+ * no warnings for enums
+ */
+enum Smorgasbord {
+  STARTER,
+  ALPHA = 3,
+  BETA = 1 << 5,
+};
+
+const float FloatPiConstant = 3.1415926535f;
+const double DoublePiConstant = 6.283185307;
+
+double DoubleZeroIsAccepted = 0.0;
+float FloatZeroIsAccepted = 0.0f;
+
+namespace geometry {
+
+template 
+struct Point {
+  T x;
+  T y;
+
+  explicit Point(T xval, T yval) noexcept : x{xval}, y{yval} {
+  }
+};
+

[PATCH] D49518: [VFS] Emit an error when a file isn't located in any directory.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

Need to double check what tests we have when using relative path names at the 
root level. I'd like to make the behavior consistent because a file name is a 
specific case of relative paths. So far there are no assertions and no errors 
but file lookup doesn't seem to be working.


https://reviews.llvm.org/D49518



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

[PATCH] D49502: [CMake] Support statically linking dependencies only to shared or static library

[Petr Hosek via Phabricator via cfe-commits]

phosek added inline comments.



Comment at: libcxx/lib/CMakeLists.txt:269
+AND (TARGET cxxabi_static OR HAVE_LIBCXXABI))
+#if ((TARGET ${LIBCXX_CXX_ABI_LIBRARY}) OR
+#(${LIBCXX_CXX_ABI_LIBRARY} MATCHES "cxxabi(_static|_shared)?" AND 
HAVE_LIBCXXABI))

ldionne wrote:
> I don't understand why any of this needs to change -- can you please explain? 
> Also, you probably didn't mean to leave the commented-out lines.
The reason this change is needed the case when we're linking shared libc++abi 
into shared libc++ in which case `${LIBCXX_CXX_ABI_LIBRARY}` will be set to 
`cxxabi_shared` in `HandleLibCXXABI.cmake` but we cannot merge `libc++abi.so` 
into `libc++.a`, so instead we force the use of `cxxabi_static` here.

Alternatively, we could modify `HandleLibCXXABI.cmake` to set two dependencies, 
one for the static case and one for the shared case and use the former one here.

Removed the commented out lines.


Repository:
  rCXX libc++

https://reviews.llvm.org/D49502



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

[PATCH] D49502: [CMake] Support statically linking dependencies only to shared or static library

[Petr Hosek via Phabricator via cfe-commits]

phosek updated this revision to Diff 156201.
phosek marked 2 inline comments as done.

Repository:
  rCXX libc++

https://reviews.llvm.org/D49502

Files:
  libcxx/CMakeLists.txt
  libcxx/cmake/Modules/HandleLibCXXABI.cmake
  libcxx/lib/CMakeLists.txt
  libcxxabi/CMakeLists.txt
  libcxxabi/src/CMakeLists.txt


Index: libcxxabi/src/CMakeLists.txt
===
--- libcxxabi/src/CMakeLists.txt
+++ libcxxabi/src/CMakeLists.txt
@@ -155,7 +155,7 @@
 # Build the static library.
 if (LIBCXXABI_ENABLE_STATIC)
   set(cxxabi_static_sources $)
-  if (LIBCXXABI_USE_LLVM_UNWINDER AND LIBCXXABI_ENABLE_STATIC_UNWINDER)
+  if (LIBCXXABI_USE_LLVM_UNWINDER AND 
LIBCXXABI_STATICALLY_LINK_UNWINDER_IN_STATIC_LIBRARY)
 if (TARGET unwind_static OR HAVE_LIBUNWIND)
   list(APPEND cxxabi_static_sources $)
 endif()
Index: libcxxabi/CMakeLists.txt
===
--- libcxxabi/CMakeLists.txt
+++ libcxxabi/CMakeLists.txt
@@ -84,6 +84,13 @@
 option(LIBCXXABI_ENABLE_SHARED "Build libc++abi as a shared library." ON)
 option(LIBCXXABI_ENABLE_STATIC "Build libc++abi as a static library." ON)
 
+cmake_dependent_option(LIBCXXABI_STATICALLY_LINK_UNWINDER_IN_STATIC_LIBRARY
+  "Statically link the LLVM unwinder to static library" ON
+  "LIBCXXABI_ENABLE_STATIC_UNWINDER;LIBCXXABI_ENABLE_STATIC" OFF)
+cmake_dependent_option(LIBCXXABI_STATICALLY_LINK_UNWINDER_IN_SHARED_LIBRARY
+  "Statically link the LLVM unwinder to shared library" ON
+  "LIBCXXABI_ENABLE_STATIC_UNWINDER;LIBCXXABI_ENABLE_SHARED" OFF)
+
 option(LIBCXXABI_BAREMETAL "Build libc++abi for baremetal targets." OFF)
 # The default terminate handler attempts to demangle uncaught exceptions, which
 # causes extra I/O and demangling code to be pulled in.
Index: libcxx/lib/CMakeLists.txt
===
--- libcxx/lib/CMakeLists.txt
+++ libcxx/lib/CMakeLists.txt
@@ -44,7 +44,7 @@
   set(LIBCXX_OSX_REEXPORT_SYSTEM_ABI_LIBRARY ON)
 endif()
 
-if (LIBCXX_ENABLE_STATIC_ABI_LIBRARY)
+if (LIBCXX_STATICALLY_LINK_ABI_IN_SHARED_LIBRARY)
   add_library_flags("-Wl,--whole-archive" "-Wl,-Bstatic")
   add_library_flags("${LIBCXX_CXX_ABI_LIBRARY}")
   add_library_flags("-Wl,-Bdynamic" "-Wl,--no-whole-archive")
@@ -259,14 +259,14 @@
 
   list(APPEND LIBCXX_TARGETS "cxx_static")
   # Attempt to merge the libc++.a archive and the ABI library archive into one.
-  if (LIBCXX_ENABLE_STATIC_ABI_LIBRARY)
+  if (LIBCXX_STATICALLY_LINK_ABI_IN_STATIC_LIBRARY)
 set(MERGE_ARCHIVES_SEARCH_PATHS "")
 if (LIBCXX_CXX_ABI_LIBRARY_PATH)
   set(MERGE_ARCHIVES_SEARCH_PATHS "-L${LIBCXX_CXX_ABI_LIBRARY_PATH}")
 endif()
-if ((TARGET ${LIBCXX_CXX_ABI_LIBRARY}) OR
-(${LIBCXX_CXX_ABI_LIBRARY} MATCHES "cxxabi(_static|_shared)?" AND 
HAVE_LIBCXXABI))
-  set(MERGE_ARCHIVES_ABI_TARGET 
"$")
+if (${LIBCXX_CXX_ABI_LIBRARY} MATCHES "cxxabi(_static|_shared)?"
+AND (TARGET cxxabi_static OR HAVE_LIBCXXABI))
+  set(MERGE_ARCHIVES_ABI_TARGET "$")
 else()
   set(MERGE_ARCHIVES_ABI_TARGET
   
"${CMAKE_STATIC_LIBRARY_PREFIX}${LIBCXX_CXX_ABI_LIBRARY}${CMAKE_STATIC_LIBRARY_SUFFIX}")
Index: libcxx/cmake/Modules/HandleLibCXXABI.cmake
===
--- libcxx/cmake/Modules/HandleLibCXXABI.cmake
+++ libcxx/cmake/Modules/HandleLibCXXABI.cmake
@@ -104,10 +104,10 @@
 elseif ("${LIBCXX_CXX_ABI_LIBNAME}" STREQUAL "libcxxabi")
   if (LIBCXX_CXX_ABI_INTREE)
 # Link against just-built "cxxabi" target.
-if (LIBCXX_ENABLE_STATIC_ABI_LIBRARY)
-set(CXXABI_LIBNAME cxxabi_static)
+if (LIBCXX_STATICALLY_LINK_ABI_IN_SHARED_LIBRARY)
+  set(CXXABI_LIBNAME cxxabi_static)
 else()
-set(CXXABI_LIBNAME cxxabi_shared)
+  set(CXXABI_LIBNAME cxxabi_shared)
 endif()
 set(LIBCXX_LIBCPPABI_VERSION "2" PARENT_SCOPE)
   else()
Index: libcxx/CMakeLists.txt
===
--- libcxx/CMakeLists.txt
+++ libcxx/CMakeLists.txt
@@ -155,6 +155,14 @@
 # cannot be used with LIBCXX_ENABLE_ABI_LINKER_SCRIPT.
 option(LIBCXX_ENABLE_STATIC_ABI_LIBRARY "Statically link the ABI library" OFF)
 
+cmake_dependent_option(LIBCXX_STATICALLY_LINK_ABI_IN_STATIC_LIBRARY
+  "Statically link the ABI library to static library" ON
+  "LIBCXX_ENABLE_STATIC_ABI_LIBRARY;LIBCXX_ENABLE_STATIC" OFF)
+
+cmake_dependent_option(LIBCXX_STATICALLY_LINK_ABI_IN_SHARED_LIBRARY
+  "Statically link the ABI library to shared library" ON
+  "LIBCXX_ENABLE_STATIC_ABI_LIBRARY;LIBCXX_ENABLE_STATIC" OFF)
+
 # Generate and install a linker script inplace of libc++.so. The linker script
 # will link libc++ to the correct ABI library. This option is on by default
 # on UNIX platforms other than Apple unless 'LIBCXX_ENABLE_STATIC_ABI_LIBRARY'


Index: libcxxabi/src/CMakeLists.txt
===
--- 

[PATCH] D49526: Updated llvm-proto-fuzzer to execute the compiled code

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added inline comments.



Comment at: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp:209
+
+// Helper function that converts ELF relocatable into raw machine code that
+// can be executed in memory. Returns size of machine code.

Did you look at using LLVM's JIT infrastructure to do this?


Repository:
  rC Clang

https://reviews.llvm.org/D49526



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

[PATCH] D47894: [clang]: Add support for "-fno-delete-null-pointer-checks"

[Manoj Gupta via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC337433: [clang]: Add support for 
-fno-delete-null-pointer-checks (authored by manojgupta, committed 
by ).

Changed prior to commit:
  https://reviews.llvm.org/D47894?vs=155891=156195#toc

Repository:
  rC Clang

https://reviews.llvm.org/D47894

Files:
  docs/ClangCommandLineReference.rst
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.def
  lib/CodeGen/CGCall.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/delete-null-pointer-checks.c
  test/CodeGen/nonnull.c
  test/CodeGen/vla.c
  test/CodeGenCXX/address-space-ref.cpp
  test/CodeGenCXX/constructors.cpp
  test/CodeGenCXX/temporaries.cpp
  test/Driver/clang_f_opts.c
  test/Sema/nonnull.c

Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -746,6 +746,8 @@
 
   Opts.NoUseJumpTables = Args.hasArg(OPT_fno_jump_tables);
 
+  Opts.NullPointerIsValid = Args.hasArg(OPT_fno_delete_null_pointer_checks);
+
   Opts.ProfileSampleAccurate = Args.hasArg(OPT_fprofile_sample_accurate);
 
   Opts.PrepareForLTO = Args.hasArg(OPT_flto, OPT_flto_EQ);
Index: lib/CodeGen/CGCall.cpp
===
--- lib/CodeGen/CGCall.cpp
+++ lib/CodeGen/CGCall.cpp
@@ -1734,6 +1734,8 @@
 FuncAttrs.addAttribute("less-precise-fpmad",
llvm::toStringRef(CodeGenOpts.LessPreciseFPMAD));
 
+if (CodeGenOpts.NullPointerIsValid)
+  FuncAttrs.addAttribute("null-pointer-is-valid", "true");
 if (!CodeGenOpts.FPDenormalMode.empty())
   FuncAttrs.addAttribute("denormal-fp-math", CodeGenOpts.FPDenormalMode);
 
@@ -1867,7 +1869,8 @@
 }
 if (TargetDecl->hasAttr())
   RetAttrs.addAttribute(llvm::Attribute::NoAlias);
-if (TargetDecl->hasAttr())
+if (TargetDecl->hasAttr() &&
+!CodeGenOpts.NullPointerIsValid)
   RetAttrs.addAttribute(llvm::Attribute::NonNull);
 if (TargetDecl->hasAttr())
   FuncAttrs.addAttribute("no_caller_saved_registers");
@@ -1974,7 +1977,8 @@
 if (!PTy->isIncompleteType() && PTy->isConstantSizeType())
   RetAttrs.addDereferenceableAttr(getContext().getTypeSizeInChars(PTy)
 .getQuantity());
-else if (getContext().getTargetAddressSpace(PTy) == 0)
+else if (getContext().getTargetAddressSpace(PTy) == 0 &&
+ !CodeGenOpts.NullPointerIsValid)
   RetAttrs.addAttribute(llvm::Attribute::NonNull);
   }
 
@@ -2083,7 +2087,8 @@
   if (!PTy->isIncompleteType() && PTy->isConstantSizeType())
 Attrs.addDereferenceableAttr(getContext().getTypeSizeInChars(PTy)
.getQuantity());
-  else if (getContext().getTargetAddressSpace(PTy) == 0)
+  else if (getContext().getTargetAddressSpace(PTy) == 0 &&
+   !CodeGenOpts.NullPointerIsValid)
 Attrs.addAttribute(llvm::Attribute::NonNull);
 }
 
@@ -2343,7 +2348,8 @@
 
 if (const ParmVarDecl *PVD = dyn_cast(Arg)) {
   if (getNonNullAttr(CurCodeDecl, PVD, PVD->getType(),
- PVD->getFunctionScopeIndex()))
+ PVD->getFunctionScopeIndex()) &&
+  !CGM.getCodeGenOpts().NullPointerIsValid)
 AI->addAttr(llvm::Attribute::NonNull);
 
   QualType OTy = PVD->getOriginalType();
@@ -2362,7 +2368,8 @@
 Attrs.addDereferenceableAttr(
   getContext().getTypeSizeInChars(ETy).getQuantity()*ArrSize);
 AI->addAttrs(Attrs);
-  } else if (getContext().getTargetAddressSpace(ETy) == 0) {
+  } else if (getContext().getTargetAddressSpace(ETy) == 0 &&
+ !CGM.getCodeGenOpts().NullPointerIsValid) {
 AI->addAttr(llvm::Attribute::NonNull);
   }
 }
@@ -2372,7 +2379,8 @@
 // we can't use the dereferenceable attribute, but in addrspace(0)
 // we know that it must be nonnull.
 if (ArrTy->getSizeModifier() == VariableArrayType::Static &&
-!getContext().getTargetAddressSpace(ArrTy->getElementType()))
+!getContext().getTargetAddressSpace(ArrTy->getElementType()) &&
+!CGM.getCodeGenOpts().NullPointerIsValid)
   AI->addAttr(llvm::Attribute::NonNull);
   }
 
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -3348,6 +3348,10 @@
options::OPT_fno_merge_all_constants, false))
 CmdArgs.push_back("-fmerge-all-constants");
 
+  if (Args.hasFlag(options::OPT_fno_delete_null_pointer_checks,
+   

r337433 - [clang]: Add support for "-fno-delete-null-pointer-checks"

[Manoj Gupta via cfe-commits]

Author: manojgupta
Date: Wed Jul 18 17:44:52 2018
New Revision: 337433

URL: http://llvm.org/viewvc/llvm-project?rev=337433=rev
Log:
[clang]: Add support for "-fno-delete-null-pointer-checks"

Summary:
Support for this option is needed for building Linux kernel.
This is a very frequently requested feature by kernel developers.

More details : https://lkml.org/lkml/2018/4/4/601

GCC option description for -fdelete-null-pointer-checks:
This Assume that programs cannot safely dereference null pointers,
and that no code or data element resides at address zero.

-fno-delete-null-pointer-checks is the inverse of this implying that
null pointer dereferencing is not undefined.

This feature is implemented in as the function attribute
"null-pointer-is-valid"="true".
This CL only adds the attribute on the function.
It also strips "nonnull" attributes from function arguments but
keeps the related warnings unchanged.

Corresponding LLVM change rL336613 already updated the
optimizations to not treat null pointer dereferencing
as undefined if the attribute is present.

Reviewers: t.p.northover, efriedma, jyknight, chandlerc, rnk, srhines, void, 
george.burgess.iv

Reviewed By: jyknight

Subscribers: drinkcat, xbolva00, cfe-commits

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

Added:
cfe/trunk/test/CodeGen/delete-null-pointer-checks.c
Modified:
cfe/trunk/docs/ClangCommandLineReference.rst
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/include/clang/Frontend/CodeGenOptions.def
cfe/trunk/lib/CodeGen/CGCall.cpp
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/test/CodeGen/nonnull.c
cfe/trunk/test/CodeGen/vla.c
cfe/trunk/test/CodeGenCXX/address-space-ref.cpp
cfe/trunk/test/CodeGenCXX/constructors.cpp
cfe/trunk/test/CodeGenCXX/temporaries.cpp
cfe/trunk/test/Driver/clang_f_opts.c
cfe/trunk/test/Sema/nonnull.c

Modified: cfe/trunk/docs/ClangCommandLineReference.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ClangCommandLineReference.rst?rev=337433=337432=337433=diff
==
--- cfe/trunk/docs/ClangCommandLineReference.rst (original)
+++ cfe/trunk/docs/ClangCommandLineReference.rst Wed Jul 18 17:44:52 2018
@@ -1543,6 +1543,14 @@ Specifies the largest alignment guarante
 
 Disable implicit builtin knowledge of a specific function
 
+.. option:: -fdelete-null-pointer-checks, -fno-delete-null-pointer-checks
+
+When enabled, treat null pointer dereference, creation of a reference to null,
+or passing a null pointer to a function parameter annotated with the "nonnull"
+attribute as undefined behavior. (And, thus the optimizer may assume that any
+pointer used in such a way must not have been null and optimize away the
+branches accordingly.) On by default.
+
 .. option:: -fno-elide-type
 
 Do not elide types when printing diagnostics

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=337433=337432=337433=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Wed Jul 18 17:44:52 2018
@@ -1080,6 +1080,13 @@ def frewrite_imports : Flag<["-"], "frew
   Flags<[CC1Option]>;
 def fno_rewrite_imports : Flag<["-"], "fno-rewrite-imports">, Group;
 
+def fdelete_null_pointer_checks : Flag<["-"],
+  "fdelete-null-pointer-checks">, Group,
+  HelpText<"Treat usage of null pointers as undefined behavior.">;
+def fno_delete_null_pointer_checks : Flag<["-"],
+  "fno-delete-null-pointer-checks">, Group, Flags<[CC1Option]>,
+  HelpText<"Do not treat usage of null pointers as undefined behavior.">;
+
 def frewrite_map_file : Separate<["-"], "frewrite-map-file">,
 Group,
 Flags<[ DriverOption, CC1Option ]>;
@@ -2855,8 +2862,6 @@ defm reorder_blocks : BooleanFFlag<"reor
 defm eliminate_unused_debug_types : 
BooleanFFlag<"eliminate-unused-debug-types">, Group;
 defm branch_count_reg : BooleanFFlag<"branch-count-reg">, 
Group;
 defm default_inline : BooleanFFlag<"default-inline">, 
Group;
-defm delete_null_pointer_checks : BooleanFFlag<"delete-null-pointer-checks">,
-Group;
 defm fat_lto_objects : BooleanFFlag<"fat-lto-objects">, 
Group;
 defm float_store : BooleanFFlag<"float-store">, 
Group;
 defm friend_injection : BooleanFFlag<"friend-injection">, 
Group;

Modified: cfe/trunk/include/clang/Frontend/CodeGenOptions.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/CodeGenOptions.def?rev=337433=337432=337433=diff
==
--- cfe/trunk/include/clang/Frontend/CodeGenOptions.def (original)
+++ cfe/trunk/include/clang/Frontend/CodeGenOptions.def Wed Jul 18 17:44:52 2018
@@ -130,6 +130,7 @@ 

[PATCH] D49526: Updated llvm-proto-fuzzer to execute the compiled code

[Emmett Neyman via Phabricator via cfe-commits]

emmettneyman added a comment.

The files

  Object.h
  Object.cpp
  llvm-objcopy.h

are from llvm/tools/llvm-obj-copy with only slight modifications, mostly 
deleting irrelevant parts.


Repository:
  rC Clang

https://reviews.llvm.org/D49526



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

[PATCH] D49526: Updated llvm-proto-fuzzer to execute the compiled code

[Emmett Neyman via Phabricator via cfe-commits]

emmettneyman created this revision.
emmettneyman added reviewers: morehouse, kcc.
Herald added subscribers: cfe-commits, mgorny.
Herald added a reviewer: alexshap.

Made changes to the llvm-proto-fuzzer

- Added loop vectorizer optimization pass in order to have two IR versions
- Updated old fuzz target to handle two different IR versions
- Wrote code to execute both versions in memory


Repository:
  rC Clang

https://reviews.llvm.org/D49526

Files:
  clang/tools/clang-fuzzer/CMakeLists.txt
  clang/tools/clang-fuzzer/ExampleClangLLVMProtoFuzzer.cpp
  clang/tools/clang-fuzzer/handle-llvm/CMakeLists.txt
  clang/tools/clang-fuzzer/handle-llvm/Object.cpp
  clang/tools/clang-fuzzer/handle-llvm/Object.h
  clang/tools/clang-fuzzer/handle-llvm/fuzzer_initialize.cpp
  clang/tools/clang-fuzzer/handle-llvm/fuzzer_initialize.h
  clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp
  clang/tools/clang-fuzzer/handle-llvm/handle_llvm.h
  clang/tools/clang-fuzzer/handle-llvm/llvm-objcopy.h

Index: clang/tools/clang-fuzzer/handle-llvm/llvm-objcopy.h
===
--- /dev/null
+++ clang/tools/clang-fuzzer/handle-llvm/llvm-objcopy.h
@@ -0,0 +1,42 @@
+//===- llvm-objcopy.h ---*- C++ -*-===//
+//
+//  The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//  Modified version of llvm/tools/llvm-objcopy/llvm-objcopy.h
+//
+//===--===//
+
+#ifndef LLVM_TOOLS_OBJCOPY_OBJCOPY_H
+#define LLVM_TOOLS_OBJCOPY_OBJCOPY_H
+
+#include "llvm/ADT/Twine.h"
+#include "llvm/Support/Compiler.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/raw_ostream.h"
+#include 
+
+namespace llvm {
+
+LLVM_ATTRIBUTE_NORETURN extern void error(Twine Message);
+LLVM_ATTRIBUTE_NORETURN extern void reportError(StringRef File, Error E);
+LLVM_ATTRIBUTE_NORETURN extern void reportError(StringRef File,
+std::error_code EC);
+
+// This is taken from llvm-readobj.
+// [see here](llvm/tools/llvm-readobj/llvm-readobj.h:38)
+template  T unwrapOrError(Expected EO) {
+  if (EO)
+return *EO;
+  std::string Buf;
+  raw_string_ostream OS(Buf);
+  logAllUnhandledErrors(EO.takeError(), OS, "");
+  OS.flush();
+  error(Buf);
+}
+
+} // end namespace llvm
+
+#endif // LLVM_TOOLS_OBJCOPY_OBJCOPY_H
Index: clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp
===
--- clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp
+++ clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp
@@ -7,33 +7,56 @@
 //
 //===--===//
 //
-// Implements HandleLLVM for use by the Clang fuzzers. Mimics the llc tool to
-// compile an LLVM IR file to X86_64 assembly.
+// Implements HandleLLVM for use by the Clang fuzzers. First runs an loop
+// vectorizer optimization pass over the given IR code. Then mimics llc on both
+// versions of the IR code to genereate machine code for each version. Inserts
+// both versions of the code into memory and executes both functions on dummy
+// inputs.
 //
 //===--===//
 
 #include "handle_llvm.h"
+#include "Object.h"
 
 #include "llvm/ADT/Triple.h"
 #include "llvm/Analysis/TargetLibraryInfo.h"
+#include "llvm/Analysis/TargetTransformInfo.h"
+#include "llvm/BinaryFormat/ELF.h"
 #include "llvm/CodeGen/CommandFlags.inc"
 #include "llvm/CodeGen/MachineModuleInfo.h"
+#include "llvm/CodeGen/TargetPassConfig.h"
+#include "llvm/IR/IRPrintingPasses.h"
 #include "llvm/IR/LegacyPassManager.h"
+#include "llvm/IR/LegacyPassNameParser.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/Module.h"
 #include "llvm/IR/Verifier.h"
 #include "llvm/IRReader/IRReader.h"
+#include "llvm/Object/Binary.h"
+#include "llvm/Object/ELFObjectFile.h"
+#include "llvm/Object/ELFTypes.h"
+#include "llvm/Pass.h"
 #include "llvm/PassRegistry.h"
-#include "llvm/Support/InitLLVM.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/SourceMgr.h"
 #include "llvm/Support/TargetRegistry.h"
+#include "llvm/Support/TargetSelect.h"
 #include "llvm/Target/TargetMachine.h"
+#include "llvm/Transforms/IPO/PassManagerBuilder.h"
+#include "llvm/Transforms/IPO.h"
 
 #include 
 
 using namespace llvm;
+using namespace object;
+using namespace ELF;
 
+static cl::list
+PassList(cl::desc("Optimizations available:"));
+
+static std::string OptIR;
+
+// Helper function to parse command line args and find the optimization level
 static void getOptLevel(const std::vector ,
   CodeGenOpt::Level ) {
   // Find the optimization level from the command line args
@@ -53,23 +76,243 @@
   }
 }
 
-void clang_fuzzer::HandleLLVM(const std::string ,
-   

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

[Florin Iucha via Phabricator via cfe-commits]

0x8000- added a comment.

I think this requires a separate discussion - do we accept magic values only 
when they are used directly to initialize a constant of the same type? Or do we 
allow them generically when they are used to initialize any constant? Or do we 
only accept several layers of nesting, but not too much?

a) const int Foo = 42;
b) #define FROB 66
c) cont int Bar[] = { 43, 44 };
d) const geometry::Rectangle 
mandelbrotCanvas{geometry::Point{-2.5, -1}, 
geometry::Dimension{3.5, 2}};

Which of these should produce a warning?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D49114



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

[PATCH] D45898: [SemaCXX] Mark destructor as referenced

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

ping


Repository:
  rC Clang

https://reviews.llvm.org/D45898



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

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

[Florin Iucha via Phabricator via cfe-commits]

0x8000- marked 8 inline comments as done.
0x8000- added inline comments.



Comment at: docs/clang-tidy/checks/readability-magic-numbers.rst:33
+
+  return -3; // FILENOTFOUND
+   }

Quuxplusone wrote:
> I notice you changed `-1` to `-3`. Is `return -1` not an example of the "no 
> magic numbers" rule? If not, why not — and can you put something in the 
> documentation about it?
> At least add a unit test proving that `return -1;` is accepted without any 
> diagnostic, if that's the intended behavior.
-1 is parsed as "UnaryOperator" - "IntegerLiteral" 1. I think -1 should be 
acceptable by default.



Comment at: test/clang-tidy/readability-magic-numbers.cpp:16
+void BuggyFunction() {
+  int BadLocalInt = 6;
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: magic number integer literal 6 
[readability-magic-numbers]

Quuxplusone wrote:
> Please add test cases immediately following this one, for
> 
> const int BadLocalConstInt = 6;
> constexpr int BadLocalConstexprInt = 6;
> static const int BadLocalStaticConstInt = 6;
> static constexpr int BadLocalStaticConstexprInt = 6;
> 
> (except of course changing "Bad" to "Good" in any cases where 6 is acceptable 
> as an initializer).
> 
> Also
> 
> std::vector BadLocalVector(6);
> const std::vector BadLocalConstVector(6);
> 
> etc. etc.
Again... all the "const .* (\d)+" patterns should be acceptable. We're 
initializing a constant. Would you prefer an explicit option?



Comment at: test/clang-tidy/readability-magic-numbers.cpp:151
+
+const geometry::Rectangle 
mandelbrotCanvas{geometry::Point{-2.5, -1}, 
geometry::Dimension{3.5, 2}};

Quuxplusone wrote:
> Surely you should expect some diagnostics on this line!
I am using those values to initialize a constant, as described earlier.

We can disable this - but this will be a terribly noisy checker.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D49114



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

[PATCH] D49083: [HIP] Register/unregister device fat binary only once

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 156184.
yaxunl marked 4 inline comments as done.
yaxunl added a comment.

Added comments about thread safety of ctor functions.


https://reviews.llvm.org/D49083

Files:
  lib/CodeGen/CGCUDANV.cpp
  test/CodeGenCUDA/device-stub.cu

Index: test/CodeGenCUDA/device-stub.cu
===
--- test/CodeGenCUDA/device-stub.cu
+++ test/CodeGenCUDA/device-stub.cu
@@ -19,7 +19,7 @@
 // RUN:   | FileCheck -allow-deprecated-dag-overlap %s -check-prefixes=NOGLOBALS,HIPNOGLOBALS
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm %s \
 // RUN: -fcuda-rdc -fcuda-include-gpubinary %t -o - -x hip \
-// RUN:   | FileCheck -allow-deprecated-dag-overlap %s --check-prefixes=ALL,RDC,HIP,HIPRDC
+// RUN:   | FileCheck -allow-deprecated-dag-overlap %s --check-prefixes=ALL,NORDC,HIP
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm %s -o - -x hip\
 // RUN:   | FileCheck -allow-deprecated-dag-overlap %s -check-prefix=NOGPUBIN
 
@@ -79,11 +79,11 @@
 // CUDA-SAME: section ".nvFatBinSegment"
 // HIP-SAME: section ".hipFatBinSegment"
 // * variable to save GPU binary handle after initialization
-// NORDC: @__[[PREFIX]]_gpubin_handle = internal global i8** null
+// CUDANORDC: @__[[PREFIX]]_gpubin_handle = internal global i8** null
+// HIP: @__[[PREFIX]]_gpubin_handle = linkonce global i8** null
 // * constant unnamed string with NVModuleID
 // RDC: [[MODULE_ID_GLOBAL:@.*]] = private constant
 // CUDARDC-SAME: c"[[MODULE_ID:.+]]\00", section "__nv_module_id", align 32
-// HIPRDC-SAME: c"[[MODULE_ID:.+]]\00", section "__hip_module_id", align 32
 // * Make sure our constructor was added to global ctor list.
 // ALL: @llvm.global_ctors = appending global {{.*}}@__[[PREFIX]]_module_ctor
 // * Alias to global symbol containing the NVModuleID.
@@ -120,10 +120,18 @@
 // ALL: define internal void @__[[PREFIX]]_module_ctor
 
 // In separate mode it calls __[[PREFIX]]RegisterFatBinary(&__[[PREFIX]]_fatbin_wrapper)
+// HIP only register fat binary once.
+// HIP: load i8**, i8*** @__hip_gpubin_handle
+// HIP-NEXT: icmp eq i8** {{.*}}, null
+// HIP-NEXT: br i1 {{.*}}, label %if, label %exit
+// HIP: if:
 // NORDC: call{{.*}}[[PREFIX]]RegisterFatBinary{{.*}}__[[PREFIX]]_fatbin_wrapper
 //   .. stores return value in __[[PREFIX]]_gpubin_handle
 // NORDC-NEXT: store{{.*}}__[[PREFIX]]_gpubin_handle
 //   .. and then calls __[[PREFIX]]_register_globals
+// HIP-NEXT: br label %exit
+// HIP: exit:
+// HIP-NEXT: load i8**, i8*** @__hip_gpubin_handle
 // NORDC-NEXT: call void @__[[PREFIX]]_register_globals
 // * In separate mode we also register a destructor.
 // NORDC-NEXT: call i32 @atexit(void (i8*)* @__[[PREFIX]]_module_dtor)
@@ -136,7 +144,14 @@
 // Test that we've created destructor.
 // NORDC: define internal void @__[[PREFIX]]_module_dtor
 // NORDC: load{{.*}}__[[PREFIX]]_gpubin_handle
-// NORDC-NEXT: call void @__[[PREFIX]]UnregisterFatBinary
+// CUDANORDC-NEXT: call void @__[[PREFIX]]UnregisterFatBinary
+// HIP-NEXT: icmp ne i8** {{.*}}, null
+// HIP-NEXT: br i1 {{.*}}, label %if, label %exit
+// HIP: if:
+// HIP-NEXT: call void @__[[PREFIX]]UnregisterFatBinary
+// HIP-NEXT: store i8** null, i8*** @__hip_gpubin_handle
+// HIP-NEXT: br label %exit
+// HIP: exit:
 
 // There should be no __[[PREFIX]]_register_globals if we have no
 // device-side globals, but we still need to register GPU binary.
Index: lib/CodeGen/CGCUDANV.cpp
===
--- lib/CodeGen/CGCUDANV.cpp
+++ lib/CodeGen/CGCUDANV.cpp
@@ -427,22 +427,58 @@
   /*constant*/ true);
   FatbinWrapper->setSection(FatbinSectionName);
 
-  // Register binary with CUDA/HIP runtime. This is substantially different in
-  // default mode vs. separate compilation!
-  if (!RelocatableDeviceCode) {
-// GpuBinaryHandle = __{cuda|hip}RegisterFatBinary();
+  // There is only one HIP fat binary per linked module, however there are
+  // multiple constructor functions. Make sure the fat binary is registered
+  // only once. The constructor functions are executed by the dynamic loader
+  // before the program gains control. The dynamic loader cannot execute the
+  // constructor functions concurrently since doing that would not guarantee
+  // thread safety of the loaded program. Therefore we can assume sequential
+  // execution of constructor functions here.
+  if (IsHIP) {
+llvm::BasicBlock *IfBlock =
+llvm::BasicBlock::Create(Context, "if", ModuleCtorFunc);
+llvm::BasicBlock *ExitBlock =
+llvm::BasicBlock::Create(Context, "exit", ModuleCtorFunc);
+GpuBinaryHandle = new llvm::GlobalVariable(
+TheModule, VoidPtrPtrTy, /*isConstant=*/false,
+llvm::GlobalValue::LinkOnceAnyLinkage,
+/*Initializer=*/llvm::ConstantPointerNull::get(VoidPtrPtrTy),
+"__hip_gpubin_handle");
+auto HandleValue =
+CtorBuilder.CreateAlignedLoad(GpuBinaryHandle, CGM.getPointerAlign());
+

[PATCH] D49523: [clangd] Add support for per-file override compilation command

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

Btw, the "extraFlags" extension is still usable with "compilationCommand".


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D49523



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

[PATCH] D49523: [clangd] Add support for per-file override compilation command

[Alex Lorenz via Phabricator via cfe-commits]

arphaman created this revision.
arphaman added reviewers: jkorous, sammccall, ilya-biryukov.
arphaman added a project: clang-tools-extra.
Herald added subscribers: dexonsmith, MaskRay, ioeric.

This patch builds on top of the "extra flags" extension added in r307241.
It adds the ability to specify user-defined custom compilation command for an 
opened file through the LSP layer. This is a non-standard extension to the 
protocol.

The particular use-case is follows: our clients do not have a compilation 
database and will specify all compilation commands dynamically when a file is 
open.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D49523

Files:
  clangd/ClangdLSPServer.cpp
  clangd/GlobalCompilationDatabase.cpp
  clangd/GlobalCompilationDatabase.h
  clangd/Protocol.cpp
  clangd/Protocol.h
  test/clangd/override-compile-command.test

Index: test/clangd/override-compile-command.test
===
--- /dev/null
+++ test/clangd/override-compile-command.test
@@ -0,0 +1,76 @@
+# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
+---
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///foo.c","languageId":"c","version":1,"text":"int main() { int i; return i; }"},"metadata":{"compilationCommand":["clang", "-c", "foo.c", "-Wall", "-Werror"]}}}
+#  CHECK:  "method": "textDocument/publishDiagnostics",
+# CHECK-NEXT:  "params": {
+# CHECK-NEXT:"diagnostics": [
+# CHECK-NEXT:  {
+# CHECK-NEXT:"message": "variable 'i' is uninitialized when used here",
+# CHECK-NEXT:"range": {
+# CHECK-NEXT:  "end": {
+# CHECK-NEXT:"character": 28,
+# CHECK-NEXT:"line": 0
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "start": {
+# CHECK-NEXT:"character": 27,
+# CHECK-NEXT:"line": 0
+# CHECK-NEXT:  }
+# CHECK-NEXT:},
+# CHECK-NEXT:"severity": 1
+# CHECK-NEXT:  }
+# CHECK-NEXT:],
+# CHECK-NEXT:"uri": "file://{{.*}}/foo.c"
+# CHECK-NEXT:  }
+---
+{"jsonrpc":"2.0","method":"textDocument/didChange","params":{"textDocument":{"uri":"test:///foo.c","version":2},"contentChanges":[{"text":"int main() { int i; return i; }"}]}}
+#  CHECK:  "method": "textDocument/publishDiagnostics",
+# CHECK-NEXT:  "params": {
+# CHECK-NEXT:"diagnostics": [
+# CHECK-NEXT:  {
+# CHECK-NEXT:"message": "variable 'i' is uninitialized when used here",
+# CHECK-NEXT:"range": {
+# CHECK-NEXT:  "end": {
+# CHECK-NEXT:"character": 28,
+# CHECK-NEXT:"line": 0
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "start": {
+# CHECK-NEXT:"character": 27,
+# CHECK-NEXT:"line": 0
+# CHECK-NEXT:  }
+# CHECK-NEXT:},
+# CHECK-NEXT:"severity": 1
+# CHECK-NEXT:  }
+# CHECK-NEXT:],
+# CHECK-NEXT:"uri": "file://{{.*}}/foo.c"
+# CHECK-NEXT:  }
+
+# extraFlags works together with compilationCommand!
+---
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///bar.c","languageId":"c","version":1,"text":"int main() { int i; return i; }"},"metadata":{"compilationCommand":["clang", "-c", "bar.c", "-Wall", "-Werror"], "extraFlags":["-Wno-error=uninitialized"]}}}
+#  CHECK:  "method": "textDocument/publishDiagnostics",
+# CHECK-NEXT:  "params": {
+# CHECK-NEXT:"diagnostics": [
+# CHECK-NEXT:  {
+# CHECK-NEXT:"message": "variable 'i' is uninitialized when used here",
+# CHECK-NEXT:"range": {
+# CHECK-NEXT:  "end": {
+# CHECK-NEXT:"character": 28,
+# CHECK-NEXT:"line": 0
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "start": {
+# CHECK-NEXT:"character": 27,
+# CHECK-NEXT:"line": 0
+# CHECK-NEXT:  }
+# CHECK-NEXT:},
+# CHECK-NEXT:"severity": 2
+# CHECK-NEXT:  }
+# CHECK-NEXT:],
+# CHECK-NEXT:"uri": "file://{{.*}}/bar.c"
+# CHECK-NEXT:  }
+---
+{"jsonrpc":"2.0","id":5,"method":"shutdown"}
+---
+{"jsonrpc":"2.0":"method":"exit"}
+
+
Index: clangd/Protocol.h
===
--- clangd/Protocol.h
+++ clangd/Protocol.h
@@ -160,6 +160,7 @@
 
 struct Metadata {
   std::vector extraFlags;
+  std::vector compilationCommand;
 };
 bool fromJSON(const llvm::json::Value &, Metadata &);
 
Index: clangd/Protocol.cpp
===
--- clangd/Protocol.cpp
+++ clangd/Protocol.cpp
@@ -122,6 +122,7 @@
   if (!O)
 return false;
   O.map("extraFlags", R.extraFlags);
+  O.map("compilationCommand", R.compilationCommand);
   return true;
 }
 
Index: clangd/GlobalCompilationDatabase.h
===
--- 

[PATCH] D49112: [Sema] Implement -Wmemset-transposed-args

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington added inline comments.



Comment at: clang/lib/Sema/SemaChecking.cpp:7984
+  if (isa(ThirdArg) &&
+  cast(ThirdArg)->getValue() == 0) {
+WarningKind = 0;

Quuxplusone wrote:
> > Suppress the diagnostic in cases like `memset(ptr, 0xff, PADDING)`, when 
> > `PADDING` is a macro defined to be `0`.
> 
> Would it suffice to treat a literal `0xFF` in the exact same way you treat a 
> literal `0`? That is,
> ```
> if (SecondArg is integer literal 0 or 255 || ThirdArg involves sizeof) {
> do nothing
> } else if (ThirdArg is integer literal 0 or 255 || SecondArg involves sizeof) 
> {
> diagnose it
> }
> ```
> You should add a test case proving that `memset(x, 0, 0)` doesn't get 
> diagnosed (assuming the two `0`s come from two different macro expansions, 
> for example).
> My sketch above would fail to diagnose `memset(x, 0, 255)`. I don't know if 
> this is a feature or a bug. :)  You probably ought to have a test case for 
> that, either way, just to demonstrate the expected behavior.
I definitely don't think that ThirdArg being 255 should lead to a diagnostic, 
regardless of what SecondArg is. I'm fine with omitting a diagnostic in 
`memset(ptr, 255, 0)`, but if the user explicitly spelled out `0` then I would 
probably prefer to diagnose. FWIW, for the case I found where we emitted a 
false-positive the second argument was 0xd9 or something.


https://reviews.llvm.org/D49112



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

[PATCH] D49112: [Sema] Implement -Wmemset-transposed-args

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington updated this revision to Diff 156178.
erik.pilkington marked 5 inline comments as done.
erik.pilkington added a comment.
This revision is now accepted and ready to land.

In this new patch:

- Add support for __builtin_bzero (-Wsuspicious-bzero), improve diagnostics for 
platforms that define bzero to __builtin_memset
- Suppress the diagnostic when the `0` in memset(ptr, something, 0) came from a 
macro expansion.
- Address review comments

Thanks!


https://reviews.llvm.org/D49112

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/transpose-memset.c

Index: clang/test/Sema/transpose-memset.c
===
--- /dev/null
+++ clang/test/Sema/transpose-memset.c
@@ -0,0 +1,60 @@
+// RUN: %clang_cc1   -Wmemset-transposed-args -verify %s
+// RUN: %clang_cc1 -xc++ -Wmemset-transposed-args -verify %s
+
+#define memset(...) __builtin_memset(__VA_ARGS__)
+#define bzero(x,y) __builtin_memset(x, 0, y)
+#define real_bzero(x,y) __builtin_bzero(x,y)
+
+int array[10];
+int *ptr;
+
+int main() {
+  memset(array, sizeof(array), 0); // expected-warning{{'size' argument to memset is '0'; did you mean to transpose the last two arguments?}} expected-note{{parenthesize the third argument to silence}}
+  memset(array, sizeof(array), 0xff); // expected-warning{{setting buffer to a 'sizeof' expression; did you mean to transpose the last two arguments?}} expected-note{{cast the second argument to 'int' to silence}}
+  memset(ptr, sizeof(ptr), 0); // expected-warning{{'size' argument to memset is '0'; did you mean to transpose the last two arguments?}} expected-note{{parenthesize the third argument to silence}}
+  memset(ptr, sizeof(*ptr) * 10, 1); // expected-warning{{setting buffer to a 'sizeof' expression; did you mean to transpose the last two arguments?}} expected-note{{cast the second argument to 'int' to silence}}
+  memset(ptr, 10 * sizeof(int *), 1); // expected-warning{{setting buffer to a 'sizeof' expression; did you mean to transpose the last two arguments?}} expected-note{{cast the second argument to 'int' to silence}}
+  memset(ptr, 10 * sizeof(int *) + 10, 0xff); // expected-warning{{setting buffer to a 'sizeof' expression; did you mean to transpose the last two arguments?}} expected-note{{cast the second argument to 'int' to silence}}
+  memset(ptr, sizeof(char) * sizeof(int *), 0xff); // expected-warning{{setting buffer to a 'sizeof' expression; did you mean to transpose the last two arguments?}} expected-note{{cast the second argument to 'int' to silence}}
+  memset(array, sizeof(array), sizeof(array)); // Uh... fine I guess.
+  memset(array, 0, sizeof(array));
+  memset(ptr, 0, sizeof(int *) * 10);
+  memset(array, (int)sizeof(array), (0)); // no warning
+  memset(array, (int)sizeof(array), 32); // no warning
+  memset(array, 32, (0)); // no warning
+
+  bzero(ptr, 0); // expected-warning{{'size' argument to bzero is '0'}} expected-note{{parenthesize the second argument to silence}}
+  real_bzero(ptr, 0); // expected-warning{{'size' argument to bzero is '0'}} expected-note{{parenthesize the second argument to silence}}
+}
+
+void macros() {
+#define ZERO 0
+  int array[10];
+  memset(array, 0xff, ZERO); // no warning
+  // Still emit a diagnostic for memsetting a sizeof expression:
+  memset(array, sizeof(array), ZERO); // expected-warning{{'sizeof'}} expected-note{{cast}}
+  bzero(array, ZERO); // no warning
+  real_bzero(array, ZERO); // no warning
+#define NESTED_DONT_DIAG\
+  memset(array, 0xff, ZERO);\
+  real_bzero(array, ZERO);
+
+  NESTED_DONT_DIAG;
+
+#define NESTED_DO_DIAG  \
+  memset(array, 0xff, 0);   \
+  real_bzero(array, 0)
+
+  NESTED_DO_DIAG; // expected-warning{{'size' argument to memset}} expected-warning{{'size' argument to bzero}} expected-note2{{parenthesize}}
+
+#define FN_MACRO(p) \
+  memset(array, 0xff, p)
+
+  FN_MACRO(ZERO);
+  FN_MACRO(0); // FIXME: should we diagnose this?
+
+  __builtin_memset(array, 0, ZERO); // no warning
+  __builtin_bzero(array, ZERO);
+  __builtin_memset(array, 0, 0); // expected-warning{{'size' argument to memset}} // expected-note{{parenthesize}}
+  __builtin_bzero(array, 0); // expected-warning{{'size' argument to bzero}} // expected-note{{parenthesize}}
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -8629,24 +8629,26 @@
   return nullptr;
 }
 
+static const UnaryExprOrTypeTraitExpr *getAsSizeOfExpr(const Expr *E) {
+  if (const auto *Unary = dyn_cast(E))
+if (Unary->getKind() == UETT_SizeOf)
+  return Unary;
+  return nullptr;
+}
+
 /// If E is a sizeof expression, returns its argument expression,
 /// otherwise returns 

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

[Florin Iucha via Phabricator via cfe-commits]

0x8000- marked 17 inline comments as done.
0x8000- added inline comments.



Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:30-32
+  for (const std::string  : IngnoredValuesInput) {
+IngnoredValues.push_back(std::stoll(IgnoredValue));
+  }

aaron.ballman wrote:
> This can be replaced with `llvm::transform(IgnoredValuesInput, 
> IgnoredValues.begin(), std::stoll);`
/home/florin/tools/llvm/tools/clang/tools/extra/clang-tidy/readability/MagicNumbersCheck.cpp:30:3:
 error: no matching function for call to 'transform'

  llvm::transform(IgnoredValuesInput, IgnoredIntegerValues.begin(), std::stoll);
  ^~~
/home/florin/tools/llvm/include/llvm/ADT/STLExtras.h:990:10: note: candidate 
template ignored: couldn't infer template argument 'UnaryPredicate' 
 
OutputIt transform(R &, OutputIt d_first, UnaryPredicate P) {
 ^
1 error generated.

Shall I wrap it in a lambda?



Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:41-42
+void MagicNumbersCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(integerLiteral().bind("ii"), this);
+  Finder->addMatcher(floatLiteral().bind("ff"), this);
+}

aaron.ballman wrote:
> The names `ii` and `ff` could be a bit more user-friendly. Also, this can be 
> written using a single matcher, I think.
> `anyOf(integerLiteral().bind("integer"), floatLiteral().bind("float"))`
addMatcher(anyOf(...), this) is ambiguous. Is there any benefit over the two 
statements?



Comment at: clang-tidy/readability/ReadabilityTidyModule.cpp:69-70
 "readability-inconsistent-declaration-parameter-name");
+CheckFactories.registerCheck(
+"readability-magic-numbers");
 CheckFactories.registerCheck(

aaron.ballman wrote:
> I think this should also be registered as a C++ Core Guideline checker, as I 
> believe it covers at least the integer and floating-point parts of 
> https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Res-magic
I have it as an alias in my branch.  I will check why it was not exported in 
the patch.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D49114



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

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

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

Quuxplusone added inline comments.



Comment at: clang-tidy/readability/MagicNumbersCheck.h:52
+
+  const std::vector IngnoredValuesInput;
+  std::vector IngnoredValues;

`Ignored`. Please spellcheck throughout.



Comment at: docs/clang-tidy/checks/readability-magic-numbers.rst:33
+
+  return -3; // FILENOTFOUND
+   }

I notice you changed `-1` to `-3`. Is `return -1` not an example of the "no 
magic numbers" rule? If not, why not — and can you put something in the 
documentation about it?
At least add a unit test proving that `return -1;` is accepted without any 
diagnostic, if that's the intended behavior.



Comment at: test/clang-tidy/readability-magic-numbers.cpp:16
+void BuggyFunction() {
+  int BadLocalInt = 6;
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: magic number integer literal 6 
[readability-magic-numbers]

Please add test cases immediately following this one, for

const int BadLocalConstInt = 6;
constexpr int BadLocalConstexprInt = 6;
static const int BadLocalStaticConstInt = 6;
static constexpr int BadLocalStaticConstexprInt = 6;

(except of course changing "Bad" to "Good" in any cases where 6 is acceptable 
as an initializer).

Also

std::vector BadLocalVector(6);
const std::vector BadLocalConstVector(6);

etc. etc.



Comment at: test/clang-tidy/readability-magic-numbers.cpp:84
+void SolidFunction() {
+  const int GoodLocalIntContant = 43;
+

`Constant`. Please spellcheck throughout.



Comment at: test/clang-tidy/readability-magic-numbers.cpp:151
+
+const geometry::Rectangle 
mandelbrotCanvas{geometry::Point{-2.5, -1}, 
geometry::Dimension{3.5, 2}};

Surely you should expect some diagnostics on this line!


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D49114



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

[PATCH] D35388: [libc++] Give extern templates default visibility on gcc

[Louis Dionne via Phabricator via cfe-commits]

ldionne added a comment.

In https://reviews.llvm.org/D35388#1167315, @thomasanderson wrote:

> In https://reviews.llvm.org/D35388#1167305, @ldionne wrote:
>
> > ABI-wise, I think this change is OK. Indeed, if `__type_visibility__` is 
> > not supported, we're already marking the base template as 
> > `__visibility__("default")`, so marking the extern template declaration 
> > with `__visibility__("default")` is not a problem. If `__type_visibility__` 
> > is supported, then there's no change in behavior. So I think there is no 
> > change in behavior either way, and I'm actually wondering why 
> > @thomasanderson wants this change in if the behavior is always the same. 
> > Perhaps I am missing something?
>
>
> It's been a while, but IIRC this fixes the Chromium build with gcc and 
> libc++, since std::vector_base_common::__throw_length_error() wasn't 
> getting exported otherwise.


Ah, well of course! That's because I assumed that 
`_LIBCPP_EXTERN_TEMPLATE_TYPE_VIS` was always used on extern template 
declarations where the base template had been declared with 
`_LIBCPP_TEMPLATE_VIS`, which isn't true for `__vector_base_common`.

> See the comment in the Chromium source code:
>  
> https://cs.chromium.org/chromium/src/buildtools/third_party/libc%2B%2B/BUILD.gn?rcl=0dd5c6f980d22be96b728155249df2da355989d9=88
> 
> And also this CL https://reviews.llvm.org/D35326

I would think that both this change and https://reviews.llvm.org/D35326 are 
good.


https://reviews.llvm.org/D35388



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

[PATCH] D49518: [VFS] Emit an error when a file isn't located in any directory.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added inline comments.



Comment at: clang/lib/Basic/VirtualFileSystem.cpp:1416
+if (NameValueNode)
+  error(NameValueNode, "file is not located in any directory");
+return nullptr;

Not happy with the error message but didn't come up with anything better. 
Suggestions are welcome.


https://reviews.llvm.org/D49518



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

r337431 - Documentation: fix a typo in the AST Matcher Reference docs.

[James Dennett via cfe-commits]

Author: jdennett
Date: Wed Jul 18 16:21:31 2018
New Revision: 337431

URL: http://llvm.org/viewvc/llvm-project?rev=337431=rev
Log:
Documentation: fix a typo in the AST Matcher Reference docs.

Modified:
cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h

Modified: cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h?rev=337431=337430=337431=diff
==
--- cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h (original)
+++ cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h Wed Jul 18 16:21:31 2018
@@ -3015,7 +3015,7 @@ AST_MATCHER_P_OVERLOAD(QualType, pointsT
 ///   class A {};
 ///   using B = A;
 /// \endcode
-/// The matcher type(hasUnqualifeidDesugaredType(recordType())) matches
+/// The matcher type(hasUnqualifiedDesugaredType(recordType())) matches
 /// both B and A.
 AST_MATCHER_P(Type, hasUnqualifiedDesugaredType, internal::Matcher,
   InnerMatcher) {


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

r337430 - Reapply r336660: [Modules] Autoload subdirectory modulemaps with specific LangOpts

[Bruno Cardoso Lopes via cfe-commits]

Author: bruno
Date: Wed Jul 18 16:21:19 2018
New Revision: 337430

URL: http://llvm.org/viewvc/llvm-project?rev=337430=rev
Log:
Reapply r336660: [Modules] Autoload subdirectory modulemaps with specific 
LangOpts

Summary:
Reproducer and errors:
https://bugs.llvm.org/show_bug.cgi?id=37878

lookupModule was falling back to loadSubdirectoryModuleMaps when it couldn't
find ModuleName in (proper) search paths. This was causing iteration over all
files in the search path subdirectories for example "/usr/include/foobar" in
bugzilla case.

Users don't expect Clang to load modulemaps in subdirectories implicitly, and
also the disk access is not cheap.

if (AllowExtraModuleMapSearch) true with ObjC with @import ModuleName.

Reviewers: rsmith, aprantl, bruno

Subscribers: cfe-commits, teemperor, v.g.vassilev

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

Added:
cfe/trunk/test/Modules/Inputs/autoload-subdirectory/a.h
cfe/trunk/test/Modules/Inputs/autoload-subdirectory/b.h
cfe/trunk/test/Modules/Inputs/autoload-subdirectory/c.h
cfe/trunk/test/Modules/Inputs/autoload-subdirectory/include/module.modulemap
cfe/trunk/test/Modules/Inputs/autoload-subdirectory/module.modulemap
cfe/trunk/test/Modules/autoload-subdirectory.cpp
Modified:
cfe/trunk/include/clang/Lex/HeaderSearch.h
cfe/trunk/lib/Frontend/CompilerInstance.cpp
cfe/trunk/lib/Lex/HeaderSearch.cpp
cfe/trunk/lib/Serialization/ASTReader.cpp

Modified: cfe/trunk/include/clang/Lex/HeaderSearch.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/HeaderSearch.h?rev=337430=337429=337430=diff
==
--- cfe/trunk/include/clang/Lex/HeaderSearch.h (original)
+++ cfe/trunk/include/clang/Lex/HeaderSearch.h Wed Jul 18 16:21:19 2018
@@ -529,8 +529,12 @@ public:
   /// search directories to produce a module definition. If not, this lookup
   /// will only return an already-known module.
   ///
+  /// \param AllowExtraModuleMapSearch Whether we allow to search modulemaps
+  /// in subdirectories.
+  ///
   /// \returns The module with the given name.
-  Module *lookupModule(StringRef ModuleName, bool AllowSearch = true);
+  Module *lookupModule(StringRef ModuleName, bool AllowSearch = true,
+   bool AllowExtraModuleMapSearch = false);
 
   /// Try to find a module map file in the given directory, returning
   /// \c nullptr if none is found.
@@ -595,8 +599,12 @@ private:
   /// but for compatibility with some buggy frameworks, additional attempts
   /// may be made to find the module under a related-but-different search-name.
   ///
+  /// \param AllowExtraModuleMapSearch Whether we allow to search modulemaps
+  /// in subdirectories.
+  ///
   /// \returns The module named ModuleName.
-  Module *lookupModule(StringRef ModuleName, StringRef SearchName);
+  Module *lookupModule(StringRef ModuleName, StringRef SearchName,
+   bool AllowExtraModuleMapSearch = false);
 
   /// Retrieve a module with the given name, which may be part of the
   /// given framework.

Modified: cfe/trunk/lib/Frontend/CompilerInstance.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInstance.cpp?rev=337430=337429=337430=diff
==
--- cfe/trunk/lib/Frontend/CompilerInstance.cpp (original)
+++ cfe/trunk/lib/Frontend/CompilerInstance.cpp Wed Jul 18 16:21:19 2018
@@ -1653,8 +1653,10 @@ CompilerInstance::loadModule(SourceLocat
 // Retrieve the cached top-level module.
 Module = Known->second;
   } else if (ModuleName == getLangOpts().CurrentModule) {
-// This is the module we're building. 
-Module = PP->getHeaderSearchInfo().lookupModule(ModuleName);
+// This is the module we're building.
+Module = PP->getHeaderSearchInfo().lookupModule(
+ModuleName, /*AllowSearch*/ true,
+/*AllowExtraModuleMapSearch*/ !IsInclusionDirective);
 /// FIXME: perhaps we should (a) look for a module using the module name
 //  to file map (PrebuiltModuleFiles) and (b) diagnose if still not found?
 //if (Module == nullptr) {
@@ -1666,7 +1668,8 @@ CompilerInstance::loadModule(SourceLocat
 Known = KnownModules.insert(std::make_pair(Path[0].first, Module)).first;
   } else {
 // Search for a module with the given name.
-Module = PP->getHeaderSearchInfo().lookupModule(ModuleName);
+Module = PP->getHeaderSearchInfo().lookupModule(ModuleName, true,
+!IsInclusionDirective);
 HeaderSearchOptions  =
 PP->getHeaderSearchInfo().getHeaderSearchOpts();
 
@@ -1743,7 +1746,8 @@ CompilerInstance::loadModule(SourceLocat
ImportLoc, ARRFlags)) {
 case ASTReader::Success: {
   if (Source != ModuleCache && !Module) {
-Module = PP->getHeaderSearchInfo().lookupModule(ModuleName);
+Module = 

[PATCH] D49518: [VFS] Emit an error when a file isn't located in any directory.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai created this revision.
vsapsai added reviewers: bruno, benlangmuir.
Herald added a subscriber: dexonsmith.

Orphaned files prevent us from building a file system tree and cause the
assertion

> Assertion failed: (NewParentE && "Parent entry must exist"), function 
> uniqueOverlayTree, file clang/lib/Basic/VirtualFileSystem.cpp, line 1303.

rdar://problem/28990865


https://reviews.llvm.org/D49518

Files:
  clang/lib/Basic/VirtualFileSystem.cpp
  clang/test/VFS/Inputs/invalid-file.yaml
  clang/test/VFS/parse-errors.c


Index: clang/test/VFS/parse-errors.c
===
--- clang/test/VFS/parse-errors.c
+++ clang/test/VFS/parse-errors.c
@@ -12,3 +12,7 @@
 // RUN: not %clang_cc1 -ivfsoverlay %S/Inputs/unknown-value.yaml -fsyntax-only 
%s 2>&1 | FileCheck -check-prefix=CHECK-UNKNOWN-VALUE %s
 // CHECK-UNKNOWN-VALUE: expected boolean value
 // CHECK-UNKNOWN-VALUE: invalid virtual filesystem overlay file
+
+// RUN: not %clang_cc1 -ivfsoverlay %S/Inputs/invalid-file.yaml -fsyntax-only 
%s 2>&1 | FileCheck -check-prefix=CHECK-INVALID-FILE %s
+// CHECK-INVALID-FILE: file is not located in any directory
+// CHECK-INVALID-FILE: invalid virtual filesystem overlay file
Index: clang/test/VFS/Inputs/invalid-file.yaml
===
--- /dev/null
+++ clang/test/VFS/Inputs/invalid-file.yaml
@@ -0,0 +1,8 @@
+{
+  'version': 0,
+  'roots': [
+{ 'name': 'file-not-in-directory.h', 'type': 'file',
+  'external-contents': 'foo'
+}
+  ]
+}
Index: clang/lib/Basic/VirtualFileSystem.cpp
===
--- clang/lib/Basic/VirtualFileSystem.cpp
+++ clang/lib/Basic/VirtualFileSystem.cpp
@@ -1244,7 +1244,8 @@
 }
   }
 
-  std::unique_ptr parseEntry(yaml::Node *N, RedirectingFileSystem *FS) {
+  std::unique_ptr parseEntry(yaml::Node *N, RedirectingFileSystem *FS,
+bool IsRootEntry) {
 auto *M = dyn_cast(N);
 if (!M) {
   error(N, "expected mapping node for file or directory entry");
@@ -1265,6 +1266,7 @@
 std::vector> EntryArrayContents;
 std::string ExternalContentsPath;
 std::string Name;
+yaml::Node *NameValueNode;
 auto UseExternalName = RedirectingFileEntry::NK_NotSet;
 EntryKind Kind;
 
@@ -1284,6 +1286,7 @@
 if (!parseScalarString(I.getValue(), Value, Buffer))
   return nullptr;
 
+NameValueNode = I.getValue();
 if (FS->UseCanonicalizedPaths) {
   SmallString<256> Path(Value);
   // Guarantee that old YAML files containing paths with ".." and "."
@@ -1320,7 +1323,8 @@
 }
 
 for (auto  : *Contents) {
-  if (std::unique_ptr E = parseEntry(, FS))
+  if (std::unique_ptr E =
+  parseEntry(, FS, /*IsRootEntry*/ false))
 EntryArrayContents.push_back(std::move(E));
   else
 return nullptr;
@@ -1405,8 +1409,15 @@
 }
 
 StringRef Parent = sys::path::parent_path(Trimmed);
-if (Parent.empty())
+if (Parent.empty()) {
+  // File entry is at the root level, outside of any directory.
+  if (IsRootEntry && Kind == EK_File) {
+if (NameValueNode)
+  error(NameValueNode, "file is not located in any directory");
+return nullptr;
+  }
   return Result;
+}
 
 // if 'name' contains multiple components, create implicit directory 
entries
 for (sys::path::reverse_iterator I = sys::path::rbegin(Parent),
@@ -1463,7 +1474,8 @@
 }
 
 for (auto  : *Roots) {
-  if (std::unique_ptr E = parseEntry(, FS))
+  if (std::unique_ptr E =
+  parseEntry(, FS, /*IsRootEntry*/ true))
 RootEntries.push_back(std::move(E));
   else
 return false;


Index: clang/test/VFS/parse-errors.c
===
--- clang/test/VFS/parse-errors.c
+++ clang/test/VFS/parse-errors.c
@@ -12,3 +12,7 @@
 // RUN: not %clang_cc1 -ivfsoverlay %S/Inputs/unknown-value.yaml -fsyntax-only %s 2>&1 | FileCheck -check-prefix=CHECK-UNKNOWN-VALUE %s
 // CHECK-UNKNOWN-VALUE: expected boolean value
 // CHECK-UNKNOWN-VALUE: invalid virtual filesystem overlay file
+
+// RUN: not %clang_cc1 -ivfsoverlay %S/Inputs/invalid-file.yaml -fsyntax-only %s 2>&1 | FileCheck -check-prefix=CHECK-INVALID-FILE %s
+// CHECK-INVALID-FILE: file is not located in any directory
+// CHECK-INVALID-FILE: invalid virtual filesystem overlay file
Index: clang/test/VFS/Inputs/invalid-file.yaml
===
--- /dev/null
+++ clang/test/VFS/Inputs/invalid-file.yaml
@@ -0,0 +1,8 @@
+{
+  'version': 0,
+  'roots': [
+{ 'name': 'file-not-in-directory.h', 'type': 'file',
+  'external-contents': 'foo'
+}
+  ]
+}
Index: clang/lib/Basic/VirtualFileSystem.cpp

[PATCH] D48786: [Preprocessor] Stop entering included files after hitting a fatal error.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added inline comments.



Comment at: clang/lib/Lex/PPDirectives.cpp:1875
+  // Stop further preprocessing if a fatal error has occurred. Any diagnostics
+  // we might have raised will not be visible.
+  if (ShouldEnter && Diags->hasFatalErrorOccurred())

jkorous wrote:
> I am not sure I understand this - does that mean that we are not displaying 
> diagnostics that were produced before "now"?
I can see how the wording can cause the confusion. But I'm not entirely sure it 
is misguiding, I've copy-pasted it from 
[Sema::InstantiatingTemplate::InstantiatingTemplate 
](https://github.com/llvm-mirror/clang/blob/580f7daabc7696d50ad09d9643b2afeadbd387d8/lib/Sema/SemaTemplateInstantiate.cpp#L218-L220).
 Let me explain my reasoning in a different way to see if it makes sense. 
Entering a file is observable if it produces diagnostics or some other build 
artifact (object file in most cases). So when we encounter a fatal error, there 
is no visible indication of entering subsequent files. That's why we can skip 
entering those files: no difference in output and less work to do.


https://reviews.llvm.org/D48786



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

[PATCH] D48559: [clangd] refactoring for XPC transport layer [NFCI]

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

In https://reviews.llvm.org/D48559#1165511, @sammccall wrote:

> In https://reviews.llvm.org/D48559#1165172, @jkorous wrote:
>
> > Hi Sam, thanks for your feedback!
> >
> > In general I agree that explicitly factoring out the transport layer and 
> > improving layering would be a good thing. Unfortunately it's highly 
> > probable that we'd like to drop JSON completely from XPC dispatch (XPC -> 
> > JSON -> ProtocolCallbacks -> JSON -> XPC) in not too distant future. 
> > (Please don't take this as a promise as our priorities might change but 
> > it's probable enough that I don't recommend to base any common abstraction 
> > on JSON.) I originally tried to create similar abstraction but ultimately 
> > gave up as it was way too complex. Not saying it's impossible or that I 
> > would not like to though!
>
>
> That makes sense and is good to know. I definitely don't want to add a 
> transport abstraction that isn't a good fit for your long-term plans.
>
> I think we need to clearly see what the goal is to get the design right 
> though - it doesn't make sense to refactor in order to share code 
> temporarily. Let's take code completion as an example, as it's one of the 
> ones we've fleshed out most for our internal (non-LSP) clients.
>
> - if the goal is to mirror JSON-RPC with a different encoding (current 
> patches), and you're just concerned about the *efficiency* of 
> `CodeCompletion` -> `CompletionItem` -> `json::Value` -> `xpc_object_t`, this 
> seems like premature optimization to me, but let's discuss!
> - if the goal is to basically follow the LSP (same textDocument/completion 
> for), using the `CompletionItem` structs in `Protocol.h` but exposing 
> more/fewer fields, renaming them etc, then we should probably have the 
> feature code express replies as protocol structs, and make the transport 
> define a way to transform specific objects (CompletionItem) into generic ones 
> whose type (json::Value or xpc_object_t) are transport-specific
> - if the goal is to maintain a stable protocol that can be evolved 
> independently of LSP (different methods or semantics) then this isn't really 
> a transport, it's a new protocol. I think the best option is probably to skip 
> `ClangdLSPServer` and have the xpc service wrap `ClangdServer` instead, with 
> a separate main entrypoint. The litmus test here: as people make changes to 
> clangd that affect the LSP surface, do you want the XPC service's interface 
> to change too? This is the hardest model to support as the C++ API 
> (ClangdServer) is not a stable one, but it's what we do for the other non-LSP 
> clients.
>
>   Does one of these seem close to what you want? Any thoughts on the 
> conclusions?


Hi Sam,
Thanks for your feedback!

Generally right now will be mirroring JSON-RPC with a different encoding, but 
in the future we will send some optimized payloads for a subset of replies 
which are constructed out of things like `CompletionItem` directly (so 
bypassing the JSON step).

I believe that your patch (https://reviews.llvm.org/D49389) is implementing the 
JSON-RPC with a choice of encoding, which will work for us right now, and it 
doesn't look like we will have problems optimizing our payloads later when we 
choose to do so.
However, the second option that you proposed does sound quite compelling as 
well (i.e. `then we should probably have the feature code express replies as 
protocol structs`), as it would give us more flexibility. However, I think it's 
orthogonal to the first option (JSON-RPC with a choice of encoding), so maybe 
we can leave it as something that will be on the table if we need it later? I 
think for the transport layer idea you proposed in 
https://reviews.llvm.org/D49389 makes sense and will work for us, and in the 
future we always can implement the second option that your proposed on top of 
of your patch, or use a simpler way to optimize our payloads without much 
disruption.

Thanks,
Alex


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D48559



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

[PATCH] D49303: [CodeGen][ObjC] Treat non-escaping blocks as global blocks to make copy/dispose a no-op

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak planned changes to this revision.
ahatanak added a comment.

In https://reviews.llvm.org/D49303#1165902, @rjmccall wrote:

> Please test that we still copy captures correctly into the block object and 
> destroy them when the block object is destroyed.


There is a bug in my patch where the capture cleanups aren't pushed when 
doesNotEscape() returns true. I'm working on a fix.


Repository:
  rC Clang

https://reviews.llvm.org/D49303



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

[PATCH] D48661: [Fixed Point Arithmetic] Fixed Point Constant

[Leonard Chan via Phabricator via cfe-commits]

leonardchan updated this revision to Diff 156174.
leonardchan marked 3 inline comments as done.

Repository:
  rC Clang

https://reviews.llvm.org/D48661

Files:
  include/clang/AST/ASTContext.h
  include/clang/Basic/FixedPoint.h
  include/clang/Basic/TargetInfo.h
  lib/AST/ASTContext.cpp
  lib/Basic/CMakeLists.txt
  lib/Basic/FixedPoint.cpp
  lib/Sema/SemaExpr.cpp
  test/Frontend/fixed_point_declarations.c
  unittests/Basic/CMakeLists.txt
  unittests/Basic/FixedPointTest.cpp

Index: unittests/Basic/FixedPointTest.cpp
===
--- /dev/null
+++ unittests/Basic/FixedPointTest.cpp
@@ -0,0 +1,683 @@
+//===- unittests/Basic/FixedPointTest.cpp -- fixed point number tests -===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "clang/Basic/FixedPoint.h"
+#include "llvm/ADT/APSInt.h"
+#include "gtest/gtest.h"
+
+using clang::APFixedPoint;
+using clang::FixedPointSemantics;
+using llvm::APInt;
+using llvm::APSInt;
+
+namespace {
+
+FixedPointSemantics Saturated(FixedPointSemantics Sema) {
+  Sema.setSaturated(true);
+  return Sema;
+}
+
+FixedPointSemantics getSAccumSema() {
+  return FixedPointSemantics(/*width=*/16, /*scale=*/7, /*isSigned=*/true,
+ /*isSaturated=*/false,
+ /*hasUnsignedPadding=*/false);
+}
+
+FixedPointSemantics getAccumSema() {
+  return FixedPointSemantics(/*width=*/32, /*scale=*/15, /*isSigned=*/true,
+ /*isSaturated=*/false,
+ /*hasUnsignedPadding=*/false);
+}
+
+FixedPointSemantics getLAccumSema() {
+  return FixedPointSemantics(/*width=*/64, /*scale=*/31, /*isSigned=*/true,
+ /*isSaturated=*/false,
+ /*hasUnsignedPadding=*/false);
+}
+
+FixedPointSemantics getSFractSema() {
+  return FixedPointSemantics(/*width=*/8, /*scale=*/7, /*isSigned=*/true,
+ /*isSaturated=*/false,
+ /*hasUnsignedPadding=*/false);
+}
+
+FixedPointSemantics getFractSema() {
+  return FixedPointSemantics(/*width=*/16, /*scale=*/15, /*isSigned=*/true,
+ /*isSaturated=*/false,
+ /*hasUnsignedPadding=*/false);
+}
+
+FixedPointSemantics getLFractSema() {
+  return FixedPointSemantics(/*width=*/32, /*scale=*/31, /*isSigned=*/true,
+ /*isSaturated=*/false,
+ /*hasUnsignedPadding=*/false);
+}
+
+FixedPointSemantics getUSAccumSema() {
+  return FixedPointSemantics(/*width=*/16, /*scale=*/8, /*isSigned=*/false,
+ /*isSaturated=*/false,
+ /*hasUnsignedPadding=*/false);
+}
+
+FixedPointSemantics getUAccumSema() {
+  return FixedPointSemantics(/*width=*/32, /*scale=*/16, /*isSigned=*/false,
+ /*isSaturated=*/false,
+ /*hasUnsignedPadding=*/false);
+}
+
+FixedPointSemantics getULAccumSema() {
+  return FixedPointSemantics(/*width=*/64, /*scale=*/32, /*isSigned=*/false,
+ /*isSaturated=*/false,
+ /*hasUnsignedPadding=*/false);
+}
+
+FixedPointSemantics getUSFractSema() {
+  return FixedPointSemantics(/*width=*/8, /*scale=*/8, /*isSigned=*/false,
+ /*isSaturated=*/false,
+ /*hasUnsignedPadding=*/false);
+}
+
+FixedPointSemantics getUFractSema() {
+  return FixedPointSemantics(/*width=*/16, /*scale=*/16, /*isSigned=*/false,
+ /*isSaturated=*/false,
+ /*hasUnsignedPadding=*/false);
+}
+
+FixedPointSemantics getULFractSema() {
+  return FixedPointSemantics(/*width=*/32, /*scale=*/32, /*isSigned=*/false,
+ /*isSaturated=*/false,
+ /*hasUnsignedPadding=*/false);
+}
+
+FixedPointSemantics getPadUSAccumSema() {
+  return FixedPointSemantics(/*width=*/16, /*scale=*/7, /*isSigned=*/false,
+ /*isSaturated=*/false,
+ /*hasUnsignedPadding=*/true);
+}
+
+FixedPointSemantics getPadUAccumSema() {
+  return FixedPointSemantics(/*width=*/32, /*scale=*/15, /*isSigned=*/false,
+ /*isSaturated=*/false,
+ /*hasUnsignedPadding=*/true);
+}
+
+FixedPointSemantics getPadULAccumSema() {
+  return FixedPointSemantics(/*width=*/64, /*scale=*/31, /*isSigned=*/false,
+ /*isSaturated=*/false,
+ /*hasUnsignedPadding=*/true);
+}
+
+FixedPointSemantics getPadUSFractSema() {
+  return FixedPointSemantics(/*width=*/8, /*scale=*/7, /*isSigned=*/false,
+ 

[PATCH] D35388: [libc++] Give extern templates default visibility on gcc

[Tom Anderson via Phabricator via cfe-commits]

thomasanderson added a comment.

In https://reviews.llvm.org/D35388#1167305, @ldionne wrote:

> ABI-wise, I think this change is OK. Indeed, if `__type_visibility__` is not 
> supported, we're already marking the base template as 
> `__visibility__("default")`, so marking the extern template declaration with 
> `__visibility__("default")` is not a problem. If `__type_visibility__` is 
> supported, then there's no change in behavior. So I think there is no change 
> in behavior either way, and I'm actually wondering why @thomasanderson wants 
> this change in if the behavior is always the same. Perhaps I am missing 
> something?


It's been a while, but IIRC this fixes the Chromium build with gcc and libc++, 
since std::vector_base_common::__throw_length_error() wasn't getting 
exported otherwise.

See the comment in the Chromium source code:
https://cs.chromium.org/chromium/src/buildtools/third_party/libc%2B%2B/BUILD.gn?rcl=0dd5c6f980d22be96b728155249df2da355989d9=88

And also this CL https://reviews.llvm.org/D35326

> As a diversion, I'm confused by the very fact that we need to apply 
> `_LIBCPP_EXTERN_TEMPLATE_TYPE_VIS` to the _extern template declaration_. I 
> would have expected instead that, if anything, we need to specify the 
> visibility on the _explicit instantiation_ in the dylib.




https://reviews.llvm.org/D35388



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

[PATCH] D41938: [Analyzer] SValBuilder Comparison Rearrangement (with Restrictions and Analyzer Option)

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.
Herald added a subscriber: mikhail.ramalho.

There are still performance regressions coming in, and this time it doesn't 
look like it's my fault: https://bugs.llvm.org/show_bug.cgi?id=38208

I suspect that this might be because we aren't enforcing complexity thresholds 
over all the symbols we're constructing, but that's not certain, we need to get 
to the bottom of it some day.

I suggest reverting the patch or putting it behind an off-by-default flag until 
we debug these cases.


Repository:
  rL LLVM

https://reviews.llvm.org/D41938



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

[PATCH] D49511: [Sema/Attribute] Check for noderef attribute

[Leonard Chan via Phabricator via cfe-commits]

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

- Added checks for expressions in statements other than declarations or 
expression statements


Repository:
  rC Clang

https://reviews.llvm.org/D49511

Files:
  include/clang/AST/Type.h
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Sema.h
  lib/AST/Type.cpp
  lib/AST/TypePrinter.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaStmt.cpp
  lib/Sema/SemaType.cpp
  test/Frontend/noderef.c

Index: test/Frontend/noderef.c
===
--- /dev/null
+++ test/Frontend/noderef.c
@@ -0,0 +1,161 @@
+// RUN: %clang_cc1 -x c -verify %s
+// RUN: %clang_cc1 -x c++ -verify %s
+
+#define NODEREF __attribute__((noderef))
+
+struct S {
+  int a;
+  int b;
+};
+
+struct S2 {
+  int a[2];
+  int NODEREF a2[2];
+  int *b;
+  int NODEREF *b2;
+  struct S *s;
+  struct S NODEREF *s2;
+};
+
+int NODEREF *func(int NODEREF *arg) {
+  int y = *arg; // expected-warning{{dereference of noderef expression}}
+  return arg;
+}
+
+void func2(int x) {}
+
+int test() {
+  int NODEREF *p;
+  int *p2;
+
+  int x = *p;   // expected-warning{{dereference of noderef expression}}
+  x = *((int NODEREF *)p2); // expected-warning{{dereference of noderef expression}}
+
+  int NODEREF **q;
+  int *NODEREF *q2;
+
+  // Indirection
+  p2 = *q;  // ok
+  x = **q;  // expected-warning{{dereference of noderef expression}}
+  p2 = *q2; // expected-warning{{dereference of noderef expression}}
+
+  **q; // expected-warning{{dereference of noderef expression}}
+   // expected-warning@-1{{expression result unused}}
+
+  p = *&*q;
+  p = **
+  q = &**
+  p = &*p;
+  p = *
+  p = &(*p);
+  p = *();
+  x = ** // expected-warning{{dereference of noderef expression}}
+
+  *p = 2;   // expected-warning{{dereference of noderef expression}}
+  *q = p;   // ok
+  **q = 2;  // expected-warning{{dereference of noderef expression}}
+  *q2 = p2; // expected-warning{{dereference of noderef expression}}
+
+  p2 = p;
+  p = p + 1;
+  p = &*(p + 1);
+
+  // Struct member access
+  struct S NODEREF *s;
+  x = s->a;   // expected-warning{{dereference of noderef expression}}
+  x = (*s).b; // expected-warning{{dereference of noderef expression}}
+  p = >a;
+  p = &(*s).b;
+
+  // Nested struct access
+  struct S2 NODEREF *s2_noderef;
+  p = s2_noderef->a;  // ok since result is an array in a struct
+  p = s2_noderef->a2; // ok
+  p = s2_noderef->b;  // expected-warning{{dereference of noderef expression}}
+  p = s2_noderef->b2; // expected-warning{{dereference of noderef expression}}
+  s = s2_noderef->s;  // expected-warning{{dereference of noderef expression}}
+  s = s2_noderef->s2; // expected-warning{{dereference of noderef expression}}
+  p = s2_noderef->a + 1;
+
+  struct S2 *s2;
+  p = s2->a;
+  p = s2->a2;
+  p = s2->b;
+  p = s2->b2;
+  s = s2->s;
+  s = s2->s2;
+
+  // Subscript access
+  x = p[1];// expected-warning{{dereference of noderef expression}}
+  p2 = q[0];   // ok
+  x = q[0][0]; // expected-warning{{dereference of noderef expression}}
+  p2 = q2[0];  // expected-warning{{dereference of noderef expression}}
+
+  int NODEREF arr[10];
+  x = arr[1]; // expected-warning{{dereference of noderef expression}}
+
+  int NODEREF *(arr2[10]);
+  int NODEREF *elem = *arr2;
+
+  int NODEREF(*arr3)[10];
+  elem = *arr3;
+
+  // Combinations between indirection, subscript, and member access
+  struct S2 NODEREF *s2_arr[10];
+  s2 = s2_arr[1];
+  struct S2 NODEREF *s2_arr2[10][10];
+  s2 = s2_arr2[1][1];
+
+  p = s2_arr[1]->a;
+  p = s2_arr[1]->b; // expected-warning{{dereference of noderef expression}}
+  int **bptr = _arr[1]->b;
+
+  x = s2->s2->a;// expected-warning{{dereference of noderef expression}}
+  x = s2_noderef->a[1]; // expected-warning{{dereference of noderef expression}}
+  p = _noderef->a[1];
+
+  // Functions
+  x = *(func(p)); // expected-warning{{dereference of noderef expression}}
+
+  // Casting is ok
+  q = (int NODEREF **)
+  q = (int NODEREF **)
+  q = 
+  q = 
+  x = s2->s2->a; // expected-warning{{dereference of noderef expression}}
+
+  // Other expressions
+  func2(*p); // expected-warning{{dereference of noderef expression}}
+  func2(*p + 1); // expected-warning{{dereference of noderef expression}}
+  func2(!*p);// expected-warning{{dereference of noderef expression}}
+  func2((x = *p));   // expected-warning{{dereference of noderef expression}}
+  func2((char)(*p)); // expected-warning{{dereference of noderef expression}}
+
+  // Other statements
+  if (*p) {}  // expected-warning{{dereference of noderef expression}}
+  else if (*p) {} // expected-warning{{dereference of noderef expression}}
+  switch (*p){}   // expected-warning{{dereference of noderef expression}}
+  for (*p; *p; *p){}  // expected-warning{{dereference of noderef expression}}
+  // 

[PATCH] D35388: [libc++] Give extern templates default visibility on gcc

[Louis Dionne via Phabricator via cfe-commits]

ldionne added a comment.

ABI-wise, I think this change is OK. Indeed, if `__type_visibility__` is not 
supported, we're already marking the base template as 
`__visibility__("default")`, so marking the extern template declaration with 
`__visibility__("default")` is not a problem. If `__type_visibility__` is 
supported, then there's no change in behavior. So I think there is no change in 
behavior either way, and I'm actually wondering why @thomasanderson wants this 
change in if the behavior is always the same. Perhaps I am missing something?

As a diversion, I'm confused by the very fact that we need to apply 
`_LIBCPP_EXTERN_TEMPLATE_TYPE_VIS` to the _extern template declaration_. I 
would have expected instead that, if anything, we need to specify the 
visibility on the _explicit instantiation_ in the dylib.


https://reviews.llvm.org/D35388



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

[PATCH] D49492: Run bounds checking sanitizer earlier to make it easier to optimize away its checks.

[Joel Galenson via Phabricator via cfe-commits]

jgalenson added a comment.

Good call; I had figured that running it earlier might just impede other 
optimizations, but I forgot that it would also hide size information.

I thought this was the easiest approach compared to changing the pass pipeline 
or adding extra checks in here.  But I hadn't realized I could use SCEV that 
easily.  From trying it quickly, I think this can remove the check in this 
testcase (and even some others this couldn't remove).  I'll work on making a 
patch that does that instead.

Thanks for the quick feedback and suggestions!


Repository:
  rC Clang

https://reviews.llvm.org/D49492



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

[PATCH] D49476: [Index] Set OrigD before D is changed.

[Argyrios Kyrtzidis via Phabricator via cfe-commits]

akyrtzi added a comment.

Is it possible to add a regression test case ? I assume this is fixing some 
issue, we should make sure we don't regress again.


Repository:
  rC Clang

https://reviews.llvm.org/D49476



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

[PATCH] D49466: Initial implementation of -fmacro-prefix-mapand -ffile-prefix-map

[Dan McGregor via Phabricator via cfe-commits]

dankm updated this revision to Diff 156164.
dankm retitled this revision from "Initial implementation of -fmacro-prefix-map
and -ffile-prefix-map" to "Initial implementation of -fmacro-prefix-mapand 
-ffile-prefix-map".
dankm added a comment.

Address some of the comments by erichkeane and joerg.


Repository:
  rC Clang

https://reviews.llvm.org/D49466

Files:
  include/clang/Basic/DiagnosticDriverKinds.td
  include/clang/Driver/Options.td
  include/clang/Lex/PreprocessorOptions.h
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Lex/PPMacroExpansion.cpp

Index: lib/Lex/PPMacroExpansion.cpp
===
--- lib/Lex/PPMacroExpansion.cpp
+++ lib/Lex/PPMacroExpansion.cpp
@@ -28,6 +28,7 @@
 #include "clang/Lex/MacroInfo.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Lex/PreprocessorLexer.h"
+#include "clang/Lex/PreprocessorOptions.h"
 #include "clang/Lex/PTHLexer.h"
 #include "clang/Lex/Token.h"
 #include "llvm/ADT/ArrayRef.h"
@@ -1453,6 +1454,15 @@
   return TI.getTriple().getEnvironment() == Env.getEnvironment();
 }
 
+static std::string remapMacroPath(StringRef Path,
+   const std::map ) {
+  for (const auto  : MacroPrefixMap)
+if (Path.startswith(Entry.first))
+  return (Twine(Entry.second) + Path.substr(Entry.first.size())).str();
+  return Path.str();
+}
+
 /// ExpandBuiltinMacro - If an identifier token is read that is to be expanded
 /// as a builtin macro, handle it and return the next token as 'Tok'.
 void Preprocessor::ExpandBuiltinMacro(Token ) {
@@ -1516,9 +1526,11 @@
 }
 
 // Escape this filename.  Turn '\' -> '\\' '"' -> '\"'
-SmallString<128> FN;
+std::string FN;
 if (PLoc.isValid()) {
   FN += PLoc.getFilename();
+  // TODO remap macro prefix here
+  FN = remapMacroPath(FN, PPOpts->MacroPrefixMap);
   Lexer::Stringify(FN);
   OS << '"' << FN << '"';
 }
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -2845,6 +2845,9 @@
   for (const auto *A : Args.filtered(OPT_error_on_deserialized_pch_decl))
 Opts.DeserializedPCHDeclsToErrorOn.insert(A->getValue());
 
+  for (const auto  : Args.getAllArgValues(OPT_fmacro_prefix_map_EQ))
+Opts.MacroPrefixMap.insert(StringRef(A).split('='));
+
   if (const Arg *A = Args.getLastArg(OPT_preamble_bytes_EQ)) {
 StringRef Value(A->getValue());
 size_t Comma = Value.find(',');
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -606,16 +606,34 @@
 
 /// Add a CC1 and CC1AS option to specify the debug file path prefix map.
 static void addDebugPrefixMapArg(const Driver , const ArgList , ArgStringList ) {
-  for (const Arg *A : Args.filtered(options::OPT_fdebug_prefix_map_EQ)) {
+  for (const Arg *A : Args.filtered(options::OPT_ffile_prefix_map_EQ,
+options::OPT_fdebug_prefix_map_EQ)) {
 StringRef Map = A->getValue();
-if (Map.find('=') == StringRef::npos)
+if (Map.find('=') == StringRef::npos) {
   D.Diag(diag::err_drv_invalid_argument_to_fdebug_prefix_map) << Map;
+  continue;
+}
 else
   CmdArgs.push_back(Args.MakeArgString("-fdebug-prefix-map=" + Map));
 A->claim();
   }
 }
 
+/// Add a CC1 option to specify the macro file path prefix map.
+static void addMacroPrefixMapArg(const Driver , const ArgList , ArgStringList ) {
+  for (const Arg *A : Args.filtered(options::OPT_ffile_prefix_map_EQ,
+options::OPT_fmacro_prefix_map_EQ)) {
+StringRef Map = A->getValue();
+if (Map.find('=') == StringRef::npos) {
+  D.Diag(diag::err_drv_invalid_argument_to_fmacro_prefix_map) << Map;
+  continue;
+}
+else
+  CmdArgs.push_back(Args.MakeArgString("-fmacro-prefix-map=" + Map));
+A->claim();
+  }
+}
+
 /// Vectorize at all optimization levels greater than 1 except for -Oz.
 /// For -Oz the loop vectorizer is disable, while the slp vectorizer is enabled.
 static bool shouldEnableVectorizerAtOLevel(const ArgList , bool isSlpVec) {
@@ -1223,6 +1241,8 @@
 // For IAMCU add special include arguments.
 getToolChain().AddIAMCUIncludeArgs(Args, CmdArgs);
   }
+
+  addMacroPrefixMapArg(D, Args, CmdArgs);
 }
 
 // FIXME: Move to target hook.
Index: include/clang/Lex/PreprocessorOptions.h
===
--- include/clang/Lex/PreprocessorOptions.h
+++ include/clang/Lex/PreprocessorOptions.h
@@ -167,6 +167,9 @@
   /// build it again.
   std::shared_ptr FailedModules;
 
+  /// A prefix map for __FILE__ and __BASEFILE__
+  std::map MacroPrefixMap;
+
 public:
   PreprocessorOptions() : PrecompiledPreambleBytes(0, false) {}
 
Index: 

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

[Florin Iucha via Phabricator via cfe-commits]

0x8000- added inline comments.



Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:23
+
+const char DefaultIgnoredValues[] = "0;1;2;10;100;";
+

aaron.ballman wrote:
> lebedev.ri wrote:
> > aaron.ballman wrote:
> > > Why 2, 10, and 100?
> > These really should be a config option.
> These are the default values for the config option. I think 0 and 1 make a 
> lot of sense to have in the default list given how prevalent they are in real 
> world code. But the other three seem to be used far less often.
> 
> I think if we wanted to have any values other than 0 and 1 (and perhaps -1) 
> as defaults, I'd want to see some data on large amounts of code to justify 
> anything else.
No need for -1, as it is covered by 1, plus unary operator.

I know 100 is used often for converting percentages.

10 is used for number parsing (yes, many people still do that by hand instead 
of libraries).

But this is the configuration - and I'm ok with reverting to 0 and 1 as I had 
originally.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D49114



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

[PATCH] D49466: Initial implementation of -fmacro-prefix-mapand -ffile-prefix-map

[Dan McGregor via Phabricator via cfe-commits]

dankm added inline comments.



Comment at: lib/Driver/ToolChains/Clang.cpp:609
 static void addDebugPrefixMapArg(const Driver , const ArgList , 
ArgStringList ) {
+  for (const Arg *A : Args.filtered(options::OPT_ffile_prefix_map_EQ)) {
+StringRef Map = A->getValue();

joerg wrote:
> I find it confusing that `-ffile_prefix_map` implies `-fdebug-prefix-map`. 
> I'm not sure that is desirable in every case. It seems better to have a 
> combined option that explicitly does both.
-ffile-prefix-map is the combined option. -fmacro-prefix-map is the 
preprocessor option, and -fdebug-prefix-map is the codegen option.



Comment at: lib/Driver/ToolChains/Clang.cpp:628
+/// Add a CC1 option to specify the macro file path prefix map.
+static void addMacroPrefixMapArg(const Driver , const ArgList , 
ArgStringList ) {
+  for (const Arg *A : Args.filtered(options::OPT_ffile_prefix_map_EQ)) {

joerg wrote:
> erichkeane wrote:
> > See advice above.
> > 
> > Additionally/alternatively, I wonder if these two functions could be 
> > trivially combined.
> Or at least the for loop could be refactored into a small helper function 
> that takes the option name, output option and error as argument.
Good ideas. I'll look into them.



Comment at: lib/Driver/ToolChains/Clang.cpp:1255
+
+  addMacroPrefixMapArg(D, Args, CmdArgs);
 }

erichkeane wrote:
> Is there a good reason for this to not live with the call to 
> addDebugPrefixMapArg?
Mostly because this is the function that adds preprocessor specific options. 
There's no other reason why it couldn't be done alongside addDebugPrefixMapArg 
in this file.



Comment at: lib/Frontend/CompilerInvocation.cpp:2848
 
+  for (const auto  : Args.getAllArgValues(OPT_fmacro_prefix_map_EQ))
+  {

erichkeane wrote:
> Again, this is so much like the debug-prefix otpion, it should probably just 
> be right next to it.
> 
> Additionally, we don't use curley brackets on single-line bodies.
This is here because it's a preprocessor option. This function handles those. 
The DebugPrefixMap handling is a codegen option.



Comment at: lib/Lex/PPMacroExpansion.cpp:1457
+static std::string remapMacroPath(StringRef Path,
+   llvm::SmallDenseMap ) {

erichkeane wrote:
> make MacroPrefixMap const.
I shall.



Comment at: lib/Lex/PPMacroExpansion.cpp:1460
+  for (const auto  : MacroPrefixMap)
+if (Path.startswith(Entry.first))
+  return (Twine(Entry.second) + Path.substr(Entry.first.size())).str();

joerg wrote:
> This doesn't handle directory vs string prefix prefix correctly, does it? At 
> the very least, it should have a test case :)
Good catch. I expect not. But on the other hand, it's exactly what 
debug-prefix-map does :)

I'll add test cases in a future review. My first goal was getting something 
sort-of working.



Comment at: lib/Lex/PPMacroExpansion.cpp:1528
 // Escape this filename.  Turn '\' -> '\\' '"' -> '\"'
-SmallString<128> FN;
+std::string FN;
 if (PLoc.isValid()) {

erichkeane wrote:
> This change shouldn't be necessary, SmallString is still likely the right 
> answer here.
I tried that long ago. It didn't work, I don't remember exactly why. But I 
agree that SmallString should be enough. I'll dig more.



Comment at: lib/Lex/Preprocessor.cpp:160
+
+  for (const auto  : this->PPOpts->MacroPrefixMap)
+MacroPrefixMap[KV.first] = KV.second;

erichkeane wrote:
> I'm unconvinced that this is necessary.  ExpandBuiltinMacro is in 
> Preprocessor, so it has access to PPOpts directly.
It has access to PPOpts, but the implementation file doesn't have a full 
definition of PreprocessorOptions. I could add that to this file, then this 
becomes redundant.


Repository:
  rC Clang

https://reviews.llvm.org/D49466



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

[PATCH] D49492: Run bounds checking sanitizer earlier to make it easier to optimize away its checks.

[Evgenii Stepanov via Phabricator via cfe-commits]

eugenis added a comment.

Some checks can be removed by asking SCEV for offset bounds, see 
SafeStack::IsAccessSafe for an example.


Repository:
  rC Clang

https://reviews.llvm.org/D49492



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

[PATCH] D49511: [Sema/Attribute] Check for noderef attribute

[Leonard Chan via Phabricator via cfe-commits]

leonardchan created this revision.
leonardchan added reviewers: phosek, mcgrathr.
leonardchan added a project: clang.

This patch adds the `noderef` attribute in clang and checks for dereferences of 
types that have this attribute. This attribute is currently used by sparse and 
would like to be ported to clang.

The following are examples of when a warning would be raised on dereferencing a 
noderef type.

  int __attribute__((noderef)) *p;
  int x = *p;  // warning: dereference of noderef expression 
[-Wignored-attributes]
  
  int __attribute__((noderef)) **p2;
  x = **p2;  // warning: dereference of noderef expression 
[-Wignored-attributes]
  
  int * __attribute__((noderef)) *p3;
  p = *p3;  // warning: dereference of noderef expression [-Wignored-attributes]
  
  struct S {
int a;
  };
  struct S __attribute__((noderef)) *s;
  x = s->a;// warning: dereference of noderef expression 
[-Wignored-attributes]
  x = (*s).a;   // warning: dereference of noderef expression 
[-Wignored-attributes]

Not all accesses may raise a warning if the value directed by the pointer may 
not be accessed. The following are examples where no warning may be raised:

  int *q;
  int __attribute__((noderef)) *p;
  q = &*p;
  q = *
  
  struct S {
int a;
  };
  struct S __attribute__((noderef)) *s;
  p = >a;
  p = &(*s).a;

More examples of existing usage of noderef in sparse can be found in 
https://git.kernel.org/pub/scm/devel/sparse/sparse.git/tree/validation/noderef.c


Repository:
  rC Clang

https://reviews.llvm.org/D49511

Files:
  include/clang/AST/Type.h
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Sema.h
  lib/AST/Type.cpp
  lib/AST/TypePrinter.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaStmt.cpp
  lib/Sema/SemaType.cpp
  test/Frontend/noderef.c

Index: test/Frontend/noderef.c
===
--- /dev/null
+++ test/Frontend/noderef.c
@@ -0,0 +1,133 @@
+// RUN: %clang_cc1 -x c -verify %s
+// RUN: %clang_cc1 -x c++ -verify %s
+
+#define NODEREF __attribute__((noderef))
+
+struct S {
+  int a;
+  int b;
+};
+
+struct S2 {
+  int a[2];
+  int NODEREF a2[2];
+  int *b;
+  int NODEREF *b2;
+  struct S *s;
+  struct S NODEREF *s2;
+};
+
+int NODEREF *func(int NODEREF *arg) {
+  int y = *arg; // expected-warning{{dereference of noderef expression}}
+  return arg;
+}
+
+void func2(int x) {}
+
+void test() {
+  int NODEREF *p;
+  int *p2;
+
+  int x = *p;   // expected-warning{{dereference of noderef expression}}
+  x = *((int NODEREF *)p2); // expected-warning{{dereference of noderef expression}}
+
+  int NODEREF **q;
+  int *NODEREF *q2;
+
+  // Indirection
+  p2 = *q;  // ok
+  x = **q;  // expected-warning{{dereference of noderef expression}}
+  p2 = *q2; // expected-warning{{dereference of noderef expression}}
+
+  **q; // expected-warning{{dereference of noderef expression}}
+   // expected-warning@-1{{expression result unused}}
+
+  p = *&*q;
+  p = **
+  q = &**
+  p = &*p;
+  p = *
+  p = &(*p);
+  p = *();
+  x = ** // expected-warning{{dereference of noderef expression}}
+
+  *p = 2;   // expected-warning{{dereference of noderef expression}}
+  *q = p;   // ok
+  **q = 2;  // expected-warning{{dereference of noderef expression}}
+  *q2 = p2; // expected-warning{{dereference of noderef expression}}
+
+  p2 = p;
+  p = p + 1;
+  p = &*(p + 1);
+
+  // Struct member access
+  struct S NODEREF *s;
+  x = s->a;   // expected-warning{{dereference of noderef expression}}
+  x = (*s).b; // expected-warning{{dereference of noderef expression}}
+  p = >a;
+  p = &(*s).b;
+
+  // Nested struct access
+  struct S2 NODEREF *s2_noderef;
+  p = s2_noderef->a;  // ok since result is an array in a struct
+  p = s2_noderef->a2; // ok
+  p = s2_noderef->b;  // expected-warning{{dereference of noderef expression}}
+  p = s2_noderef->b2; // expected-warning{{dereference of noderef expression}}
+  s = s2_noderef->s;  // expected-warning{{dereference of noderef expression}}
+  s = s2_noderef->s2; // expected-warning{{dereference of noderef expression}}
+  p = s2_noderef->a + 1;
+
+  struct S2 *s2;
+  p = s2->a;
+  p = s2->a2;
+  p = s2->b;
+  p = s2->b2;
+  s = s2->s;
+  s = s2->s2;
+
+  // Subscript access
+  x = p[1];// expected-warning{{dereference of noderef expression}}
+  p2 = q[0];   // ok
+  x = q[0][0]; // expected-warning{{dereference of noderef expression}}
+  p2 = q2[0];  // expected-warning{{dereference of noderef expression}}
+
+  int NODEREF arr[10];
+  x = arr[1]; // expected-warning{{dereference of noderef expression}}
+
+  int NODEREF *(arr2[10]);
+  int NODEREF *elem = *arr2;
+
+  int NODEREF(*arr3)[10];
+  elem = *arr3;
+
+  // Combinations between indirection, subscript, and member access
+  struct S2 NODEREF *s2_arr[10];
+  s2 = s2_arr[1];
+  struct S2 NODEREF *s2_arr2[10][10];
+  s2 = s2_arr2[1][1];
+
+  p = s2_arr[1]->a;
+  p = s2_arr[1]->b; // 

[PATCH] D49492: Run bounds checking sanitizer earlier to make it easier to optimize away its checks.

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

This sanitizer has a bit of a strange design compared to other sanitizers; it 
tries to compute the size of the base object using the IR at the point the pass 
runs.  So the later it runs, the more information it has.  Trivial example:

  static int accumulate(int* foo, int n) {
int sum = 0;
for (unsigned i = 0; i < n; i++)
  sum += foo[i];
return sum;
  }
  extern void fill(int *arr, int n);
  int dosum(int n) {
int foo[1000];
fill(foo, n);
return accumulate(foo, n);
  }


Repository:
  rC Clang

https://reviews.llvm.org/D49492



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

[PATCH] D49492: Run bounds checking sanitizer earlier to make it easier to optimize away its checks.

[Evgenii Stepanov via Phabricator via cfe-commits]

eugenis added a comment.

ObjectSizeOffsetEvaluator may fail to trace the address back to the underlying 
object, and we will end up not inserting the check.
Does ChecksUnable statistic go up with this change?


Repository:
  rC Clang

https://reviews.llvm.org/D49492



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

[PATCH] D49508: [CodeGen] VisitMaterializeTemporaryExpr(): don't skip NoOp Casts.

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

skipRValueSubobjectAdjustments has to match the rules in `[class.temporary]` in 
the standard, which includes skipping over certain explicit casts.

Would it be enough to accumulate the skipped casts into a SmallVector, like we 
do for the skipped comma operators?




Comment at: test/CodeGenCXX/const-init-cxx11.cpp:229
   // This creates a non-const temporary and binds a reference to it.
-  // CHECK: @[[TEMP:.*]] = internal global {{.*}} { i32 5 }, align 4
+  // CHECK: @[[TEMP:.*]] = internal constant {{.*}} { i32 5 }, align 4
   // CHECK: @_ZN16LiteralReference3litE = constant {{.*}} @[[TEMP]], align 8

This change is suspect; note the comment "this creates a non-const temporary".


Repository:
  rC Clang

https://reviews.llvm.org/D49508



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

[PATCH] D49492: Run bounds checking sanitizer earlier to make it easier to optimize away its checks.

[Joel Galenson via Phabricator via cfe-commits]

jgalenson added a comment.

In https://reviews.llvm.org/D49492#1167064, @efriedma wrote:

> Are you sure this will actually do what you want, in general?  I suspect it 
> will end up missing bounds checks in some cases because it's running it too 
> early (before mem2reg/inlining/etc).


No, I'm not sure; that's one reason I'm asking for comments.  But I don't see 
any specific problems.  For example, I don't see why inlining would matter; the 
checks should still be added, just before inlining instead of after (which of 
course affects the inlining heuristic, but that's another matter).  I don't 
understand mem2reg as well, though.  Do you have specific examples you think 
might fail?

I was thinking about this in terms of other sanitizers I know, specifically the 
integer overflow sanitizer.  That adds overflow checks in Clang, which is 
before all of these LLVM passes.  So my thought was that moving bounds checks 
to be inserted earlier brings it closer to how the integer overflow sanitizer 
works.


Repository:
  rC Clang

https://reviews.llvm.org/D49492



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

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:23
+
+const char DefaultIgnoredValues[] = "0;1;2;10;100;";
+

lebedev.ri wrote:
> aaron.ballman wrote:
> > Why 2, 10, and 100?
> These really should be a config option.
These are the default values for the config option. I think 0 and 1 make a lot 
of sense to have in the default list given how prevalent they are in real world 
code. But the other three seem to be used far less often.

I think if we wanted to have any values other than 0 and 1 (and perhaps -1) as 
defaults, I'd want to see some data on large amounts of code to justify 
anything else.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D49114



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

[PATCH] D46230: For x86_64, gcc 7.2 under Amazon Linux AMI sets its paths to x86_64-amazon-linux

[Jiading Gai via Phabricator via cfe-commits]

gaijiading added a comment.

In https://reviews.llvm.org/D46230#1167023, @echristo wrote:

> In https://reviews.llvm.org/D46230#1166958, @gaijiading wrote:
>
> > In https://reviews.llvm.org/D46230#1166919, @echristo wrote:
> >
> > > LGTM.
> > >
> > > -eric
> >
> >
> > Hi Eric, I do not have commit access to trunk. Could you commit the change 
> > for me? Thanks.
>
>
> The binaries make that hard, can you email me a tarball please?


Sure, I can do that. Please let me know what’s the email address I should send 
this to. Thanks.


Repository:
  rC Clang

https://reviews.llvm.org/D46230



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

[PATCH] D49360: [analyzer] Add support for more basic_string API in DanglingInternalBufferChecker

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: test/Analysis/dangling-internal-buffer.cpp:175
   std::string s;
-  {
-c = s.c_str();
-  }
-  consume(c); // no-warning
+  c = s.c_str(); // expected-note {{Pointer to dangling buffer was obtained 
here}}
+  s.clear(); // expected-note {{Method call is allowed to invalidate the 
internal buffer}}

rnkovacs wrote:
> dcoughlin wrote:
> > In other parts of clang we use the term "inner pointer" to mean a pointer 
> > that will be invalidated if its containing object is destroyed 
> > https://clang.llvm.org/docs/AutomaticReferenceCounting.html#interior-pointers.
> >  There are existing attributes that use this term to specify that a method 
> > returns an inner pointer.
> > 
> > I think it would be good to use the same terminology here. So the 
> > diagnostic could be something like "Dangling inner pointer obtained here".
> I feel like I should also retitle the checker to `DanglingInnerBuffer` to 
> remain consistent. What do you think?
My intuition suggests that we should remove the word "Dangling" from the 
checker name, because our checker names are usually indicating what they check, 
not what bugs they find. Eg., `MallocChecker` doesn't find all mallocs, it 
checks that mallocs are used correctly. This checker checks that pointers to 
inner buffers are used correctly, so we may call it `InnerPointerChecker` or 
something like that.


https://reviews.llvm.org/D49360



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

[PATCH] D49360: [analyzer] Add support for more basic_string API in DanglingInternalBufferChecker

[Artem Dergachev via Phabricator via cfe-commits]

NoQ accepted this revision.
NoQ added a comment.

Let's commit this patch and make another round later, as we gather ideas for 
better names and messages.


https://reviews.llvm.org/D49360



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

[PATCH] D49466: Initial implementation of -fmacro-prefix-mapand -ffile-prefix-map

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added inline comments.



Comment at: lib/Driver/ToolChains/Clang.cpp:609
 static void addDebugPrefixMapArg(const Driver , const ArgList , 
ArgStringList ) {
+  for (const Arg *A : Args.filtered(options::OPT_ffile_prefix_map_EQ)) {
+StringRef Map = A->getValue();

I find it confusing that `-ffile_prefix_map` implies `-fdebug-prefix-map`. I'm 
not sure that is desirable in every case. It seems better to have a combined 
option that explicitly does both.



Comment at: lib/Driver/ToolChains/Clang.cpp:612
+if (Map.find('=') == StringRef::npos)
+  D.Diag(diag::err_drv_invalid_argument_to_fdebug_prefix_map) << Map;
+else

I'd prefer the bailout style here, i.e. `if (...) { D.diag(...); continue }`



Comment at: lib/Driver/ToolChains/Clang.cpp:628
+/// Add a CC1 option to specify the macro file path prefix map.
+static void addMacroPrefixMapArg(const Driver , const ArgList , 
ArgStringList ) {
+  for (const Arg *A : Args.filtered(options::OPT_ffile_prefix_map_EQ)) {

erichkeane wrote:
> See advice above.
> 
> Additionally/alternatively, I wonder if these two functions could be 
> trivially combined.
Or at least the for loop could be refactored into a small helper function that 
takes the option name, output option and error as argument.



Comment at: lib/Lex/PPMacroExpansion.cpp:1460
+  for (const auto  : MacroPrefixMap)
+if (Path.startswith(Entry.first))
+  return (Twine(Entry.second) + Path.substr(Entry.first.size())).str();

This doesn't handle directory vs string prefix prefix correctly, does it? At 
the very least, it should have a test case :)


Repository:
  rC Clang

https://reviews.llvm.org/D49466



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

[PATCH] D49421: [CodeComplete] Fix accessibilty of protected members from base class.

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: lib/Sema/SemaAccess.cpp:1871-1873
+// The access should be AS_none as we don't know how the member was
+// accessed - `AccessedEntity::getAccess` describes what access was used to
+// access an entity.

aaron.ballman wrote:
> ioeric wrote:
> > aaron.ballman wrote:
> > > This seems to contradict the function-level comment that says the check 
> > > seems to only take the access specifiers into account and not how the 
> > > member is accessed.
> > > not how the member is accessed.
> > It's not very clear to me what exactly this means.  But I think we are 
> > still not taking how member was accessed into account. The access in 
> > `DeclAccessPair` describes how a member was accessed, and this should be 
> > None instead of the access specifier of the member as we don't have this 
> > information here. 
> > 
> > I updated the wording in the comment to make this a bit clearer.
> I'm still wondering about the removal of the public access check -- if the 
> declaration has a public access specifier, that early return saves work. So 
> `Entity` may still require passing `AS_none` but we might want to continue to 
> check `Decl->getAccess() == AS_public` for the early return?
> 
> (Drive-by nit that's not yours: `Decl` is the name of a type, so if you 
> wanted to rename the parameter to something that isn't a type name, I would 
> not be sad, but you're not required to do it.)
Added `Decl->getAccess() == AS_public` back to preserve the behavior.

s/Decl/Target/


Repository:
  rC Clang

https://reviews.llvm.org/D49421



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

[PATCH] D49421: [CodeComplete] Fix accessibilty of protected members from base class.

[Eric Liu via Phabricator via cfe-commits]

ioeric updated this revision to Diff 156146.
ioeric marked an inline comment as done.
ioeric added a comment.

- addressed review comments.
- Addressed review comments.


Repository:
  rC Clang

https://reviews.llvm.org/D49421

Files:
  lib/Sema/SemaAccess.cpp
  lib/Sema/SemaCodeComplete.cpp
  test/Index/complete-access-checks.cpp

Index: test/Index/complete-access-checks.cpp
===
--- test/Index/complete-access-checks.cpp
+++ test/Index/complete-access-checks.cpp
@@ -36,10 +36,10 @@
 
 // CHECK-SUPER-ACCESS: CXXMethod:{ResultType void}{TypedText doSomething}{LeftParen (}{RightParen )} (34)
 // CHECK-SUPER-ACCESS: CXXMethod:{ResultType void}{Informative X::}{TypedText func1}{LeftParen (}{RightParen )} (36)
-// CHECK-SUPER-ACCESS: CXXMethod:{ResultType void}{Informative X::}{TypedText func2}{LeftParen (}{RightParen )} (36) (inaccessible)
+// CHECK-SUPER-ACCESS: CXXMethod:{ResultType void}{Informative X::}{TypedText func2}{LeftParen (}{RightParen )} (36)
 // CHECK-SUPER-ACCESS: CXXMethod:{ResultType void}{Informative X::}{TypedText func3}{LeftParen (}{RightParen )} (36) (inaccessible)
 // CHECK-SUPER-ACCESS: FieldDecl:{ResultType int}{Informative X::}{TypedText member1} (37)
-// CHECK-SUPER-ACCESS: FieldDecl:{ResultType int}{Informative X::}{TypedText member2} (37) (inaccessible)
+// CHECK-SUPER-ACCESS: FieldDecl:{ResultType int}{Informative X::}{TypedText member2} (37)
 // CHECK-SUPER-ACCESS: FieldDecl:{ResultType int}{Informative X::}{TypedText member3} (37) (inaccessible)
 // CHECK-SUPER-ACCESS: CXXMethod:{ResultType Y &}{TypedText operator=}{LeftParen (}{Placeholder const Y &}{RightParen )} (79)
 // CHECK-SUPER-ACCESS: CXXMethod:{ResultType X &}{Text X::}{TypedText operator=}{LeftParen (}{Placeholder const X &}{RightParen )} (81)
@@ -87,3 +87,26 @@
 // CHECK-USING-ACCESSIBLE: ClassDecl:{TypedText Q}{Text ::} (75)
 // CHECK-USING-ACCESSIBLE: CXXDestructor:{ResultType void}{Informative P::}{TypedText ~P}{LeftParen (}{RightParen )} (81)
 // CHECK-USING-ACCESSIBLE: CXXDestructor:{ResultType void}{TypedText ~Q}{LeftParen (}{RightParen )} (79)
+
+class B {
+protected:
+  int member;
+};
+
+class C : private B {};
+
+
+class D : public C {
+ public:
+  void f(::B *b);
+};
+
+void D::f(::B *that) {
+  // RUN: c-index-test -code-completion-at=%s:106:9 %s | FileCheck -check-prefix=CHECK-PRIVATE-SUPER-THIS %s
+  this->;
+// CHECK-PRIVATE-SUPER-THIS: FieldDecl:{ResultType int}{Informative B::}{TypedText member} (37) (inaccessible)
+
+  // RUN: c-index-test -code-completion-at=%s:110:9 %s | FileCheck -check-prefix=CHECK-PRIVATE-SUPER-THAT %s
+  that->;
+// CHECK-PRIVATE-SUPER-THAT: FieldDecl:{ResultType int}{TypedText member} (35) (inaccessible)
+}
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -1303,8 +1303,33 @@
 void FoundDecl(NamedDecl *ND, NamedDecl *Hiding, DeclContext *Ctx,
bool InBaseClass) override {
   bool Accessible = true;
-  if (Ctx)
-Accessible = Results.getSema().IsSimplyAccessible(ND, Ctx);
+  if (Ctx) {
+DeclContext *AccessingCtx = Ctx;
+// If ND comes from a base class, set the naming class back to the
+// derived class if the search starts from the derived class (i.e.
+// InBaseClass is true).
+//
+// Example:
+//   class B { protected: int X; }
+//   class D : public B { void f(); }
+//   void D::f() { this->^; }
+// The completion after "this->" will have `InBaseClass` set to true and
+// `Ctx` set to "B", when looking up in `B`. We need to set the actual
+// accessing context (i.e. naming class) to "D" so that access can be
+// calculated correctly.
+if (InBaseClass && isa(Ctx)) {
+  CXXRecordDecl *RC = nullptr;
+  // Get the enclosing record.
+  for (DeclContext *DC = CurContext; !DC->isFileContext();
+   DC = DC->getParent()) {
+if ((RC = dyn_cast(DC)))
+  break;
+  }
+  if (RC)
+AccessingCtx = RC;
+}
+Accessible = Results.getSema().IsSimplyAccessible(ND, AccessingCtx);
+  }
 
   ResultBuilder::Result Result(ND, Results.getBasePriority(ND), nullptr,
false, Accessible, FixIts);
Index: lib/Sema/SemaAccess.cpp
===
--- lib/Sema/SemaAccess.cpp
+++ lib/Sema/SemaAccess.cpp
@@ -11,6 +11,7 @@
 //
 //===--===//
 
+#include "clang/Basic/Specifiers.h"
 #include "clang/Sema/SemaInternal.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/CXXInheritance.h"
@@ -1856,29 +1857,31 @@
   }
 }
 
-/// Checks access to Decl from the given class. The check will take access
+/// Checks access 

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:23
+
+const char DefaultIgnoredValues[] = "0;1;2;10;100;";
+

aaron.ballman wrote:
> Why 2, 10, and 100?
These really should be a config option.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D49114



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

[PATCH] D49508: [CodeGen] VisitMaterializeTemporaryExpr(): don't skip NoOp Casts.

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri created this revision.
lebedev.ri added reviewers: rsmith, rjmccall, majnemer, efriedma.
Herald added a subscriber: cfe-commits.

As discussed in PR38166 , we need 
to be able to distinqush whether the cast
we are visiting is actually a cast, or part of an `ExplicitCast`.
There are at least two ways to get there:

1. Introduce a new `CastKind`, and use it instead of `IntegralCast` if we are 
in `ExplicitCast`.

  Would work, but does not scale - what if we will need more of these cast 
kinds?
2. Fix `ScalarExprEmitter::VisitCastExpr()` to visit these `NoOp` casts.

  As pointed out by @rsmith, CodeGenFunction::EmitMaterializeTemporaryExpr calls

  skipRValueSubobjectAdjustments, which steps over the CK_NoOp cast`,

  which explains why we currently don't visit those.

Now, the problem is, this is a very specific area of clang.
I'm not familiar with it.
At first, when i saw these test regressions, i immediately panicked,
But now that i have actually looked at them in detail, at least in the case of 
`test/CodeGenCXX/cxx0x-initializer-*`,
as far as i can tell, only the IR is different, the assembly is still exactly 
the same. Or i'm really failing to look..
So i'm not sure whether these test changes are really ok or not, thus i'm 
posting this differential.


Repository:
  rC Clang

https://reviews.llvm.org/D49508

Files:
  include/clang/AST/Expr.h
  lib/AST/Expr.cpp
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CGExprConstant.cpp
  test/CodeGenCXX/const-init-cxx11.cpp
  test/CodeGenCXX/cxx0x-initializer-references.cpp
  test/CodeGenCXX/cxx0x-initializer-stdinitializerlist.cpp

Index: test/CodeGenCXX/cxx0x-initializer-stdinitializerlist.cpp
===
--- test/CodeGenCXX/cxx0x-initializer-stdinitializerlist.cpp
+++ test/CodeGenCXX/cxx0x-initializer-stdinitializerlist.cpp
@@ -501,7 +501,7 @@
   void PR22940_helper(const pair&) { }
   void PR22940() {
 // CHECK-LABEL: @_ZN9B197730107PR22940Ev
-// CHECK: call {{.*}} @_ZN9B197730104pairIPviEC{{.}}Ev(
+// CHECK-NOT: call {{.*}} @_ZN9B197730104pairIPviEC{{.}}Ev(
 // CHECK: call {{.*}} @_ZN9B1977301014PR22940_helperERKNS_4pairIPviEE(
 PR22940_helper(pair());
   }
Index: test/CodeGenCXX/cxx0x-initializer-references.cpp
===
--- test/CodeGenCXX/cxx0x-initializer-references.cpp
+++ test/CodeGenCXX/cxx0x-initializer-references.cpp
@@ -94,9 +94,9 @@
 }
 
 void foo() {
-// CHECK-LABEL: @_ZN7PR231653fooEv
-// CHECK: call {{.*}} @_ZN7PR2316510ChildClassC1Ev
-// CHECK: call void @_ZN7PR231656helperERKNS_13AbstractClassE
+  // CHECK-LABEL: @_ZN7PR231653fooEv
+  // CHECK-NOT: call {{.*}} @_ZN7PR2316510ChildClassC1Ev
+  // CHECK: call void @_ZN7PR231656helperERKNS_13AbstractClassE
   helper(ChildClass());
 }
 
Index: test/CodeGenCXX/const-init-cxx11.cpp
===
--- test/CodeGenCXX/const-init-cxx11.cpp
+++ test/CodeGenCXX/const-init-cxx11.cpp
@@ -226,7 +226,7 @@
   };
 
   // This creates a non-const temporary and binds a reference to it.
-  // CHECK: @[[TEMP:.*]] = internal global {{.*}} { i32 5 }, align 4
+  // CHECK: @[[TEMP:.*]] = internal constant {{.*}} { i32 5 }, align 4
   // CHECK: @_ZN16LiteralReference3litE = constant {{.*}} @[[TEMP]], align 8
   const Lit  = Lit();
 
Index: lib/CodeGen/CGExprConstant.cpp
===
--- lib/CodeGen/CGExprConstant.cpp
+++ lib/CodeGen/CGExprConstant.cpp
@@ -1835,8 +1835,8 @@
   assert(E->getStorageDuration() == SD_Static);
   SmallVector CommaLHSs;
   SmallVector Adjustments;
-  const Expr *Inner = E->GetTemporaryExpr()
-  ->skipRValueSubobjectAdjustments(CommaLHSs, Adjustments);
+  const Expr *Inner = E->GetTemporaryExpr()->skipRValueSubobjectAdjustments(
+  CommaLHSs, Adjustments, /*CanSkipNoOpCasts*/ false);
   return CGM.GetAddrOfGlobalTemporary(E, Inner);
 }
 
Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -467,7 +467,8 @@
 
   SmallVector CommaLHSs;
   SmallVector Adjustments;
-  E = E->skipRValueSubobjectAdjustments(CommaLHSs, Adjustments);
+  E = E->skipRValueSubobjectAdjustments(CommaLHSs, Adjustments,
+/*CanSkipNoOpCasts*/ false);
 
   for (const auto  : CommaLHSs)
 EmitIgnoredExpr(Ignored);
Index: lib/AST/Expr.cpp
===
--- lib/AST/Expr.cpp
+++ lib/AST/Expr.cpp
@@ -76,7 +76,8 @@
 
 const Expr *Expr::skipRValueSubobjectAdjustments(
 SmallVectorImpl ,
-SmallVectorImpl ) const {
+SmallVectorImpl ,
+bool CanSkipNoOpCasts) const {
   const Expr *E = this;
   while (true) {
 E = E->IgnoreParens();
@@ -92,10 +93,16 @@
 continue;
   }
 
-  if (CE->getCastKind() == CK_NoOp) {
+   

[PATCH] D47474: Implement cpu_dispatch/cpu_specific Multiversioning

[Aaron Ballman via Phabricator via cfe-commits]

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

Aside from an `assert` than can be removed, this LGTM on the attribute side of 
things.




Comment at: lib/CodeGen/CodeGenModule.cpp:2446
+  const auto *FD = cast(GD.getDecl());
+  assert(FD && "Not a FunctionDecl?");
+  const auto *DD = FD->getAttr();

aaron.ballman wrote:
> `cast<>` already asserts this.
I don't think this is done -- the assert can be removed.


https://reviews.llvm.org/D47474



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

[PATCH] D49457: DR330: when determining whether a cast casts away constness, consider qualifiers from all levels matching a multidimensional array

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

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rC337422: DR330: when determining whether a cast casts away 
constness, consider (authored by rsmith, committed by ).

Repository:
  rC Clang

https://reviews.llvm.org/D49457

Files:
  include/clang/AST/ASTContext.h
  lib/AST/ASTContext.cpp
  lib/Sema/SemaCast.cpp
  test/CXX/drs/dr3xx.cpp

Index: include/clang/AST/ASTContext.h
===
--- include/clang/AST/ASTContext.h
+++ include/clang/AST/ASTContext.h
@@ -2289,6 +2289,7 @@
const ObjCMethodDecl *MethodImp);
 
   bool UnwrapSimilarTypes(QualType , QualType );
+  bool UnwrapSimilarArrayTypes(QualType , QualType );
 
   /// Determine if two types are similar, according to the C++ rules. That is,
   /// determine if they are the same other than qualifiers on the initial
Index: test/CXX/drs/dr3xx.cpp
===
--- test/CXX/drs/dr3xx.cpp
+++ test/CXX/drs/dr3xx.cpp
@@ -403,6 +403,28 @@
 (void) reinterpret_cast(q); // expected-error {{casts away qualifiers}}
 (void) reinterpret_cast(t);
   }
+
+  namespace swift_17882 {
+typedef const char P[72];
+typedef int *Q;
+void f(P , P *pp) {
+  (void) reinterpret_cast(pr);
+  (void) reinterpret_cast(pp);
+}
+
+struct X {};
+typedef const volatile int A[1][2][3];
+typedef int *const X::*volatile *B1;
+typedef int *const X::* *B2;
+typedef int *X::*  volatile *B3;
+typedef volatile int *(*const B4)[4];
+void f(A *a) {
+  (void) reinterpret_cast(a);
+  (void) reinterpret_cast(a); // expected-error {{casts away qualifiers}}
+  (void) reinterpret_cast(a); // expected-error {{casts away qualifiers}}
+  (void) reinterpret_cast(a);
+}
+  }
 }
 
 namespace dr331 { // dr331: yes
Index: lib/AST/ASTContext.cpp
===
--- lib/AST/ASTContext.cpp
+++ lib/AST/ASTContext.cpp
@@ -5008,28 +5008,29 @@
 /// Attempt to unwrap two types that may both be array types with the same bound
 /// (or both be array types of unknown bound) for the purpose of comparing the
 /// cv-decomposition of two types per C++ [conv.qual].
-static void unwrapSimilarArrayTypes(ASTContext , QualType ,
-QualType ) {
+bool ASTContext::UnwrapSimilarArrayTypes(QualType , QualType ) {
+  bool UnwrappedAny = false;
   while (true) {
-auto *AT1 = Ctx.getAsArrayType(T1);
-if (!AT1) return;
+auto *AT1 = getAsArrayType(T1);
+if (!AT1) return UnwrappedAny;
 
-auto *AT2 = Ctx.getAsArrayType(T2);
-if (!AT2) return;
+auto *AT2 = getAsArrayType(T2);
+if (!AT2) return UnwrappedAny;
 
 // If we don't have two array types with the same constant bound nor two
 // incomplete array types, we've unwrapped everything we can.
 if (auto *CAT1 = dyn_cast(AT1)) {
   auto *CAT2 = dyn_cast(AT2);
   if (!CAT2 || CAT1->getSize() != CAT2->getSize())
-return;
+return UnwrappedAny;
 } else if (!isa(AT1) ||
!isa(AT2)) {
-  return;
+  return UnwrappedAny;
 }
 
 T1 = AT1->getElementType();
 T2 = AT2->getElementType();
+UnwrappedAny = true;
   }
 }
 
@@ -5046,16 +5047,16 @@
 /// \return \c true if a pointer type was unwrapped, \c false if we reached a
 /// pair of types that can't be unwrapped further.
 bool ASTContext::UnwrapSimilarTypes(QualType , QualType ) {
-  unwrapSimilarArrayTypes(*this, T1, T2);
+  UnwrapSimilarArrayTypes(T1, T2);
 
   const auto *T1PtrType = T1->getAs();
   const auto *T2PtrType = T2->getAs();
   if (T1PtrType && T2PtrType) {
 T1 = T1PtrType->getPointeeType();
 T2 = T2PtrType->getPointeeType();
 return true;
   }
-  
+
   const auto *T1MPType = T1->getAs();
   const auto *T2MPType = T2->getAs();
   if (T1MPType && T2MPType && 
Index: lib/Sema/SemaCast.cpp
===
--- lib/Sema/SemaCast.cpp
+++ lib/Sema/SemaCast.cpp
@@ -459,50 +459,83 @@
 /// Like Sema::UnwrapSimilarTypes, this removes one level of indirection from
 /// both types, provided that they're both pointer-like or array-like. Unlike
 /// the Sema function, doesn't care if the unwrapped pieces are related.
+///
+/// This function may remove additional levels as necessary for correctness:
+/// the resulting T1 is unwrapped sufficiently that it is never an array type,
+/// so that its qualifiers can be directly compared to those of T2 (which will
+/// have the combined set of qualifiers from all indermediate levels of T2),
+/// as (effectively) required by [expr.const.cast]p7 replacing T1's qualifiers
+/// with those from T2.
 static CastAwayConstnessKind
 unwrapCastAwayConstnessLevel(ASTContext , QualType , QualType ) {
-  // Note, even if 

[PATCH] D49504: Enable C++2a Chrono Literals

[Erich Keane via Phabricator via cfe-commits]

erichkeane updated this revision to Diff 156143.
erichkeane added a comment.

@mclow.lists s comments.


https://reviews.llvm.org/D49504

Files:
  lib/Lex/LiteralSupport.cpp
  test/SemaCXX/cxx2a-user-defined-literals.cpp


Index: lib/Lex/LiteralSupport.cpp
===
--- lib/Lex/LiteralSupport.cpp
+++ lib/Lex/LiteralSupport.cpp
@@ -751,7 +751,8 @@
 
   // If we have a hex digit other than 'e' (which denotes a FP exponent) then
   // the code is using an incorrect base.
-  if (isHexDigit(*s) && *s != 'e' && *s != 'E') {
+  if (isHexDigit(*s) && *s != 'e' && *s != 'E' &&
+  !isValidUDSuffix(PP.getLangOpts(), StringRef(s, ThisTokEnd - s))) {
 PP.Diag(PP.AdvanceToTokenCharacter(TokLoc, s-ThisTokBegin),
 diag::err_invalid_digit) << StringRef(s, 1) << (radix == 8 ? 1 : 
0);
 hadError = true;
@@ -804,12 +805,14 @@
   if (!LangOpts.CPlusPlus14)
 return false;
 
-  // In C++1y, "s", "h", "min", "ms", "us", and "ns" are used in the library.
+  // In C++14, "s", "h", "min", "ms", "us", and "ns" are used in the library.
   // Per tweaked N3660, "il", "i", and "if" are also used in the library.
+  // In C++2a "d" and "y" are used in the library.
   return llvm::StringSwitch(Suffix)
   .Cases("h", "min", "s", true)
   .Cases("ms", "us", "ns", true)
   .Cases("il", "i", "if", true)
+  .Cases("d", "y", LangOpts.CPlusPlus2a)
   .Default(false);
 }
 
@@ -922,7 +925,9 @@
 s = SkipBinaryDigits(s);
 if (s == ThisTokEnd) {
   // Done.
-} else if (isHexDigit(*s)) {
+} else if (isHexDigit(*s) &&
+   !isValidUDSuffix(PP.getLangOpts(),
+StringRef(s, ThisTokEnd - s))) {
   PP.Diag(PP.AdvanceToTokenCharacter(TokLoc, s-ThisTokBegin),
   diag::err_invalid_digit) << StringRef(s, 1) << 2;
   hadError = true;
Index: test/SemaCXX/cxx2a-user-defined-literals.cpp
===
--- test/SemaCXX/cxx2a-user-defined-literals.cpp
+++ test/SemaCXX/cxx2a-user-defined-literals.cpp
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 -std=c++2a %s -include %s -verify
+
+#ifndef INCLUDED
+#define INCLUDED
+
+#pragma clang system_header
+namespace std {
+  namespace chrono {
+struct day{};
+struct year{};
+  }
+  using size_t = decltype(sizeof(0));
+  constexpr chrono::day operator"" d(unsigned long long d) noexcept;
+  constexpr chrono::year operator"" y(unsigned long long y) noexcept;
+}
+
+#else
+
+using namespace std;
+chrono::day dec_d = 5d;
+chrono::day oct_d = 05d;
+chrono::day bin_d = 0b011d;
+// expected-error@+3{{no viable conversion from 'int' to 'chrono::day'}}
+// expected-note@9{{candidate constructor (the implicit copy constructor)}}
+// expected-note@9{{candidate constructor (the implicit move constructor)}}
+chrono::day hex_d = 0x44d;
+chrono::year y  = 10y;
+#endif


Index: lib/Lex/LiteralSupport.cpp
===
--- lib/Lex/LiteralSupport.cpp
+++ lib/Lex/LiteralSupport.cpp
@@ -751,7 +751,8 @@
 
   // If we have a hex digit other than 'e' (which denotes a FP exponent) then
   // the code is using an incorrect base.
-  if (isHexDigit(*s) && *s != 'e' && *s != 'E') {
+  if (isHexDigit(*s) && *s != 'e' && *s != 'E' &&
+  !isValidUDSuffix(PP.getLangOpts(), StringRef(s, ThisTokEnd - s))) {
 PP.Diag(PP.AdvanceToTokenCharacter(TokLoc, s-ThisTokBegin),
 diag::err_invalid_digit) << StringRef(s, 1) << (radix == 8 ? 1 : 0);
 hadError = true;
@@ -804,12 +805,14 @@
   if (!LangOpts.CPlusPlus14)
 return false;
 
-  // In C++1y, "s", "h", "min", "ms", "us", and "ns" are used in the library.
+  // In C++14, "s", "h", "min", "ms", "us", and "ns" are used in the library.
   // Per tweaked N3660, "il", "i", and "if" are also used in the library.
+  // In C++2a "d" and "y" are used in the library.
   return llvm::StringSwitch(Suffix)
   .Cases("h", "min", "s", true)
   .Cases("ms", "us", "ns", true)
   .Cases("il", "i", "if", true)
+  .Cases("d", "y", LangOpts.CPlusPlus2a)
   .Default(false);
 }
 
@@ -922,7 +925,9 @@
 s = SkipBinaryDigits(s);
 if (s == ThisTokEnd) {
   // Done.
-} else if (isHexDigit(*s)) {
+} else if (isHexDigit(*s) &&
+   !isValidUDSuffix(PP.getLangOpts(),
+StringRef(s, ThisTokEnd - s))) {
   PP.Diag(PP.AdvanceToTokenCharacter(TokLoc, s-ThisTokBegin),
   diag::err_invalid_digit) << StringRef(s, 1) << 2;
   hadError = true;
Index: test/SemaCXX/cxx2a-user-defined-literals.cpp
===
--- test/SemaCXX/cxx2a-user-defined-literals.cpp
+++ test/SemaCXX/cxx2a-user-defined-literals.cpp
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 -std=c++2a %s -include %s -verify
+
+#ifndef INCLUDED
+#define INCLUDED
+
+#pragma clang system_header
+namespace std {
+  namespace chrono {
+

r337422 - DR330: when determining whether a cast casts away constness, consider

[Richard Smith via cfe-commits]

Author: rsmith
Date: Wed Jul 18 13:13:36 2018
New Revision: 337422

URL: http://llvm.org/viewvc/llvm-project?rev=337422=rev
Log:
DR330: when determining whether a cast casts away constness, consider
qualifiers from all levels matching a multidimensional array.

For example, this allows casting from
  pointer to   array ofarray of   const volatile int
to
  pointer to const pointer to volatile pointer toint
because the multidimensional array part of the source type corresponds
to a part of the destination type that contains both 'const' and
'volatile'.

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

Modified:
cfe/trunk/include/clang/AST/ASTContext.h
cfe/trunk/lib/AST/ASTContext.cpp
cfe/trunk/lib/Sema/SemaCast.cpp
cfe/trunk/test/CXX/drs/dr3xx.cpp

Modified: cfe/trunk/include/clang/AST/ASTContext.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTContext.h?rev=337422=337421=337422=diff
==
--- cfe/trunk/include/clang/AST/ASTContext.h (original)
+++ cfe/trunk/include/clang/AST/ASTContext.h Wed Jul 18 13:13:36 2018
@@ -2289,6 +2289,7 @@ public:
const ObjCMethodDecl *MethodImp);
 
   bool UnwrapSimilarTypes(QualType , QualType );
+  bool UnwrapSimilarArrayTypes(QualType , QualType );
 
   /// Determine if two types are similar, according to the C++ rules. That is,
   /// determine if they are the same other than qualifiers on the initial

Modified: cfe/trunk/lib/AST/ASTContext.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=337422=337421=337422=diff
==
--- cfe/trunk/lib/AST/ASTContext.cpp (original)
+++ cfe/trunk/lib/AST/ASTContext.cpp Wed Jul 18 13:13:36 2018
@@ -5008,28 +5008,29 @@ QualType ASTContext::getUnqualifiedArray
 /// Attempt to unwrap two types that may both be array types with the same 
bound
 /// (or both be array types of unknown bound) for the purpose of comparing the
 /// cv-decomposition of two types per C++ [conv.qual].
-static void unwrapSimilarArrayTypes(ASTContext , QualType ,
-QualType ) {
+bool ASTContext::UnwrapSimilarArrayTypes(QualType , QualType ) {
+  bool UnwrappedAny = false;
   while (true) {
-auto *AT1 = Ctx.getAsArrayType(T1);
-if (!AT1) return;
+auto *AT1 = getAsArrayType(T1);
+if (!AT1) return UnwrappedAny;
 
-auto *AT2 = Ctx.getAsArrayType(T2);
-if (!AT2) return;
+auto *AT2 = getAsArrayType(T2);
+if (!AT2) return UnwrappedAny;
 
 // If we don't have two array types with the same constant bound nor two
 // incomplete array types, we've unwrapped everything we can.
 if (auto *CAT1 = dyn_cast(AT1)) {
   auto *CAT2 = dyn_cast(AT2);
   if (!CAT2 || CAT1->getSize() != CAT2->getSize())
-return;
+return UnwrappedAny;
 } else if (!isa(AT1) ||
!isa(AT2)) {
-  return;
+  return UnwrappedAny;
 }
 
 T1 = AT1->getElementType();
 T2 = AT2->getElementType();
+UnwrappedAny = true;
   }
 }
 
@@ -5046,7 +5047,7 @@ static void unwrapSimilarArrayTypes(ASTC
 /// \return \c true if a pointer type was unwrapped, \c false if we reached a
 /// pair of types that can't be unwrapped further.
 bool ASTContext::UnwrapSimilarTypes(QualType , QualType ) {
-  unwrapSimilarArrayTypes(*this, T1, T2);
+  UnwrapSimilarArrayTypes(T1, T2);
 
   const auto *T1PtrType = T1->getAs();
   const auto *T2PtrType = T2->getAs();
@@ -5055,7 +5056,7 @@ bool ASTContext::UnwrapSimilarTypes(Qual
 T2 = T2PtrType->getPointeeType();
 return true;
   }
-  
+
   const auto *T1MPType = T1->getAs();
   const auto *T2MPType = T2->getAs();
   if (T1MPType && T2MPType && 

Modified: cfe/trunk/lib/Sema/SemaCast.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCast.cpp?rev=337422=337421=337422=diff
==
--- cfe/trunk/lib/Sema/SemaCast.cpp (original)
+++ cfe/trunk/lib/Sema/SemaCast.cpp Wed Jul 18 13:13:36 2018
@@ -459,50 +459,83 @@ enum CastAwayConstnessKind {
 /// Like Sema::UnwrapSimilarTypes, this removes one level of indirection from
 /// both types, provided that they're both pointer-like or array-like. Unlike
 /// the Sema function, doesn't care if the unwrapped pieces are related.
+///
+/// This function may remove additional levels as necessary for correctness:
+/// the resulting T1 is unwrapped sufficiently that it is never an array type,
+/// so that its qualifiers can be directly compared to those of T2 (which will
+/// have the combined set of qualifiers from all indermediate levels of T2),
+/// as (effectively) required by [expr.const.cast]p7 replacing T1's qualifiers
+/// with those from T2.
 static CastAwayConstnessKind
 unwrapCastAwayConstnessLevel(ASTContext , QualType , QualType ) {
-  // 

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:23
+
+const char DefaultIgnoredValues[] = "0;1;2;10;100;";
+

Why 2, 10, and 100?



Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:30-32
+  for (const std::string  : IngnoredValuesInput) {
+IngnoredValues.push_back(std::stoll(IgnoredValue));
+  }

This can be replaced with `llvm::transform(IgnoredValuesInput, 
IgnoredValues.begin(), std::stoll);`



Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:33
+  }
+  std::sort(IngnoredValues.begin(), IngnoredValues.end());
+}

Please use `llvm::sort()` instead to help find non-determinism bugs. (I'm a bit 
surprised we don't have a range-based sort though.)



Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:37
+void MagicNumbersCheck::storeOptions(ClangTidyOptions::OptionMap ) {
+  Options.store(Opts, "IgnoredValues", DefaultIgnoredValues);
+}

`IgnoredIntegerValues` as the public facing option as well, unless you want to 
allow users to specify ignored floating-point values too (which could be useful 
as well).



Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:41-42
+void MagicNumbersCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(integerLiteral().bind("ii"), this);
+  Finder->addMatcher(floatLiteral().bind("ff"), this);
+}

The names `ii` and `ff` could be a bit more user-friendly. Also, this can be 
written using a single matcher, I think.
`anyOf(integerLiteral().bind("integer"), floatLiteral().bind("float"))`



Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:54
+  Result.Nodes.getNodeAs("ii");
+  if (MatchedInteger) {
+

Rather than indent everything for the typical case, I'd prefer this to be an 
early return.



Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:72
+
+diag(MatchedInteger->getLocation(), "magic number integer literal %0")
+<< Str.data();

This diagnostic text doesn't really tell the user what's wrong with their code 
or how to fix it. Also, this function is largely duplicated in 
`checkFloatMatch()`. I think they should be combined where possible and the 
diagnostic should read: `'%0' is a magic number; consider replacing with a 
named constant` or something along those lines.



Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:118
+const ast_type_traits::DynTypedNode ) const {
+  const VarDecl *AsVarDecl = Node.get();
+  if (AsVarDecl) {

Can use `const auto *` here as the type is spelled out in the initialization.



Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:119-123
+  if (AsVarDecl) {
+if (AsVarDecl->getType().isConstQualified()) {
+  return true;
+}
+  }

The `if` statements can be combined into a single statement and the braces can 
be elided.



Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:124-126
+  const FieldDecl *AsFieldDecl = Node.get();
+  if (AsFieldDecl) {
+if (AsFieldDecl->getType().isConstQualified()) {

Same here for `auto` and single statement.



Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:135-140
+  const SubstNonTypeTemplateParmExpr *AsTemplateArgument =
+  Node.get();
+  if (AsTemplateArgument) {
+return true;
+  }
+  return false;

`return Node.get != nullptr;` However, that makes 
me wonder why this should be a function at all.



Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:147
+
+  if (isConstantDefinition(Node)) {
+return true;

Elide braces



Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:151-158
+  for (const ast_type_traits::DynTypedNode  :
+   Result.Context->getParents(Node)) {
+if (isEventuallyConstant(Result, Parent)) {
+  return true;
+}
+  }
+

I think this could be replaced by a call to `llvm::any_of()`



Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:164
+const ast_type_traits::DynTypedNode ) const {
+  const EnumConstantDecl *AsEnumConstantDecl = Node.get();
+  if (AsEnumConstantDecl)

No need for this local variable.



Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:168-174
+  for (const ast_type_traits::DynTypedNode  :
+   Result.Context->getParents(Node)) {
+if (isEnumerationDefinition(Result, Parent))
+  return true;
+  }
+
+  return false;

`llvm::any_of()`



Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:183-186
+/*
+ * Ignore this instance, because this is the report where the template is
+ * defined, not where it is instantiated.
+ */

[PATCH] D49148: [DEBUGINFO] Disable unsupported debug info options for NVPTX target.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev updated this revision to Diff 156139.
ABataev added a comment.

Renamed function supportsNonDefaultDebugOptions() -> 
supportsDebugInfoOption(const Arg*). Instead of the enum it accepts and should 
check one of the debug info options.


Repository:
  rC Clang

https://reviews.llvm.org/D49148

Files:
  include/clang/Basic/DiagnosticDriverKinds.td
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Driver/ToolChain.h
  lib/Driver/ToolChains/Clang.cpp
  lib/Driver/ToolChains/Cuda.h
  test/Driver/cuda-unsupported-debug-options.cu
  test/Driver/openmp-unsupported-debug-options.c

Index: test/Driver/openmp-unsupported-debug-options.c
===
--- /dev/null
+++ test/Driver/openmp-unsupported-debug-options.c
@@ -0,0 +1,21 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// REQUIRES: nvptx-registered-target
+
+// RUN: %clang -### -target x86_64-linux-gnu -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda -c %s -g -gz 2>&1 | FileCheck %s
+// RUN: %clang -### -target x86_64-linux-gnu -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda -c %s -g -fdebug-info-for-profiling 2>&1 | FileCheck %s
+// RUN: %clang -### -target x86_64-linux-gnu -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda -c %s -g -gsplit-dwarf 2>&1 | FileCheck %s
+// RUN: %clang -### -target x86_64-linux-gnu -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda -c %s -g -glldb 2>&1 | FileCheck %s
+// RUN: %clang -### -target x86_64-linux-gnu -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda -c %s -g -gcodeview 2>&1 | FileCheck %s
+// RUN: %clang -### -target x86_64-linux-gnu -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda -c %s -g -gmodules 2>&1 | FileCheck %s
+// RUN: %clang -### -target x86_64-linux-gnu -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda -c %s -g -gembed-source -gdwarf-5 2>&1 | FileCheck %s
+// RUN: %clang -### -target x86_64-linux-gnu -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda -c %s -g -fdebug-macro 2>&1 | FileCheck %s
+// RUN: %clang -### -target x86_64-linux-gnu -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda -c %s -g -ggnu-pubnames 2>&1 | FileCheck %s
+// RUN: %clang -### -target x86_64-linux-gnu -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda -c %s -g -gdwarf-aranges 2>&1 | FileCheck %s
+// RUN: %clang -### -target x86_64-linux-gnu -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda -c %s -g -fdebug-types-section 2>&1 | FileCheck %s
+// CHECK: debug information option '{{-gz|-fdebug-info-for-profiling|-gsplit-dwarf|-glldb|-gcodeview|-gmodules|-gembed-source|-fdebug-macro|-ggnu-pubnames|-gdwarf-aranges|-fdebug-types-section}}' is not supported for target 'nvptx64-nvidia-cuda' [-Wunsupported-target-opt]
+// CHECK-NOT: debug information option '{{.*}}' is not supported for target 'x86
+// CHECK: "-triple" "nvptx64-nvidia-cuda"
+// CHECK-NOT: {{-compress-debug|-fdebug-info-for-profiling|split-dwarf|lldb|codeview|module-format|embed-source|debug-info-macro|gnu-pubnames|generate-arange-section|generate-type-units}}
+// CHECK: "-triple" "x86_64
+// CHECK-SAME: {{-compress-debug|-fdebug-info-for-profiling|split-dwarf|lldb|codeview|module-format|embed-source|debug-info-macro|gnu-pubnames|generate-arange-section|generate-type-units}}
Index: test/Driver/cuda-unsupported-debug-options.cu
===
--- /dev/null
+++ test/Driver/cuda-unsupported-debug-options.cu
@@ -0,0 +1,21 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// REQUIRES: nvptx-registered-target
+
+// RUN: %clang -### -target x86_64-linux-gnu -c %s -g -gz 2>&1 | FileCheck %s
+// RUN: %clang -### -target x86_64-linux-gnu -c %s -g -fdebug-info-for-profiling 2>&1 | FileCheck %s
+// RUN: %clang -### -target x86_64-linux-gnu -c %s -g -gsplit-dwarf 2>&1 | FileCheck %s
+// RUN: %clang -### -target x86_64-linux-gnu -c %s -g -glldb 2>&1 | FileCheck %s
+// RUN: %clang -### -target x86_64-linux-gnu -c %s -g -gcodeview 2>&1 | FileCheck %s
+// RUN: %clang -### -target x86_64-linux-gnu -c %s -g -gmodules 2>&1 | FileCheck %s
+// RUN: %clang -### -target x86_64-linux-gnu -c %s -g -gembed-source -gdwarf-5 2>&1 | FileCheck %s
+// RUN: %clang -### -target x86_64-linux-gnu -c %s -g -fdebug-macro 2>&1 | FileCheck %s
+// RUN: %clang -### -target x86_64-linux-gnu -c %s -g -ggnu-pubnames 2>&1 | FileCheck %s
+// RUN: %clang -### -target x86_64-linux-gnu -c %s -g -gdwarf-aranges 2>&1 | FileCheck %s
+// RUN: %clang -### -target x86_64-linux-gnu -c %s -g -fdebug-types-section 2>&1 | FileCheck %s
+// CHECK: debug information option '{{-gz|-fdebug-info-for-profiling|-gsplit-dwarf|-glldb|-gcodeview|-gmodules|-gembed-source|-fdebug-macro|-ggnu-pubnames|-gdwarf-aranges|-fdebug-types-section}}' is not supported for target 'nvptx64-nvidia-cuda' [-Wunsupported-target-opt]
+// CHECK-NOT: debug information option '{{.*}}' is not supported for target 'x86
+// CHECK: "-triple" "nvptx64-nvidia-cuda"
+// CHECK-NOT: 

r337420 - Add support for __declspec(code_seg("segname"))

[Erich Keane via cfe-commits]

Author: erichkeane
Date: Wed Jul 18 13:04:48 2018
New Revision: 337420

URL: http://llvm.org/viewvc/llvm-project?rev=337420=rev
Log:
Add support for __declspec(code_seg("segname"))

This patch uses CodeSegAttr to represent __declspec(code_seg) rather than 
building on the existing support for #pragma code_seg.
The code_seg declspec is applied on functions and classes. This attribute 
enables the placement of code into separate named segments, including compiler-
generated codes and template instantiations.

For more information, please see the following:
https://msdn.microsoft.com/en-us/library/dn636922.aspx

This patch fixes the regression for the support for attribute ((section).
https://github.com/llvm-mirror/clang/commit/746b78de7812bc785fbb5207b788348040b23fa7

Patch by Soumi Manna (Manna)
Differential Revision: https://reviews.llvm.org/D48841


Added:
cfe/trunk/test/CodeGenCXX/code-seg.cpp
cfe/trunk/test/CodeGenCXX/code-seg1.cpp
cfe/trunk/test/CodeGenCXX/code-seg2.cpp
cfe/trunk/test/CodeGenCXX/code-seg3.cpp
cfe/trunk/test/SemaCXX/code-seg.cpp
cfe/trunk/test/SemaCXX/code-seg1.cpp
Modified:
cfe/trunk/include/clang/Basic/Attr.td
cfe/trunk/include/clang/Basic/AttrDocs.td
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/CodeGen/CodeGenModule.cpp
cfe/trunk/lib/Sema/SemaDecl.cpp
cfe/trunk/lib/Sema/SemaDeclAttr.cpp
cfe/trunk/lib/Sema/SemaDeclCXX.cpp
cfe/trunk/lib/Sema/SemaLambda.cpp

Modified: cfe/trunk/include/clang/Basic/Attr.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=337420=337419=337420=diff
==
--- cfe/trunk/include/clang/Basic/Attr.td (original)
+++ cfe/trunk/include/clang/Basic/Attr.td Wed Jul 18 13:04:48 2018
@@ -1770,6 +1770,13 @@ def Section : InheritableAttr {
   let Documentation = [SectionDocs];
 }
 
+def CodeSeg : InheritableAttr {
+  let Spellings = [Declspec<"code_seg">];
+  let Args = [StringArgument<"Name">];
+  let Subjects = SubjectList<[Function, CXXRecord], ErrorDiag>;
+  let Documentation = [CodeSegDocs];
+}
+
 def PragmaClangBSSSection : InheritableAttr {
   // This attribute has no spellings as it is only ever created implicitly.
   let Spellings = [];

Modified: cfe/trunk/include/clang/Basic/AttrDocs.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/AttrDocs.td?rev=337420=337419=337420=diff
==
--- cfe/trunk/include/clang/Basic/AttrDocs.td (original)
+++ cfe/trunk/include/clang/Basic/AttrDocs.td Wed Jul 18 13:04:48 2018
@@ -306,6 +306,18 @@ An example of how to use ``alloc_size``
   }];
 }
 
+def CodeSegDocs : Documentation {
+  let Category = DocCatFunction;
+  let Content = [{
+The ``__declspec(code_seg)`` attribute enables the placement of code into 
separate
+named segments that can be paged or locked in memory individually. This 
attribute
+is used to control the placement of instantiated templates and 
compiler-generated
+code. See the documentation for `__declspec(code_seg)`_ on MSDN.
+
+.. _`__declspec(code_seg)`: 
http://msdn.microsoft.com/en-us/library/dn636922.aspx
+  }];
+}
+
 def AllocAlignDocs : Documentation {
   let Category = DocCatFunction;
   let Content = [{

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=337420=337419=337420=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Jul 18 13:04:48 
2018
@@ -2668,11 +2668,19 @@ def err_only_annotate_after_access_spec
   "access specifier can only have annotation attributes">;
 
 def err_attribute_section_invalid_for_target : Error<
-  "argument to 'section' attribute is not valid for this target: %0">;
+  "argument to %select{'code_seg'|'section'}1 attribute is not valid for this 
target: %0">;
 def warn_mismatched_section : Warning<
-  "section does not match previous declaration">, InGroup;
+  "%select{codeseg|section}0 does not match previous declaration">, 
InGroup;
 def warn_attribute_section_on_redeclaration : Warning<
   "section attribute is specified on redeclared variable">, InGroup;
+def err_mismatched_code_seg_base : Error<
+  "derived class must specify the same code segment as its base classes">;
+def err_mismatched_code_seg_override : Error<
+  "overriding virtual function must specify the same code segment as its 
overridden function">;
+def err_conflicting_codeseg_attribute : Error<
+  "conflicting code segment specifiers">;
+def warn_duplicate_codeseg_attribute : Warning<
+  "duplicate code segment specifiers">, InGroup;
 
 def err_anonymous_property: Error<
   "anonymous property is not supported">;

[PATCH] D49492: Run bounds checking sanitizer earlier to make it easier to optimize away its checks.

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

Are you sure this will actually do what you want, in general?  I suspect it 
will end up missing bounds checks in some cases because it's running it too 
early (before mem2reg/inlining/etc).


Repository:
  rC Clang

https://reviews.llvm.org/D49492



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

[PATCH] D49504: Enable C++2a Chrono Literals

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In https://reviews.llvm.org/D49504#1167045, @mclow.lists wrote:

> This looks ok to me - I'm assuming that somewhere there's a test for `int foo 
> = 0x123d`, so we're sure that didn't get broken.


The hex_d SHOULD be testing that, it ensures that the RHS is an integer.




Comment at: test/SemaCXX/cxx2a-user-defined-literals.cpp:8
+namespace std {
+  struct string {};
+  namespace chrono {

mclow.lists wrote:
> Do you need `string` here, or is this a copy-pasto?
Gah, good catch.  I was testing something else briefly.


Repository:
  rC Clang

https://reviews.llvm.org/D49504



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

[PATCH] D47474: Implement cpu_dispatch/cpu_specific Multiversioning

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: include/clang/Basic/Attr.td:851
+  let Spellings = [Clang<"cpu_specific">];
+  let Args = [VariadicIdentifierArgument<"Cpus">];
+  let Subjects = SubjectList<[Function]>;

aaron.ballman wrote:
> erichkeane wrote:
> > aaron.ballman wrote:
> > > `Cpus` -> `CPUs` ?
> > So, I agree with you we should be consistent with spelling.  However, in 
> > this case, I'd prefer not changing this one.  This is what the generated 
> > code in Attrs.inc looks like if I change it:
> > 
> > typedef IdentifierInfo ** cPUs_iterator;
> > cPUs_iterator cPUs_begin() const { return cPUs_; }
> > cPUs_iterator cPUs_end() const { return cPUs_ + cPUs_Size; }
> > unsigned cPUs_size() const { return cPUs_Size; }
> > llvm::iterator_range cPUs() const { return 
> > llvm::make_range(cPUs_begin(), cPUs_end()); }
> > 
> > I think having "Cpus" in 2 places is way better than having to spell it as 
> > cPUs_begin.  Your thoughts?
> Oh wow that is disgusting. Yes, please leave as `Cpus` here.
:-D  Already Done!


https://reviews.llvm.org/D47474



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

[PATCH] D49504: Enable C++2a Chrono Literals

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists added a comment.

This looks ok to me - I'm assuming that somewhere there's a test for `int foo = 
0x123d`, so we're sure that didn't get broken.




Comment at: test/SemaCXX/cxx2a-user-defined-literals.cpp:8
+namespace std {
+  struct string {};
+  namespace chrono {

Do you need `string` here, or is this a copy-pasto?


Repository:
  rC Clang

https://reviews.llvm.org/D49504



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

Re: r337381 - Mention clang-cl improvements from r335466 and r336379 in ReleaseNotes.rst

[Kim Gräsman via cfe-commits]

This is lovely! Found a bit inline...

On Wed, Jul 18, 2018, 14:00 Nico Weber via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: nico
> Date: Wed Jul 18 04:55:03 2018
> New Revision: 337381
>
> URL: http://llvm.org/viewvc/llvm-project?rev=337381=rev
> Log:
> Mention clang-cl improvements from r335466 and r336379 in ReleaseNotes.rst
>
> Modified:
> cfe/trunk/docs/ReleaseNotes.rst
>
> Modified: cfe/trunk/docs/ReleaseNotes.rst
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ReleaseNotes.rst?rev=337381=337380=337381=diff
>
> ==
> --- cfe/trunk/docs/ReleaseNotes.rst (original)
> +++ cfe/trunk/docs/ReleaseNotes.rst Wed Jul 18 04:55:03 2018
> @@ -166,7 +166,19 @@ Attribute Changes in Clang
>  Windows Support
>  ---
>
> -Clang's support for building native Windows programs ...
> +- clang-cl's support for precompiled headers has been much improved:
> +
> +   - When using a pch file, clang-cl now no longer redundantly emits
> inline
> + methods that are already stored in the obj that was built together
> with
> + the pch file (matching cl.exe).  This speeds up builds using pch
> files
> + by around 30%.
> +
> +   - The /Ycfoo.h and /Yufoo.h flags an now be used without /FIfoo.h when
>
> typo: an - > can
>
> - Kim
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49504: Enable C++2a Chrono Literals

[Erich Keane via Phabricator via cfe-commits]

erichkeane created this revision.
erichkeane added reviewers: rsmith, gribozavr, ddunbar, eli.friedman.
Herald added a subscriber: cfe-commits.

C++2a via http://wg21.link/p0355 permits the library
literals of 'd' and 'y'.  This patch enables them in the
Lexer so that they can be properly parsed.

Note that 'd' gets confused with the hex character, so 
modifications to how octal, binary, and decimal numbers are
parsed were required. Since this is simply making previously 
invalid code legal, this should be fine.

Hex still greedily parses the 'd' as a hexit, since it would 
a: violate [lex.ext]p1
b: break existing code.


Repository:
  rC Clang

https://reviews.llvm.org/D49504

Files:
  lib/Lex/LiteralSupport.cpp
  test/SemaCXX/cxx2a-user-defined-literals.cpp


Index: lib/Lex/LiteralSupport.cpp
===
--- lib/Lex/LiteralSupport.cpp
+++ lib/Lex/LiteralSupport.cpp
@@ -751,7 +751,8 @@
 
   // If we have a hex digit other than 'e' (which denotes a FP exponent) then
   // the code is using an incorrect base.
-  if (isHexDigit(*s) && *s != 'e' && *s != 'E') {
+  if (isHexDigit(*s) && *s != 'e' && *s != 'E' &&
+  !isValidUDSuffix(PP.getLangOpts(), StringRef(s, ThisTokEnd - s))) {
 PP.Diag(PP.AdvanceToTokenCharacter(TokLoc, s-ThisTokBegin),
 diag::err_invalid_digit) << StringRef(s, 1) << (radix == 8 ? 1 : 
0);
 hadError = true;
@@ -804,12 +805,14 @@
   if (!LangOpts.CPlusPlus14)
 return false;
 
-  // In C++1y, "s", "h", "min", "ms", "us", and "ns" are used in the library.
+  // In C++14, "s", "h", "min", "ms", "us", and "ns" are used in the library.
   // Per tweaked N3660, "il", "i", and "if" are also used in the library.
+  // In C++2a "d" and "y" are used in the library.
   return llvm::StringSwitch(Suffix)
   .Cases("h", "min", "s", true)
   .Cases("ms", "us", "ns", true)
   .Cases("il", "i", "if", true)
+  .Cases("d", "y", LangOpts.CPlusPlus2a)
   .Default(false);
 }
 
@@ -922,7 +925,9 @@
 s = SkipBinaryDigits(s);
 if (s == ThisTokEnd) {
   // Done.
-} else if (isHexDigit(*s)) {
+} else if (isHexDigit(*s) &&
+   !isValidUDSuffix(PP.getLangOpts(),
+StringRef(s, ThisTokEnd - s))) {
   PP.Diag(PP.AdvanceToTokenCharacter(TokLoc, s-ThisTokBegin),
   diag::err_invalid_digit) << StringRef(s, 1) << 2;
   hadError = true;
Index: test/SemaCXX/cxx2a-user-defined-literals.cpp
===
--- test/SemaCXX/cxx2a-user-defined-literals.cpp
+++ test/SemaCXX/cxx2a-user-defined-literals.cpp
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -std=c++2a %s -include %s -verify
+
+#ifndef INCLUDED
+#define INCLUDED
+
+#pragma clang system_header
+namespace std {
+  struct string {};
+  namespace chrono {
+struct day{};
+struct year{};
+  }
+  using size_t = decltype(sizeof(0));
+  constexpr chrono::day operator"" d(unsigned long long d) noexcept;
+  constexpr chrono::year operator"" y(unsigned long long y) noexcept;
+}
+
+#else
+
+using namespace std;
+chrono::day dec_d = 5d;
+chrono::day oct_d = 05d;
+chrono::day bin_d = 0b011d;
+// expected-error@+3{{no viable conversion from 'int' to 'chrono::day'}}
+// expected-note@11{{candidate constructor (the implicit copy constructor)}}
+// expected-note@11{{candidate constructor (the implicit move constructor)}}
+chrono::day hex_d = 0x44d;
+chrono::year y  = 10y;
+#endif


Index: lib/Lex/LiteralSupport.cpp
===
--- lib/Lex/LiteralSupport.cpp
+++ lib/Lex/LiteralSupport.cpp
@@ -751,7 +751,8 @@
 
   // If we have a hex digit other than 'e' (which denotes a FP exponent) then
   // the code is using an incorrect base.
-  if (isHexDigit(*s) && *s != 'e' && *s != 'E') {
+  if (isHexDigit(*s) && *s != 'e' && *s != 'E' &&
+  !isValidUDSuffix(PP.getLangOpts(), StringRef(s, ThisTokEnd - s))) {
 PP.Diag(PP.AdvanceToTokenCharacter(TokLoc, s-ThisTokBegin),
 diag::err_invalid_digit) << StringRef(s, 1) << (radix == 8 ? 1 : 0);
 hadError = true;
@@ -804,12 +805,14 @@
   if (!LangOpts.CPlusPlus14)
 return false;
 
-  // In C++1y, "s", "h", "min", "ms", "us", and "ns" are used in the library.
+  // In C++14, "s", "h", "min", "ms", "us", and "ns" are used in the library.
   // Per tweaked N3660, "il", "i", and "if" are also used in the library.
+  // In C++2a "d" and "y" are used in the library.
   return llvm::StringSwitch(Suffix)
   .Cases("h", "min", "s", true)
   .Cases("ms", "us", "ns", true)
   .Cases("il", "i", "if", true)
+  .Cases("d", "y", LangOpts.CPlusPlus2a)
   .Default(false);
 }
 
@@ -922,7 +925,9 @@
 s = SkipBinaryDigits(s);
 if (s == ThisTokEnd) {
   // Done.
-} else if (isHexDigit(*s)) {
+} else if (isHexDigit(*s) &&
+   !isValidUDSuffix(PP.getLangOpts(),
+StringRef(s, 

[PATCH] D46230: For x86_64, gcc 7.2 under Amazon Linux AMI sets its paths to x86_64-amazon-linux

[Eric Christopher via Phabricator via cfe-commits]

echristo added a comment.

In https://reviews.llvm.org/D46230#1166958, @gaijiading wrote:

> In https://reviews.llvm.org/D46230#1166919, @echristo wrote:
>
> > LGTM.
> >
> > -eric
>
>
> Hi Eric, I do not have commit access to trunk. Could you commit the change 
> for me? Thanks.


The binaries make that hard, can you email me a tarball please?


Repository:
  rC Clang

https://reviews.llvm.org/D46230



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

[PATCH] D49421: [CodeComplete] Fix accessibilty of protected members from base class.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: lib/Sema/SemaAccess.cpp:1871-1873
+// The access should be AS_none as we don't know how the member was
+// accessed - `AccessedEntity::getAccess` describes what access was used to
+// access an entity.

ioeric wrote:
> aaron.ballman wrote:
> > This seems to contradict the function-level comment that says the check 
> > seems to only take the access specifiers into account and not how the 
> > member is accessed.
> > not how the member is accessed.
> It's not very clear to me what exactly this means.  But I think we are still 
> not taking how member was accessed into account. The access in 
> `DeclAccessPair` describes how a member was accessed, and this should be None 
> instead of the access specifier of the member as we don't have this 
> information here. 
> 
> I updated the wording in the comment to make this a bit clearer.
I'm still wondering about the removal of the public access check -- if the 
declaration has a public access specifier, that early return saves work. So 
`Entity` may still require passing `AS_none` but we might want to continue to 
check `Decl->getAccess() == AS_public` for the early return?

(Drive-by nit that's not yours: `Decl` is the name of a type, so if you wanted 
to rename the parameter to something that isn't a type name, I would not be 
sad, but you're not required to do it.)



Comment at: lib/Sema/SemaCodeComplete.cpp:1316-1317
+//   void D::f() { this->^; }
+// The completion after "this->" will have `InBaseClass` set to true 
and
+// `Ctx` set to "B". We need to set the actual accessing context (i.e.
+// naming class) to "D" so that access can be calculated correctly.

ioeric wrote:
> aaron.ballman wrote:
> > This makes it sound like the caller has messed something up, but I'm also 
> > pretty unfamiliar with the code completion logic. Why would `InBaseClass` 
> > be true for that completion point and why would the context be set to `B`?
> It's a bit messy, but this is roughly how lookup inside a class, say D,works:
> 1. lookup inside the class (InBaseClass = false, Context=D).
> 2. Run the same lookup on all base classes (InBaseClass=true, Context=B). 
> (https://github.com/llvm-mirror/clang/blob/master/lib/Sema/SemaLookup.cpp#L3584)
> 
> So, `InBaseClass` would be true when a lookup starts from a derived class. 
Ah, thank you for the explanation; that helps.


Repository:
  rC Clang

https://reviews.llvm.org/D49421



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

[PATCH] D49502: [CMake] Support statically linking dependencies only to shared or static library

[Louis Dionne via Phabricator via cfe-commits]

ldionne added inline comments.



Comment at: libcxx/CMakeLists.txt:158
 
+cmake_dependent_option(LIBCXX_ENABLE_STATIC_ABI_LIBRARY_STATIC
+  "Statically link the ABI library to static library" ON

phosek wrote:
> Maybe `LIBCXX_ENABLE_STATIC_ABI_LIBRARY_IN_STATIC_LIBRARY` would be more 
> accurate?
`LIBCXX_STATICALLY_LINK_ABI_IN_STATIC_LIBRARY` and 
`LIBCXX_STATICALLY_LINK_ABI_IN_SHARED_LIBRARY`?

And then 
`LIBCXXABI_STATICALLY_LINK_UNWINDER_IN_STATIC_LIBRARY`/`LIBCXXABI_STATICALLY_LINK_UNWINDER_IN_SHARED_LIBRARY`.



Comment at: libcxx/lib/CMakeLists.txt:269
+AND (TARGET cxxabi_static OR HAVE_LIBCXXABI))
+#if ((TARGET ${LIBCXX_CXX_ABI_LIBRARY}) OR
+#(${LIBCXX_CXX_ABI_LIBRARY} MATCHES "cxxabi(_static|_shared)?" AND 
HAVE_LIBCXXABI))

I don't understand why any of this needs to change -- can you please explain? 
Also, you probably didn't mean to leave the commented-out lines.


Repository:
  rCXX libc++

https://reviews.llvm.org/D49502



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

[PATCH] D49502: [CMake] Support statically linking dependencies only to shared or static library

[Petr Hosek via Phabricator via cfe-commits]

phosek added inline comments.



Comment at: libcxx/CMakeLists.txt:158
 
+cmake_dependent_option(LIBCXX_ENABLE_STATIC_ABI_LIBRARY_STATIC
+  "Statically link the ABI library to static library" ON

Maybe `LIBCXX_ENABLE_STATIC_ABI_LIBRARY_IN_STATIC_LIBRARY` would be more 
accurate?


Repository:
  rCXX libc++

https://reviews.llvm.org/D49502



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

[PATCH] D49502: [CMake] Support statically linking dependencies only to shared or static library

[Petr Hosek via Phabricator via cfe-commits]

phosek added a comment.

This came up in Fuchsia. Clang driver, add `-lc++` as a dependency. This is 
sufficient for shared libc++, but for static user has to manually pass 
`-lc++abi -lunwind`, so for static libc++ we would actually like to combine 
both libc++abi and libunwind into a single archive.


Repository:
  rCXX libc++

https://reviews.llvm.org/D49502



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

[PATCH] D49502: [CMake] Support statically linking dependencies only to shared or static library

[Petr Hosek via Phabricator via cfe-commits]

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

Currently it's possible to select whether to statically link unwinder
or the C++ ABI library, but this option applies to both the shared
and static library. However, in some scenarios it may be desirable to
only statically link unwinder and C++ ABI library into static C++
library since for shared C++ library we can rely on dynamic linking
and linker scripts. This change enables selectively enabling or
disabling statically linking only to shared or static library.


Repository:
  rCXX libc++

https://reviews.llvm.org/D49502

Files:
  libcxx/CMakeLists.txt
  libcxx/cmake/Modules/HandleLibCXXABI.cmake
  libcxx/lib/CMakeLists.txt
  libcxxabi/CMakeLists.txt
  libcxxabi/src/CMakeLists.txt

Index: libcxxabi/src/CMakeLists.txt
===
--- libcxxabi/src/CMakeLists.txt
+++ libcxxabi/src/CMakeLists.txt
@@ -61,9 +61,9 @@
   # Prefer using the in-tree version of libunwind, either shared or static. If
   # none are found fall back to using -lunwind.
   # FIXME: Is it correct to prefer the static version of libunwind?
-  if (NOT LIBCXXABI_ENABLE_STATIC_UNWINDER AND (TARGET unwind_shared OR HAVE_LIBUNWIND))
+  if (NOT LIBCXXABI_ENABLE_STATIC_UNWINDER_SHARED AND (TARGET unwind_shared OR HAVE_LIBUNWIND))
 list(APPEND LIBCXXABI_LIBRARIES unwind_shared)
-  elseif (LIBCXXABI_ENABLE_STATIC_UNWINDER AND (TARGET unwind_static OR HAVE_LIBUNWIND))
+  elseif (LIBCXXABI_ENABLE_STATIC_UNWINDER_STATIC AND (TARGET unwind_static OR HAVE_LIBUNWIND))
 list(APPEND LIBCXXABI_LIBRARIES unwind_static)
   else()
 list(APPEND LIBCXXABI_LIBRARIES unwind)
@@ -155,7 +155,7 @@
 # Build the static library.
 if (LIBCXXABI_ENABLE_STATIC)
   set(cxxabi_static_sources $)
-  if (LIBCXXABI_USE_LLVM_UNWINDER AND LIBCXXABI_ENABLE_STATIC_UNWINDER)
+  if (LIBCXXABI_USE_LLVM_UNWINDER AND LIBCXXABI_ENABLE_STATIC_UNWINDER_STATIC)
 if (TARGET unwind_static OR HAVE_LIBUNWIND)
   list(APPEND cxxabi_static_sources $)
 endif()
Index: libcxxabi/CMakeLists.txt
===
--- libcxxabi/CMakeLists.txt
+++ libcxxabi/CMakeLists.txt
@@ -84,6 +84,13 @@
 option(LIBCXXABI_ENABLE_SHARED "Build libc++abi as a shared library." ON)
 option(LIBCXXABI_ENABLE_STATIC "Build libc++abi as a static library." ON)
 
+cmake_dependent_option(LIBCXXABI_ENABLE_STATIC_UNWINDER_STATIC
+  "Statically link the LLVM unwinder to static library" ON
+  "LIBCXXABI_ENABLE_STATIC_UNWINDER;LIBCXXABI_ENABLE_STATIC" OFF)
+cmake_dependent_option(LIBCXXABI_ENABLE_STATIC_UNWINDER_SHARED
+  "Statically link the LLVM unwinder to shared library" ON
+  "LIBCXXABI_ENABLE_STATIC_UNWINDER;LIBCXXABI_ENABLE_SHARED" OFF)
+
 option(LIBCXXABI_BAREMETAL "Build libc++abi for baremetal targets." OFF)
 # The default terminate handler attempts to demangle uncaught exceptions, which
 # causes extra I/O and demangling code to be pulled in.
Index: libcxx/lib/CMakeLists.txt
===
--- libcxx/lib/CMakeLists.txt
+++ libcxx/lib/CMakeLists.txt
@@ -44,7 +44,7 @@
   set(LIBCXX_OSX_REEXPORT_SYSTEM_ABI_LIBRARY ON)
 endif()
 
-if (LIBCXX_ENABLE_STATIC_ABI_LIBRARY)
+if (LIBCXX_ENABLE_STATIC_ABI_LIBRARY_SHARED)
   add_library_flags("-Wl,--whole-archive" "-Wl,-Bstatic")
   add_library_flags("${LIBCXX_CXX_ABI_LIBRARY}")
   add_library_flags("-Wl,-Bdynamic" "-Wl,--no-whole-archive")
@@ -259,14 +259,16 @@
 
   list(APPEND LIBCXX_TARGETS "cxx_static")
   # Attempt to merge the libc++.a archive and the ABI library archive into one.
-  if (LIBCXX_ENABLE_STATIC_ABI_LIBRARY)
+  if (LIBCXX_ENABLE_STATIC_ABI_LIBRARY_STATIC)
 set(MERGE_ARCHIVES_SEARCH_PATHS "")
 if (LIBCXX_CXX_ABI_LIBRARY_PATH)
   set(MERGE_ARCHIVES_SEARCH_PATHS "-L${LIBCXX_CXX_ABI_LIBRARY_PATH}")
 endif()
-if ((TARGET ${LIBCXX_CXX_ABI_LIBRARY}) OR
-(${LIBCXX_CXX_ABI_LIBRARY} MATCHES "cxxabi(_static|_shared)?" AND HAVE_LIBCXXABI))
-  set(MERGE_ARCHIVES_ABI_TARGET "$")
+if (${LIBCXX_CXX_ABI_LIBRARY} MATCHES "cxxabi(_static|_shared)?"
+AND (TARGET cxxabi_static OR HAVE_LIBCXXABI))
+#if ((TARGET ${LIBCXX_CXX_ABI_LIBRARY}) OR
+#(${LIBCXX_CXX_ABI_LIBRARY} MATCHES "cxxabi(_static|_shared)?" AND HAVE_LIBCXXABI))
+  set(MERGE_ARCHIVES_ABI_TARGET "$")
 else()
   set(MERGE_ARCHIVES_ABI_TARGET
   "${CMAKE_STATIC_LIBRARY_PREFIX}${LIBCXX_CXX_ABI_LIBRARY}${CMAKE_STATIC_LIBRARY_SUFFIX}")
Index: libcxx/cmake/Modules/HandleLibCXXABI.cmake
===
--- libcxx/cmake/Modules/HandleLibCXXABI.cmake
+++ libcxx/cmake/Modules/HandleLibCXXABI.cmake
@@ -104,10 +104,10 @@
 elseif ("${LIBCXX_CXX_ABI_LIBNAME}" STREQUAL "libcxxabi")
   if (LIBCXX_CXX_ABI_INTREE)
 # Link against just-built "cxxabi" target.
-if (LIBCXX_ENABLE_STATIC_ABI_LIBRARY)
-   

[PATCH] D47474: Implement cpu_dispatch/cpu_specific Multiversioning

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: include/clang/Basic/Attr.td:851
+  let Spellings = [Clang<"cpu_specific">];
+  let Args = [VariadicIdentifierArgument<"Cpus">];
+  let Subjects = SubjectList<[Function]>;

erichkeane wrote:
> aaron.ballman wrote:
> > `Cpus` -> `CPUs` ?
> So, I agree with you we should be consistent with spelling.  However, in this 
> case, I'd prefer not changing this one.  This is what the generated code in 
> Attrs.inc looks like if I change it:
> 
> typedef IdentifierInfo ** cPUs_iterator;
> cPUs_iterator cPUs_begin() const { return cPUs_; }
> cPUs_iterator cPUs_end() const { return cPUs_ + cPUs_Size; }
> unsigned cPUs_size() const { return cPUs_Size; }
> llvm::iterator_range cPUs() const { return 
> llvm::make_range(cPUs_begin(), cPUs_end()); }
> 
> I think having "Cpus" in 2 places is way better than having to spell it as 
> cPUs_begin.  Your thoughts?
Oh wow that is disgusting. Yes, please leave as `Cpus` here.


https://reviews.llvm.org/D47474



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

[PATCH] D46230: For x86_64, gcc 7.2 under Amazon Linux AMI sets its paths to x86_64-amazon-linux

[Jiading Gai via Phabricator via cfe-commits]

gaijiading added a comment.

In https://reviews.llvm.org/D46230#1166919, @echristo wrote:

> LGTM.
>
> -eric


Hi Eric, I do not have commit access to trunk. Could you commit the change for 
me? Thanks.


Repository:
  rC Clang

https://reviews.llvm.org/D46230



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

r337417 - [analyzer] Remove a debug print that was accidentally left around.

[Artem Dergachev via cfe-commits]

Author: dergachev
Date: Wed Jul 18 11:44:40 2018
New Revision: 337417

URL: http://llvm.org/viewvc/llvm-project?rev=337417=rev
Log:
[analyzer] Remove a debug print that was accidentally left around.

No functional change intended.

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

Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp?rev=337417=337416=337417=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp Wed Jul 18 11:44:40 2018
@@ -2562,7 +2562,6 @@ generateVisitorsDiagnostics(BugReport *R
   assert(!LastPiece &&
  "There can only be one final piece in a diagnostic.");
   LastPiece = std::move(Piece);
-  llvm::errs() << "Writing to last piece" << "\n";
   (*Notes)[ErrorNode].push_back(LastPiece);
 }
   }


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

[PATCH] D46230: For x86_64, gcc 7.2 under Amazon Linux AMI sets its paths to x86_64-amazon-linux

[Jiading Gai via Phabricator via cfe-commits]

gaijiading added a comment.

Thanks very much for the comments and the review.


Repository:
  rC Clang

https://reviews.llvm.org/D46230



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

[PATCH] D46230: For x86_64, gcc 7.2 under Amazon Linux AMI sets its paths to x86_64-amazon-linux

[Eric Christopher via Phabricator via cfe-commits]

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

LGTM.

-eric


Repository:
  rC Clang

https://reviews.llvm.org/D46230



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

[PATCH] D46230: For x86_64, gcc 7.2 under Amazon Linux AMI sets its paths to x86_64-amazon-linux

[Jiading Gai via Phabricator via cfe-commits]

gaijiading updated this revision to Diff 156117.
gaijiading added a comment.

Include the full context in the update diff.


Repository:
  rC Clang

https://reviews.llvm.org/D46230

Files:
  lib/Driver/ToolChains/Gnu.cpp
  test/Driver/Inputs/ami_linux_tree/usr/lib/gcc/x86_64-amazon-linux/7/crtbegin.o
  
test/Driver/Inputs/ami_linux_tree/usr/lib/gcc/x86_64-amazon-linux/7/crtbeginT.o
  test/Driver/Inputs/ami_linux_tree/usr/lib/gcc/x86_64-amazon-linux/7/crtend.o
  test/Driver/Inputs/ami_linux_tree/usr/lib64/crt1.o
  test/Driver/Inputs/ami_linux_tree/usr/lib64/crti.o
  test/Driver/Inputs/ami_linux_tree/usr/lib64/crtn.o
  test/Driver/linux-ld.c


Index: test/Driver/linux-ld.c
===
--- test/Driver/linux-ld.c
+++ test/Driver/linux-ld.c
@@ -1792,3 +1792,25 @@
 // CHECK-LD-RHLE7-DTS: Selected GCC installation: 
[[GCC_INSTALL:[[SYSROOT]]/lib/gcc/x86_64-redhat-linux/7]]
 // CHECK-LD-RHEL7-DTS-NOT: /usr/bin/ld
 // CHECK-LD-RHLE7-DTS: [[GCC_INSTALL]/../../../bin/ld
+
+// Check whether gcc7 install works fine on Amazon Linux AMI
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=x86_64-amazon-linux -rtlib=libgcc \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/ami_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-LD-AMI %s
+// CHECK-LD-AMI-NOT: warning:
+// CHECK-LD-AMI: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-LD-AMI: "--eh-frame-hdr"
+// CHECK-LD-AMI: "-m" "elf_x86_64"
+// CHECK-LD-AMI: "-dynamic-linker"
+// CHECK-LD-AMI: "{{.*}}/usr/lib/gcc/x86_64-amazon-linux/7{{/|}}crtbegin.o"
+// CHECK-LD-AMI: "-L[[SYSROOT]]/usr/lib/gcc/x86_64-amazon-linux/7"
+// CHECK-LD-AMI: 
"-L[[SYSROOT]]/usr/lib/gcc/x86_64-amazon-linux/7/../../../../lib64"
+// CHECK-LD-AMI: "-L[[SYSROOT]]/usr/lib/gcc/x86_64-amazon-linux/7/../../.."
+// CHECK-LD-AMI: "-L[[SYSROOT]]/lib"
+// CHECK-LD-AMI: "-L[[SYSROOT]]/usr/lib"
+// CHECK-LD-AMI: "-lgcc" "--as-needed" "-lgcc_s" "--no-as-needed"
+// CHECK-LD-AMI: "-lc"
+// CHECK-LD-AMI: "-lgcc" "--as-needed" "-lgcc_s" "--no-as-needed"
+//
Index: lib/Driver/ToolChains/Gnu.cpp
===
--- lib/Driver/ToolChains/Gnu.cpp
+++ lib/Driver/ToolChains/Gnu.cpp
@@ -1838,7 +1838,8 @@
   "x86_64-pc-linux-gnu","x86_64-redhat-linux6E",
   "x86_64-redhat-linux","x86_64-suse-linux",
   "x86_64-manbo-linux-gnu", "x86_64-linux-gnu",
-  "x86_64-slackware-linux", "x86_64-unknown-linux"};
+  "x86_64-slackware-linux", "x86_64-unknown-linux",
+  "x86_64-amazon-linux"};
   static const char *const X32LibDirs[] = {"/libx32"};
   static const char *const X86LibDirs[] = {"/lib32", "/lib"};
   static const char *const X86Triples[] = {


Index: test/Driver/linux-ld.c
===
--- test/Driver/linux-ld.c
+++ test/Driver/linux-ld.c
@@ -1792,3 +1792,25 @@
 // CHECK-LD-RHLE7-DTS: Selected GCC installation: [[GCC_INSTALL:[[SYSROOT]]/lib/gcc/x86_64-redhat-linux/7]]
 // CHECK-LD-RHEL7-DTS-NOT: /usr/bin/ld
 // CHECK-LD-RHLE7-DTS: [[GCC_INSTALL]/../../../bin/ld
+
+// Check whether gcc7 install works fine on Amazon Linux AMI
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=x86_64-amazon-linux -rtlib=libgcc \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/ami_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-LD-AMI %s
+// CHECK-LD-AMI-NOT: warning:
+// CHECK-LD-AMI: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-LD-AMI: "--eh-frame-hdr"
+// CHECK-LD-AMI: "-m" "elf_x86_64"
+// CHECK-LD-AMI: "-dynamic-linker"
+// CHECK-LD-AMI: "{{.*}}/usr/lib/gcc/x86_64-amazon-linux/7{{/|}}crtbegin.o"
+// CHECK-LD-AMI: "-L[[SYSROOT]]/usr/lib/gcc/x86_64-amazon-linux/7"
+// CHECK-LD-AMI: "-L[[SYSROOT]]/usr/lib/gcc/x86_64-amazon-linux/7/../../../../lib64"
+// CHECK-LD-AMI: "-L[[SYSROOT]]/usr/lib/gcc/x86_64-amazon-linux/7/../../.."
+// CHECK-LD-AMI: "-L[[SYSROOT]]/lib"
+// CHECK-LD-AMI: "-L[[SYSROOT]]/usr/lib"
+// CHECK-LD-AMI: "-lgcc" "--as-needed" "-lgcc_s" "--no-as-needed"
+// CHECK-LD-AMI: "-lc"
+// CHECK-LD-AMI: "-lgcc" "--as-needed" "-lgcc_s" "--no-as-needed"
+//
Index: lib/Driver/ToolChains/Gnu.cpp
===
--- lib/Driver/ToolChains/Gnu.cpp
+++ lib/Driver/ToolChains/Gnu.cpp
@@ -1838,7 +1838,8 @@
   "x86_64-pc-linux-gnu","x86_64-redhat-linux6E",
   "x86_64-redhat-linux","x86_64-suse-linux",
   "x86_64-manbo-linux-gnu", "x86_64-linux-gnu",
-  "x86_64-slackware-linux", "x86_64-unknown-linux"};
+  "x86_64-slackware-linux", "x86_64-unknown-linux",
+  "x86_64-amazon-linux"};
   static const char *const X32LibDirs[] = {"/libx32"};
   static const char *const X86LibDirs[] = {"/lib32", "/lib"};
   static const char *const X86Triples[] = {
___
cfe-commits mailing list

[PATCH] D49148: [DEBUGINFO] Disable unsupported debug info options for NVPTX target.

[Eric Christopher via Phabricator via cfe-commits]

echristo added a comment.

In https://reviews.llvm.org/D49148#1166859, @echristo wrote:

> In https://reviews.llvm.org/D49148#1166429, @ABataev wrote:
>
> > In https://reviews.llvm.org/D49148#1165826, @echristo wrote:
> >
> > > I think you should break it out on an option by option basis. Just 
> > > warning on "non-standard" options won't make as much sense to end users. 
> > > Perhaps a "this option is unsupported on the target you're compiling for" 
> > > etc.
> > >
> > > Thoughts?
> >
> >
> >
> >
> > 1. I can split it, no problems.
> > 2. Hmm, actually this what the warning already says. If the option is not 
> > supported it says 'debug option '-xxx' is not supported for target 
> > 'xxx-xxx-xxx''. It does not seem to me like a warning on non-standard 
> > option.
>
>
> Let me try to elaborate a bit, I agree that I'm not very clear above.
>
> I'm not a fan of the generic "non default debug options". It feels 
> misleading. I think we probably want to separate it by "dwarf extensions", 
> and "dwarf version".
>
> As far as the error message itself: "debug option" sounds like an option to 
> debug clang rather than a debug information option. Perhaps say "debug 
> information option" rather than "debug option"?


Summarizing offline (irc) conversation:

Let's keep the generic solution that Alexey was using here, but instead migrate 
it to a "supports debug flag" routine that uses enums to check for which things 
we support. This way targets that don't quite support various levels of debug 
info or extensions can define them for themselves and even check OS versions.

(This would probably have been useful for darwin in the past and still might 
be).


Repository:
  rC Clang

https://reviews.llvm.org/D49148



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

[PATCH] D49458: Support implicit _Atomic struct load / store

[JF Bastien via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL337410: Support implicit _Atomic struct load / store 
(authored by jfb, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D49458

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


Index: cfe/trunk/test/CodeGen/atomic-ops.c
===
--- cfe/trunk/test/CodeGen/atomic-ops.c
+++ cfe/trunk/test/CodeGen/atomic-ops.c
@@ -183,6 +183,18 @@
   double x;
 };
 
+void implicit_store(_Atomic(struct S) *a, struct S s) {
+  // CHECK-LABEL: @implicit_store(
+  // CHECK: store atomic i64 %{{.*}}, i64* %{{.*}} seq_cst, align 8
+  *a = s;
+}
+
+struct S implicit_load(_Atomic(struct S) *a) {
+  // CHECK-LABEL: @implicit_load(
+  // CHECK: load atomic i64, i64* %{{.*}} seq_cst, align 8
+  return *a;
+}
+
 struct S fd1(struct S *a) {
   // CHECK-LABEL: @fd1
   // CHECK: [[RETVAL:%.*]] = alloca %struct.S, align 4
Index: cfe/trunk/lib/Sema/Sema.cpp
===
--- cfe/trunk/lib/Sema/Sema.cpp
+++ cfe/trunk/lib/Sema/Sema.cpp
@@ -481,6 +481,7 @@
 case CK_ArrayToPointerDecay:
 case CK_FunctionToPointerDecay:
 case CK_ToVoid:
+case CK_NonAtomicToAtomic:
   break;
 }
   }


Index: cfe/trunk/test/CodeGen/atomic-ops.c
===
--- cfe/trunk/test/CodeGen/atomic-ops.c
+++ cfe/trunk/test/CodeGen/atomic-ops.c
@@ -183,6 +183,18 @@
   double x;
 };
 
+void implicit_store(_Atomic(struct S) *a, struct S s) {
+  // CHECK-LABEL: @implicit_store(
+  // CHECK: store atomic i64 %{{.*}}, i64* %{{.*}} seq_cst, align 8
+  *a = s;
+}
+
+struct S implicit_load(_Atomic(struct S) *a) {
+  // CHECK-LABEL: @implicit_load(
+  // CHECK: load atomic i64, i64* %{{.*}} seq_cst, align 8
+  return *a;
+}
+
 struct S fd1(struct S *a) {
   // CHECK-LABEL: @fd1
   // CHECK: [[RETVAL:%.*]] = alloca %struct.S, align 4
Index: cfe/trunk/lib/Sema/Sema.cpp
===
--- cfe/trunk/lib/Sema/Sema.cpp
+++ cfe/trunk/lib/Sema/Sema.cpp
@@ -481,6 +481,7 @@
 case CK_ArrayToPointerDecay:
 case CK_FunctionToPointerDecay:
 case CK_ToVoid:
+case CK_NonAtomicToAtomic:
   break;
 }
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r337410 - Support implicit _Atomic struct load / store

[JF Bastien via cfe-commits]

Author: jfb
Date: Wed Jul 18 11:01:41 2018
New Revision: 337410

URL: http://llvm.org/viewvc/llvm-project?rev=337410=rev
Log:
Support implicit _Atomic struct load / store

Summary:
Using _Atomic to do implicit load / store is just a seq_cst atomic_load / 
atomic_store. Stores currently assert in Sema::ImpCastExprToType with 'can't 
implicitly cast lvalue to rvalue with this cast kind', but that's erroneous. 
The codegen is fine as the test shows.

While investigating I found that Richard had found the problem here: 
https://reviews.llvm.org/D46112#1113557



Reviewers: dexonsmith

Subscribers: cfe-commits, efriedma, rsmith, aaron.ballman

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

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

Modified: cfe/trunk/lib/Sema/Sema.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Sema.cpp?rev=337410=337409=337410=diff
==
--- cfe/trunk/lib/Sema/Sema.cpp (original)
+++ cfe/trunk/lib/Sema/Sema.cpp Wed Jul 18 11:01:41 2018
@@ -481,6 +481,7 @@ ExprResult Sema::ImpCastExprToType(Expr
 case CK_ArrayToPointerDecay:
 case CK_FunctionToPointerDecay:
 case CK_ToVoid:
+case CK_NonAtomicToAtomic:
   break;
 }
   }

Modified: cfe/trunk/test/CodeGen/atomic-ops.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/atomic-ops.c?rev=337410=337409=337410=diff
==
--- cfe/trunk/test/CodeGen/atomic-ops.c (original)
+++ cfe/trunk/test/CodeGen/atomic-ops.c Wed Jul 18 11:01:41 2018
@@ -183,6 +183,18 @@ struct S {
   double x;
 };
 
+void implicit_store(_Atomic(struct S) *a, struct S s) {
+  // CHECK-LABEL: @implicit_store(
+  // CHECK: store atomic i64 %{{.*}}, i64* %{{.*}} seq_cst, align 8
+  *a = s;
+}
+
+struct S implicit_load(_Atomic(struct S) *a) {
+  // CHECK-LABEL: @implicit_load(
+  // CHECK: load atomic i64, i64* %{{.*}} seq_cst, align 8
+  return *a;
+}
+
 struct S fd1(struct S *a) {
   // CHECK-LABEL: @fd1
   // CHECK: [[RETVAL:%.*]] = alloca %struct.S, align 4


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

[PATCH] D49458: Support implicit _Atomic struct load / store

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

I'm sure there are other bugs around `_Atomic`, please file a bug an CC me if 
that's the case. I'll commit this fix for now.


Repository:
  rC Clang

https://reviews.llvm.org/D49458



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

[PATCH] D49083: [HIP] Register/unregister device fat binary only once

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added inline comments.



Comment at: lib/CodeGen/CGCUDANV.cpp:444
+auto HandleValue =
+CtorBuilder.CreateAlignedLoad(GpuBinaryHandle, CGM.getPointerAlign());
+llvm::Constant *Zero = 
llvm::Constant::getNullValue(HandleValue->getType());

rjmccall wrote:
> yaxunl wrote:
> > rjmccall wrote:
> > > Do you not need to worry about concurrency here?
> > The ctor functions are executed by the dynamic loader before the program 
> > gains the control. The dynamic loader cannot excute the ctor functions 
> > concurrently since doing that would not gurantee thread safety of the 
> > loaded program. Therefore we can assume sequential execution of ctor 
> > functions here.
> Okay.  That's worth a comment.
> 
> Is the name here specified by some ABI document, or is it just a conventional 
> name that we're picking now?
Will add a comment for that.

You mean `__hip_gpubin_handle`? It is an implementation detail. It is not 
defined by ABI or other documentation. Since it is only used internally by ctor 
functions, it is not a visible elf symbol. Its name is by convention since the 
cuda corresponding one was named __cuda_gpubin_handle.


https://reviews.llvm.org/D49083



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

[PATCH] D46230: For x86_64, gcc 7.2 under Amazon Linux AMI sets its paths to x86_64-amazon-linux

[Eric Christopher via Phabricator via cfe-commits]

echristo added a comment.

It's usually preferred to submit patches with full context - it'll make this a 
bit easier if you do please?


Repository:
  rC Clang

https://reviews.llvm.org/D46230



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

[PATCH] D49148: [DEBUGINFO] Disable unsupported debug info options for NVPTX target.

[Eric Christopher via Phabricator via cfe-commits]

echristo added a comment.

In https://reviews.llvm.org/D49148#1166429, @ABataev wrote:

> In https://reviews.llvm.org/D49148#1165826, @echristo wrote:
>
> > I think you should break it out on an option by option basis. Just warning 
> > on "non-standard" options won't make as much sense to end users. Perhaps a 
> > "this option is unsupported on the target you're compiling for" etc.
> >
> > Thoughts?
>
>
>
>
> 1. I can split it, no problems.
> 2. Hmm, actually this what the warning already says. If the option is not 
> supported it says 'debug option '-xxx' is not supported for target 
> 'xxx-xxx-xxx''. It does not seem to me like a warning on non-standard option.


Let me try to elaborate a bit, I agree that I'm not very clear above.

I'm not a fan of the generic "non default debug options". It feels misleading. 
I think we probably want to separate it by "dwarf extensions", and "dwarf 
version".

As far as the error message itself: "debug option" sounds like an option to 
debug clang rather than a debug information option. Perhaps say "debug 
information option" rather than "debug option"?


Repository:
  rC Clang

https://reviews.llvm.org/D49148



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

[PATCH] D49083: [HIP] Register/unregister device fat binary only once

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/CodeGen/CGCUDANV.cpp:444
+auto HandleValue =
+CtorBuilder.CreateAlignedLoad(GpuBinaryHandle, CGM.getPointerAlign());
+llvm::Constant *Zero = 
llvm::Constant::getNullValue(HandleValue->getType());

yaxunl wrote:
> rjmccall wrote:
> > Do you not need to worry about concurrency here?
> The ctor functions are executed by the dynamic loader before the program 
> gains the control. The dynamic loader cannot excute the ctor functions 
> concurrently since doing that would not gurantee thread safety of the loaded 
> program. Therefore we can assume sequential execution of ctor functions here.
Okay.  That's worth a comment.

Is the name here specified by some ABI document, or is it just a conventional 
name that we're picking now?


https://reviews.llvm.org/D49083



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

[PATCH] D49294: Sema: Fix explicit address space cast in C++

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/Sema/SemaOverload.cpp:3150
+  !getLangOpts().OpenCLCPlusPlus)
+return false;
+

yaxunl wrote:
> rjmccall wrote:
> > It's not really OpenCL C++ that's special here, it's the possibility of 
> > promotions between address spaces.
> For OpenCL C++, there is language rule about address space promotion.
> 
> For other languages, there is no generic rule about adderss space promotion, 
> since the meaning of address space and their relations are target dependent. 
> Do we want to introduce a target hook Target::canPromote(AddressSpace Src, 
> AddressSpace Dest, LanguageOptions LangOpts) to represent this? Or do we just 
> assume a simple rule, that is, all address space can be promoted to default 
> address space 0, otherwise it is not allowed?
A target-specific hook for handling target-specific address spaces makes sense 
to me.  I don't think there's a generic rule allowing promotion into the 
default AS.


https://reviews.llvm.org/D49294



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

[libcxx] r337406 - Update the synopsis for for C++20. No functional change.

[Marshall Clow via cfe-commits]

Author: marshall
Date: Wed Jul 18 10:37:51 2018
New Revision: 337406

URL: http://llvm.org/viewvc/llvm-project?rev=337406=rev
Log:
Update the synopsis for  for C++20. No functional change.


Modified:
libcxx/trunk/include/chrono

Modified: libcxx/trunk/include/chrono
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/chrono?rev=337406=337405=337406=diff
==
--- libcxx/trunk/include/chrono (original)
+++ libcxx/trunk/include/chrono Wed Jul 18 10:37:51 2018
@@ -147,6 +147,11 @@ template  struct is_clock;  // C++20
+template inline constexpr bool is_clock_v = is_clock::value;   // 
C++20
+
+
 // duration arithmetic
 template 
   constexpr
@@ -204,6 +209,8 @@ template 
 constexpr ToDuration round(const duration& d);// C++17
 
+// duration I/O is elsewhere
+
 // time_point arithmetic (all constexpr in C++14)
 template 
   time_point>::type>
@@ -251,6 +258,7 @@ template 
 constexpr duration abs(duration d);  // C++17
+
 // Clocks
 
 class system_clock
@@ -267,6 +275,28 @@ public:
 static time_point from_time_t(time_t __t) noexcept;
 };
 
+template 
+  using sys_time  = time_point; // C++20
+using sys_seconds = sys_time;  // C++20
+using sys_days= sys_time; // C++20
+
+class utc_clock;// C++20
+
+template 
+  using utc_time  = time_point;// C++20
+using utc_seconds = utc_time;  // C++20
+
+class tai_clock;// C++20
+
+template 
+  using tai_time  = time_point;// C++20
+using tai_seconds = tai_time;  // C++20
+
+class file_clock;   // C++20
+
+template
+  using file_time = time_point;   // C++20
+
 class steady_clock
 {
 public:
@@ -281,8 +311,466 @@ public:
 
 typedef steady_clock high_resolution_clock;
 
+// 25.7.8, local time   // C++20
+struct local_t {};
+template
+  using local_time  = time_point;
+using local_seconds = local_time;
+using local_days= local_time;
+
+// 25.7.9, time_point conversions template 
   // C++20
+struct clock_time_conversion;
+
+template
+  auto clock_cast(const time_point& t);
+  
+// 25.8.2, class last_spec// C++20
+struct last_spec;
+
+// 25.8.3, class day  // C++20
+
+class day;
+constexpr bool operator==(const day& x, const day& y) noexcept;
+constexpr bool operator!=(const day& x, const day& y) noexcept;
+constexpr bool operator< (const day& x, const day& y) noexcept;
+constexpr bool operator> (const day& x, const day& y) noexcept;
+constexpr bool operator<=(const day& x, const day& y) noexcept;
+constexpr bool operator>=(const day& x, const day& y) noexcept;
+constexpr day  operator+(const day&  x, const days& y) noexcept;
+constexpr day  operator+(const days& x, const day&  y) noexcept;
+constexpr day  operator-(const day&  x, const days& y) noexcept;
+constexpr days operator-(const day&  x, const day&  y) noexcept;
+
+// 25.8.4, class month// C++20
+class month;
+constexpr bool operator==(const month& x, const month& y) noexcept;
+constexpr bool operator!=(const month& x, const month& y) noexcept;
+constexpr bool operator< (const month& x, const month& y) noexcept;
+constexpr bool operator> (const month& x, const month& y) noexcept;
+constexpr bool operator<=(const month& x, const month& y) noexcept;
+constexpr bool operator>=(const month& x, const month& y) noexcept;
+constexpr month  operator+(const month&  x, const months& y) noexcept;
+constexpr month  operator+(const months& x,  const month& y) noexcept;
+constexpr month  operator-(const month&  x, const months& y) noexcept;
+constexpr months operator-(const month&  x,  const month& y) noexcept;
+
+// 25.8.5, class year// C++20
+class year;
+constexpr bool operator==(const year& x, const year& y) noexcept;
+constexpr bool operator!=(const year& x, const year& y) noexcept;
+constexpr bool operator< (const year& x, const year& y) noexcept;
+constexpr bool operator> (const year& x, const year& y) noexcept;
+constexpr bool operator<=(const year& x, const year& y) noexcept;
+constexpr bool operator>=(const year& x, const year& y) noexcept;
+constexpr year  operator+(const year&  x, const years& y) noexcept;
+constexpr year  operator+(const years& x, const year&  y) noexcept;
+constexpr year  operator-(const year&  x, const years& y) noexcept;
+constexpr years operator-(const year&  x, const year&  y) noexcept;
+
+// 25.8.6, class weekday// C++20
+class weekday;
+
+constexpr bool operator==(const weekday& x, const weekday& y) noexcept;
+constexpr bool operator!=(const weekday& x, const weekday& y) noexcept;
+constexpr weekday operator+(const weekday& x, const days&y) noexcept;
+constexpr weekday operator+(const days&x, const weekday& y) noexcept;
+constexpr weekday operator-(const weekday& x, const days&y) noexcept;
+constexpr daysoperator-(const weekday& x, const weekday& y) 

[PATCH] D48862: [OpenEmbedded] Fix lib paths for OpenEmbedded targets

[Mandeep Singh Grang via Phabricator via cfe-commits]

mgrang added a comment.

Ping 3 for reviews please.


https://reviews.llvm.org/D48862



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

[PATCH] D49466: Initial implementation of -fmacro-prefix-mapand -ffile-prefix-map

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: include/clang/Lex/Preprocessor.h:155
+  /// A prefix map for __FILE__ and __BASEFILE__
+  llvm::SmallDenseMap MacroPrefixMap;
+

Scrolling back up, put this implementation as close to the debug implementation 
as you can, they are so ridiculously related that having them far enough apart 
that future changes could cause them to divert is troublesome to me.



Comment at: include/clang/Lex/PreprocessorOptions.h:171
+  /// A prefix map for __FILE__ and __BASEFILE__
+  std::map MacroPrefixMap;
+

It seems this can be StringRefs as well.



Comment at: lib/Driver/ToolChains/Clang.cpp:617
+  }
   for (const Arg *A : Args.filtered(options::OPT_fdebug_prefix_map_EQ)) {
 StringRef Map = A->getValue();

filtered can take multiple options, you should be able to not add anything here 
except adding OPT_ffile_prefix_map_EQ to the filtered line, plus a ternary in 
the Diag line.



Comment at: lib/Driver/ToolChains/Clang.cpp:628
+/// Add a CC1 option to specify the macro file path prefix map.
+static void addMacroPrefixMapArg(const Driver , const ArgList , 
ArgStringList ) {
+  for (const Arg *A : Args.filtered(options::OPT_ffile_prefix_map_EQ)) {

See advice above.

Additionally/alternatively, I wonder if these two functions could be trivially 
combined.



Comment at: lib/Driver/ToolChains/Clang.cpp:1255
+
+  addMacroPrefixMapArg(D, Args, CmdArgs);
 }

Is there a good reason for this to not live with the call to 
addDebugPrefixMapArg?



Comment at: lib/Frontend/CompilerInvocation.cpp:2848
 
+  for (const auto  : Args.getAllArgValues(OPT_fmacro_prefix_map_EQ))
+  {

Again, this is so much like the debug-prefix otpion, it should probably just be 
right next to it.

Additionally, we don't use curley brackets on single-line bodies.



Comment at: lib/Lex/PPMacroExpansion.cpp:1457
+static std::string remapMacroPath(StringRef Path,
+   llvm::SmallDenseMap ) {

make MacroPrefixMap const.



Comment at: lib/Lex/PPMacroExpansion.cpp:1528
 // Escape this filename.  Turn '\' -> '\\' '"' -> '\"'
-SmallString<128> FN;
+std::string FN;
 if (PLoc.isValid()) {

This change shouldn't be necessary, SmallString is still likely the right 
answer here.



Comment at: lib/Lex/Preprocessor.cpp:160
+
+  for (const auto  : this->PPOpts->MacroPrefixMap)
+MacroPrefixMap[KV.first] = KV.second;

I'm unconvinced that this is necessary.  ExpandBuiltinMacro is in Preprocessor, 
so it has access to PPOpts directly.


Repository:
  rC Clang

https://reviews.llvm.org/D49466



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

[PATCH] D46190: For a used declaration, mark any associated usings as referenced.

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

A bunch of style comments.  Maybe try clang-format-diff.




Comment at: include/clang/Sema/SemaInternal.h:91
   Var->markUsed(SemaRef.Context);
+  SemaRef.MarkUsingReferenced(Var, Loc, /*CXXScopeSpec*=*/nullptr, RefExpr);
 }

The comments on a nullptr parameter usually use the formal parameter name, not 
its type.



Comment at: lib/Sema/SemaDeclCXX.cpp:15549
+  R.setAddUsingDirectives();
+  LookupName(R,getCurScope());
+

Space after the comma.



Comment at: lib/Sema/SemaDeclCXX.cpp:15554
+
+  for (LookupResult::iterator I = R.begin(), E = R.end(); I != E; ++I) {
+NamedDecl *D = (*I)->getUnderlyingDecl();

Could this be a range-based for loop?



Comment at: lib/Sema/SemaDeclCXX.cpp:15566
+// Check if the declaration was introduced by a 'using-directive'.
+if (auto *UDir = dyn_cast(D))
+  if (UDir->getNominatedNamespace() == Target) {

else if



Comment at: lib/Sema/SemaExpr.cpp:14460
   Func->markUsed(Context);
+  MarkUsingReferenced(Func, Loc, /*CXXScopeSpec*=*/nullptr, E);
 }

Parameter comments usually use the formal parameter name, not its type.



Comment at: lib/Sema/SemaExpr.cpp:15374
+  SemaRef.MarkAnyDeclReferenced(Loc, D, MightBeOdrUse,
+/*CXXScopeSpec*=*/nullptr, E);
 

Parameter comments usually use the formal parameter name, not its type.



Comment at: lib/Sema/SemaExpr.cpp:15396
+SemaRef.MarkAnyDeclReferenced(Loc, DM, MightBeOdrUse,
+  /*CXXScopeSpec*=*/nullptr, E);
 }

Parameter comments usually use the formal parameter name, not its type.



Comment at: lib/Sema/SemaLookup.cpp:196
+void addUsingDirective(LookupResult ) {
+  for (auto I = usings.begin(), E = usings.end(); I != E; ++I)
+R.addDecl((*I));

Can this be a range-based for loop?



Comment at: lib/Sema/SemaLookup.cpp:1064
+static void
+AddUsingDirectives(LookupResult ,UnqualUsingDirectiveSet ) {
+  if (R.isAddUsingDirectives())

Space after the comma.
`UDirs` has a different meaning elsewhere in this file, maybe `UsingDirs` 
instead?



Comment at: lib/Sema/SemaLookup.cpp:1236
 UDirs.done();
+AddUsingDirectives(R,UDirs);
 

Space after the comma.



Comment at: lib/Sema/SemaLookup.cpp:1277
 UDirs.done();
+AddUsingDirectives(R,UDirs);
   }

Space after the comma.


https://reviews.llvm.org/D46190



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

[PATCH] D49274: [CUDA] Provide integer SIMD functions for CUDA-9.2

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

I'm in the middle of writing the tests for these as it's very easy to mess 
things up. I'll update the patch once I run it through the tests.

Another problem with the patch in the current form is that these instructions 
apparently do not accept immediate arguments. PTX is a never ending source of 
surprises...


https://reviews.llvm.org/D49274



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

[PATCH] D49058: [analyzer] Move DanglingInternalBufferChecker out of alpha

[Reka Kovacs via Phabricator via cfe-commits]

rnkovacs added a comment.

Two more reports on Ceph that seem to be true positives (no other reports from 
this checker):

1. Here  (or if it does not work, the bug is on L130 
here ).
2. Here  (or L363 and L373 here 
).


https://reviews.llvm.org/D49058



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

[PATCH] D49492: Run bounds checking sanitizer earlier to make it easier to optimize away its checks.

[Joel Galenson via Phabricator via cfe-commits]

jgalenson created this revision.
jgalenson added reviewers: eugenis, chandlerc.
Herald added a subscriber: cfe-commits.

Running the bounds checking sanitizer earlier makes it easier for other 
optimizations to remove the checks it inserts. 
 While it could also inhibit other optimizations, I ran a few benchmarks and 
this did seem to improve performance and code size slightly.

Note that I'm not sure how to hook this up to the new PM, as I couldn't find a 
similar hook into the beginning of the pipeline.  Are there any suggestions on 
what to do about that, or is it fine for this to work just on the old PM?


Repository:
  rC Clang

https://reviews.llvm.org/D49492

Files:
  lib/CodeGen/BackendUtil.cpp
  test/CodeGen/bounds-checking-opt.c


Index: test/CodeGen/bounds-checking-opt.c
===
--- /dev/null
+++ test/CodeGen/bounds-checking-opt.c
@@ -0,0 +1,20 @@
+// RUN: %clang -S -fsanitize=local-bounds -emit-llvm -target x86_64-- %s -o - 
| FileCheck -check-prefix=O0 %s
+// RUN: %clang -S -fsanitize=local-bounds -emit-llvm -target x86_64-- -O2 %s 
-o - | FileCheck -check-prefix=O2 %s
+// RUN: %clang -S -fsanitize=local-bounds -emit-llvm -target aarch64-- %s -o - 
| FileCheck -check-prefix=O0 %s
+// RUN: %clang -S -fsanitize=local-bounds -emit-llvm -target aarch64-- -O2 %s 
-o - | FileCheck -check-prefix=O2 %s
+
+extern void fill(int *arr);
+
+// CHECK-LABEL: @f
+int f(int x) {
+  // O0: call {{.*}} @llvm.trap
+  // O2-NOT: call {{.*}} @llvm.trap
+  int foo[1000];
+  fill(foo);
+  int sum = 0;
+  #pragma clang loop vectorize(disable)
+  #pragma clang loop unroll(disable)
+  for (unsigned i = 0; i < 1000; i++)
+sum += foo[i];
+  return sum;
+}
Index: lib/CodeGen/BackendUtil.cpp
===
--- lib/CodeGen/BackendUtil.cpp
+++ lib/CodeGen/BackendUtil.cpp
@@ -562,7 +562,7 @@
 addCoroutinePassesToExtensionPoints(PMBuilder);
 
   if (LangOpts.Sanitize.has(SanitizerKind::LocalBounds)) {
-PMBuilder.addExtension(PassManagerBuilder::EP_ScalarOptimizerLate,
+PMBuilder.addExtension(PassManagerBuilder::EP_EarlyAsPossible,
addBoundsCheckingPass);
 PMBuilder.addExtension(PassManagerBuilder::EP_EnabledOnOptLevel0,
addBoundsCheckingPass);


Index: test/CodeGen/bounds-checking-opt.c
===
--- /dev/null
+++ test/CodeGen/bounds-checking-opt.c
@@ -0,0 +1,20 @@
+// RUN: %clang -S -fsanitize=local-bounds -emit-llvm -target x86_64-- %s -o - | FileCheck -check-prefix=O0 %s
+// RUN: %clang -S -fsanitize=local-bounds -emit-llvm -target x86_64-- -O2 %s -o - | FileCheck -check-prefix=O2 %s
+// RUN: %clang -S -fsanitize=local-bounds -emit-llvm -target aarch64-- %s -o - | FileCheck -check-prefix=O0 %s
+// RUN: %clang -S -fsanitize=local-bounds -emit-llvm -target aarch64-- -O2 %s -o - | FileCheck -check-prefix=O2 %s
+
+extern void fill(int *arr);
+
+// CHECK-LABEL: @f
+int f(int x) {
+  // O0: call {{.*}} @llvm.trap
+  // O2-NOT: call {{.*}} @llvm.trap
+  int foo[1000];
+  fill(foo);
+  int sum = 0;
+  #pragma clang loop vectorize(disable)
+  #pragma clang loop unroll(disable)
+  for (unsigned i = 0; i < 1000; i++)
+sum += foo[i];
+  return sum;
+}
Index: lib/CodeGen/BackendUtil.cpp
===
--- lib/CodeGen/BackendUtil.cpp
+++ lib/CodeGen/BackendUtil.cpp
@@ -562,7 +562,7 @@
 addCoroutinePassesToExtensionPoints(PMBuilder);
 
   if (LangOpts.Sanitize.has(SanitizerKind::LocalBounds)) {
-PMBuilder.addExtension(PassManagerBuilder::EP_ScalarOptimizerLate,
+PMBuilder.addExtension(PassManagerBuilder::EP_EarlyAsPossible,
addBoundsCheckingPass);
 PMBuilder.addExtension(PassManagerBuilder::EP_EnabledOnOptLevel0,
addBoundsCheckingPass);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits