[libunwind] r323499 - Don't enable _LIBUNWIND_BUILD_ZERO_COST_APIS if building the SJLJ APIs

[Martin Storsjo via cfe-commits]

Author: mstorsjo
Date: Thu Jan 25 22:50:07 2018
New Revision: 323499

URL: http://llvm.org/viewvc/llvm-project?rev=323499=rev
Log:
Don't enable _LIBUNWIND_BUILD_ZERO_COST_APIS if building the SJLJ APIs

Otherwise, a shared library build with SJLJ APIs enabled would
end up with duplicate symbols.

This didn't occur for the apple && arm case due to specifically
checking for that in the surrounding ifdef.

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

Modified:
libunwind/trunk/src/config.h

Modified: libunwind/trunk/src/config.h
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/config.h?rev=323499=323498=323499=diff
==
--- libunwind/trunk/src/config.h (original)
+++ libunwind/trunk/src/config.h Thu Jan 25 22:50:07 2018
@@ -72,8 +72,10 @@
 (!defined(__APPLE__) && defined(__arm__)) ||   
\
 (defined(__arm64__) || defined(__aarch64__)) ||
\
 defined(__mips__)
+#if !defined(_LIBUNWIND_BUILD_SJLJ_APIS)
 #define _LIBUNWIND_BUILD_ZERO_COST_APIS
 #endif
+#endif
 
 #if defined(__powerpc64__) && defined(_ARCH_PWR8)
 #define PPC64_HAS_VMX


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

[PATCH] D42555: [libunwind] Don't enable _LIBUNWIND_BUILD_ZERO_COST_APIS if building the SJLJ APIs

[Martin Storsjö via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL323499: Dont enable _LIBUNWIND_BUILD_ZERO_COST_APIS if 
building the SJLJ APIs (authored by mstorsjo, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D42555?vs=131500=131541#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D42555

Files:
  libunwind/trunk/src/config.h


Index: libunwind/trunk/src/config.h
===
--- libunwind/trunk/src/config.h
+++ libunwind/trunk/src/config.h
@@ -72,8 +72,10 @@
 (!defined(__APPLE__) && defined(__arm__)) ||   
\
 (defined(__arm64__) || defined(__aarch64__)) ||
\
 defined(__mips__)
+#if !defined(_LIBUNWIND_BUILD_SJLJ_APIS)
 #define _LIBUNWIND_BUILD_ZERO_COST_APIS
 #endif
+#endif
 
 #if defined(__powerpc64__) && defined(_ARCH_PWR8)
 #define PPC64_HAS_VMX


Index: libunwind/trunk/src/config.h
===
--- libunwind/trunk/src/config.h
+++ libunwind/trunk/src/config.h
@@ -72,8 +72,10 @@
 (!defined(__APPLE__) && defined(__arm__)) ||   \
 (defined(__arm64__) || defined(__aarch64__)) ||\
 defined(__mips__)
+#if !defined(_LIBUNWIND_BUILD_SJLJ_APIS)
 #define _LIBUNWIND_BUILD_ZERO_COST_APIS
 #endif
+#endif
 
 #if defined(__powerpc64__) && defined(_ARCH_PWR8)
 #define PPC64_HAS_VMX
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42354: Fix libcxx MSVC C++17 redefinition of 'align_val_t'

[Force.Charlie-I via Phabricator via cfe-commits]

fcharlie added a comment.

In https://reviews.llvm.org/D42354#988656, @mclow.lists wrote:

> I'm pretty sure I don't want to know what MSFT is doing putting `align_val_t` 
> in *that* header file.
>
> Now I'm wondering if those values `__zero` and `__max` are actually used.


Use

>   grep "__zero" -Rn .

There's no place to use ** __zero** and **__max**.


Repository:
  rCXX libc++

https://reviews.llvm.org/D42354



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

[PATCH] D42354: Fix libcxx MSVC C++17 redefinition of 'align_val_t'

[Force.Charlie-I via Phabricator via cfe-commits]

fcharlie added a comment.

In https://reviews.llvm.org/D42354#988656, @mclow.lists wrote:

> I'm pretty sure I don't want to know what MSFT is doing putting `align_val_t` 
> in *that* header file.
>
> Now I'm wondering if those values `__zero` and `__max` are actually used.


Use

>   grep "__zero" -Rn .

There's no place to use ** __zero** and **__max**.


Repository:
  rCXX libc++

https://reviews.llvm.org/D42354



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

[PATCH] D42461: [cmake] [libunwind] Call llvm_setup_rpath() when adding shared libraries.

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL323496: [cmake] [libunwind] Call llvm_setup_rpath() when 
adding shared libraries. (authored by dhinton, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D42461

Files:
  libunwind/trunk/src/CMakeLists.txt


Index: libunwind/trunk/src/CMakeLists.txt
===
--- libunwind/trunk/src/CMakeLists.txt
+++ libunwind/trunk/src/CMakeLists.txt
@@ -108,6 +108,9 @@
 # Build the shared library.
 if (LIBUNWIND_ENABLE_SHARED)
   add_library(unwind_shared SHARED $)
+  if(LLVM_FOUND)
+llvm_setup_rpath(unwind_shared)
+  endif()
   target_link_libraries(unwind_shared ${libraries})
   set_target_properties(unwind_shared
 PROPERTIES


Index: libunwind/trunk/src/CMakeLists.txt
===
--- libunwind/trunk/src/CMakeLists.txt
+++ libunwind/trunk/src/CMakeLists.txt
@@ -108,6 +108,9 @@
 # Build the shared library.
 if (LIBUNWIND_ENABLE_SHARED)
   add_library(unwind_shared SHARED $)
+  if(LLVM_FOUND)
+llvm_setup_rpath(unwind_shared)
+  endif()
   target_link_libraries(unwind_shared ${libraries})
   set_target_properties(unwind_shared
 PROPERTIES
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[libunwind] r323496 - [cmake] [libunwind] Call llvm_setup_rpath() when adding shared libraries.

[Don Hinton via cfe-commits]

Author: dhinton
Date: Thu Jan 25 20:01:50 2018
New Revision: 323496

URL: http://llvm.org/viewvc/llvm-project?rev=323496=rev
Log:
[cmake] [libunwind] Call llvm_setup_rpath() when adding shared libraries.

Clang and llvm already use llvm_setup_rpath(), so this change will
help standarize rpath usage across all projects.

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

Modified:
libunwind/trunk/src/CMakeLists.txt

Modified: libunwind/trunk/src/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/CMakeLists.txt?rev=323496=323495=323496=diff
==
--- libunwind/trunk/src/CMakeLists.txt (original)
+++ libunwind/trunk/src/CMakeLists.txt Thu Jan 25 20:01:50 2018
@@ -108,6 +108,9 @@ set(LIBUNWIND_TARGETS)
 # Build the shared library.
 if (LIBUNWIND_ENABLE_SHARED)
   add_library(unwind_shared SHARED $)
+  if(LLVM_FOUND)
+llvm_setup_rpath(unwind_shared)
+  endif()
   target_link_libraries(unwind_shared ${libraries})
   set_target_properties(unwind_shared
 PROPERTIES


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

[PATCH] D42461: [cmake] [libunwind] Call llvm_setup_rpath() when adding shared libraries.

[Don Hinton via Phabricator via cfe-commits]

hintonda updated this revision to Diff 131537.
hintonda added a comment.

Only call llvm_setup_rpath() if LLVM_FOUND is true.


Repository:
  rUNW libunwind

https://reviews.llvm.org/D42461

Files:
  src/CMakeLists.txt


Index: src/CMakeLists.txt
===
--- src/CMakeLists.txt
+++ src/CMakeLists.txt
@@ -108,6 +108,9 @@
 # Build the shared library.
 if (LIBUNWIND_ENABLE_SHARED)
   add_library(unwind_shared SHARED $)
+  if(LLVM_FOUND)
+llvm_setup_rpath(unwind_shared)
+  endif()
   target_link_libraries(unwind_shared ${libraries})
   set_target_properties(unwind_shared
 PROPERTIES


Index: src/CMakeLists.txt
===
--- src/CMakeLists.txt
+++ src/CMakeLists.txt
@@ -108,6 +108,9 @@
 # Build the shared library.
 if (LIBUNWIND_ENABLE_SHARED)
   add_library(unwind_shared SHARED $)
+  if(LLVM_FOUND)
+llvm_setup_rpath(unwind_shared)
+  endif()
   target_link_libraries(unwind_shared ${libraries})
   set_target_properties(unwind_shared
 PROPERTIES
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42460: [cmake] [libcxxabi] Call llvm_setup_rpath() when adding shared libraries.

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL323495: Reland: (authored by dhinton, committed by ).

Repository:
  rL LLVM

https://reviews.llvm.org/D42460

Files:
  libcxxabi/trunk/src/CMakeLists.txt


Index: libcxxabi/trunk/src/CMakeLists.txt
===
--- libcxxabi/trunk/src/CMakeLists.txt
+++ libcxxabi/trunk/src/CMakeLists.txt
@@ -127,6 +127,9 @@
 # Build the shared library.
 if (LIBCXXABI_ENABLE_SHARED)
   add_library(cxxabi_shared SHARED $)
+  if(LLVM_FOUND)
+llvm_setup_rpath(cxxabi_shared)
+  endif()
   target_link_libraries(cxxabi_shared ${LIBCXXABI_LIBRARIES})
   set_target_properties(cxxabi_shared
 PROPERTIES


Index: libcxxabi/trunk/src/CMakeLists.txt
===
--- libcxxabi/trunk/src/CMakeLists.txt
+++ libcxxabi/trunk/src/CMakeLists.txt
@@ -127,6 +127,9 @@
 # Build the shared library.
 if (LIBCXXABI_ENABLE_SHARED)
   add_library(cxxabi_shared SHARED $)
+  if(LLVM_FOUND)
+llvm_setup_rpath(cxxabi_shared)
+  endif()
   target_link_libraries(cxxabi_shared ${LIBCXXABI_LIBRARIES})
   set_target_properties(cxxabi_shared
 PROPERTIES
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[libcxxabi] r323495 - Reland:

[Don Hinton via cfe-commits]

Author: dhinton
Date: Thu Jan 25 19:41:58 2018
New Revision: 323495

URL: http://llvm.org/viewvc/llvm-project?rev=323495=rev
Log:
Reland:
[cmake] [libcxxabi] Call llvm_setup_rpath() when adding shared libraries.

Clang and llvm already use llvm_setup_rpath(), so this change will
help standarize rpath usage across all projects.

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

Modified:
libcxxabi/trunk/src/CMakeLists.txt

Modified: libcxxabi/trunk/src/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/libcxxabi/trunk/src/CMakeLists.txt?rev=323495=323494=323495=diff
==
--- libcxxabi/trunk/src/CMakeLists.txt (original)
+++ libcxxabi/trunk/src/CMakeLists.txt Thu Jan 25 19:41:58 2018
@@ -127,6 +127,9 @@ set(LIBCXXABI_TARGETS)
 # Build the shared library.
 if (LIBCXXABI_ENABLE_SHARED)
   add_library(cxxabi_shared SHARED $)
+  if(LLVM_FOUND)
+llvm_setup_rpath(cxxabi_shared)
+  endif()
   target_link_libraries(cxxabi_shared ${LIBCXXABI_LIBRARIES})
   set_target_properties(cxxabi_shared
 PROPERTIES


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

[PATCH] D41885: [libcxxabi][demangler] Improve handling of variadic templates

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington added inline comments.



Comment at: src/cxa_demangle.cpp:260-261
+
+#if 0
+  void dump() const {
+char *Buffer = static_cast(std::malloc(1024));

dexonsmith wrote:
> Why is this behind `#if 0`?  Should you just use something like 
> `LLVM_DUMP_METHOD`?
Do you mean conditionally defined based on NDEBUG and marked 
__attribute__((noinline,used)) for debuggers? I was just using it as a quick & 
simple little helper for debugging.



Comment at: src/cxa_demangle.cpp:1412-1414
+setCachedRHSComponent(false);
+setCachedArray(false);
+setCachedFunction(false);

dexonsmith wrote:
> This seems like a super-common sequence in constructors of subclasses of 
> `Node`.  Can you pull out a common subclass of `Node` (say, `CachingNode`)?
I had a hard time finding a nice way of abstracting the cache from the 
individual ctors, they all need to set it up based on their parameters. The new 
patch just defaults to setting the three cached values to false, with each ctor 
overriding when necessary. This gets rid of a lot of the duplication.



Comment at: test/test_demangle.pass.cpp:29607
 {"_ZTHN3fooE", "thread-local initialization routine for foo"},
-{"_Z4algoIJiiiEEvZ1gEUlT_E_", "void algo(g::'lambda'(int, 
int, int))"},
+{"_Z4algoIJiiiEEvZ1gEUlDpT_E_", "void algo(g::'lambda'(int, 
int, int))"},
 // attribute abi_tag

dexonsmith wrote:
> erik.pilkington wrote:
> > I just generated this symbol by hand because I couldn't get clang to do it 
> > without crashing. Turns out I forgot the Dp!
> Why was it crashing?  (Did you file a bug?)
Yep, I filed https://bugs.llvm.org/show_bug.cgi?id=33092 to track this at the 
time. I didn't really look too far into the crash, but it seems pretty 
low-priority.


https://reviews.llvm.org/D41885



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

[PATCH] D41885: [libcxxabi][demangler] Improve handling of variadic templates

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington updated this revision to Diff 131535.
erik.pilkington marked 2 inline comments as done.
erik.pilkington added a comment.

In this new patch:

- Make the cached values use a 3-way bool type, and default it to false. This 
simplifies all the Node ctors.
- Remove some minor style fixes to clean up the diff.


https://reviews.llvm.org/D41885

Files:
  src/cxa_demangle.cpp
  test/test_demangle.pass.cpp
  test/unittest_demangle.pass.cpp

Index: test/unittest_demangle.pass.cpp
===
--- test/unittest_demangle.pass.cpp
+++ test/unittest_demangle.pass.cpp
@@ -82,44 +82,6 @@
   }
 }
 
-void testSubstitutionTable() {
-  {
-SubstitutionTable<2> Tab;
-
-NameType Names[] = {{"MERP"}, {"MARP"}, {"MAMP"}};
-Tab.pushPack();
-Tab.pushSubstitutionIntoPack([0]);
-Tab.pushSubstitutionIntoPack([1]);
-Tab.pushSubstitutionIntoPack([2]);
-
-int Index = 0;
-for (Node* N : Tab.nthSubstitution(0)) {
-  assert(static_cast(N)->getName() == Names[Index].getName());
-  ++Index;
-}
-assert(Index == 3);
-
-Tab.popPack();
-assert(Tab.empty() && Tab.size() == 0);
-Tab.pushSubstitution([0]);
-Tab.pushSubstitution([1]);
-assert(!Tab.empty() && Tab.size() == 2);
-
-int I = 0;
-for (Node* N : Tab.nthSubstitution(0)) {
-  assert(static_cast(N)->getName() == "MERP");
-  assert(I == 0);
-  ++I;
-}
-for (Node* N : Tab.nthSubstitution(1)) {
-  assert(static_cast(N)->getName() == "MARP");
-  assert(I == 1);
-  ++I;
-}
-  }
-}
-
 int main() {
   testPODSmallVector();
-  testSubstitutionTable();
 }
Index: test/test_demangle.pass.cpp
===
--- test/test_demangle.pass.cpp
+++ test/test_demangle.pass.cpp
@@ -29585,7 +29585,7 @@
 {"_Z1fPU11objcproto1A11objc_object", "f(id)"},
 {"_Z1fPKU11objcproto1A7NSArray", "f(NSArray const*)"},
 {"_ZNK1AIJ1Z1Y1XEEcv1BIJDpPT_EEIJS2_S1_S0_EEEv", "A::operator B() const"},
-{"_ZNK3Ncr6Silver7Utility6detail12CallOnThreadIZ53-[DeploymentSetupController handleManualServerEntry:]E3$_5EclIJEEEDTclclL_ZNS2_4getTIS4_EERT_vEEspclsr3stdE7forwardIT_Efp_EEEDpOSA_", "decltype(-[DeploymentSetupController handleManualServerEntry:]::$_5& Ncr::Silver::Utility::detail::getT<-[DeploymentSetupController handleManualServerEntry:]::$_5>()()(std::forward<-[DeploymentSetupController handleManualServerEntry:]::$_5>(fp))) Ncr::Silver::Utility::detail::CallOnThread<-[DeploymentSetupController handleManualServerEntry:]::$_5>::operator()<>(-[DeploymentSetupController handleManualServerEntry:]::$_5&&) const"},
+{"_ZNK3Ncr6Silver7Utility6detail12CallOnThreadIZ53-[DeploymentSetupController handleManualServerEntry:]E3$_5EclIJEEEDTclclL_ZNS2_4getTIS4_EERT_vEEspclsr3stdE7forwardIT_Efp_EEEDpOSA_", "decltype(-[DeploymentSetupController handleManualServerEntry:]::$_5& Ncr::Silver::Utility::detail::getT<-[DeploymentSetupController handleManualServerEntry:]::$_5>()()(std::forward<-[DeploymentSetupController handleManualServerEntry:]::$_5>(fp)...)) Ncr::Silver::Utility::detail::CallOnThread<-[DeploymentSetupController handleManualServerEntry:]::$_5>::operator()<>(-[DeploymentSetupController handleManualServerEntry:]::$_5&&...) const"},
 {"_Zli2_xy", "operator\"\" _x(unsigned long long)"},
 {"_Z1fIiEDcT_", "decltype(auto) f(int)"},
 {"_ZZ4testvEN1g3fooE5Point", "test()::g::foo(Point)"},
@@ -29604,13 +29604,19 @@
 {"PFvRmOE", "void (*)(unsigned long&) &&"},
 {"_ZTW1x", "thread-local wrapper routine for x"},
 {"_ZTHN3fooE", "thread-local initialization routine for foo"},
-{"_Z4algoIJiiiEEvZ1gEUlT_E_", "void algo(g::'lambda'(int, int, int))"},
+{"_Z4algoIJiiiEEvZ1gEUlDpT_E_", "void algo(g::'lambda'(int, int, int))"},
 // attribute abi_tag
 {"_Z1fB3foov", "f[abi:foo]()"},
 {"_Z1fB3fooB3barv", "f[abi:foo][abi:bar]()"},
 {"_ZN1SB5outer1fB5innerEv", "S[abi:outer]::f[abi:inner]()"},
 {"_ZN1SC2B8ctor_tagEv", "S::S[abi:ctor_tag]()"},
 {"_ZplB4MERP1SS_", "operator+[abi:MERP](S, S)"},
+
+{"_Z1fIJifcEEvDp5unaryIT_E", "void f(unary, unary, unary)"},
+{"_Z1fIJEJiEEvDpT_DpT0_", "void f(int)"},
+{"_Z1fIJicEEvDp7MuncherIAstT__S1_E", "void f(Muncher, Muncher)"},
+{"_ZN1SIJifcEE1fIJdjEEEiDp4MerpIJifcT_EE", "int S::f(Merp, Merp)"},
+{"_Z1pIJicEEiDp4MerpIXsZT_EJT_EE", "int p(Merp, Merp)"},
 };
 
 const unsigned N = sizeof(cases) / sizeof(cases[0]);
Index: src/cxa_demangle.cpp
===
--- src/cxa_demangle.cpp
+++ src/cxa_demangle.cpp
@@ -10,16 +10,17 @@
 // FIXME: (possibly) incomplete list of features 

[PATCH] D42354: Fix libcxx MSVC C++17 redefinition of 'align_val_t'

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists added a comment.

I'm pretty sure I don't want to know what MSFT is doing putting `align_val_t` 
in *that* header file.

Now I'm wondering if those values `__zero` and `__max` are actually used.


Repository:
  rCXX libc++

https://reviews.llvm.org/D42354



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

[PATCH] D42460: [cmake] [libcxxabi] Call llvm_setup_rpath() when adding shared libraries.

[Don Hinton via Phabricator via cfe-commits]

hintonda updated this revision to Diff 131528.
hintonda added a comment.

Only call llvm_setup_rpath() if LLVM_FOUND is true.


Repository:
  rCXXA libc++abi

https://reviews.llvm.org/D42460

Files:
  src/CMakeLists.txt


Index: src/CMakeLists.txt
===
--- src/CMakeLists.txt
+++ src/CMakeLists.txt
@@ -127,6 +127,9 @@
 # Build the shared library.
 if (LIBCXXABI_ENABLE_SHARED)
   add_library(cxxabi_shared SHARED $)
+  if(LLVM_FOUND)
+llvm_setup_rpath(cxxabi_shared)
+  endif()
   target_link_libraries(cxxabi_shared ${LIBCXXABI_LIBRARIES})
   set_target_properties(cxxabi_shared
 PROPERTIES


Index: src/CMakeLists.txt
===
--- src/CMakeLists.txt
+++ src/CMakeLists.txt
@@ -127,6 +127,9 @@
 # Build the shared library.
 if (LIBCXXABI_ENABLE_SHARED)
   add_library(cxxabi_shared SHARED $)
+  if(LLVM_FOUND)
+llvm_setup_rpath(cxxabi_shared)
+  endif()
   target_link_libraries(cxxabi_shared ${LIBCXXABI_LIBRARIES})
   set_target_properties(cxxabi_shared
 PROPERTIES
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42459: [cmake] [libcxx] Call llvm_setup_rpath() when adding shared libraries.

[Phabricator 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 rL323492: Reland: [cmake] [libcxx] Call llvm_setup_rpath() 
when adding shared libraries. (authored by dhinton, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D42459

Files:
  libcxx/trunk/lib/CMakeLists.txt


Index: libcxx/trunk/lib/CMakeLists.txt
===
--- libcxx/trunk/lib/CMakeLists.txt
+++ libcxx/trunk/lib/CMakeLists.txt
@@ -222,6 +222,9 @@
 # Build the shared library.
 if (LIBCXX_ENABLE_SHARED)
   add_library(cxx_shared SHARED $)
+  if(LLVM_FOUND)
+llvm_setup_rpath(cxx_shared)
+  endif()
   target_link_libraries(cxx_shared ${LIBCXX_LIBRARIES})
   set_target_properties(cxx_shared
 PROPERTIES


Index: libcxx/trunk/lib/CMakeLists.txt
===
--- libcxx/trunk/lib/CMakeLists.txt
+++ libcxx/trunk/lib/CMakeLists.txt
@@ -222,6 +222,9 @@
 # Build the shared library.
 if (LIBCXX_ENABLE_SHARED)
   add_library(cxx_shared SHARED $)
+  if(LLVM_FOUND)
+llvm_setup_rpath(cxx_shared)
+  endif()
   target_link_libraries(cxx_shared ${LIBCXX_LIBRARIES})
   set_target_properties(cxx_shared
 PROPERTIES
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42532: [OpenCL] Add "cles_khr_int64" extension.

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia accepted this revision.
Anastasia added a comment.

Oh I guess it can be used to exclude the code with long? LGTM!


Repository:
  rC Clang

https://reviews.llvm.org/D42532



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

[PATCH] D42459: [cmake] [libcxx] Call llvm_setup_rpath() when adding shared libraries.

[Phabricator 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 rCXX323492: Reland: [cmake] [libcxx] Call llvm_setup_rpath() 
when adding shared libraries. (authored by dhinton, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D42459?vs=131506=131526#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D42459

Files:
  lib/CMakeLists.txt


Index: lib/CMakeLists.txt
===
--- lib/CMakeLists.txt
+++ lib/CMakeLists.txt
@@ -222,6 +222,9 @@
 # Build the shared library.
 if (LIBCXX_ENABLE_SHARED)
   add_library(cxx_shared SHARED $)
+  if(LLVM_FOUND)
+llvm_setup_rpath(cxx_shared)
+  endif()
   target_link_libraries(cxx_shared ${LIBCXX_LIBRARIES})
   set_target_properties(cxx_shared
 PROPERTIES


Index: lib/CMakeLists.txt
===
--- lib/CMakeLists.txt
+++ lib/CMakeLists.txt
@@ -222,6 +222,9 @@
 # Build the shared library.
 if (LIBCXX_ENABLE_SHARED)
   add_library(cxx_shared SHARED $)
+  if(LLVM_FOUND)
+llvm_setup_rpath(cxx_shared)
+  endif()
   target_link_libraries(cxx_shared ${LIBCXX_LIBRARIES})
   set_target_properties(cxx_shared
 PROPERTIES
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42513: [CUDA] Added partial support for CUDA-9.1

[Justin Lebar via Phabricator via cfe-commits]

jlebar added inline comments.



Comment at: clang/lib/Headers/__clang_cuda_device_functions.h:32
+
+#define __DEVICE__ static __device__ __forceinline__
+// There are number of functions that became compiler builtins in CUDA-9 and 
are

tra wrote:
> jlebar wrote:
> > tra wrote:
> > > jlebar wrote:
> > > > I don't think we should need `__forceinline__`?  I'm sure they had that 
> > > > in their headers, but clang doesn't rely on its inliner for 
> > > > correctness, unlike some *other* compilers.  :)
> > > > 
> > > > I'm also not sure about `static`.  I think we mark all CUDA code as 
> > > > internal, so it shouldn't be necessary?  If they're not static, they 
> > > > should be marked as `inline`, though.
> > > The idea is to make those wrappers always inlined, so they are 
> > > effectively replaced with a call to __nv_* libdevice function. Whether 
> > > the callee itself get inlined is controlled by the attributes in the 
> > > libdevice bitcode.
> > > 
> > > `__attribute__((always_inline))` which `__forceinline__` expands to is 
> > > handled differently from regular inline. We have 
> > > Transforms/IPO/AlwaysInliner.cpp which handles them with -O1. Higher opt 
> > > levels will presumably inline these functions, too. Without always_inline 
> > > these wrappers will remain uninlined at -O1. Replacing it with just 
> > > `inline` will work OK (only affects -O1 in a minor way), but 
> > > `__forceinline__` is a bit closer to what I want from these wrappers.
> > > 
> > > As for the `static`, unused non-static functions will be emitted by clang 
> > > -- as far as clang is concerned they are externally visible. I've checked 
> > > and we do end up emitting them in PTX, even though they don't have 
> > > .visible directive (I think that's what you referred to when you 
> > > mentioned 'mark as internal'). I don't think we want that.
> > > 
> > > I'll keep this combination for now. We can adjust it later, if necessary.
> > Can you help me understand what makes you want this particular inlining 
> > behavior (i.e., always inline, even at -O0) for these wrappers?
> > 
> > Does e.g. libc++ do the same thing for the functions it has that are thin 
> > wrappers around e.g. libc functions?
> > 
> > > As for the static, unused non-static functions will be emitted by clang 
> > > -- as far as clang is concerned they are externally visible. I've checked 
> > > and we do end up emitting them in PTX, even though they don't have 
> > > .visible directive (I think that's what you referred to when you 
> > > mentioned 'mark as internal'). I don't think we want that.
> > 
> > Agree.  This is kind of unfortunate, though -- it's not How You Write 
> > Headers.  Could we add a comment?
> libc++ does seem to use _LIBCPP_ALWAYS_INLINE for a lot of wrapper-like 
> functions:
> 
> E.g. libcxx/include/__locale:
> ```
> 535:_LIBCPP_ALWAYS_INLINE
> 536-const char_type* tolower(char_type* __low, const char_type* __high) 
> const
> 537-{
> 538-return do_tolower(__low, __high);
> 539-}
> ```
> 
> 
sgtm then!


https://reviews.llvm.org/D42513



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

[PATCH] D42513: [CUDA] Added partial support for CUDA-9.1

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.



Comment at: clang/lib/Headers/__clang_cuda_device_functions.h:32
+
+#define __DEVICE__ static __device__ __forceinline__
+// There are number of functions that became compiler builtins in CUDA-9 and 
are

jlebar wrote:
> tra wrote:
> > jlebar wrote:
> > > I don't think we should need `__forceinline__`?  I'm sure they had that 
> > > in their headers, but clang doesn't rely on its inliner for correctness, 
> > > unlike some *other* compilers.  :)
> > > 
> > > I'm also not sure about `static`.  I think we mark all CUDA code as 
> > > internal, so it shouldn't be necessary?  If they're not static, they 
> > > should be marked as `inline`, though.
> > The idea is to make those wrappers always inlined, so they are effectively 
> > replaced with a call to __nv_* libdevice function. Whether the callee 
> > itself get inlined is controlled by the attributes in the libdevice bitcode.
> > 
> > `__attribute__((always_inline))` which `__forceinline__` expands to is 
> > handled differently from regular inline. We have 
> > Transforms/IPO/AlwaysInliner.cpp which handles them with -O1. Higher opt 
> > levels will presumably inline these functions, too. Without always_inline 
> > these wrappers will remain uninlined at -O1. Replacing it with just 
> > `inline` will work OK (only affects -O1 in a minor way), but 
> > `__forceinline__` is a bit closer to what I want from these wrappers.
> > 
> > As for the `static`, unused non-static functions will be emitted by clang 
> > -- as far as clang is concerned they are externally visible. I've checked 
> > and we do end up emitting them in PTX, even though they don't have .visible 
> > directive (I think that's what you referred to when you mentioned 'mark as 
> > internal'). I don't think we want that.
> > 
> > I'll keep this combination for now. We can adjust it later, if necessary.
> Can you help me understand what makes you want this particular inlining 
> behavior (i.e., always inline, even at -O0) for these wrappers?
> 
> Does e.g. libc++ do the same thing for the functions it has that are thin 
> wrappers around e.g. libc functions?
> 
> > As for the static, unused non-static functions will be emitted by clang -- 
> > as far as clang is concerned they are externally visible. I've checked and 
> > we do end up emitting them in PTX, even though they don't have .visible 
> > directive (I think that's what you referred to when you mentioned 'mark as 
> > internal'). I don't think we want that.
> 
> Agree.  This is kind of unfortunate, though -- it's not How You Write 
> Headers.  Could we add a comment?
libc++ does seem to use _LIBCPP_ALWAYS_INLINE for a lot of wrapper-like 
functions:

E.g. libcxx/include/__locale:
```
535:_LIBCPP_ALWAYS_INLINE
536-const char_type* tolower(char_type* __low, const char_type* __high) 
const
537-{
538-return do_tolower(__low, __high);
539-}
```




https://reviews.llvm.org/D42513



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

[PATCH] D42532: [OpenCL] Add "cles_khr_int64" extension.

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

I am just a bit unclear about the use of this extension in the kernel language. 
Could you please clarify?


Repository:
  rC Clang

https://reviews.llvm.org/D42532



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

[PATCH] D42220: libcxx: Use vcruntime declarations for typeinfo on Windows.

[Peter Collingbourne via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCXX323491: libcxx: Use vcruntime declarations for typeinfo on 
Windows. (authored by pcc, committed by ).
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D42220?vs=130343=131524#toc

Repository:
  rCXX libc++

https://reviews.llvm.org/D42220

Files:
  include/typeinfo
  src/support/runtime/exception_msvc.ipp
  src/typeinfo.cpp


Index: src/typeinfo.cpp
===
--- src/typeinfo.cpp
+++ src/typeinfo.cpp
@@ -9,7 +9,7 @@
 
 #include "typeinfo"
 
-#if defined(_LIBCPP_ABI_MICROSOFT)
+#if defined(_LIBCPP_ABI_MICROSOFT) && defined(_LIBCPP_NO_VCRUNTIME)
 #include 
 
 int std::type_info::__compare(const type_info &__rhs) const _NOEXCEPT {
@@ -49,7 +49,8 @@
 // FIXME: Remove __APPLE__ default here once buildit is gone.
 // FIXME: Remove the _LIBCPP_BUILDING_HAS_NO_ABI_LIBRARY configuration.
 #if (!defined(LIBCXX_BUILDING_LIBCXXABI) && !defined(LIBCXXRT) &&  
\
- !defined(__GLIBCXX__) && !defined(__APPLE__)) ||  
\
+ !defined(__GLIBCXX__) && !defined(__APPLE__) &&   
\
+ !(defined(_LIBCPP_ABI_MICROSOFT) && !defined(_LIBCPP_NO_VCRUNTIME))) ||   
\
 defined(_LIBCPP_BUILDING_HAS_NO_ABI_LIBRARY)
 std::type_info::~type_info()
 {
Index: src/support/runtime/exception_msvc.ipp
===
--- src/support/runtime/exception_msvc.ipp
+++ src/support/runtime/exception_msvc.ipp
@@ -97,6 +97,7 @@
 return "bad_array_length";
 }
 
+#if defined(_LIBCPP_NO_VCRUNTIME)
 bad_cast::bad_cast() _NOEXCEPT
 {
 }
@@ -125,7 +126,6 @@
   return "std::bad_typeid";
 }
 
-#if defined(_LIBCPP_NO_VCRUNTIME)
 exception::~exception() _NOEXCEPT
 {
 }
Index: include/typeinfo
===
--- include/typeinfo
+++ include/typeinfo
@@ -69,6 +69,10 @@
 #pragma GCC system_header
 #endif
 
+#if defined(_LIBCPP_ABI_MICROSOFT) && !defined(_LIBCPP_NO_VCRUNTIME)
+#include 
+#else
+
 #if !defined(_LIBCPP_ABI_MICROSOFT)
 #if defined(_LIBCPP_NONUNIQUE_RTTI_BIT)
 #define _LIBCPP_HAS_NONUNIQUE_TYPEINFO
@@ -219,6 +223,8 @@
 
 }  // std
 
+#endif // defined(_LIBCPP_ABI_MICROSOFT) && !defined(_LIBCPP_NO_VCRUNTIME)
+
 _LIBCPP_BEGIN_NAMESPACE_STD
 _LIBCPP_NORETURN inline _LIBCPP_ALWAYS_INLINE
 void __throw_bad_cast()


Index: src/typeinfo.cpp
===
--- src/typeinfo.cpp
+++ src/typeinfo.cpp
@@ -9,7 +9,7 @@
 
 #include "typeinfo"
 
-#if defined(_LIBCPP_ABI_MICROSOFT)
+#if defined(_LIBCPP_ABI_MICROSOFT) && defined(_LIBCPP_NO_VCRUNTIME)
 #include 
 
 int std::type_info::__compare(const type_info &__rhs) const _NOEXCEPT {
@@ -49,7 +49,8 @@
 // FIXME: Remove __APPLE__ default here once buildit is gone.
 // FIXME: Remove the _LIBCPP_BUILDING_HAS_NO_ABI_LIBRARY configuration.
 #if (!defined(LIBCXX_BUILDING_LIBCXXABI) && !defined(LIBCXXRT) &&  \
- !defined(__GLIBCXX__) && !defined(__APPLE__)) ||  \
+ !defined(__GLIBCXX__) && !defined(__APPLE__) &&   \
+ !(defined(_LIBCPP_ABI_MICROSOFT) && !defined(_LIBCPP_NO_VCRUNTIME))) ||   \
 defined(_LIBCPP_BUILDING_HAS_NO_ABI_LIBRARY)
 std::type_info::~type_info()
 {
Index: src/support/runtime/exception_msvc.ipp
===
--- src/support/runtime/exception_msvc.ipp
+++ src/support/runtime/exception_msvc.ipp
@@ -97,6 +97,7 @@
 return "bad_array_length";
 }
 
+#if defined(_LIBCPP_NO_VCRUNTIME)
 bad_cast::bad_cast() _NOEXCEPT
 {
 }
@@ -125,7 +126,6 @@
   return "std::bad_typeid";
 }
 
-#if defined(_LIBCPP_NO_VCRUNTIME)
 exception::~exception() _NOEXCEPT
 {
 }
Index: include/typeinfo
===
--- include/typeinfo
+++ include/typeinfo
@@ -69,6 +69,10 @@
 #pragma GCC system_header
 #endif
 
+#if defined(_LIBCPP_ABI_MICROSOFT) && !defined(_LIBCPP_NO_VCRUNTIME)
+#include 
+#else
+
 #if !defined(_LIBCPP_ABI_MICROSOFT)
 #if defined(_LIBCPP_NONUNIQUE_RTTI_BIT)
 #define _LIBCPP_HAS_NONUNIQUE_TYPEINFO
@@ -219,6 +223,8 @@
 
 }  // std
 
+#endif // defined(_LIBCPP_ABI_MICROSOFT) && !defined(_LIBCPP_NO_VCRUNTIME)
+
 _LIBCPP_BEGIN_NAMESPACE_STD
 _LIBCPP_NORETURN inline _LIBCPP_ALWAYS_INLINE
 void __throw_bad_cast()
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42513: [CUDA] Added partial support for CUDA-9.1

[Justin Lebar via Phabricator via cfe-commits]

jlebar added inline comments.



Comment at: clang/lib/Headers/__clang_cuda_device_functions.h:32
+
+#define __DEVICE__ static __device__ __forceinline__
+// There are number of functions that became compiler builtins in CUDA-9 and 
are

tra wrote:
> jlebar wrote:
> > I don't think we should need `__forceinline__`?  I'm sure they had that in 
> > their headers, but clang doesn't rely on its inliner for correctness, 
> > unlike some *other* compilers.  :)
> > 
> > I'm also not sure about `static`.  I think we mark all CUDA code as 
> > internal, so it shouldn't be necessary?  If they're not static, they should 
> > be marked as `inline`, though.
> The idea is to make those wrappers always inlined, so they are effectively 
> replaced with a call to __nv_* libdevice function. Whether the callee itself 
> get inlined is controlled by the attributes in the libdevice bitcode.
> 
> `__attribute__((always_inline))` which `__forceinline__` expands to is 
> handled differently from regular inline. We have 
> Transforms/IPO/AlwaysInliner.cpp which handles them with -O1. Higher opt 
> levels will presumably inline these functions, too. Without always_inline 
> these wrappers will remain uninlined at -O1. Replacing it with just `inline` 
> will work OK (only affects -O1 in a minor way), but `__forceinline__` is a 
> bit closer to what I want from these wrappers.
> 
> As for the `static`, unused non-static functions will be emitted by clang -- 
> as far as clang is concerned they are externally visible. I've checked and we 
> do end up emitting them in PTX, even though they don't have .visible 
> directive (I think that's what you referred to when you mentioned 'mark as 
> internal'). I don't think we want that.
> 
> I'll keep this combination for now. We can adjust it later, if necessary.
Can you help me understand what makes you want this particular inlining 
behavior (i.e., always inline, even at -O0) for these wrappers?

Does e.g. libc++ do the same thing for the functions it has that are thin 
wrappers around e.g. libc functions?

> As for the static, unused non-static functions will be emitted by clang -- as 
> far as clang is concerned they are externally visible. I've checked and we do 
> end up emitting them in PTX, even though they don't have .visible directive 
> (I think that's what you referred to when you mentioned 'mark as internal'). 
> I don't think we want that.

Agree.  This is kind of unfortunate, though -- it's not How You Write Headers.  
Could we add a comment?



Comment at: clang/lib/Headers/__clang_cuda_runtime_wrapper.h:144
+// Declare or define device-side functions that particular CUDA version relies
+// on but does (no longer) declare in its headers. E.g. some of the device-side
+// functions that used to be implemented in a header in CUDA-8, became NVCC's

tra wrote:
> jlebar wrote:
> > Sentence doesn't read right if you skip the parens.
> > 
> > But also: Our header provides *everything* from 
> > cuda/device_functions_decls.h, not just the things that were once in that 
> > header and, in newer CUDA versions, are no longer there, right?  I'm 
> > actually even more confused now that I read the big header, because here we 
> > say the header "defines or declares" these functions, but that suggests 
> > that for every function, it decides whether to define or to declare it, but 
> > that's not the case...
> I should rephrase that then.
> 
> `__clang_cuda_device_functions.h` does two jobs. It always includes 
> `__clang_cuda_libdevice_declares.h` which provides **declarations** for the 
> libdevice functions. Those are gone in CUDA-9.1.
> 
> It also provides **definitions** for the standard library and __* device-side 
> functions that call libdevice.
> 
> I can explicitly include _clang_cuda_libdevice_declares.h  here and update 
> the comment to make more sense.
I think that would help clarify things, for sure.


https://reviews.llvm.org/D42513



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

[libcxx] r323491 - libcxx: Use vcruntime declarations for typeinfo on Windows.

[Peter Collingbourne via cfe-commits]

Author: pcc
Date: Thu Jan 25 17:22:17 2018
New Revision: 323491

URL: http://llvm.org/viewvc/llvm-project?rev=323491=rev
Log:
libcxx: Use vcruntime declarations for typeinfo on Windows.

We need to use the vcruntime declarations on Windows to avoid an
ODR violation involving rtti.obj, which provides the definition of
the runtime function implementing dynamic_cast and depends on the
vcruntime implementations of bad_cast and bad_typeid.

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

Modified:
libcxx/trunk/include/typeinfo
libcxx/trunk/src/support/runtime/exception_msvc.ipp
libcxx/trunk/src/typeinfo.cpp

Modified: libcxx/trunk/include/typeinfo
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/typeinfo?rev=323491=323490=323491=diff
==
--- libcxx/trunk/include/typeinfo (original)
+++ libcxx/trunk/include/typeinfo Thu Jan 25 17:22:17 2018
@@ -69,6 +69,10 @@ public:
 #pragma GCC system_header
 #endif
 
+#if defined(_LIBCPP_ABI_MICROSOFT) && !defined(_LIBCPP_NO_VCRUNTIME)
+#include 
+#else
+
 #if !defined(_LIBCPP_ABI_MICROSOFT)
 #if defined(_LIBCPP_NONUNIQUE_RTTI_BIT)
 #define _LIBCPP_HAS_NONUNIQUE_TYPEINFO
@@ -219,6 +223,8 @@ public:
 
 }  // std
 
+#endif // defined(_LIBCPP_ABI_MICROSOFT) && !defined(_LIBCPP_NO_VCRUNTIME)
+
 _LIBCPP_BEGIN_NAMESPACE_STD
 _LIBCPP_NORETURN inline _LIBCPP_ALWAYS_INLINE
 void __throw_bad_cast()

Modified: libcxx/trunk/src/support/runtime/exception_msvc.ipp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/support/runtime/exception_msvc.ipp?rev=323491=323490=323491=diff
==
--- libcxx/trunk/src/support/runtime/exception_msvc.ipp (original)
+++ libcxx/trunk/src/support/runtime/exception_msvc.ipp Thu Jan 25 17:22:17 2018
@@ -97,6 +97,7 @@ bad_array_length::what() const _NOEXCEPT
 return "bad_array_length";
 }
 
+#if defined(_LIBCPP_NO_VCRUNTIME)
 bad_cast::bad_cast() _NOEXCEPT
 {
 }
@@ -125,7 +126,6 @@ bad_typeid::what() const _NOEXCEPT
   return "std::bad_typeid";
 }
 
-#if defined(_LIBCPP_NO_VCRUNTIME)
 exception::~exception() _NOEXCEPT
 {
 }

Modified: libcxx/trunk/src/typeinfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/typeinfo.cpp?rev=323491=323490=323491=diff
==
--- libcxx/trunk/src/typeinfo.cpp (original)
+++ libcxx/trunk/src/typeinfo.cpp Thu Jan 25 17:22:17 2018
@@ -9,7 +9,7 @@
 
 #include "typeinfo"
 
-#if defined(_LIBCPP_ABI_MICROSOFT)
+#if defined(_LIBCPP_ABI_MICROSOFT) && defined(_LIBCPP_NO_VCRUNTIME)
 #include 
 
 int std::type_info::__compare(const type_info &__rhs) const _NOEXCEPT {
@@ -49,7 +49,8 @@ size_t std::type_info::hash_code() const
 // FIXME: Remove __APPLE__ default here once buildit is gone.
 // FIXME: Remove the _LIBCPP_BUILDING_HAS_NO_ABI_LIBRARY configuration.
 #if (!defined(LIBCXX_BUILDING_LIBCXXABI) && !defined(LIBCXXRT) &&  
\
- !defined(__GLIBCXX__) && !defined(__APPLE__)) ||  
\
+ !defined(__GLIBCXX__) && !defined(__APPLE__) &&   
\
+ !(defined(_LIBCPP_ABI_MICROSOFT) && !defined(_LIBCPP_NO_VCRUNTIME))) ||   
\
 defined(_LIBCPP_BUILDING_HAS_NO_ABI_LIBRARY)
 std::type_info::~type_info()
 {


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

Re: [libcxx] r323453 - [cmake] [libcxx] Call llvm_setup_rpath() when adding shared libraries.

[Duncan P. N. Exon Smith via cfe-commits]

> On Jan 25, 2018, at 17:09, Don Hinton  wrote:
> 
> 
> 
> On Thu, Jan 25, 2018 at 4:21 PM, Duncan P. N. Exon Smith 
> > wrote:
> 
> 
>> On Jan 25, 2018, at 16:18, Duncan P. N. Exon Smith via cfe-commits 
>> > wrote:
>> 
>> 
>> 
>>> On Jan 25, 2018, at 13:07, Don Hinton >> > wrote:
>>> 
>>> On Thu, Jan 25, 2018 at 12:54 PM, Chris Bieneman >> > wrote:
>>> Historically we have supported building libcxx without llvm-config 
>>> available on the system.
>>> 
>>> In all likelihood the bots didn't fail because the bots do have llvm-config 
>>> because we usually require an llvm build or source checkout in order to 
>>> build and run the tests since tests depend on lit and gtest.
>>> 
>>> Ah, okay, but if the bots don't test/enforce this, does it make sense to 
>>> continue to support building without llvm-config?
>> 
>> I didn't realize we didn't have a bot enforcing this.  I just made one:
>> http://lab.llvm.org:8080/green/view/Libcxx/job/libcxx_master_cmake_standalone
>>  
>> 
>> 
>>> I don't want to break upstream users, but the alternative is to duplicate 
>>> this logic across multiple sub-projects.  (see 
>>> http://lists.llvm.org/pipermail/llvm-dev/2018-January/120707.html 
>>>  for a 
>>> related rpath issue).
>> 
>> It's temporary though.  In the brave new world on GitHub with monorepo, the 
>> runtime libraries will be able to access a shared set of CMake configuration 
>> scripts.
> 
> Note that there's other stuff duplicated in the runtime libraries, such as 
> preprocessor definitions (LLVM_... => __LIBCXX_...), that we'll never really 
> be able to collapse.
> 
> Great, I've changed it to this:
> 
> +  if(LLVM_FOUND)
> +llvm_setup_rpath(cxx_shared)
> +  endif()
> 
> Which should work for both cases.  Is that okay?

SGTM.

>  
> 
>>> 
>>>  
>>> 
>>> 
>>> -Chris
>>> 
>>> 
 On Jan 25, 2018, at 12:51 PM, Don Hinton > wrote:
 
 On Thu, Jan 25, 2018 at 12:29 PM, Duncan P. N. Exon Smith 
 > wrote:
 I don't really understand why, but our bots seemed to survive this:
 http://lab.llvm.org:8080/green/view/Libcxx/job/libcxx_master_cmake/2137/ 
 
 
 Console output is here:
 http://lab.llvm.org:8080/green/view/Libcxx/job/libcxx_master_cmake/2137/consoleFull
  
 
 
 That doesn't necessarily mean it's safe.  Does anyone know why this might 
 have "worked"?
 
 I'm not an expert, but llvm installs all of these cmake modules along side 
 llvm-config, so if you find llvm-config, you find these modules.
 
 So, unless I'm missing something, it looks like AddLLVM.cmake is always 
 available.
  
 
> On Jan 25, 2018, at 11:02, Shoaib Meenai  > wrote:
> 
> This is going to break building libc++ standalone (i.e. without any LLVM 
> repository or even its CMake modules), right? Some upstream users care a 
> lot about that use case (CC beanz and Duncan).
>  
> From: cfe-commits  > on behalf of Don Hinton via 
> cfe-commits  >
> Reply-To: Don Hinton >
> Date: Thursday, January 25, 2018 at 10:15 AM
> To: "cfe-commits@lists.llvm.org " 
> >
> Subject: [libcxx] r323453 - [cmake] [libcxx] Call llvm_setup_rpath() when 
> adding shared libraries.
>  
> Author: dhinton
> Date: Thu Jan 25 10:13:26 2018
> New Revision: 323453
>  
> URL: 
> https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject-3Frev-3D323453-26view-3Drev=DwIGaQ=5VD0RTtNlTh3ycd41b3MUw=o3kDXzdBUE3ljQXKeTWOMw=zwvSmk9ZIfMARYg3JvYvNuA0Iz-PEq3H2i_ICF6vsR8=LDCu89byjNdZWoCIYHGHaPr3IamIdHLF0JwbnYE92vM=
>  
> 
> Log:
> [cmake] [libcxx] Call llvm_setup_rpath() when adding shared libraries.
>  
> Clang and llvm already use llvm_setup_rpath(), 

[libcxx] r323490 - libcxx: Move #include_next out of header guard in wrapper header.

[Peter Collingbourne via cfe-commits]

Author: pcc
Date: Thu Jan 25 17:19:23 2018
New Revision: 323490

URL: http://llvm.org/viewvc/llvm-project?rev=323490=rev
Log:
libcxx: Move #include_next  out of header guard in wrapper header.

Code on Windows expects to be able to do:

 #define _USE_MATH_DEFINES
 #include 

and receive the definitions of mathematical constants, even if 
has previously been included. To support this scenario, re-include
 every time the wrapper header is included.

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

Added:
libcxx/trunk/test/libcxx/depr/depr.c.headers/math_h.sh.cpp
Modified:
libcxx/trunk/include/math.h

Modified: libcxx/trunk/include/math.h
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/math.h?rev=323490=323489=323490=diff
==
--- libcxx/trunk/include/math.h (original)
+++ libcxx/trunk/include/math.h Thu Jan 25 17:19:23 2018
@@ -8,6 +8,16 @@
 //
 
//===--===//
 
+// This include lives outside the header guard in order to support an MSVC
+// extension which allows users to do:
+//
+// #define _USE_MATH_DEFINES
+// #include 
+//
+// and receive the definitions of mathematical constants, even if 
+// has previously been included.
+#include_next 
+
 #ifndef _LIBCPP_MATH_H
 #define _LIBCPP_MATH_H
 
@@ -298,8 +308,6 @@ long doubletruncl(long double x);
 #pragma GCC system_header
 #endif
 
-#include_next 
-
 #ifdef __cplusplus
 
 // We support including .h headers inside 'extern "C"' contexts, so switch

Added: libcxx/trunk/test/libcxx/depr/depr.c.headers/math_h.sh.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/libcxx/depr/depr.c.headers/math_h.sh.cpp?rev=323490=auto
==
--- libcxx/trunk/test/libcxx/depr/depr.c.headers/math_h.sh.cpp (added)
+++ libcxx/trunk/test/libcxx/depr/depr.c.headers/math_h.sh.cpp Thu Jan 25 
17:19:23 2018
@@ -0,0 +1,23 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// RUN: %compile -fsyntax-only
+
+#ifdef _MSC_VER
+
+#include 
+
+#define _USE_MATH_DEFINES
+#include 
+
+#ifndef M_PI
+#error M_PI not defined
+#endif
+
+#endif


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

[PATCH] D42513: [CUDA] Added partial support for CUDA-9.1

[Artem Belevich via Phabricator via cfe-commits]

tra marked 2 inline comments as done.
tra added inline comments.



Comment at: clang/lib/Headers/__clang_cuda_device_functions.h:32
+
+#define __DEVICE__ static __device__ __forceinline__
+// There are number of functions that became compiler builtins in CUDA-9 and 
are

jlebar wrote:
> I don't think we should need `__forceinline__`?  I'm sure they had that in 
> their headers, but clang doesn't rely on its inliner for correctness, unlike 
> some *other* compilers.  :)
> 
> I'm also not sure about `static`.  I think we mark all CUDA code as internal, 
> so it shouldn't be necessary?  If they're not static, they should be marked 
> as `inline`, though.
The idea is to make those wrappers always inlined, so they are effectively 
replaced with a call to __nv_* libdevice function. Whether the callee itself 
get inlined is controlled by the attributes in the libdevice bitcode.

`__attribute__((always_inline))` which `__forceinline__` expands to is handled 
differently from regular inline. We have Transforms/IPO/AlwaysInliner.cpp which 
handles them with -O1. Higher opt levels will presumably inline these 
functions, too. Without always_inline these wrappers will remain uninlined at 
-O1. Replacing it with just `inline` will work OK (only affects -O1 in a minor 
way), but `__forceinline__` is a bit closer to what I want from these wrappers.

As for the `static`, unused non-static functions will be emitted by clang -- as 
far as clang is concerned they are externally visible. I've checked and we do 
end up emitting them in PTX, even though they don't have .visible directive (I 
think that's what you referred to when you mentioned 'mark as internal'). I 
don't think we want that.

I'll keep this combination for now. We can adjust it later, if necessary.



Comment at: clang/lib/Headers/__clang_cuda_device_functions.h:51
+__DEVICE__ void __brkpt() { asm volatile("brkpt;"); }
+__DEVICE__ void __brkpt(int __a) { __brkpt(); }
+__DEVICE__ unsigned int __byte_perm(unsigned int __a, unsigned int __b,

jlebar wrote:
> Sanity check: Ignoring the __a argument here is correct?
Yes. It's a leftover from the old versions and has been deprecated in CUDA-8:
```
__DEPRECATED__("Please use __brkpt() instead.") void brkpt(int c = 0)
```

CUDA-9.1 does not have it at all.



Comment at: clang/lib/Headers/__clang_cuda_runtime_wrapper.h:144
+// Declare or define device-side functions that particular CUDA version relies
+// on but does (no longer) declare in its headers. E.g. some of the device-side
+// functions that used to be implemented in a header in CUDA-8, became NVCC's

jlebar wrote:
> Sentence doesn't read right if you skip the parens.
> 
> But also: Our header provides *everything* from 
> cuda/device_functions_decls.h, not just the things that were once in that 
> header and, in newer CUDA versions, are no longer there, right?  I'm actually 
> even more confused now that I read the big header, because here we say the 
> header "defines or declares" these functions, but that suggests that for 
> every function, it decides whether to define or to declare it, but that's not 
> the case...
I should rephrase that then.

`__clang_cuda_device_functions.h` does two jobs. It always includes 
`__clang_cuda_libdevice_declares.h` which provides **declarations** for the 
libdevice functions. Those are gone in CUDA-9.1.

It also provides **definitions** for the standard library and __* device-side 
functions that call libdevice.

I can explicitly include _clang_cuda_libdevice_declares.h  here and update the 
comment to make more sense.



Comment at: clang/lib/Headers/__clang_cuda_runtime_wrapper.h:155
+#undef __THROW
+#define __THROW
 

jlebar wrote:
> Should we be pushing/popping the macro?  That is, do we want this macro 
> exposed to user code?
We do  push __THROW at the beginning of this header and pop it closer to the 
end, so this redefinition is not visible outside.



https://reviews.llvm.org/D42513



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

Re: [libcxx] r323453 - [cmake] [libcxx] Call llvm_setup_rpath() when adding shared libraries.

[Don Hinton via cfe-commits]

On Thu, Jan 25, 2018 at 4:21 PM, Duncan P. N. Exon Smith <
dexonsm...@apple.com> wrote:

>
>
> On Jan 25, 2018, at 16:18, Duncan P. N. Exon Smith via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>
>
> On Jan 25, 2018, at 13:07, Don Hinton  wrote:
>
> On Thu, Jan 25, 2018 at 12:54 PM, Chris Bieneman  wrote:
>
>> Historically we have supported building libcxx without llvm-config
>> available on the system.
>>
>> In all likelihood the bots didn't fail because the bots do have
>> llvm-config because we usually require an llvm build or source checkout in
>> order to build and run the tests since tests depend on lit and gtest.
>>
>
> Ah, okay, but if the bots don't test/enforce this, does it make sense to
> continue to support building without llvm-config?
>
>
> I didn't realize we didn't have a bot enforcing this.  I just made one:
> http://lab.llvm.org:8080/green/view/Libcxx/job/libcxx_
> master_cmake_standalone
>
> I don't want to break upstream users, but the alternative is to duplicate
> this logic across multiple sub-projects.  (see http://lists.llvm.org/
> pipermail/llvm-dev/2018-January/120707.html for a related rpath issue).
>
>
> It's temporary though.  In the brave new world on GitHub with monorepo,
> the runtime libraries will be able to access a shared set of CMake
> configuration scripts.
>
>
> Note that there's other stuff duplicated in the runtime libraries, such as
> preprocessor definitions (LLVM_... => __LIBCXX_...), that we'll never
> really be able to collapse.
>

Great, I've changed it to this:

+  if(LLVM_FOUND)
+llvm_setup_rpath(cxx_shared)
+  endif()

Which should work for both cases.  Is that okay?


>
>
>
>
>>
>>
>> -Chris
>>
>>
>> On Jan 25, 2018, at 12:51 PM, Don Hinton  wrote:
>>
>> On Thu, Jan 25, 2018 at 12:29 PM, Duncan P. N. Exon Smith <
>> dexonsm...@apple.com> wrote:
>>
>>> I don't really understand why, but our bots seemed to survive this:
>>> http://lab.llvm.org:8080/green/view/Libcxx/job/libcxx_master_cmake/2137/
>>>
>>> Console output is here:
>>> http://lab.llvm.org:8080/green/view/Libcxx/job/libcxx_master
>>> _cmake/2137/consoleFull
>>>
>>> That doesn't necessarily mean it's safe.  Does anyone know why this
>>> might have "worked"?
>>>
>>
>> I'm not an expert, but llvm installs all of these cmake modules along
>> side llvm-config, so if you find llvm-config, you find these modules.
>>
>> So, unless I'm missing something, it looks like AddLLVM.cmake is always
>> available.
>>
>>
>>>
>>> On Jan 25, 2018, at 11:02, Shoaib Meenai  wrote:
>>>
>>> This is going to break building libc++ standalone (i.e. without any LLVM
>>> repository or even its CMake modules), right? Some upstream users care a
>>> lot about that use case (CC beanz and Duncan).
>>>
>>> *From: *cfe-commits  on behalf of
>>> Don Hinton via cfe-commits 
>>> *Reply-To: *Don Hinton 
>>> *Date: *Thursday, January 25, 2018 at 10:15 AM
>>> *To: *"cfe-commits@lists.llvm.org" 
>>> *Subject: *[libcxx] r323453 - [cmake] [libcxx] Call llvm_setup_rpath()
>>> when adding shared libraries.
>>>
>>> Author: dhinton
>>> Date: Thu Jan 25 10:13:26 2018
>>> New Revision: 323453
>>>
>>> URL: https://urldefense.proofpoint.com/v2/url?u=http-3A__llv
>>> m.org_viewvc_llvm-2Dproject-3Frev-3D323453-26view-3Drev=
>>> DwIGaQ=5VD0RTtNlTh3ycd41b3MUw=o3kDXzdBUE3ljQXKeTWOMw=
>>> zwvSmk9ZIfMARYg3JvYvNuA0Iz-PEq3H2i_ICF6vsR8=LDCu89byjNdZWo
>>> CIYHGHaPr3IamIdHLF0JwbnYE92vM=
>>> Log:
>>> [cmake] [libcxx] Call llvm_setup_rpath() when adding shared libraries.
>>>
>>> Clang and llvm already use llvm_setup_rpath(), so this change will
>>> help standarize rpath usage across all projects.
>>>
>>> Differential Revision: https://urldefense.p
>>> roofpoint.com/v2/url?u=https-3A__reviews.llvm.org_D42459=D
>>> wIGaQ=5VD0RTtNlTh3ycd41b3MUw=o3kDXzdBUE3ljQXKeTWOMw=zw
>>> vSmk9ZIfMARYg3JvYvNuA0Iz-PEq3H2i_ICF6vsR8=YCR-YJBua5p-4IK0
>>> 5GjHoZUU7aN8UJAFzL2xaz7byyw=
>>>
>>> Modified:
>>> libcxx/trunk/lib/CMakeLists.txt
>>>
>>> Modified: libcxx/trunk/lib/CMakeLists.txt
>>> URL: https://urldefense.proofpoint.com/v2/url?u=http-3A__llv
>>> m.org_viewvc_llvm-2Dproject_libcxx_trunk_lib_CMakeLists.tx
>>> t-3Frev-3D323453-26r1-3D323452-26r2-3D323453-26view-3Ddiff&
>>> d=DwIGaQ=5VD0RTtNlTh3ycd41b3MUw=o3kDXzdBUE3ljQXKeTWOMw&
>>> m=zwvSmk9ZIfMARYg3JvYvNuA0Iz-PEq3H2i_ICF6vsR8=Rd7xc2GWS8o
>>> yDcm0QshNPTqM3BglPY5_aHMIuCRUt1s=
>>> 
>>> ==
>>> --- libcxx/trunk/lib/CMakeLists.txt (original)
>>> +++ libcxx/trunk/lib/CMakeLists.txt Thu Jan 25 10:13:26 2018
>>> @@ -222,6 +222,7 @@ set(LIBCXX_TARGETS)
>>> # Build the shared library.
>>> if (LIBCXX_ENABLE_SHARED)
>>>add_library(cxx_shared SHARED $)
>>> +  llvm_setup_rpath(cxx_shared)
>>>target_link_libraries(cxx_shared ${LIBCXX_LIBRARIES})
>>>

[PATCH] D42555: [libunwind] Don't enable _LIBUNWIND_BUILD_ZERO_COST_APIS if building the SJLJ APIs

[Saleem Abdulrasool via Phabricator via cfe-commits]

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

Ugh, I really wish that we could simplify that wrapping conditional.


https://reviews.llvm.org/D42555



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

[PATCH] D42561: [PR36008] Avoid -Wsign-compare warning for enum constants in typeof expressions

[Alex Lorenz via Phabricator via cfe-commits]

arphaman created this revision.
arphaman added a reviewer: lebedev.ri.

This patch looks through typeof type at the original expression when diagnosing 
-Wsign-compare to avoid an unfriendly diagnostic.


Repository:
  rC Clang

https://reviews.llvm.org/D42561

Files:
  lib/Sema/SemaChecking.cpp
  test/Sema/compare.c


Index: test/Sema/compare.c
===
--- test/Sema/compare.c
+++ test/Sema/compare.c
@@ -391,3 +391,15 @@
 void test12(unsigned a) {
   if (0 && -1 > a) { }
 }
+
+// PR36008
+
+enum PR36008EnumTest {
+kPR36008Value = 0,
+};
+
+void pr36008(enum PR36008EnumTest lhs) {
+  __typeof__(lhs) x = lhs;
+  __typeof__(kPR36008Value) y = (kPR36008Value);
+  if (x == y) x = y; // no warning
+}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -8955,6 +8955,11 @@
   LHS = LHS->IgnoreParenImpCasts();
   RHS = RHS->IgnoreParenImpCasts();
 
+  if (!S.getLangOpts().CPlusPlus) {
+if (const TypeOfExprType *TET = dyn_cast(RHS->getType()))
+  RHS = TET->getUnderlyingExpr()->IgnoreParenImpCasts();
+  }
+
   // Check to see if one of the (unmodified) operands is of different
   // signedness.
   Expr *signedOperand, *unsignedOperand;


Index: test/Sema/compare.c
===
--- test/Sema/compare.c
+++ test/Sema/compare.c
@@ -391,3 +391,15 @@
 void test12(unsigned a) {
   if (0 && -1 > a) { }
 }
+
+// PR36008
+
+enum PR36008EnumTest {
+kPR36008Value = 0,
+};
+
+void pr36008(enum PR36008EnumTest lhs) {
+  __typeof__(lhs) x = lhs;
+  __typeof__(kPR36008Value) y = (kPR36008Value);
+  if (x == y) x = y; // no warning
+}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -8955,6 +8955,11 @@
   LHS = LHS->IgnoreParenImpCasts();
   RHS = RHS->IgnoreParenImpCasts();
 
+  if (!S.getLangOpts().CPlusPlus) {
+if (const TypeOfExprType *TET = dyn_cast(RHS->getType()))
+  RHS = TET->getUnderlyingExpr()->IgnoreParenImpCasts();
+  }
+
   // Check to see if one of the (unmodified) operands is of different
   // signedness.
   Expr *signedOperand, *unsignedOperand;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42276: [Driver] Add an -fexperimental-isel driver option to enable/disable GlobalISel

[Amara Emerson via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC323485: [Driver] Add an -fexperimental-isel driver option to 
enable/disable GlobalISel. (authored by aemerson, committed by ).

Repository:
  rC Clang

https://reviews.llvm.org/D42276

Files:
  include/clang/Basic/DiagnosticDriverKinds.td
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Driver/Options.td
  lib/Driver/ToolChains/Clang.cpp
  test/Driver/global-isel.c

Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -4694,6 +4694,37 @@
 CmdArgs.push_back("-fwhole-program-vtables");
   }
 
+  if (Arg *A = Args.getLastArg(options::OPT_fexperimental_isel,
+   options::OPT_fno_experimental_isel)) {
+CmdArgs.push_back("-mllvm");
+if (A->getOption().matches(options::OPT_fexperimental_isel)) {
+  CmdArgs.push_back("-global-isel=1");
+
+  // GISel is on by default on AArch64 -O0, so don't bother adding
+  // the fallback remarks for it. Other combinations will add a warning of
+  // some kind.
+  bool IsArchSupported = Triple.getArch() == llvm::Triple::aarch64;
+  bool IsOptLevelSupported = false;
+
+  Arg *A = Args.getLastArg(options::OPT_O_Group);
+  if (Triple.getArch() == llvm::Triple::aarch64) {
+if (!A || A->getOption().matches(options::OPT_O0))
+  IsOptLevelSupported = true;
+  }
+  if (!IsArchSupported || !IsOptLevelSupported) {
+CmdArgs.push_back("-mllvm");
+CmdArgs.push_back("-global-isel-abort=2");
+
+if (!IsArchSupported)
+  D.Diag(diag::warn_drv_experimental_isel_incomplete) << Triple.getArchName();
+else
+  D.Diag(diag::warn_drv_experimental_isel_incomplete_opt);
+  }
+} else {
+  CmdArgs.push_back("-global-isel=0");
+}
+  }
+
   // Finally add the compile command to the compilation.
   if (Args.hasArg(options::OPT__SLASH_fallback) &&
   Output.getType() == types::TY_Object &&
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -1033,6 +1033,8 @@
 def finline_hint_functions: Flag<["-"], "finline-hint-functions">, Group, Flags<[CC1Option]>,
   HelpText<"Inline functions which are (explicitly or implicitly) marked inline">;
 def finline : Flag<["-"], "finline">, Group;
+def fexperimental_isel : Flag<["-"], "fexperimental-isel">, Group,
+  HelpText<"Enables the experimental global instruction selector">;
 def fexperimental_new_pass_manager : Flag<["-"], "fexperimental-new-pass-manager">,
   Group, Flags<[CC1Option]>,
   HelpText<"Enables an experimental new pass manager in LLVM.">;
@@ -1244,6 +1246,8 @@
 def fno_gnu_keywords : Flag<["-"], "fno-gnu-keywords">, Group, Flags<[CC1Option]>;
 def fno_inline_functions : Flag<["-"], "fno-inline-functions">, Group, Flags<[CC1Option]>;
 def fno_inline : Flag<["-"], "fno-inline">, Group, Flags<[CC1Option]>;
+def fno_experimental_isel : Flag<["-"], "fno-experimental-isel">, Group,
+  HelpText<"Disables the experimental global instruction selector">;
 def fno_experimental_new_pass_manager : Flag<["-"], "fno-experimental-new-pass-manager">,
   Group, Flags<[CC1Option]>,
   HelpText<"Disables an experimental new pass manager in LLVM.">;
Index: include/clang/Basic/DiagnosticDriverKinds.td
===
--- include/clang/Basic/DiagnosticDriverKinds.td
+++ include/clang/Basic/DiagnosticDriverKinds.td
@@ -361,4 +361,12 @@
 def note_drv_verify_prefix_spelling : Note<
   "-verify prefixes must start with a letter and contain only alphanumeric"
   " characters, hyphens, and underscores">;
+
+def warn_drv_experimental_isel_incomplete : Warning<
+  "-fexperimental-isel support for the '%0' architecture is incomplete">,
+  InGroup;
+
+def warn_drv_experimental_isel_incomplete_opt : Warning<
+  "-fexperimental-isel support is incomplete for this architecture at the current optimization level">,
+  InGroup;
 }
Index: include/clang/Basic/DiagnosticGroups.td
===
--- include/clang/Basic/DiagnosticGroups.td
+++ include/clang/Basic/DiagnosticGroups.td
@@ -985,3 +985,6 @@
 // A warning group for warnings about code that clang accepts when
 // compiling OpenCL C/C++ but which is not compatible with the SPIR spec.
 def SpirCompat : DiagGroup<"spir-compat">;
+
+// Warning for the experimental-isel options.
+def ExperimentalISel : DiagGroup<"experimental-isel">;
Index: test/Driver/global-isel.c
===
--- test/Driver/global-isel.c
+++ test/Driver/global-isel.c
@@ -0,0 +1,24 @@
+// REQUIRES: x86-registered-target,aarch64-registered-target
+
+// RUN: %clang -fexperimental-isel -S -### %s 2>&1 | 

Re: [libcxx] r323453 - [cmake] [libcxx] Call llvm_setup_rpath() when adding shared libraries.

[Duncan P. N. Exon Smith via cfe-commits]

> On Jan 25, 2018, at 16:18, Duncan P. N. Exon Smith via cfe-commits 
>  wrote:
> 
> 
> 
>> On Jan 25, 2018, at 13:07, Don Hinton > > wrote:
>> 
>> On Thu, Jan 25, 2018 at 12:54 PM, Chris Bieneman > > wrote:
>> Historically we have supported building libcxx without llvm-config available 
>> on the system.
>> 
>> In all likelihood the bots didn't fail because the bots do have llvm-config 
>> because we usually require an llvm build or source checkout in order to 
>> build and run the tests since tests depend on lit and gtest.
>> 
>> Ah, okay, but if the bots don't test/enforce this, does it make sense to 
>> continue to support building without llvm-config?
> 
> I didn't realize we didn't have a bot enforcing this.  I just made one:
> http://lab.llvm.org:8080/green/view/Libcxx/job/libcxx_master_cmake_standalone 
> 
> 
>> I don't want to break upstream users, but the alternative is to duplicate 
>> this logic across multiple sub-projects.  (see 
>> http://lists.llvm.org/pipermail/llvm-dev/2018-January/120707.html 
>>  for a 
>> related rpath issue).
> 
> It's temporary though.  In the brave new world on GitHub with monorepo, the 
> runtime libraries will be able to access a shared set of CMake configuration 
> scripts.

Note that there's other stuff duplicated in the runtime libraries, such as 
preprocessor definitions (LLVM_... => __LIBCXX_...), that we'll never really be 
able to collapse.

>> 
>>  
>> 
>> 
>> -Chris
>> 
>> 
>>> On Jan 25, 2018, at 12:51 PM, Don Hinton >> > wrote:
>>> 
>>> On Thu, Jan 25, 2018 at 12:29 PM, Duncan P. N. Exon Smith 
>>> > wrote:
>>> I don't really understand why, but our bots seemed to survive this:
>>> http://lab.llvm.org:8080/green/view/Libcxx/job/libcxx_master_cmake/2137/ 
>>> 
>>> 
>>> Console output is here:
>>> http://lab.llvm.org:8080/green/view/Libcxx/job/libcxx_master_cmake/2137/consoleFull
>>>  
>>> 
>>> 
>>> That doesn't necessarily mean it's safe.  Does anyone know why this might 
>>> have "worked"?
>>> 
>>> I'm not an expert, but llvm installs all of these cmake modules along side 
>>> llvm-config, so if you find llvm-config, you find these modules.
>>> 
>>> So, unless I'm missing something, it looks like AddLLVM.cmake is always 
>>> available.
>>>  
>>> 
 On Jan 25, 2018, at 11:02, Shoaib Meenai > wrote:
 
 This is going to break building libc++ standalone (i.e. without any LLVM 
 repository or even its CMake modules), right? Some upstream users care a 
 lot about that use case (CC beanz and Duncan).
  
 From: cfe-commits > on behalf of Don Hinton via 
 cfe-commits >
 Reply-To: Don Hinton >
 Date: Thursday, January 25, 2018 at 10:15 AM
 To: "cfe-commits@lists.llvm.org " 
 >
 Subject: [libcxx] r323453 - [cmake] [libcxx] Call llvm_setup_rpath() when 
 adding shared libraries.
  
 Author: dhinton
 Date: Thu Jan 25 10:13:26 2018
 New Revision: 323453
  
 URL: 
 https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject-3Frev-3D323453-26view-3Drev=DwIGaQ=5VD0RTtNlTh3ycd41b3MUw=o3kDXzdBUE3ljQXKeTWOMw=zwvSmk9ZIfMARYg3JvYvNuA0Iz-PEq3H2i_ICF6vsR8=LDCu89byjNdZWoCIYHGHaPr3IamIdHLF0JwbnYE92vM=
  
 
 Log:
 [cmake] [libcxx] Call llvm_setup_rpath() when adding shared libraries.
  
 Clang and llvm already use llvm_setup_rpath(), so this change will
 help standarize rpath usage across all projects.
  
 Differential Revision: 
 https://urldefense.proofpoint.com/v2/url?u=https-3A__reviews.llvm.org_D42459=DwIGaQ=5VD0RTtNlTh3ycd41b3MUw=o3kDXzdBUE3ljQXKeTWOMw=zwvSmk9ZIfMARYg3JvYvNuA0Iz-PEq3H2i_ICF6vsR8=YCR-YJBua5p-4IK05GjHoZUU7aN8UJAFzL2xaz7byyw=
  
 

Re: [libcxx] r323453 - [cmake] [libcxx] Call llvm_setup_rpath() when adding shared libraries.

[Duncan P. N. Exon Smith via cfe-commits]

> On Jan 25, 2018, at 13:07, Don Hinton  wrote:
> 
> On Thu, Jan 25, 2018 at 12:54 PM, Chris Bieneman  > wrote:
> Historically we have supported building libcxx without llvm-config available 
> on the system.
> 
> In all likelihood the bots didn't fail because the bots do have llvm-config 
> because we usually require an llvm build or source checkout in order to build 
> and run the tests since tests depend on lit and gtest.
> 
> Ah, okay, but if the bots don't test/enforce this, does it make sense to 
> continue to support building without llvm-config?

I didn't realize we didn't have a bot enforcing this.  I just made one:
http://lab.llvm.org:8080/green/view/Libcxx/job/libcxx_master_cmake_standalone 


> I don't want to break upstream users, but the alternative is to duplicate 
> this logic across multiple sub-projects.  (see 
> http://lists.llvm.org/pipermail/llvm-dev/2018-January/120707.html 
>  for a 
> related rpath issue).

It's temporary though.  In the brave new world on GitHub with monorepo, the 
runtime libraries will be able to access a shared set of CMake configuration 
scripts.

> 
>  
> 
> 
> -Chris
> 
> 
>> On Jan 25, 2018, at 12:51 PM, Don Hinton > > wrote:
>> 
>> On Thu, Jan 25, 2018 at 12:29 PM, Duncan P. N. Exon Smith 
>> > wrote:
>> I don't really understand why, but our bots seemed to survive this:
>> http://lab.llvm.org:8080/green/view/Libcxx/job/libcxx_master_cmake/2137/ 
>> 
>> 
>> Console output is here:
>> http://lab.llvm.org:8080/green/view/Libcxx/job/libcxx_master_cmake/2137/consoleFull
>>  
>> 
>> 
>> That doesn't necessarily mean it's safe.  Does anyone know why this might 
>> have "worked"?
>> 
>> I'm not an expert, but llvm installs all of these cmake modules along side 
>> llvm-config, so if you find llvm-config, you find these modules.
>> 
>> So, unless I'm missing something, it looks like AddLLVM.cmake is always 
>> available.
>>  
>> 
>>> On Jan 25, 2018, at 11:02, Shoaib Meenai >> > wrote:
>>> 
>>> This is going to break building libc++ standalone (i.e. without any LLVM 
>>> repository or even its CMake modules), right? Some upstream users care a 
>>> lot about that use case (CC beanz and Duncan).
>>>  
>>> From: cfe-commits >> > on behalf of Don Hinton via 
>>> cfe-commits >
>>> Reply-To: Don Hinton >
>>> Date: Thursday, January 25, 2018 at 10:15 AM
>>> To: "cfe-commits@lists.llvm.org " 
>>> >
>>> Subject: [libcxx] r323453 - [cmake] [libcxx] Call llvm_setup_rpath() when 
>>> adding shared libraries.
>>>  
>>> Author: dhinton
>>> Date: Thu Jan 25 10:13:26 2018
>>> New Revision: 323453
>>>  
>>> URL: 
>>> https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject-3Frev-3D323453-26view-3Drev=DwIGaQ=5VD0RTtNlTh3ycd41b3MUw=o3kDXzdBUE3ljQXKeTWOMw=zwvSmk9ZIfMARYg3JvYvNuA0Iz-PEq3H2i_ICF6vsR8=LDCu89byjNdZWoCIYHGHaPr3IamIdHLF0JwbnYE92vM=
>>>  
>>> 
>>> Log:
>>> [cmake] [libcxx] Call llvm_setup_rpath() when adding shared libraries.
>>>  
>>> Clang and llvm already use llvm_setup_rpath(), so this change will
>>> help standarize rpath usage across all projects.
>>>  
>>> Differential Revision: 
>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__reviews.llvm.org_D42459=DwIGaQ=5VD0RTtNlTh3ycd41b3MUw=o3kDXzdBUE3ljQXKeTWOMw=zwvSmk9ZIfMARYg3JvYvNuA0Iz-PEq3H2i_ICF6vsR8=YCR-YJBua5p-4IK05GjHoZUU7aN8UJAFzL2xaz7byyw=
>>>  
>>> 
>>>  
>>> Modified:
>>> libcxx/trunk/lib/CMakeLists.txt
>>>  
>>> Modified: libcxx/trunk/lib/CMakeLists.txt
>>> URL: 
>>> 

[PATCH] D42508: AST: support protocol conformances on id/class/interfaces in MS ABI

[David Majnemer via Phabricator via cfe-commits]

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

LGTM


Repository:
  rC Clang

https://reviews.llvm.org/D42508



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

Re: [libcxx] r323453 - [cmake] [libcxx] Call llvm_setup_rpath() when adding shared libraries.

[Chris Bieneman via cfe-commits]

We have lots of duplicated functionality across the LLVM runtime libraries to 
continue supporting building them without LLVM sources or installations.

Despite my history of being a strong advocate for the runtimes being used 
without LLVM, I don't really have a horse in this race anymore. Duncan might, 
so I'll defer to him.

-Chris

> On Jan 25, 2018, at 1:07 PM, Don Hinton  wrote:
> 
> On Thu, Jan 25, 2018 at 12:54 PM, Chris Bieneman  > wrote:
> Historically we have supported building libcxx without llvm-config available 
> on the system.
> 
> In all likelihood the bots didn't fail because the bots do have llvm-config 
> because we usually require an llvm build or source checkout in order to build 
> and run the tests since tests depend on lit and gtest.
> 
> Ah, okay, but if the bots don't test/enforce this, does it make sense to 
> continue to support building without llvm-config?
> 
> I don't want to break upstream users, but the alternative is to duplicate 
> this logic across multiple sub-projects.  (see 
> http://lists.llvm.org/pipermail/llvm-dev/2018-January/120707.html 
>  for a 
> related rpath issue).
> 
>  
> 
> 
> -Chris
> 
> 
>> On Jan 25, 2018, at 12:51 PM, Don Hinton > > wrote:
>> 
>> On Thu, Jan 25, 2018 at 12:29 PM, Duncan P. N. Exon Smith 
>> > wrote:
>> I don't really understand why, but our bots seemed to survive this:
>> http://lab.llvm.org:8080/green/view/Libcxx/job/libcxx_master_cmake/2137/ 
>> 
>> 
>> Console output is here:
>> http://lab.llvm.org:8080/green/view/Libcxx/job/libcxx_master_cmake/2137/consoleFull
>>  
>> 
>> 
>> That doesn't necessarily mean it's safe.  Does anyone know why this might 
>> have "worked"?
>> 
>> I'm not an expert, but llvm installs all of these cmake modules along side 
>> llvm-config, so if you find llvm-config, you find these modules.
>> 
>> So, unless I'm missing something, it looks like AddLLVM.cmake is always 
>> available.
>>  
>> 
>>> On Jan 25, 2018, at 11:02, Shoaib Meenai >> > wrote:
>>> 
>>> This is going to break building libc++ standalone (i.e. without any LLVM 
>>> repository or even its CMake modules), right? Some upstream users care a 
>>> lot about that use case (CC beanz and Duncan).
>>>  
>>> From: cfe-commits >> > on behalf of Don Hinton via 
>>> cfe-commits >
>>> Reply-To: Don Hinton >
>>> Date: Thursday, January 25, 2018 at 10:15 AM
>>> To: "cfe-commits@lists.llvm.org " 
>>> >
>>> Subject: [libcxx] r323453 - [cmake] [libcxx] Call llvm_setup_rpath() when 
>>> adding shared libraries.
>>>  
>>> Author: dhinton
>>> Date: Thu Jan 25 10:13:26 2018
>>> New Revision: 323453
>>>  
>>> URL: 
>>> https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject-3Frev-3D323453-26view-3Drev=DwIGaQ=5VD0RTtNlTh3ycd41b3MUw=o3kDXzdBUE3ljQXKeTWOMw=zwvSmk9ZIfMARYg3JvYvNuA0Iz-PEq3H2i_ICF6vsR8=LDCu89byjNdZWoCIYHGHaPr3IamIdHLF0JwbnYE92vM=
>>>  
>>> 
>>> Log:
>>> [cmake] [libcxx] Call llvm_setup_rpath() when adding shared libraries.
>>>  
>>> Clang and llvm already use llvm_setup_rpath(), so this change will
>>> help standarize rpath usage across all projects.
>>>  
>>> Differential Revision: 
>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__reviews.llvm.org_D42459=DwIGaQ=5VD0RTtNlTh3ycd41b3MUw=o3kDXzdBUE3ljQXKeTWOMw=zwvSmk9ZIfMARYg3JvYvNuA0Iz-PEq3H2i_ICF6vsR8=YCR-YJBua5p-4IK05GjHoZUU7aN8UJAFzL2xaz7byyw=
>>>  
>>> 
>>>  
>>> Modified:
>>> libcxx/trunk/lib/CMakeLists.txt
>>>  
>>> Modified: libcxx/trunk/lib/CMakeLists.txt
>>> URL: 
>>> https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_libcxx_trunk_lib_CMakeLists.txt-3Frev-3D323453-26r1-3D323452-26r2-3D323453-26view-3Ddiff=DwIGaQ=5VD0RTtNlTh3ycd41b3MUw=o3kDXzdBUE3ljQXKeTWOMw=zwvSmk9ZIfMARYg3JvYvNuA0Iz-PEq3H2i_ICF6vsR8=Rd7xc2GWS8oyDcm0QshNPTqM3BglPY5_aHMIuCRUt1s=
>>>  

[PATCH] D42513: [CUDA] Added partial support for CUDA-9.1

[Justin Lebar via Phabricator via cfe-commits]

jlebar added a comment.

Gosh I think the chances we get through this without a copy-paste error are way 
too high.  I'm not sure what to do about it, though.  I don't have the focus to 
check this as carefully as it needs.

Perhaps we need to test this in the test-suite (eventually)?  We've seen that 
even when our own implementation is correct, sometimes nvptx breaks it, and so 
we need to test e2e *anyway*...




Comment at: clang/lib/Driver/ToolChains/Cuda.cpp:125
+if (Version >= CudaVersion::CUDA_90) {
   // CUDA-9 uses single libdevice file for all GPU variants.
   std::string FilePath = LibDevicePath + "/libdevice.10.bc";

Perhaps update to CUDA 9+



Comment at: clang/lib/Headers/__clang_cuda_device_functions.h:32
+
+#define __DEVICE__ static __device__ __forceinline__
+// There are number of functions that became compiler builtins in CUDA-9 and 
are

I don't think we should need `__forceinline__`?  I'm sure they had that in 
their headers, but clang doesn't rely on its inliner for correctness, unlike 
some *other* compilers.  :)

I'm also not sure about `static`.  I think we mark all CUDA code as internal, 
so it shouldn't be necessary?  If they're not static, they should be marked as 
`inline`, though.



Comment at: clang/lib/Headers/__clang_cuda_device_functions.h:51
+__DEVICE__ void __brkpt() { asm volatile("brkpt;"); }
+__DEVICE__ void __brkpt(int __a) { __brkpt(); }
+__DEVICE__ unsigned int __byte_perm(unsigned int __a, unsigned int __b,

Sanity check: Ignoring the __a argument here is correct?



Comment at: clang/lib/Headers/__clang_cuda_runtime_wrapper.h:143
 
-// We need decls for functions in CUDA's libdevice with __device__
-// attribute only. Alas they come either as __host__ __device__ or
-// with no attributes at all. To work around that, define __CUDA_RTC__
-// which produces HD variant and undef __host__ which gives us desided
-// decls with __device__ attribute.
-#pragma push_macro("__host__")
-#define __host__
-#define __CUDACC_RTC__
-#include "device_functions_decls.h"
-#undef __CUDACC_RTC__
+// Declare or define device-side functions that particular CUDA version relies
+// on but does (no longer) declare in its headers. E.g. some of the device-side

a particular (or "our particular", if you like)



Comment at: clang/lib/Headers/__clang_cuda_runtime_wrapper.h:144
+// Declare or define device-side functions that particular CUDA version relies
+// on but does (no longer) declare in its headers. E.g. some of the device-side
+// functions that used to be implemented in a header in CUDA-8, became NVCC's

Sentence doesn't read right if you skip the parens.

But also: Our header provides *everything* from cuda/device_functions_decls.h, 
not just the things that were once in that header and, in newer CUDA versions, 
are no longer there, right?  I'm actually even more confused now that I read 
the big header, because here we say the header "defines or declares" these 
functions, but that suggests that for every function, it decides whether to 
define or to declare it, but that's not the case...



Comment at: clang/lib/Headers/__clang_cuda_runtime_wrapper.h:155
+#undef __THROW
+#define __THROW
 

Should we be pushing/popping the macro?  That is, do we want this macro exposed 
to user code?


https://reviews.llvm.org/D42513



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

[PATCH] D42560: [analyzer] dump() ExprEngine's internal traits into the ExplodedGraph view.

[George Karpenkov via Phabricator via cfe-commits]

george.karpenkov added a comment.

Looks good




Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:392
+
+auto InitTempSet = State->get();
+if (!InitTempSet.isEmpty()) {

Actually I would put both of those into static functions: 
printInitializedTemporaries, printCXXNewAllocatorValues



Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:415
+for (auto I : NewAllocValsMap) {
+  if (I.first.second != LC)
+continue;

Can we get a variable for `I.first`?


Repository:
  rC Clang

https://reviews.llvm.org/D42560



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

[PATCH] D42560: [analyzer] dump() ExprEngine's internal traits into the ExplodedGraph view.

[Artem Dergachev via Phabricator via cfe-commits]

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

When viewing the `ExplodedGraph` through graphviz, make `ExprEngine`'s two 
private program state traits - namely C++ allocator values and C++ initialized 
temporaries - visible in the dump.

How it looks:

F5784825: Graph.png 

Unfortunately this relies on `LocationContext`, so it wouldn't be available in 
normal `ProgramState` dumps. Hmm, i guess i could add a mode to print them in 
an unsorted order.


Repository:
  rC Clang

https://reviews.llvm.org/D42560

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
  include/clang/StaticAnalyzer/Core/PathSensitive/SubEngine.h
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  lib/StaticAnalyzer/Core/ProgramState.cpp


Index: lib/StaticAnalyzer/Core/ProgramState.cpp
===
--- lib/StaticAnalyzer/Core/ProgramState.cpp
+++ lib/StaticAnalyzer/Core/ProgramState.cpp
@@ -450,7 +450,7 @@
   Mgr.getConstraintManager().print(this, Out, NL, Sep);
 
   // Print checker-specific data.
-  Mgr.getOwningEngine()->printState(Out, this, NL, Sep);
+  Mgr.getOwningEngine()->printState(Out, this, NL, Sep, LC);
 }
 
 void ProgramState::printDOT(raw_ostream , const LocationContext *LC) const 
{
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -384,7 +384,44 @@
 }
 
 void ExprEngine::printState(raw_ostream , ProgramStateRef State,
-const char *NL, const char *Sep) {
+const char *NL, const char *Sep,
+const LocationContext *LCtx) {
+  if (LCtx) {
+LangOptions LO; // FIXME.
+
+auto InitTempSet = State->get();
+if (!InitTempSet.isEmpty()) {
+  Out << Sep << "Initialized temporaries:" << NL;
+
+  LCtx->dumpStack(Out, "", NL, Sep,
+  [, , InitTempSet, NL](const LocationContext *LC) {
+for (auto I : InitTempSet) {
+  if (I.second != LC)
+continue;
+  Out << '(' << I.second << ',' << I.first << ") ";
+  I.first->printPretty(Out, nullptr, PrintingPolicy(LO));
+  Out << NL;
+}
+  });
+}
+
+auto NewAllocValsMap = State->get();
+if (!NewAllocValsMap.isEmpty()) {
+  Out << Sep << "operator new() allocator return values:" << NL;
+
+  LCtx->dumpStack(Out, "", NL, Sep,
+  [, , NewAllocValsMap, NL](const LocationContext *LC) {
+for (auto I : NewAllocValsMap) {
+  if (I.first.second != LC)
+continue;
+  Out << '(' << I.first.second << ',' << I.first.first << ") ";
+  I.first.first->printPretty(Out, nullptr, PrintingPolicy(LO));
+  Out << " : " << I.second << NL;
+}
+  });
+}
+  }
+
   getCheckerManager().runCheckersForPrintState(Out, State, NL, Sep);
 }
 
Index: include/clang/StaticAnalyzer/Core/PathSensitive/SubEngine.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/SubEngine.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/SubEngine.h
@@ -155,7 +155,8 @@
 
   /// printState - Called by ProgramStateManager to print checker-specific 
data.
   virtual void printState(raw_ostream , ProgramStateRef State,
-  const char *NL, const char *Sep) = 0;
+  const char *NL, const char *Sep,
+  const LocationContext *LCtx = nullptr) = 0;
 
   /// Called by CoreEngine when the analysis worklist is either empty or the
   //  maximum number of analysis steps have been reached.
Index: include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -299,8 +299,9 @@
const CallEvent *Call) override;
 
   /// printState - Called by ProgramStateManager to print checker-specific 
data.
-  void printState(raw_ostream , ProgramStateRef State,
-  const char *NL, const char *Sep) override;
+  void printState(raw_ostream , ProgramStateRef State, const char *NL,
+  const char *Sep,
+  const LocationContext *LCtx = nullptr) override;
 
   ProgramStateManager& getStateManager() override { return StateMgr; }
 


Index: lib/StaticAnalyzer/Core/ProgramState.cpp
===
--- lib/StaticAnalyzer/Core/ProgramState.cpp
+++ lib/StaticAnalyzer/Core/ProgramState.cpp
@@ -450,7 +450,7 @@
   Mgr.getConstraintManager().print(this, Out, 

[PATCH] D42344: [libc++] Use multi-key tree search for {map, set}::{count, equal_range}

[N via Phabricator via cfe-commits]

ng added a comment.

Ping


https://reviews.llvm.org/D42344



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

[libcxx] r323479 - Mark 2903 as complete; we already do this

[Marshall Clow via cfe-commits]

Author: marshall
Date: Thu Jan 25 14:33:17 2018
New Revision: 323479

URL: http://llvm.org/viewvc/llvm-project?rev=323479=rev
Log:
Mark 2903 as complete; we already do this

Modified:
libcxx/trunk/www/cxx1z_status.html

Modified: libcxx/trunk/www/cxx1z_status.html
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/www/cxx1z_status.html?rev=323479=323478=323479=diff
==
--- libcxx/trunk/www/cxx1z_status.html (original)
+++ libcxx/trunk/www/cxx1z_status.html Thu Jan 25 14:33:17 2018
@@ -485,7 +485,7 @@
https://wg21.link/LWG2878;>2878Missing 
DefaultConstructible requirement for istream_iterator default 
constructorKona
https://wg21.link/LWG2890;>2890The 
definition of 'object state' applies only to class 
typesKonaComplete
https://wg21.link/LWG2900;>2900The copy 
and move constructors of optional are not 
constexprKonaComplete
-   https://wg21.link/LWG2903;>2903The form 
of initialization for the emplace-constructors is not 
specifiedKona
+   https://wg21.link/LWG2903;>2903The form 
of initialization for the emplace-constructors is not 
specifiedKonaComplete
https://wg21.link/LWG2904;>2904Make 
variant move-assignment more exception 
safeKonaComplete
https://wg21.link/LWG2905;>2905is_constructible_vunique_ptrP,
 D, P, D const  should be false when D is not copy 
constructibleKonaComplete
https://wg21.link/LWG2908;>2908The 
less-than operator for shared pointers could do 
moreKona
@@ -503,7 +503,7 @@
 
   
 
-  Last Updated: 2-Jan-2018
+  Last Updated: 25-Jan-2018
 
 
 


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

[PATCH] D42464: add prefix with '_' support for property name. Corresponding apple dev doc: https://developer.apple.com/library/content/qa/qa1908/_index.html

[Yan Zhang via Phabricator via cfe-commits]

Wizard updated this revision to Diff 131511.
Wizard marked 8 inline comments as done.
Wizard added a comment.

update some documents and comments


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42464

Files:
  clang-tidy/objc/PropertyDeclarationCheck.cpp
  docs/clang-tidy/checks/objc-property-declaration.rst
  test/clang-tidy/objc-property-declaration-custom.m
  test/clang-tidy/objc-property-declaration.m

Index: test/clang-tidy/objc-property-declaration.m
===
--- test/clang-tidy/objc-property-declaration.m
+++ test/clang-tidy/objc-property-declaration.m
@@ -3,11 +3,22 @@
 
 @interface Foo
 @property(assign, nonatomic) int NotCamelCase;
-// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'NotCamelCase' should use lowerCamelCase style, according to the Apple Coding Guidelines [objc-property-declaration]
+// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'NotCamelCase' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
 // CHECK-FIXES: @property(assign, nonatomic) int notCamelCase;
 @property(assign, nonatomic) int camelCase;
 @property(strong, nonatomic) NSString *URLString;
 @property(strong, nonatomic) NSString *bundleID;
 @property(strong, nonatomic) NSString *URL_string;
-// CHECK-MESSAGES: :[[@LINE-1]]:40: warning: property name 'URL_string' should use lowerCamelCase style, according to the Apple Coding Guidelines [objc-property-declaration]
+// CHECK-MESSAGES: :[[@LINE-1]]:40: warning: property name 'URL_string' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
 @end
+
+@interface Foo (Bar)
+@property(assign, nonatomic) int abc_NotCamelCase;
+// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'abc_NotCamelCase' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
+@property(assign, nonatomic) int abc_camelCase;
+@end
+
+@interface Foo ()
+@property(assign, nonatomic) int abc_camelCase;
+// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'abc_camelCase' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
+@end
\ No newline at end of file
Index: test/clang-tidy/objc-property-declaration-custom.m
===
--- test/clang-tidy/objc-property-declaration-custom.m
+++ test/clang-tidy/objc-property-declaration-custom.m
@@ -6,9 +6,9 @@
 
 @interface Foo
 @property(assign, nonatomic) int AbcNotRealPrefix;
-// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'AbcNotRealPrefix' should use lowerCamelCase style, according to the Apple Coding Guidelines [objc-property-declaration]
+// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'AbcNotRealPrefix' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
 // CHECK-FIXES: @property(assign, nonatomic) int abcNotRealPrefix;
 @property(assign, nonatomic) int ABCCustomPrefix;
 @property(strong, nonatomic) NSString *ABC_custom_prefix;
-// CHECK-MESSAGES: :[[@LINE-1]]:40: warning: property name 'ABC_custom_prefix' should use lowerCamelCase style, according to the Apple Coding Guidelines [objc-property-declaration]
+// CHECK-MESSAGES: :[[@LINE-1]]:40: warning: property name 'ABC_custom_prefix' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
 @end
Index: docs/clang-tidy/checks/objc-property-declaration.rst
===
--- docs/clang-tidy/checks/objc-property-declaration.rst
+++ docs/clang-tidy/checks/objc-property-declaration.rst
@@ -32,6 +32,15 @@
 
 The corresponding style rule: https://developer.apple.com/library/content/documentation/Cocoa/Conceptual/CodingGuidelines/Articles/NamingIvarsAndTypes.html#//apple_ref/doc/uid/20001284-1001757
 
+The check will also accept property declared in category with a prefix of
+lowercase letters followed by a '_' to avoid naming conflict. For example:
+
+.. code-block:: objc
+@property(nonatomic, assign) int abc_lowerCamelCase;
+
+The corresponding style rule: https://developer.apple.com/library/content/qa/qa1908/_index.html
+
+
 Options
 ---
 
Index: clang-tidy/objc/PropertyDeclarationCheck.cpp
===
--- clang-tidy/objc/PropertyDeclarationCheck.cpp
+++ clang-tidy/objc/PropertyDeclarationCheck.cpp
@@ -8,20 +8,31 @@
 //===--===//
 
 #include "PropertyDeclarationCheck.h"
+#include 
 #include "../utils/OptionsUtils.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"

[PATCH] D42464: add prefix with '_' support for property name. Corresponding apple dev doc: https://developer.apple.com/library/content/qa/qa1908/_index.html

[Yan Zhang via Phabricator via cfe-commits]

Wizard added inline comments.



Comment at: clang-tidy/objc/PropertyDeclarationCheck.cpp:93
+ [](const std::string ) { return llvm::Regex::escape(s); });
   // Allow any of these names:
   // foo

hokein wrote:
> Does the comment still make sense? Seems you changed the regex below.
Yes they are actually the same. I am just replacing the prefix "::" with "^" 
according to the UsedInMatcher param.



Comment at: clang-tidy/objc/PropertyDeclarationCheck.cpp:115
+
+bool prefixedPropertyNameMatches(const llvm::StringRef ,
+ const std::vector ) {

hokein wrote:
> no need to pass const reference of `llvm::StringRef`. `StringRef` is cheap.
The original property name I directly get from ast is a const. If I remove the 
const here, I will have to make a copy of the property name before calling this.
I prefer keep this const to save that copy :-)



Comment at: clang-tidy/objc/PropertyDeclarationCheck.cpp:151
+  hasCategoryPropertyPrefix(MatchedDecl->getName())) {
+const auto *CategoryDecl = (const ObjCCategoryDecl *)(DeclContext);
+if (!prefixedPropertyNameMatches(MatchedDecl->getName(), SpecialAcronyms) 
||

hokein wrote:
> Consider using `const auto* CategoryDecl = 
> dyn_cast(DeclContext)`, we can get rid of this cast, and 
> `NodeKind` variable.
Tried that before but I encountered 2 issues:
1. 'clang::DeclContext' is not polymorphic
2. cannot use dynamic_cast with -fno-rtti
which are preventing me from using dynamic_cast


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42464



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

r323473 - Don't let test write to the source dir after r323426.

[Nico Weber via cfe-commits]

Author: nico
Date: Thu Jan 25 13:49:03 2018
New Revision: 323473

URL: http://llvm.org/viewvc/llvm-project?rev=323473=rev
Log:
Don't let test write to the source dir after r323426.

Modified:
cfe/trunk/test/Driver/cl-x86-flags.c

Modified: cfe/trunk/test/Driver/cl-x86-flags.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/cl-x86-flags.c?rev=323473=323472=323473=diff
==
--- cfe/trunk/test/Driver/cl-x86-flags.c (original)
+++ cfe/trunk/test/Driver/cl-x86-flags.c Thu Jan 25 13:49:03 2018
@@ -8,7 +8,7 @@
 // RUN: --target=i386-pc-win32 -### -- 2>&1 %s | FileCheck 
-check-prefix=MFLAGS %s
 // MFLAGS-NOT: argument unused during compilation
 
-// RUN: %clang_cl -m32 -arch:IA32 --target=i386-pc-windows /c -Xclang -verify 
-DTEST_32_ARCH_IA32 -- %s
+// RUN: %clang_cl -m32 -arch:IA32 --target=i386-pc-windows /c /Fo%t.obj 
-Xclang -verify -DTEST_32_ARCH_IA32 -- %s
 #if defined(TEST_32_ARCH_IA32)
 #if _M_IX86_FP || __AVX__ || __AVX2__ || __AVX512F__  || __AVX512BW__
 #error fail
@@ -22,7 +22,7 @@
 // RUN: %clang_cl -m64 -arch:IA32 --target=x86_64-pc-windows -### -- 2>&1 %s | 
FileCheck -check-prefix=IA3264 %s
 // IA3264: argument unused during compilation
 
-// RUN: %clang_cl -m32 -arch:SSE --target=i386-pc-windows /c -Xclang -verify 
-DTEST_32_ARCH_SSE -- %s
+// RUN: %clang_cl -m32 -arch:SSE --target=i386-pc-windows /c /Fo%t.obj -Xclang 
-verify -DTEST_32_ARCH_SSE -- %s
 #if defined(TEST_32_ARCH_SSE)
 #if _M_IX86_FP != 1 || __AVX__ || __AVX2__ || __AVX512F__  || __AVX512BW__
 #error fail
@@ -32,7 +32,7 @@
 // RUN: %clang_cl -m32 -arch:sse --target=i386-pc-windows -### -- 2>&1 %s | 
FileCheck -check-prefix=sse %s
 // sse: argument unused during compilation
 
-// RUN: %clang_cl -m32 -arch:SSE2 --target=i386-pc-windows /c -Xclang -verify 
-DTEST_32_ARCH_SSE2 -- %s
+// RUN: %clang_cl -m32 -arch:SSE2 --target=i386-pc-windows /c /Fo%t.obj 
-Xclang -verify -DTEST_32_ARCH_SSE2 -- %s
 #if defined(TEST_32_ARCH_SSE2)
 #if _M_IX86_FP != 2 || __AVX__ || __AVX2__ || __AVX512F__  || __AVX512BW__
 #error fail
@@ -48,7 +48,7 @@
 // RUN: %clang_cl -m64 -arch:SSE2 --target=x86_64-pc-windows -### -- 2>&1 %s | 
FileCheck -check-prefix=SSE264 %s
 // SSE264: argument unused during compilation
 
-// RUN: %clang_cl -m32 -arch:AVX --target=i386-pc-windows /c -Xclang -verify 
-DTEST_32_ARCH_AVX -- %s
+// RUN: %clang_cl -m32 -arch:AVX --target=i386-pc-windows /c /Fo%t.obj -Xclang 
-verify -DTEST_32_ARCH_AVX -- %s
 #if defined(TEST_32_ARCH_AVX)
 #if _M_IX86_FP != 2 || !__AVX__ || __AVX2__ || __AVX512F__  || __AVX512BW__
 #error fail
@@ -58,7 +58,7 @@
 // RUN: %clang_cl -m32 -arch:avx --target=i386-pc-windows -### -- 2>&1 %s | 
FileCheck -check-prefix=avx %s
 // avx: argument unused during compilation
 
-// RUN: %clang_cl -m32 -arch:AVX2 --target=i386-pc-windows /c -Xclang -verify 
-DTEST_32_ARCH_AVX2 -- %s
+// RUN: %clang_cl -m32 -arch:AVX2 --target=i386-pc-windows /c /Fo%t.obj 
-Xclang -verify -DTEST_32_ARCH_AVX2 -- %s
 #if defined(TEST_32_ARCH_AVX2)
 #if _M_IX86_FP != 2 || !__AVX__ || !__AVX2__ || __AVX512F__  || __AVX512BW__
 #error fail
@@ -68,7 +68,7 @@
 // RUN: %clang_cl -m32 -arch:avx2 --target=i386-pc-windows -### -- 2>&1 %s | 
FileCheck -check-prefix=avx2 %s
 // avx2: argument unused during compilation
 
-// RUN: %clang_cl -m32 -arch:AVX512F --target=i386-pc-windows /c -Xclang 
-verify -DTEST_32_ARCH_AVX512F -- %s
+// RUN: %clang_cl -m32 -arch:AVX512F --target=i386-pc-windows /c /Fo%t.obj 
-Xclang -verify -DTEST_32_ARCH_AVX512F -- %s
 #if defined(TEST_32_ARCH_AVX512F)
 #if _M_IX86_FP != 2 || !__AVX__ || !__AVX2__ || !__AVX512F__  || __AVX512BW__
 #error fail
@@ -78,7 +78,7 @@
 // RUN: %clang_cl -m32 -arch:avx512f --target=i386-pc-windows -### -- 2>&1 %s 
| FileCheck -check-prefix=avx512f %s
 // avx512f: argument unused during compilation
 
-// RUN: %clang_cl -m32 -arch:AVX512 --target=i386-pc-windows /c -Xclang 
-verify -DTEST_32_ARCH_AVX512 -- %s
+// RUN: %clang_cl -m32 -arch:AVX512 --target=i386-pc-windows /c /Fo%t.obj 
-Xclang -verify -DTEST_32_ARCH_AVX512 -- %s
 #if defined(TEST_32_ARCH_AVX512)
 #if _M_IX86_FP != 2 || !__AVX__ || !__AVX2__ || !__AVX512F__  || !__AVX512BW__
 #error fail
@@ -88,7 +88,7 @@
 // RUN: %clang_cl -m32 -arch:avx512 --target=i386-pc-windows -### -- 2>&1 %s | 
FileCheck -check-prefix=avx512 %s
 // avx512: argument unused during compilation
 
-// RUN: %clang_cl -m64 -arch:AVX --target=x86_64-pc-windows /c -Xclang -verify 
-DTEST_64_ARCH_AVX -- %s
+// RUN: %clang_cl -m64 -arch:AVX --target=x86_64-pc-windows /c /Fo%t.obj 
-Xclang -verify -DTEST_64_ARCH_AVX -- %s
 #if defined(TEST_64_ARCH_AVX)
 #if _M_IX86_FP || !__AVX__ || __AVX2__ || __AVX512F__  || __AVX512BW__
 #error fail
@@ -98,7 +98,7 @@
 // RUN: %clang_cl -m64 -arch:avx --target=x86_64-pc-windows -### -- 2>&1 %s | 
FileCheck -check-prefix=avx64 %s
 // avx64: argument unused during compilation
 
-// RUN: %clang_cl -m64 -arch:AVX2 --target=x86_64-pc-windows 

[PATCH] D42459: [cmake] [libcxx] Call llvm_setup_rpath() when adding shared libraries.

[Don Hinton via Phabricator via cfe-commits]

hintonda updated this revision to Diff 131506.
hintonda added a comment.

Only call llvm_setup_rpath() if LLVM_FOUND is true.


Repository:
  rCXX libc++

https://reviews.llvm.org/D42459

Files:
  lib/CMakeLists.txt


Index: lib/CMakeLists.txt
===
--- lib/CMakeLists.txt
+++ lib/CMakeLists.txt
@@ -222,6 +222,9 @@
 # Build the shared library.
 if (LIBCXX_ENABLE_SHARED)
   add_library(cxx_shared SHARED $)
+  if(LLVM_FOUND)
+llvm_setup_rpath(cxx_shared)
+  endif()
   target_link_libraries(cxx_shared ${LIBCXX_LIBRARIES})
   set_target_properties(cxx_shared
 PROPERTIES


Index: lib/CMakeLists.txt
===
--- lib/CMakeLists.txt
+++ lib/CMakeLists.txt
@@ -222,6 +222,9 @@
 # Build the shared library.
 if (LIBCXX_ENABLE_SHARED)
   add_library(cxx_shared SHARED $)
+  if(LLVM_FOUND)
+llvm_setup_rpath(cxx_shared)
+  endif()
   target_link_libraries(cxx_shared ${LIBCXX_LIBRARIES})
   set_target_properties(cxx_shared
 PROPERTIES
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42552: [analyzer] dump() environment frame-by-frame.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Also printing the location context pointer on every line is redundant, but 
putting it on the frame line would clamp up the crash dump with weird pointers 
and also make the frame lines in the state dump visually similar to the 
environment entry lines.


Repository:
  rC Clang

https://reviews.llvm.org/D42552



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

[PATCH] D42276: [Driver] Add an -fexperimental-isel driver option to enable/disable GlobalISel

[Quentin Colombet via Phabricator via cfe-commits]

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

LGTM


Repository:
  rC Clang

https://reviews.llvm.org/D42276



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

Re: [libcxx] r323453 - [cmake] [libcxx] Call llvm_setup_rpath() when adding shared libraries.

[Don Hinton via cfe-commits]

On Thu, Jan 25, 2018 at 12:54 PM, Chris Bieneman  wrote:

> Historically we have supported building libcxx without llvm-config
> available on the system.
>
> In all likelihood the bots didn't fail because the bots do have
> llvm-config because we usually require an llvm build or source checkout in
> order to build and run the tests since tests depend on lit and gtest.
>

Ah, okay, but if the bots don't test/enforce this, does it make sense to
continue to support building without llvm-config?

I don't want to break upstream users, but the alternative is to duplicate
this logic across multiple sub-projects.  (see
http://lists.llvm.org/pipermail/llvm-dev/2018-January/120707.html for a
related rpath issue).



>
>
> -Chris
>
>
> On Jan 25, 2018, at 12:51 PM, Don Hinton  wrote:
>
> On Thu, Jan 25, 2018 at 12:29 PM, Duncan P. N. Exon Smith <
> dexonsm...@apple.com> wrote:
>
>> I don't really understand why, but our bots seemed to survive this:
>> http://lab.llvm.org:8080/green/view/Libcxx/job/libcxx_master_cmake/2137/
>>
>> Console output is here:
>> http://lab.llvm.org:8080/green/view/Libcxx/job/libcxx_master
>> _cmake/2137/consoleFull
>>
>> That doesn't necessarily mean it's safe.  Does anyone know why this might
>> have "worked"?
>>
>
> I'm not an expert, but llvm installs all of these cmake modules along side
> llvm-config, so if you find llvm-config, you find these modules.
>
> So, unless I'm missing something, it looks like AddLLVM.cmake is always
> available.
>
>
>>
>> On Jan 25, 2018, at 11:02, Shoaib Meenai  wrote:
>>
>> This is going to break building libc++ standalone (i.e. without any LLVM
>> repository or even its CMake modules), right? Some upstream users care a
>> lot about that use case (CC beanz and Duncan).
>>
>> *From: *cfe-commits  on behalf of
>> Don Hinton via cfe-commits 
>> *Reply-To: *Don Hinton 
>> *Date: *Thursday, January 25, 2018 at 10:15 AM
>> *To: *"cfe-commits@lists.llvm.org" 
>> *Subject: *[libcxx] r323453 - [cmake] [libcxx] Call llvm_setup_rpath()
>> when adding shared libraries.
>>
>> Author: dhinton
>> Date: Thu Jan 25 10:13:26 2018
>> New Revision: 323453
>>
>> URL: https://urldefense.proofpoint.com/v2/url?u=http-3A__
>> llvm.org_viewvc_llvm-2Dproject-3Frev-3D323453-26view-3Drev&
>> d=DwIGaQ=5VD0RTtNlTh3ycd41b3MUw=o3kDXzdBUE3ljQXKeTWOMw&
>> m=zwvSmk9ZIfMARYg3JvYvNuA0Iz-PEq3H2i_ICF6vsR8=LDCu89byjNdZ
>> WoCIYHGHaPr3IamIdHLF0JwbnYE92vM=
>> Log:
>> [cmake] [libcxx] Call llvm_setup_rpath() when adding shared libraries.
>>
>> Clang and llvm already use llvm_setup_rpath(), so this change will
>> help standarize rpath usage across all projects.
>>
>> Differential Revision: https://urldefense.proofpoint.com/v2/url?u=https-3
>> A__reviews.llvm.org_D42459=DwIGaQ=5VD0RTtNlTh3ycd41b3MUw
>> =o3kDXzdBUE3ljQXKeTWOMw=zwvSmk9ZIfMARYg3JvYvNuA0Iz-PEq3H
>> 2i_ICF6vsR8=YCR-YJBua5p-4IK05GjHoZUU7aN8UJAFzL2xaz7byyw=
>>
>> Modified:
>> libcxx/trunk/lib/CMakeLists.txt
>>
>> Modified: libcxx/trunk/lib/CMakeLists.txt
>> URL: https://urldefense.proofpoint.com/v2/url?u=http-3A__
>> llvm.org_viewvc_llvm-2Dproject_libcxx_trunk_lib_CMakeLists.
>> txt-3Frev-3D323453-26r1-3D323452-26r2-3D323453-26view-
>> 3Ddiff=DwIGaQ=5VD0RTtNlTh3ycd41b3MUw=o3kDXzdBUE3ljQXKe
>> TWOMw=zwvSmk9ZIfMARYg3JvYvNuA0Iz-PEq3H2i_ICF6vsR8=Rd7xc2
>> GWS8oyDcm0QshNPTqM3BglPY5_aHMIuCRUt1s=
>> 
>> ==
>> --- libcxx/trunk/lib/CMakeLists.txt (original)
>> +++ libcxx/trunk/lib/CMakeLists.txt Thu Jan 25 10:13:26 2018
>> @@ -222,6 +222,7 @@ set(LIBCXX_TARGETS)
>> # Build the shared library.
>> if (LIBCXX_ENABLE_SHARED)
>>add_library(cxx_shared SHARED $)
>> +  llvm_setup_rpath(cxx_shared)
>>target_link_libraries(cxx_shared ${LIBCXX_LIBRARIES})
>>set_target_properties(cxx_shared
>>  PROPERTIES
>>
>>
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.ll
>> vm.org_cgi-2Dbin_mailman_listinfo_cfe-2Dcommits=DwIGaQ=5
>> VD0RTtNlTh3ycd41b3MUw=o3kDXzdBUE3ljQXKeTWOMw=zwvSmk9ZIfM
>> ARYg3JvYvNuA0Iz-PEq3H2i_ICF6vsR8=TPt8bk99XfmUCdi7MBx
>> yguYV6hJg3PsXFCTXyDfGms8=
>>
>>
>>
>>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42555: [libunwind] Don't enable _LIBUNWIND_BUILD_ZERO_COST_APIS if building the SJLJ APIs

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo created this revision.
mstorsjo added reviewers: compnerd, smeenai, phosek.
Herald added subscribers: kristof.beyls, aemerson.

Otherwise, a shared library build with SJLJ APIs enabled would end up with 
duplicate symbols.

This didn't occur for the apple && arm case due to specifically checking for 
that in the surrounding ifdef.


https://reviews.llvm.org/D42555

Files:
  src/config.h


Index: src/config.h
===
--- src/config.h
+++ src/config.h
@@ -72,8 +72,10 @@
 (!defined(__APPLE__) && defined(__arm__)) ||   
\
 (defined(__arm64__) || defined(__aarch64__)) ||
\
 defined(__mips__)
+#if !defined(_LIBUNWIND_BUILD_SJLJ_APIS)
 #define _LIBUNWIND_BUILD_ZERO_COST_APIS
 #endif
+#endif
 
 #if defined(__powerpc64__) && defined(_ARCH_PWR8)
 #define PPC64_HAS_VMX


Index: src/config.h
===
--- src/config.h
+++ src/config.h
@@ -72,8 +72,10 @@
 (!defined(__APPLE__) && defined(__arm__)) ||   \
 (defined(__arm64__) || defined(__aarch64__)) ||\
 defined(__mips__)
+#if !defined(_LIBUNWIND_BUILD_SJLJ_APIS)
 #define _LIBUNWIND_BUILD_ZERO_COST_APIS
 #endif
+#endif
 
 #if defined(__powerpc64__) && defined(_ARCH_PWR8)
 #define PPC64_HAS_VMX
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [libcxx] r323453 - [cmake] [libcxx] Call llvm_setup_rpath() when adding shared libraries.

[Chris Bieneman via cfe-commits]

Historically we have supported building libcxx without llvm-config available on 
the system.

In all likelihood the bots didn't fail because the bots do have llvm-config 
because we usually require an llvm build or source checkout in order to build 
and run the tests since tests depend on lit and gtest.

-Chris

> On Jan 25, 2018, at 12:51 PM, Don Hinton  wrote:
> 
> On Thu, Jan 25, 2018 at 12:29 PM, Duncan P. N. Exon Smith 
> > wrote:
> I don't really understand why, but our bots seemed to survive this:
> http://lab.llvm.org:8080/green/view/Libcxx/job/libcxx_master_cmake/2137/ 
> 
> 
> Console output is here:
> http://lab.llvm.org:8080/green/view/Libcxx/job/libcxx_master_cmake/2137/consoleFull
>  
> 
> 
> That doesn't necessarily mean it's safe.  Does anyone know why this might 
> have "worked"?
> 
> I'm not an expert, but llvm installs all of these cmake modules along side 
> llvm-config, so if you find llvm-config, you find these modules.
> 
> So, unless I'm missing something, it looks like AddLLVM.cmake is always 
> available.
>  
> 
>> On Jan 25, 2018, at 11:02, Shoaib Meenai > > wrote:
>> 
>> This is going to break building libc++ standalone (i.e. without any LLVM 
>> repository or even its CMake modules), right? Some upstream users care a lot 
>> about that use case (CC beanz and Duncan).
>>  
>> From: cfe-commits > > on behalf of Don Hinton via 
>> cfe-commits >
>> Reply-To: Don Hinton >
>> Date: Thursday, January 25, 2018 at 10:15 AM
>> To: "cfe-commits@lists.llvm.org " 
>> >
>> Subject: [libcxx] r323453 - [cmake] [libcxx] Call llvm_setup_rpath() when 
>> adding shared libraries.
>>  
>> Author: dhinton
>> Date: Thu Jan 25 10:13:26 2018
>> New Revision: 323453
>>  
>> URL: 
>> https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject-3Frev-3D323453-26view-3Drev=DwIGaQ=5VD0RTtNlTh3ycd41b3MUw=o3kDXzdBUE3ljQXKeTWOMw=zwvSmk9ZIfMARYg3JvYvNuA0Iz-PEq3H2i_ICF6vsR8=LDCu89byjNdZWoCIYHGHaPr3IamIdHLF0JwbnYE92vM=
>>  
>> 
>> Log:
>> [cmake] [libcxx] Call llvm_setup_rpath() when adding shared libraries.
>>  
>> Clang and llvm already use llvm_setup_rpath(), so this change will
>> help standarize rpath usage across all projects.
>>  
>> Differential Revision: 
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__reviews.llvm.org_D42459=DwIGaQ=5VD0RTtNlTh3ycd41b3MUw=o3kDXzdBUE3ljQXKeTWOMw=zwvSmk9ZIfMARYg3JvYvNuA0Iz-PEq3H2i_ICF6vsR8=YCR-YJBua5p-4IK05GjHoZUU7aN8UJAFzL2xaz7byyw=
>>  
>> 
>>  
>> Modified:
>> libcxx/trunk/lib/CMakeLists.txt
>>  
>> Modified: libcxx/trunk/lib/CMakeLists.txt
>> URL: 
>> https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_libcxx_trunk_lib_CMakeLists.txt-3Frev-3D323453-26r1-3D323452-26r2-3D323453-26view-3Ddiff=DwIGaQ=5VD0RTtNlTh3ycd41b3MUw=o3kDXzdBUE3ljQXKeTWOMw=zwvSmk9ZIfMARYg3JvYvNuA0Iz-PEq3H2i_ICF6vsR8=Rd7xc2GWS8oyDcm0QshNPTqM3BglPY5_aHMIuCRUt1s=
>>  
>> 
>> ==
>> --- libcxx/trunk/lib/CMakeLists.txt (original)
>> +++ libcxx/trunk/lib/CMakeLists.txt Thu Jan 25 10:13:26 2018
>> @@ -222,6 +222,7 @@ set(LIBCXX_TARGETS)
>> # Build the shared library.
>> if (LIBCXX_ENABLE_SHARED)
>>add_library(cxx_shared SHARED $)
>> +  llvm_setup_rpath(cxx_shared)
>>target_link_libraries(cxx_shared ${LIBCXX_LIBRARIES})
>>set_target_properties(cxx_shared
>>  PROPERTIES
>>  
>>  
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org 
>> 

Re: [libcxx] r323453 - [cmake] [libcxx] Call llvm_setup_rpath() when adding shared libraries.

[Don Hinton via cfe-commits]

On Thu, Jan 25, 2018 at 12:29 PM, Duncan P. N. Exon Smith <
dexonsm...@apple.com> wrote:

> I don't really understand why, but our bots seemed to survive this:
> http://lab.llvm.org:8080/green/view/Libcxx/job/libcxx_master_cmake/2137/
>
> Console output is here:
> http://lab.llvm.org:8080/green/view/Libcxx/job/libcxx_
> master_cmake/2137/consoleFull
>
> That doesn't necessarily mean it's safe.  Does anyone know why this might
> have "worked"?
>

I'm not an expert, but llvm installs all of these cmake modules along side
llvm-config, so if you find llvm-config, you find these modules.

So, unless I'm missing something, it looks like AddLLVM.cmake is always
available.


>
> On Jan 25, 2018, at 11:02, Shoaib Meenai  wrote:
>
> This is going to break building libc++ standalone (i.e. without any LLVM
> repository or even its CMake modules), right? Some upstream users care a
> lot about that use case (CC beanz and Duncan).
>
> *From: *cfe-commits  on behalf of Don
> Hinton via cfe-commits 
> *Reply-To: *Don Hinton 
> *Date: *Thursday, January 25, 2018 at 10:15 AM
> *To: *"cfe-commits@lists.llvm.org" 
> *Subject: *[libcxx] r323453 - [cmake] [libcxx] Call llvm_setup_rpath()
> when adding shared libraries.
>
> Author: dhinton
> Date: Thu Jan 25 10:13:26 2018
> New Revision: 323453
>
> URL: https://urldefense.proofpoint.com/v2/url?u=http-
> 3A__llvm.org_viewvc_llvm-2Dproject-3Frev-3D323453-26view-3Drev=DwIGaQ=
> 5VD0RTtNlTh3ycd41b3MUw=o3kDXzdBUE3ljQXKeTWOMw=
> zwvSmk9ZIfMARYg3JvYvNuA0Iz-PEq3H2i_ICF6vsR8=
> LDCu89byjNdZWoCIYHGHaPr3IamIdHLF0JwbnYE92vM=
> Log:
> [cmake] [libcxx] Call llvm_setup_rpath() when adding shared libraries.
>
> Clang and llvm already use llvm_setup_rpath(), so this change will
> help standarize rpath usage across all projects.
>
> Differential Revision: https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__reviews.llvm.org_D42459=DwIGaQ=5VD0RTtNlTh3ycd41b3MUw=
> o3kDXzdBUE3ljQXKeTWOMw=zwvSmk9ZIfMARYg3JvYvNuA0Iz-
> PEq3H2i_ICF6vsR8=YCR-YJBua5p-4IK05GjHoZUU7aN8UJAFzL2xaz7byyw=
>
> Modified:
> libcxx/trunk/lib/CMakeLists.txt
>
> Modified: libcxx/trunk/lib/CMakeLists.txt
> URL: https://urldefense.proofpoint.com/v2/url?u=http-
> 3A__llvm.org_viewvc_llvm-2Dproject_libcxx_trunk_lib_
> CMakeLists.txt-3Frev-3D323453-26r1-3D323452-26r2-3D323453-
> 26view-3Ddiff=DwIGaQ=5VD0RTtNlTh3ycd41b3MUw=
> o3kDXzdBUE3ljQXKeTWOMw=zwvSmk9ZIfMARYg3JvYvNuA0Iz-PEq3H2i_ICF6vsR8=
> Rd7xc2GWS8oyDcm0QshNPTqM3BglPY5_aHMIuCRUt1s=
> 
> ==
> --- libcxx/trunk/lib/CMakeLists.txt (original)
> +++ libcxx/trunk/lib/CMakeLists.txt Thu Jan 25 10:13:26 2018
> @@ -222,6 +222,7 @@ set(LIBCXX_TARGETS)
> # Build the shared library.
> if (LIBCXX_ENABLE_SHARED)
>add_library(cxx_shared SHARED $)
> +  llvm_setup_rpath(cxx_shared)
>target_link_libraries(cxx_shared ${LIBCXX_LIBRARIES})
>set_target_properties(cxx_shared
>  PROPERTIES
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.
> llvm.org_cgi-2Dbin_mailman_listinfo_cfe-2Dcommits=DwIGaQ=
> 5VD0RTtNlTh3ycd41b3MUw=o3kDXzdBUE3ljQXKeTWOMw=
> zwvSmk9ZIfMARYg3JvYvNuA0Iz-PEq3H2i_ICF6vsR8=
> TPt8bk99XfmUCdi7MBxyguYV6hJg3PsXFCTXyDfGms8=
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39571: [clangd] DidChangeConfiguration Notification

[Simon Marchi via Phabricator via cfe-commits]

simark added inline comments.



Comment at: clangd/ClangdLSPServer.cpp:302
 
+// FIXME: This function needs to be properly tested.
+void ClangdLSPServer::onChangeConfiguration(

ilya-biryukov wrote:
> simark wrote:
> > ilya-biryukov wrote:
> > > simark wrote:
> > > > ilya-biryukov wrote:
> > > > > Are you planning to to address this FIXME before checking the code in?
> > > > Following what you said here:
> > > > 
> > > > https://reviews.llvm.org/D39571?id=124024#inline-359345
> > > > 
> > > > I have not really looked into what was wrong with the test, and what is 
> > > > missing in the infrastructure to make it work.  But I assumed that the 
> > > > situation did not change since then.  Can you enlighten me on what the 
> > > > problem was, and what is missing?
> > > We usually write unittests for that kind of thing, since they allow to 
> > > plug an in-memory filesystem, but we only test `ClangdServer` (examples 
> > > are in `unittests/clangd/ClangdTests.cpp`). `ClangdLSPServer` does not 
> > > allow to plug in a virtual filesystem (vfs). Even if we add vfs, it's 
> > > still hard to unit-test because we'll have to match the json input/output 
> > > directly.
> > > 
> > > This leaves us with an option of a lit test that runs `clangd` directly, 
> > > similar to tests in `test/clangd`.
> > > The lit test would need to create a temporary directory, create proper 
> > > `compile_commands.json` there, then send the LSP commands with the path 
> > > to the test to clangd.
> > > One major complication is that in LSP we have to specify the size of each 
> > > message, but in our case the size would change depending on created temp 
> > > path. It means we'll have to patch the test input to setup proper paths 
> > > and message sizes.
> > > If we choose to go down this path, 
> > > `clang-tools-extra/test/clang-tidy/vfsoverlay.cpp` does a similar setup 
> > > (create temp-dir, patch up some configuration files to point into the 
> > > temp directory, etc) and could be used as a starting point.
> > > 
> > > It's not impossible to write that test, it's just a bit involved. Having 
> > > a test would be nice, though, to ensure we don't break this method while 
> > > doing other things. Especially given that this functionality is not used 
> > > anywhere in clangd.
> > > We usually write unittests for that kind of thing, since they allow to 
> > > plug an in-memory filesystem, but we only test ClangdServer (examples are 
> > > in unittests/clangd/ClangdTests.cpp). ClangdLSPServer does not allow to 
> > > plug in a virtual filesystem (vfs). Even if we add vfs, it's still hard 
> > > to unit-test because we'll have to match the json input/output directly.
> > 
> > What do you mean by "we'll have to match the json input/output directly"?  
> > That we'll have to match the complete JSON output textually?  Couldn't the 
> > test parse the JSON into some data structures, then we could assert 
> > specific things, like that this particular field is present and contains a 
> > certain substring, for example?
> > 
> > > This leaves us with an option of a lit test that runs clangd directly, 
> > > similar to tests in test/clangd.
> > > The lit test would need to create a temporary directory, create proper 
> > > compile_commands.json there, then send the LSP commands with the path to 
> > > the test to clangd.
> > > One major complication is that in LSP we have to specify the size of each 
> > > message, but in our case the size would change depending on created temp 
> > > path. It means we'll have to patch the test input to setup proper paths 
> > > and message sizes.
> > > If we choose to go down this path, 
> > > clang-tools-extra/test/clang-tidy/vfsoverlay.cpp does a similar setup 
> > > (create temp-dir, patch up some configuration files to point into the 
> > > temp directory, etc) and could be used as a starting point.
> > 
> > Ok, I see the complication with the Content-Length.  I am not familiar with 
> > lit yet, so I don't know what it is capable of.  But being able to craft 
> > and send arbitrary LSP messages would certainly be helpful in the future 
> > for all kinds of black box test, so having a framework that allows to do 
> > this would be helpful, I think.  I'm not familiar enough with the ecosystem 
> > to do this right now, but I'll keep it in mind.
> > 
> > One question about this particular test.  Would there be some race 
> > condition here?  If we do:
> > 
> > 1. Start clangd with compile_commands.json #1
> > 2. Ask for the definition of a function, expecting a result
> > 3. Change the configuration to compile_commands.json #2
> > 4. Ask for the definition of the same function, expecting a different result
> > 
> > Since clangd is multi-threaded and does work in the background, are we sure 
> > that we'll get the result we want in #4?
> > 
> > > It's not impossible to write that test, it's just a bit involved. Having 
> > > a test would be nice, though, to ensure 

Re: [libcxx] r323453 - [cmake] [libcxx] Call llvm_setup_rpath() when adding shared libraries.

[Duncan P. N. Exon Smith via cfe-commits]

I don't really understand why, but our bots seemed to survive this:
http://lab.llvm.org:8080/green/view/Libcxx/job/libcxx_master_cmake/2137/ 


Console output is here:
http://lab.llvm.org:8080/green/view/Libcxx/job/libcxx_master_cmake/2137/consoleFull
 


That doesn't necessarily mean it's safe.  Does anyone know why this might have 
"worked"?

> On Jan 25, 2018, at 11:02, Shoaib Meenai  wrote:
> 
> This is going to break building libc++ standalone (i.e. without any LLVM 
> repository or even its CMake modules), right? Some upstream users care a lot 
> about that use case (CC beanz and Duncan).
>  
> From: cfe-commits  > on behalf of Don Hinton via 
> cfe-commits >
> Reply-To: Don Hinton >
> Date: Thursday, January 25, 2018 at 10:15 AM
> To: "cfe-commits@lists.llvm.org " 
> >
> Subject: [libcxx] r323453 - [cmake] [libcxx] Call llvm_setup_rpath() when 
> adding shared libraries.
>  
> Author: dhinton
> Date: Thu Jan 25 10:13:26 2018
> New Revision: 323453
>  
> URL: 
> https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject-3Frev-3D323453-26view-3Drev=DwIGaQ=5VD0RTtNlTh3ycd41b3MUw=o3kDXzdBUE3ljQXKeTWOMw=zwvSmk9ZIfMARYg3JvYvNuA0Iz-PEq3H2i_ICF6vsR8=LDCu89byjNdZWoCIYHGHaPr3IamIdHLF0JwbnYE92vM=
>  
> 
> Log:
> [cmake] [libcxx] Call llvm_setup_rpath() when adding shared libraries.
>  
> Clang and llvm already use llvm_setup_rpath(), so this change will
> help standarize rpath usage across all projects.
>  
> Differential Revision: 
> https://urldefense.proofpoint.com/v2/url?u=https-3A__reviews.llvm.org_D42459=DwIGaQ=5VD0RTtNlTh3ycd41b3MUw=o3kDXzdBUE3ljQXKeTWOMw=zwvSmk9ZIfMARYg3JvYvNuA0Iz-PEq3H2i_ICF6vsR8=YCR-YJBua5p-4IK05GjHoZUU7aN8UJAFzL2xaz7byyw=
>  
> 
>  
> Modified:
> libcxx/trunk/lib/CMakeLists.txt
>  
> Modified: libcxx/trunk/lib/CMakeLists.txt
> URL: 
> https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_libcxx_trunk_lib_CMakeLists.txt-3Frev-3D323453-26r1-3D323452-26r2-3D323453-26view-3Ddiff=DwIGaQ=5VD0RTtNlTh3ycd41b3MUw=o3kDXzdBUE3ljQXKeTWOMw=zwvSmk9ZIfMARYg3JvYvNuA0Iz-PEq3H2i_ICF6vsR8=Rd7xc2GWS8oyDcm0QshNPTqM3BglPY5_aHMIuCRUt1s=
>  
> 
> ==
> --- libcxx/trunk/lib/CMakeLists.txt (original)
> +++ libcxx/trunk/lib/CMakeLists.txt Thu Jan 25 10:13:26 2018
> @@ -222,6 +222,7 @@ set(LIBCXX_TARGETS)
> # Build the shared library.
> if (LIBCXX_ENABLE_SHARED)
>add_library(cxx_shared SHARED $)
> +  llvm_setup_rpath(cxx_shared)
>target_link_libraries(cxx_shared ${LIBCXX_LIBRARIES})
>set_target_properties(cxx_shared
>  PROPERTIES
>  
>  
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org 
> https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.llvm.org_cgi-2Dbin_mailman_listinfo_cfe-2Dcommits=DwIGaQ=5VD0RTtNlTh3ycd41b3MUw=o3kDXzdBUE3ljQXKeTWOMw=zwvSmk9ZIfMARYg3JvYvNuA0Iz-PEq3H2i_ICF6vsR8=TPt8bk99XfmUCdi7MBxyguYV6hJg3PsXFCTXyDfGms8=
>  
> 
>  

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

[PATCH] D42459: [cmake] [libcxx] Call llvm_setup_rpath() when adding shared libraries.

[Don Hinton via Phabricator via cfe-commits]

hintonda requested review of this revision.
hintonda added a comment.

I probably should have put this in the summary, but the reason for this patch 
isn't just to clean up the cmake files, it's to fix a cmake configuration 
failure when cross-compiling for Linux on Darwin:

CMake Error at 
/Users/dhinton/projects/llvm_project/libcxx/lib/CMakeLists.txt:224 
(add_library):

  The install of the cxx_shared target requires changing an RPATH from the
  build tree, but this is not supported with the Ninja generator unless on an
  ELF-based platform.  The CMAKE_BUILD_WITH_INSTALL_RPATH variable may be set
  to avoid this relinking step.


Repository:
  rCXX libc++

https://reviews.llvm.org/D42459



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

[PATCH] D42552: [analyzer] dump() environment frame-by-frame.

[Artem Dergachev via Phabricator via cfe-commits]

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

Like this:

  Expressions by stack frame:
  #0 Calling bar at line 9
  (0x7fbe2a60ea10,0x7fbe2b86ca30) x : 1 S32b
  #1 Calling foo
  (0x7fbe2a60e2e0,0x7fbe2b86cc98) bar : {bar}
  (0x7fbe2a60e2e0,0x7fbe2b86cce0) x : 0 S32b

Or in exploded graph:

F5784469: Frames.png 

This way it's instantly obvious which `x` is equal to `1` and which `x` is 
equal to `0`.

Additionally, the separate section for the backtrace is removed from the 
graphviz dumps, and the respective code is deduplicated with 
`LocationContext::dumpStack()` which is also used for crash dumps.

It might be not clear where is the current location context in some cases (or 
annoying to supply location context every time you want to dump the program 
state - it is not even necessarily available), so i made a simple 
auto-detection of the freshest location context in which the environment has at 
least one entry, which would be run unless the context is specified explicitly.

The dump is a bit weird for blocks, but nevertheless it represents how the 
analyzer currently works:

F5784474: Blocks.png 


Repository:
  rC Clang

https://reviews.llvm.org/D42552

Files:
  include/clang/Analysis/AnalysisDeclContext.h
  include/clang/StaticAnalyzer/Core/PathSensitive/Environment.h
  include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
  lib/Analysis/AnalysisDeclContext.cpp
  lib/StaticAnalyzer/Core/Environment.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  lib/StaticAnalyzer/Core/ProgramState.cpp
  test/Analysis/crash-trace.c
  test/Analysis/expr-inspection.c

Index: test/Analysis/expr-inspection.c
===
--- test/Analysis/expr-inspection.c
+++ test/Analysis/expr-inspection.c
@@ -18,6 +18,8 @@
 // CHECK: Store (direct and default bindings)
 // CHECK-NEXT: (y,0,direct) : 1 S32b
 
-// CHECK: Expressions:
+// CHECK: Expressions by stack frame:
+// CHECK-NEXT: #0 Calling foo
 // CHECK-NEXT: clang_analyzer_printState : {clang_analyzer_printState}
-// CHECK-NEXT: {{(Ranges are empty.)|(Constraints:[[:space:]]*$)}}
+
+// CHECK: {{(Ranges are empty.)|(Constraints:[[:space:]]*$)}}
Index: test/Analysis/crash-trace.c
===
--- test/Analysis/crash-trace.c
+++ test/Analysis/crash-trace.c
@@ -18,6 +18,6 @@
 // CHECK: 0.	Program arguments: {{.*}}clang
 // CHECK-NEXT: 1.	 parser at end of file
 // CHECK-NEXT: 2. While analyzing stack: 
-// CHECK-NEXT:  #0 void inlined()
-// CHECK-NEXT:  #1 void test()
+// CHECK-NEXT:  #0 Calling inlined at line 15
+// CHECK-NEXT:  #1 Calling test
 // CHECK-NEXT: 3.	{{.*}}crash-trace.c:{{[0-9]+}}:3: Error evaluating statement
Index: lib/StaticAnalyzer/Core/ProgramState.cpp
===
--- lib/StaticAnalyzer/Core/ProgramState.cpp
+++ lib/StaticAnalyzer/Core/ProgramState.cpp
@@ -437,24 +437,24 @@
 //  State pretty-printing.
 //===--===//
 
-void ProgramState::print(raw_ostream ,
- const char *NL, const char *Sep) const {
+void ProgramState::print(raw_ostream , const char *NL, const char *Sep,
+ const LocationContext *LC) const {
   // Print the store.
   ProgramStateManager  = getStateManager();
   Mgr.getStoreManager().print(getStore(), Out, NL, Sep);
 
   // Print out the environment.
-  Env.print(Out, NL, Sep);
+  Env.print(Out, NL, Sep, LC);
 
   // Print out the constraints.
   Mgr.getConstraintManager().print(this, Out, NL, Sep);
 
   // Print checker-specific data.
   Mgr.getOwningEngine()->printState(Out, this, NL, Sep);
 }
 
-void ProgramState::printDOT(raw_ostream ) const {
-  print(Out, "\\l", "\\|");
+void ProgramState::printDOT(raw_ostream , const LocationContext *LC) const {
+  print(Out, "\\l", "\\|", LC);
 }
 
 LLVM_DUMP_METHOD void ProgramState::dump() const {
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -2769,12 +2769,6 @@
 << "\\l";
 }
   }
-  static void printLocation2(raw_ostream , SourceLocation SLoc) {
-if (SLoc.isFileID() && GraphPrintSourceManager->isInMainFile(SLoc))
-  Out << "line " << GraphPrintSourceManager->getExpansionLineNumber(SLoc);
-else
-  SLoc.print(Out, *GraphPrintSourceManager);
-  }
 
   static std::string getNodeLabel(const ExplodedNode *N, void*){
 
@@ -2949,40 +2943,7 @@
 Out << "\\|StateID: " << (const void*) state.get()
 << " NodeID: " << (const void*) N << "\\|";
 
-// Analysis stack backtrace.
-Out << "Location context stack (from current to outer):\\l";
-const 

r323461 - AST: inline a single-use variable (NFC)

[Saleem Abdulrasool via cfe-commits]

Author: compnerd
Date: Thu Jan 25 11:54:31 2018
New Revision: 323461

URL: http://llvm.org/viewvc/llvm-project?rev=323461=rev
Log:
AST: inline a single-use variable (NFC)

Inline the single use variable into the only use.  NFC.

Modified:
cfe/trunk/lib/AST/MicrosoftMangle.cpp

Modified: cfe/trunk/lib/AST/MicrosoftMangle.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/MicrosoftMangle.cpp?rev=323461=323460=323461=diff
==
--- cfe/trunk/lib/AST/MicrosoftMangle.cpp (original)
+++ cfe/trunk/lib/AST/MicrosoftMangle.cpp Thu Jan 25 11:54:31 2018
@@ -1559,12 +1559,11 @@ MicrosoftCXXNameMangler::mangleRefQualif
 
 void MicrosoftCXXNameMangler::manglePointerExtQualifiers(Qualifiers Quals,
  QualType PointeeType) 
{
-  bool HasRestrict = Quals.hasRestrict();
   if (PointersAre64Bit &&
   (PointeeType.isNull() || !PointeeType->isFunctionType()))
 Out << 'E';
 
-  if (HasRestrict)
+  if (Quals.hasRestrict())
 Out << 'I';
 
   if (Quals.hasUnaligned() ||


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

Re: Please subscribe "rUNW libunwind" to cfe-commits.

[Don Hinton via cfe-commits]

Ah, I think that's the issue.

local:/Users/dhinton/projects/llvm_project/libcxxabi $ cat .arcconfig
{
  "repository.callsign" : "CXXA",
  "conduit_uri" : "https://reviews.llvm.org/;
}

So, it should set repository to CXXA, not LLVM.



On Thu, Jan 25, 2018 at 11:25 AM, Ben Hamilton 
wrote:

> Yes, that's currently a submit time feature built in to Phabricator which
> i believe we'll have to fix upstream.
>
> Because both the top level LLVM SVN report and the sub repos see the same
> files, Phabricator automatically subscribes both lists to the commits.
>
> I wasn't able to find an easy way to disable this feature. If you can find
> one, please let me know! (I was punting in hopes the migration to git would
> happen first.)
>
> On Thu, Jan 25, 2018 at 11:01 AM Don Hinton  wrote:
>
>> Btw, I'm seeing other inconsistencies, e.g., I just committed
>> https://reviews.llvm.org/D42460 and herald ended up adding both
>> cfe-commits and llvm-commits to subscribers.
>>
>> When I created it, I left subscribers blank, so herald added cfe-commits
>> -- which seems to be correct.
>> Then, when I committed it, herald added llvm-commits -- which doesn't
>> seem correct.
>>
>>
>>
>> On Thu, Jan 25, 2018 at 10:04 AM, Don Hinton  wrote:
>>
>>> Thanks Ben...
>>>
>>> On Thu, Jan 25, 2018 at 9:53 AM, Ben Hamilton 
>>> wrote:
>>>
 Ah, I see from http://bcain-llvm.readthedocs.io/projects/
 libunwind/en/latest/ that libunwind review discussion does take place
 on cfe-commits.

 Added. Sorry about not doing that earlier!

 Ben

 On Thu, Jan 25, 2018 at 8:37 AM Ben Hamilton 
 wrote:

> Great question. Is there an existing separate list for libunwind?
>
> If libunwind discussion already happens on cfe-commits, then I'll make
> reviews for libunwind subscribe that list.
>
> On Tue, Jan 23, 2018, 21:40 Don Hinton  wrote:
>
>> Currently, herald doesn't subscribe libunwind to any group.  Should
>> it be added to cfe-commits?
>>
>>   https://reviews.llvm.org/H268
>>
>
>>>
>>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42460: [cmake] [libcxxabi] Call llvm_setup_rpath() when adding shared libraries.

[Don Hinton via Phabricator via cfe-commits]

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

Reopen after rollback.

@smeenai pointed out LLVM may not be available in standalone builds


Repository:
  rL LLVM

https://reviews.llvm.org/D42460



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

[PATCH] D42459: [cmake] [libcxx] Call llvm_setup_rpath() when adding shared libraries.

[Don Hinton via Phabricator via cfe-commits]

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

Reopen after rollback.

@smeenai pointed out LLVM may not be available in standalone builds


Repository:
  rCXX libc++

https://reviews.llvm.org/D42459



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

Re: Please subscribe "rUNW libunwind" to cfe-commits.

[Ben Hamilton via cfe-commits]

Yes, that's currently a submit time feature built in to Phabricator which i
believe we'll have to fix upstream.

Because both the top level LLVM SVN report and the sub repos see the same
files, Phabricator automatically subscribes both lists to the commits.

I wasn't able to find an easy way to disable this feature. If you can find
one, please let me know! (I was punting in hopes the migration to git would
happen first.)

On Thu, Jan 25, 2018 at 11:01 AM Don Hinton  wrote:

> Btw, I'm seeing other inconsistencies, e.g., I just committed
> https://reviews.llvm.org/D42460 and herald ended up adding both
> cfe-commits and llvm-commits to subscribers.
>
> When I created it, I left subscribers blank, so herald added cfe-commits
> -- which seems to be correct.
> Then, when I committed it, herald added llvm-commits -- which doesn't seem
> correct.
>
>
>
> On Thu, Jan 25, 2018 at 10:04 AM, Don Hinton  wrote:
>
>> Thanks Ben...
>>
>> On Thu, Jan 25, 2018 at 9:53 AM, Ben Hamilton 
>> wrote:
>>
>>> Ah, I see from
>>> http://bcain-llvm.readthedocs.io/projects/libunwind/en/latest/ that
>>> libunwind review discussion does take place on cfe-commits.
>>>
>>> Added. Sorry about not doing that earlier!
>>>
>>> Ben
>>>
>>> On Thu, Jan 25, 2018 at 8:37 AM Ben Hamilton 
>>> wrote:
>>>
 Great question. Is there an existing separate list for libunwind?

 If libunwind discussion already happens on cfe-commits, then I'll make
 reviews for libunwind subscribe that list.

 On Tue, Jan 23, 2018, 21:40 Don Hinton  wrote:

> Currently, herald doesn't subscribe libunwind to any group.  Should it
> be added to cfe-commits?
>
>   https://reviews.llvm.org/H268
>

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

[libcxx] r323459 - Revert [libcxx] r323453 - [cmake] [libcxx] Call llvm_setup_rpath() when adding shared libraries.

[Don Hinton via cfe-commits]

Author: dhinton
Date: Thu Jan 25 11:22:23 2018
New Revision: 323459

URL: http://llvm.org/viewvc/llvm-project?rev=323459=rev
Log:
Revert [libcxx] r323453 - [cmake] [libcxx] Call llvm_setup_rpath() when adding 
shared libraries.

Shoaib Meenai pointed out this will break standalone builds when built without 
llvm.

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

Modified:
libcxx/trunk/lib/CMakeLists.txt

Modified: libcxx/trunk/lib/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/lib/CMakeLists.txt?rev=323459=323458=323459=diff
==
--- libcxx/trunk/lib/CMakeLists.txt (original)
+++ libcxx/trunk/lib/CMakeLists.txt Thu Jan 25 11:22:23 2018
@@ -222,7 +222,6 @@ set(LIBCXX_TARGETS)
 # Build the shared library.
 if (LIBCXX_ENABLE_SHARED)
   add_library(cxx_shared SHARED $)
-  llvm_setup_rpath(cxx_shared)
   target_link_libraries(cxx_shared ${LIBCXX_LIBRARIES})
   set_target_properties(cxx_shared
 PROPERTIES


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

[libcxxabi] r323458 - Revert [libcxxabi] r323455 - [cmake] [libcxxabi] Call llvm_setup_rpath() when adding shared libraries.

[Don Hinton via cfe-commits]

Author: dhinton
Date: Thu Jan 25 11:18:51 2018
New Revision: 323458

URL: http://llvm.org/viewvc/llvm-project?rev=323458=rev
Log:
Revert [libcxxabi] r323455 - [cmake] [libcxxabi] Call llvm_setup_rpath() when 
adding shared libraries.

Shoaib Meenai pointed out this will break standalone builds can be built 
without llvm.

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

Modified:
libcxxabi/trunk/src/CMakeLists.txt

Modified: libcxxabi/trunk/src/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/libcxxabi/trunk/src/CMakeLists.txt?rev=323458=323457=323458=diff
==
--- libcxxabi/trunk/src/CMakeLists.txt (original)
+++ libcxxabi/trunk/src/CMakeLists.txt Thu Jan 25 11:18:51 2018
@@ -127,7 +127,6 @@ set(LIBCXXABI_TARGETS)
 # Build the shared library.
 if (LIBCXXABI_ENABLE_SHARED)
   add_library(cxxabi_shared SHARED $)
-  llvm_setup_rpath(cxxabi_shared)
   target_link_libraries(cxxabi_shared ${LIBCXXABI_LIBRARIES})
   set_target_properties(cxxabi_shared
 PROPERTIES


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

[PATCH] D42530: Clang permits assignment to vector/extvector elements in a const method

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/AST/ExprClassification.cpp:652
+if (Ctx.getCanonicalType(ASE->getBase()->getType()).isConstQualified())
+  return Cl::CM_ConstQualified;
+

Is there a reason why the places that compute the type of these l-value 
expressions don't propagate qualiifers?  This hardly seems restricted to 
'const'.


https://reviews.llvm.org/D42530



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

[PATCH] D42521: [CodeGen] Use the non-virtual alignment when emitting the base class subobject constructor

[John McCall via Phabricator via cfe-commits]

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

LGTM.


https://reviews.llvm.org/D42521



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

Re: [libcxxabi] r323455 - [cmake] [libcxxabi] Call llvm_setup_rpath() when adding shared libraries.

[Shoaib Meenai via cfe-commits]

Same concerns about building standalone as r323453.

From: cfe-commits  on behalf of Don Hinton 
via cfe-commits 
Reply-To: Don Hinton 
Date: Thursday, January 25, 2018 at 10:45 AM
To: "cfe-commits@lists.llvm.org" 
Subject: [libcxxabi] r323455 - [cmake] [libcxxabi] Call llvm_setup_rpath() when 
adding shared libraries.

Author: dhinton
Date: Thu Jan 25 10:43:18 2018
New Revision: 323455

URL: 
https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject-3Frev-3D323455-26view-3Drev=DwIGaQ=5VD0RTtNlTh3ycd41b3MUw=o3kDXzdBUE3ljQXKeTWOMw=hTm6KbFwg4mKjKLP4oJisCnQB5oDNYWxGe5FbWvRMG4=aQXMyZsZ0iP8FoN1WB_c71EWuSMWQxCXM-sGKrKadHQ=
Log:
[cmake] [libcxxabi] Call llvm_setup_rpath() when adding shared libraries.

Clang and llvm already use llvm_setup_rpath(), so this change will
help standarize rpath usage across all projects.

Differential Revision: 
https://urldefense.proofpoint.com/v2/url?u=https-3A__reviews.llvm.org_D42460=DwIGaQ=5VD0RTtNlTh3ycd41b3MUw=o3kDXzdBUE3ljQXKeTWOMw=hTm6KbFwg4mKjKLP4oJisCnQB5oDNYWxGe5FbWvRMG4=ar8LZPLycTnAYZym_qLrhMWtMbmSLGMTM-iF0fGYapA=

Modified:
libcxxabi/trunk/src/CMakeLists.txt

Modified: libcxxabi/trunk/src/CMakeLists.txt
URL: 
https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_libcxxabi_trunk_src_CMakeLists.txt-3Frev-3D323455-26r1-3D323454-26r2-3D323455-26view-3Ddiff=DwIGaQ=5VD0RTtNlTh3ycd41b3MUw=o3kDXzdBUE3ljQXKeTWOMw=hTm6KbFwg4mKjKLP4oJisCnQB5oDNYWxGe5FbWvRMG4=Q0L26GZkEGTO8MI8bqktkHHGjnRaeZHZd1kOIR1F8ng=
==
--- libcxxabi/trunk/src/CMakeLists.txt (original)
+++ libcxxabi/trunk/src/CMakeLists.txt Thu Jan 25 10:43:18 2018
@@ -127,6 +127,7 @@ set(LIBCXXABI_TARGETS)
# Build the shared library.
if (LIBCXXABI_ENABLE_SHARED)
   add_library(cxxabi_shared SHARED $)
+  llvm_setup_rpath(cxxabi_shared)
   target_link_libraries(cxxabi_shared ${LIBCXXABI_LIBRARIES})
   set_target_properties(cxxabi_shared
 PROPERTIES


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.llvm.org_cgi-2Dbin_mailman_listinfo_cfe-2Dcommits=DwIGaQ=5VD0RTtNlTh3ycd41b3MUw=o3kDXzdBUE3ljQXKeTWOMw=hTm6KbFwg4mKjKLP4oJisCnQB5oDNYWxGe5FbWvRMG4=BwX8iu3z7NGRyVxRMdBFsrL01j8I4y_eQ-fw8kW5bLs=

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

Re: [libcxx] r323453 - [cmake] [libcxx] Call llvm_setup_rpath() when adding shared libraries.

[Shoaib Meenai via cfe-commits]

This is going to break building libc++ standalone (i.e. without any LLVM 
repository or even its CMake modules), right? Some upstream users care a lot 
about that use case (CC beanz and Duncan).

From: cfe-commits  on behalf of Don Hinton 
via cfe-commits 
Reply-To: Don Hinton 
Date: Thursday, January 25, 2018 at 10:15 AM
To: "cfe-commits@lists.llvm.org" 
Subject: [libcxx] r323453 - [cmake] [libcxx] Call llvm_setup_rpath() when 
adding shared libraries.

Author: dhinton
Date: Thu Jan 25 10:13:26 2018
New Revision: 323453

URL: 
https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject-3Frev-3D323453-26view-3Drev=DwIGaQ=5VD0RTtNlTh3ycd41b3MUw=o3kDXzdBUE3ljQXKeTWOMw=zwvSmk9ZIfMARYg3JvYvNuA0Iz-PEq3H2i_ICF6vsR8=LDCu89byjNdZWoCIYHGHaPr3IamIdHLF0JwbnYE92vM=
Log:
[cmake] [libcxx] Call llvm_setup_rpath() when adding shared libraries.

Clang and llvm already use llvm_setup_rpath(), so this change will
help standarize rpath usage across all projects.

Differential Revision: 
https://urldefense.proofpoint.com/v2/url?u=https-3A__reviews.llvm.org_D42459=DwIGaQ=5VD0RTtNlTh3ycd41b3MUw=o3kDXzdBUE3ljQXKeTWOMw=zwvSmk9ZIfMARYg3JvYvNuA0Iz-PEq3H2i_ICF6vsR8=YCR-YJBua5p-4IK05GjHoZUU7aN8UJAFzL2xaz7byyw=

Modified:
libcxx/trunk/lib/CMakeLists.txt

Modified: libcxx/trunk/lib/CMakeLists.txt
URL: 
https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_libcxx_trunk_lib_CMakeLists.txt-3Frev-3D323453-26r1-3D323452-26r2-3D323453-26view-3Ddiff=DwIGaQ=5VD0RTtNlTh3ycd41b3MUw=o3kDXzdBUE3ljQXKeTWOMw=zwvSmk9ZIfMARYg3JvYvNuA0Iz-PEq3H2i_ICF6vsR8=Rd7xc2GWS8oyDcm0QshNPTqM3BglPY5_aHMIuCRUt1s=
==
--- libcxx/trunk/lib/CMakeLists.txt (original)
+++ libcxx/trunk/lib/CMakeLists.txt Thu Jan 25 10:13:26 2018
@@ -222,6 +222,7 @@ set(LIBCXX_TARGETS)
# Build the shared library.
if (LIBCXX_ENABLE_SHARED)
   add_library(cxx_shared SHARED $)
+  llvm_setup_rpath(cxx_shared)
   target_link_libraries(cxx_shared ${LIBCXX_LIBRARIES})
   set_target_properties(cxx_shared
 PROPERTIES


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.llvm.org_cgi-2Dbin_mailman_listinfo_cfe-2Dcommits=DwIGaQ=5VD0RTtNlTh3ycd41b3MUw=o3kDXzdBUE3ljQXKeTWOMw=zwvSmk9ZIfMARYg3JvYvNuA0Iz-PEq3H2i_ICF6vsR8=TPt8bk99XfmUCdi7MBxyguYV6hJg3PsXFCTXyDfGms8=

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

Re: Please subscribe "rUNW libunwind" to cfe-commits.

[Don Hinton via cfe-commits]

Btw, I'm seeing other inconsistencies, e.g., I just committed
https://reviews.llvm.org/D42460 and herald ended up adding both cfe-commits
and llvm-commits to subscribers.

When I created it, I left subscribers blank, so herald added cfe-commits --
which seems to be correct.
Then, when I committed it, herald added llvm-commits -- which doesn't seem
correct.



On Thu, Jan 25, 2018 at 10:04 AM, Don Hinton  wrote:

> Thanks Ben...
>
> On Thu, Jan 25, 2018 at 9:53 AM, Ben Hamilton 
> wrote:
>
>> Ah, I see from http://bcain-llvm.readthedocs.io/projects/libunwind/en/
>> latest/ that libunwind review discussion does take place on cfe-commits.
>>
>> Added. Sorry about not doing that earlier!
>>
>> Ben
>>
>> On Thu, Jan 25, 2018 at 8:37 AM Ben Hamilton 
>> wrote:
>>
>>> Great question. Is there an existing separate list for libunwind?
>>>
>>> If libunwind discussion already happens on cfe-commits, then I'll make
>>> reviews for libunwind subscribe that list.
>>>
>>> On Tue, Jan 23, 2018, 21:40 Don Hinton  wrote:
>>>
 Currently, herald doesn't subscribe libunwind to any group.  Should it
 be added to cfe-commits?

   https://reviews.llvm.org/H268

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

[PATCH] D42549: [CodeGen] Use the zero initializer instead of storing an all zero representation.

[Matt Davis via Phabricator via cfe-commits]

mattd created this revision.
mattd added a reviewer: majnemer.

This change avoids the overhead of storing, and later crawling,
an initializer list of all zeros for arrays. When LLVM
visits this (llvm/IR/Constants.cpp) ConstantArray::getImpl()
it will scan the list looking for an array of all zero.

We can avoid the store, and short-cut the scan, by detecting
all zeros when clang builds-up the initialization representation.

This was brought to my attention when investigating PR36030


https://reviews.llvm.org/D42549

Files:
  lib/CodeGen/CGExprConstant.cpp
  test/CodeGen/array-init.c

Index: test/CodeGen/array-init.c
===
--- test/CodeGen/array-init.c
+++ test/CodeGen/array-init.c
@@ -0,0 +1,65 @@
+// RUN: %clang_cc1 %s -O0 -triple x86_64-unknown-linux-gnu -emit-llvm -o - | FileCheck %s
+
+// CHECK: @test.a1 = internal global [10 x i32] [i32 0, i32 1, i32 2, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0], align 16
+// CHECK: @test.a2 = internal global [10 x i32] zeroinitializer, align 16
+// CHECK: @test.a3 = internal global [10 x i32] zeroinitializer, align 16
+// CHECK: @test.a4 = internal global [10 x i32] zeroinitializer, align 16
+// CHECK: @test.b1 = internal constant [10 x i32] [i32 0, i32 1, i32 2, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0], align 16
+// CHECK: @test.b2 = internal constant [10 x i32] zeroinitializer, align 16
+// CHECK: @test.b3 = internal constant [10 x i32] zeroinitializer, align 16
+
+// CHECK: define void @test() #0 {
+// CHECK:   %c1 = alloca [10 x i32], align 16
+// CHECK:   %c2 = alloca [10 x i32], align 16
+// CHECK:   %c3 = alloca [10 x i32], align 16
+// CHECK:   %c4 = alloca [10 x i32], align 16
+// CHECK:   %d1 = alloca [10 x i32], align 16
+// CHECK:   %d2 = alloca [10 x i32], align 16
+// CHECK:   %d3 = alloca [10 x i32], align 16
+// CHECK:   %d4 = alloca [10 x i32], align 16
+// CHECK:   %1 = bitcast [10 x i32]* %c1 to i8*
+// CHECK:   call void @llvm.memset.p0i8.i64(i8* align 16 %1, i8 0, i64 40, i1 false)
+// CHECK:   %2 = bitcast i8* %1 to [10 x i32]*
+// CHECK:   %3 = getelementptr [10 x i32], [10 x i32]* %2, i32 0, i32 1
+// CHECK:   store i32 1, i32* %3
+// CHECK:   %4 = getelementptr [10 x i32], [10 x i32]* %2, i32 0, i32 2
+// CHECK:   store i32 2, i32* %4
+// CHECK:   %5 = bitcast [10 x i32]* %c2 to i8*
+// CHECK:   call void @llvm.memset.p0i8.i64(i8* align 16 %5, i8 0, i64 40, i1 false)
+// CHECK:   %6 = bitcast [10 x i32]* %c3 to i8*
+// CHECK:   call void @llvm.memset.p0i8.i64(i8* align 16 %6, i8 0, i64 40, i1 false)
+// CHECK:   %7 = bitcast [10 x i32]* %d1 to i8*
+// CHECK:   call void @llvm.memset.p0i8.i64(i8* align 16 %7, i8 0, i64 40, i1 true)
+// CHECK:   %8 = bitcast i8* %7 to [10 x i32]*
+// CHECK:   %9 = getelementptr [10 x i32], [10 x i32]* %8, i32 0, i32 1
+// CHECK:   store volatile i32 1, i32* %9
+// CHECK:   %10 = getelementptr [10 x i32], [10 x i32]* %8, i32 0, i32 2
+// CHECK:   store volatile i32 2, i32* %10
+// CHECK:   %11 = bitcast [10 x i32]* %d2 to i8*
+// CHECK:   call void @llvm.memset.p0i8.i64(i8* align 16 %11, i8 0, i64 40, i1 true)
+// CHECK:   %12 = bitcast [10 x i32]* %d3 to i8*
+// CHECK:   call void @llvm.memset.p0i8.i64(i8* align 16 %12, i8 0, i64 40, i1 true)
+// CHECK:   ret void
+// CHECK: }
+
+void test()
+{
+  static int a1[10] = {0,1,2};
+  static int a2[10] = {0,0,0};
+  static int a3[10] = {0};
+  static int a4[10];
+
+  const int b1[10] = {0,1,2};
+  const int b2[10] = {0,0,0};
+  const int b3[10] = {0};
+  
+  int c1[10] = {0,1,2};
+  int c2[10] = {0,0,0};
+  int c3[10] = {0};
+  int c4[10];
+  
+  volatile int d1[10] = {0,1,2};
+  volatile int d2[10] = {0,0,0};
+  volatile int d3[10] = {0};
+  volatile int d4[10];
+}
Index: lib/CodeGen/CGExprConstant.cpp
===
--- lib/CodeGen/CGExprConstant.cpp
+++ lib/CodeGen/CGExprConstant.cpp
@@ -859,9 +859,10 @@
 
 // Copy initializer elements.
 SmallVector Elts;
-Elts.reserve(NumInitableElts + NumElements);
+Elts.reserve(std::max(NumInitableElts, NumElements));
 
 bool RewriteType = false;
+bool AllNullValues = true;
 for (unsigned i = 0; i < NumInitableElts; ++i) {
   Expr *Init = ILE->getInit(i);
   llvm::Constant *C = Emitter.tryEmitPrivateForMemory(Init, EltType);
@@ -869,8 +870,15 @@
 return nullptr;
   RewriteType |= (C->getType() != ElemTy);
   Elts.push_back(C);
+  if (!C->isNullValue())
+AllNullValues = false;
 }
 
+// If all initializer elements are "zero," then avoid storing NumElements
+// instances of the zero representation.
+if (AllNullValues)
+  return llvm::ConstantAggregateZero::get(AType);
+
 RewriteType |= (fillC->getType() != ElemTy);
 Elts.resize(NumElements, fillC);
 
@@ -877,7 +885,7 @@
 if (RewriteType) {
   // FIXME: Try to avoid packing the array
   std::vector Types;
-  

[PATCH] D41102: Setup clang-doc frontend framework

[Julie Hockett via Phabricator via cfe-commits]

juliehockett updated this revision to Diff 131481.
juliehockett added a comment.
Herald added a subscriber: hintonda.

Cleaning up a few unnecessary copyings


https://reviews.llvm.org/D41102

Files:
  test/CMakeLists.txt
  test/Tooling/clang-doc-basic.cpp
  test/lit.cfg.py
  tools/CMakeLists.txt
  tools/clang-doc/CMakeLists.txt
  tools/clang-doc/ClangDoc.cpp
  tools/clang-doc/ClangDoc.h
  tools/clang-doc/ClangDocReporter.cpp
  tools/clang-doc/ClangDocReporter.h
  tools/clang-doc/ClangDocYAML.h
  tools/clang-doc/tool/CMakeLists.txt
  tools/clang-doc/tool/ClangDocMain.cpp

Index: tools/clang-doc/tool/ClangDocMain.cpp
===
--- /dev/null
+++ tools/clang-doc/tool/ClangDocMain.cpp
@@ -0,0 +1,68 @@
+//===-- ClangDocMain.cpp - Clangdoc -*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "ClangDoc.h"
+#include "clang/Driver/Options.h"
+#include "clang/Frontend/FrontendActions.h"
+#include "clang/Tooling/CommonOptionsParser.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/Support/Process.h"
+#include "llvm/Support/Signals.h"
+#include 
+
+using namespace clang;
+using namespace llvm;
+
+namespace {
+
+cl::OptionCategory ClangDocCategory("clang-doc options");
+
+cl::opt
+EmitLLVM("emit-llvm",
+ cl::desc("Output in LLVM bitstream format (default is YAML)."),
+ cl::init(false), cl::cat(ClangDocCategory));
+
+cl::opt
+DoxygenOnly("doxygen",
+cl::desc("Use only doxygen-style comments to generate docs."),
+cl::init(false), cl::cat(ClangDocCategory));
+
+} // namespace
+
+int main(int argc, const char **argv) {
+  sys::PrintStackTraceOnErrorSignal(argv[0]);
+  tooling::CommonOptionsParser OptionsParser(argc, argv, ClangDocCategory);
+
+  clang::doc::OutFormat EmitFormat =
+  EmitLLVM ? doc::OutFormat::LLVM : doc::OutFormat::YAML;
+
+  // TODO: Update the source path list to only consider changed files for
+  // incremental doc updates.
+  doc::ClangDocReporter Reporter(OptionsParser.getSourcePathList());
+  doc::ClangDocContext Context{EmitFormat};
+
+  tooling::ClangTool Tool(OptionsParser.getCompilations(),
+  OptionsParser.getSourcePathList());
+
+  if (!DoxygenOnly)
+Tool.appendArgumentsAdjuster(tooling::getInsertArgumentAdjuster(
+"-fparse-all-comments", tooling::ArgumentInsertPosition::BEGIN));
+
+  doc::ClangDocActionFactory Factory(Context, Reporter);
+
+  outs() << "Parsing codebase...\n";
+  int Status = Tool.run();
+  if (Status)
+return Status;
+
+  outs() << "Writing docs...\n";
+  Reporter.serialize(EmitFormat, outs());
+
+  return 0;
+}
Index: tools/clang-doc/tool/CMakeLists.txt
===
--- /dev/null
+++ tools/clang-doc/tool/CMakeLists.txt
@@ -0,0 +1,18 @@
+include_directories(${CMAKE_CURRENT_SOURCE_DIR}/..)
+
+add_clang_executable(clang-doc
+  ClangDocMain.cpp
+  )
+
+target_link_libraries(clang-doc
+  PRIVATE
+  clangAST
+  clangASTMatchers
+  clangBasic
+  clangFormat
+  clangFrontend
+  clangDoc
+  clangRewrite
+  clangTooling
+  clangToolingCore
+  )
Index: tools/clang-doc/ClangDocYAML.h
===
--- /dev/null
+++ tools/clang-doc/ClangDocYAML.h
@@ -0,0 +1,217 @@
+//===--  ClangDocYAML.h - ClangDoc YAML -*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_DOC_CLANG_DOC_YAML_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_DOC_CLANG_DOC_YAML_H
+
+#include "llvm/Support/YAMLTraits.h"
+
+LLVM_YAML_IS_SEQUENCE_VECTOR(clang::doc::CommentInfo)
+LLVM_YAML_IS_SEQUENCE_VECTOR(clang::doc::NamedType)
+LLVM_YAML_IS_SEQUENCE_VECTOR(clang::doc::Location)
+LLVM_YAML_IS_SEQUENCE_VECTOR(clang::doc::Pair)
+LLVM_YAML_IS_SEQUENCE_VECTOR(clang::doc::Pair)
+LLVM_YAML_IS_SEQUENCE_VECTOR(clang::doc::Pair)
+LLVM_YAML_IS_SEQUENCE_VECTOR(clang::doc::Pair)
+LLVM_YAML_IS_SEQUENCE_VECTOR(clang::doc::Pair)
+
+namespace llvm {
+namespace yaml {
+
+template  struct NormalizedMap {
+  NormalizedMap(IO &) {}
+  NormalizedMap(IO &, const StringMap ) {
+for (const auto  : Map) {
+  clang::doc::Pair Pair{Entry.getKeyData(), Entry.getValue()};
+  VectorMap.push_back(Pair);
+}
+  }
+
+  StringMap denormalize(IO &) {
+StringMap Map;
+for (const auto  : VectorMap)
+  Map[Pair.Key] = Pair.Value;
+return Map;
+  }
+
+  std::vector VectorMap;
+};
+
+template 

[PATCH] D42460: [cmake] [libcxxabi] Call llvm_setup_rpath() when adding shared libraries.

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL323455: [cmake] [libcxxabi] Call llvm_setup_rpath() when 
adding shared libraries. (authored by dhinton, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D42460

Files:
  libcxxabi/trunk/src/CMakeLists.txt


Index: libcxxabi/trunk/src/CMakeLists.txt
===
--- libcxxabi/trunk/src/CMakeLists.txt
+++ libcxxabi/trunk/src/CMakeLists.txt
@@ -127,6 +127,7 @@
 # Build the shared library.
 if (LIBCXXABI_ENABLE_SHARED)
   add_library(cxxabi_shared SHARED $)
+  llvm_setup_rpath(cxxabi_shared)
   target_link_libraries(cxxabi_shared ${LIBCXXABI_LIBRARIES})
   set_target_properties(cxxabi_shared
 PROPERTIES


Index: libcxxabi/trunk/src/CMakeLists.txt
===
--- libcxxabi/trunk/src/CMakeLists.txt
+++ libcxxabi/trunk/src/CMakeLists.txt
@@ -127,6 +127,7 @@
 # Build the shared library.
 if (LIBCXXABI_ENABLE_SHARED)
   add_library(cxxabi_shared SHARED $)
+  llvm_setup_rpath(cxxabi_shared)
   target_link_libraries(cxxabi_shared ${LIBCXXABI_LIBRARIES})
   set_target_properties(cxxabi_shared
 PROPERTIES
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[libcxxabi] r323455 - [cmake] [libcxxabi] Call llvm_setup_rpath() when adding shared libraries.

[Don Hinton via cfe-commits]

Author: dhinton
Date: Thu Jan 25 10:43:18 2018
New Revision: 323455

URL: http://llvm.org/viewvc/llvm-project?rev=323455=rev
Log:
[cmake] [libcxxabi] Call llvm_setup_rpath() when adding shared libraries.

Clang and llvm already use llvm_setup_rpath(), so this change will
help standarize rpath usage across all projects.

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

Modified:
libcxxabi/trunk/src/CMakeLists.txt

Modified: libcxxabi/trunk/src/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/libcxxabi/trunk/src/CMakeLists.txt?rev=323455=323454=323455=diff
==
--- libcxxabi/trunk/src/CMakeLists.txt (original)
+++ libcxxabi/trunk/src/CMakeLists.txt Thu Jan 25 10:43:18 2018
@@ -127,6 +127,7 @@ set(LIBCXXABI_TARGETS)
 # Build the shared library.
 if (LIBCXXABI_ENABLE_SHARED)
   add_library(cxxabi_shared SHARED $)
+  llvm_setup_rpath(cxxabi_shared)
   target_link_libraries(cxxabi_shared ${LIBCXXABI_LIBRARIES})
   set_target_properties(cxxabi_shared
 PROPERTIES


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

[PATCH] D42459: [cmake] [libcxx] Call llvm_setup_rpath() when adding shared libraries.

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCXX323453: [cmake] [libcxx] Call llvm_setup_rpath() when 
adding shared libraries. (authored by dhinton, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D42459?vs=131195=131478#toc

Repository:
  rCXX libc++

https://reviews.llvm.org/D42459

Files:
  lib/CMakeLists.txt


Index: lib/CMakeLists.txt
===
--- lib/CMakeLists.txt
+++ lib/CMakeLists.txt
@@ -222,6 +222,7 @@
 # Build the shared library.
 if (LIBCXX_ENABLE_SHARED)
   add_library(cxx_shared SHARED $)
+  llvm_setup_rpath(cxx_shared)
   target_link_libraries(cxx_shared ${LIBCXX_LIBRARIES})
   set_target_properties(cxx_shared
 PROPERTIES


Index: lib/CMakeLists.txt
===
--- lib/CMakeLists.txt
+++ lib/CMakeLists.txt
@@ -222,6 +222,7 @@
 # Build the shared library.
 if (LIBCXX_ENABLE_SHARED)
   add_library(cxx_shared SHARED $)
+  llvm_setup_rpath(cxx_shared)
   target_link_libraries(cxx_shared ${LIBCXX_LIBRARIES})
   set_target_properties(cxx_shared
 PROPERTIES
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42248: Always allow "#pragma region".

[Matt Davis via Phabricator via cfe-commits]

mattd added a comment.

Thanks @majnemer!  Would you mind committing this on my behalf? I do not have 
commit access, thanks.


https://reviews.llvm.org/D42248



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

[libcxx] r323453 - [cmake] [libcxx] Call llvm_setup_rpath() when adding shared libraries.

[Don Hinton via cfe-commits]

Author: dhinton
Date: Thu Jan 25 10:13:26 2018
New Revision: 323453

URL: http://llvm.org/viewvc/llvm-project?rev=323453=rev
Log:
[cmake] [libcxx] Call llvm_setup_rpath() when adding shared libraries.

Clang and llvm already use llvm_setup_rpath(), so this change will
help standarize rpath usage across all projects.

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

Modified:
libcxx/trunk/lib/CMakeLists.txt

Modified: libcxx/trunk/lib/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/lib/CMakeLists.txt?rev=323453=323452=323453=diff
==
--- libcxx/trunk/lib/CMakeLists.txt (original)
+++ libcxx/trunk/lib/CMakeLists.txt Thu Jan 25 10:13:26 2018
@@ -222,6 +222,7 @@ set(LIBCXX_TARGETS)
 # Build the shared library.
 if (LIBCXX_ENABLE_SHARED)
   add_library(cxx_shared SHARED $)
+  llvm_setup_rpath(cxx_shared)
   target_link_libraries(cxx_shared ${LIBCXX_LIBRARIES})
   set_target_properties(cxx_shared
 PROPERTIES


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

[PATCH] D42508: AST: support protocol conformances on id/class/interfaces in MS ABI

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd updated this revision to Diff 131477.
compnerd added a comment.

Add additional test cases as discussed on IRC.  These ensure that the back 
reference handling is correct.


Repository:
  rC Clang

https://reviews.llvm.org/D42508

Files:
  lib/AST/MicrosoftMangle.cpp
  test/CodeGenObjCXX/msabi-protocol-conformance.mm


Index: test/CodeGenObjCXX/msabi-protocol-conformance.mm
===
--- /dev/null
+++ test/CodeGenObjCXX/msabi-protocol-conformance.mm
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -triple thumbv7-windows-msvc -fobjc-runtime=ios-6.0 -o - 
-emit-llvm %s | FileCheck %s
+
+@protocol P;
+@protocol Q;
+
+@class I;
+
+void f(id, id, id, id) {}
+// CHECK-LABEL: "\01?f@@YAXPAUobjc_object@@PAU?$objc_object@YP10@Z"
+
+void f(id, id) {}
+// CHECK-LABEL: "\01?f@@YAXPAU?$objc_object@YP0@Z"
+
+void f(id) {}
+// CHECK-LABEL: "\01?f@@YAXPAU?$objc_object@YP@Z"
+
+void f(id) {}
+// CHECK-LABEL: "\01?f@@YAXPAU?$objc_object@YP@@YQ@Z"
+
+void f(Class) {}
+// CHECK-LABEL: "\01?f@@YAXPAU?$objc_class@YP@Z"
+
+void f(Class) {}
+// CHECK-LABEL: "\01?f@@YAXPAU?$objc_class@YP@@YQ@Z"
+
+void f(I *) {}
+// CHECK-LABEL: "\01?f@@YAXPAU?$I@YP@Z"
+
+void f(I *) {}
+// CHECK-LABEL: "\01?f@@YAXPAU?$I@YP@@YQ@Z"
+
Index: lib/AST/MicrosoftMangle.cpp
===
--- lib/AST/MicrosoftMangle.cpp
+++ lib/AST/MicrosoftMangle.cpp
@@ -2451,9 +2451,36 @@

 void MicrosoftCXXNameMangler::mangleType(const ObjCObjectType *T, Qualifiers,
  SourceRange Range) {
-  // We don't allow overloading by different protocol qualification,
-  // so mangling them isn't necessary.
-  mangleType(T->getBaseType(), Range, QMM_Drop);
+  if (T->qual_empty())
+return mangleType(T->getBaseType(), Range, QMM_Drop);
+
+  ArgBackRefMap OuterArgsContext;
+  BackRefVec OuterTemplateContext;
+
+  TypeBackReferences.swap(OuterArgsContext);
+  NameBackReferences.swap(OuterTemplateContext);
+
+  mangleTagTypeKind(TTK_Struct);
+
+  Out << "?$";
+  if (T->isObjCId())
+mangleSourceName("objc_object");
+  else if (T->isObjCClass())
+mangleSourceName("objc_class");
+  else
+mangleSourceName(T->getInterface()->getName());
+
+  for (const auto  : T->quals()) {
+Out << 'Y'; // cointerface
+mangleSourceName(Q->getName());
+Out << '@';
+  }
+  Out << '@';
+
+  Out << '@';
+
+  TypeBackReferences.swap(OuterArgsContext);
+  NameBackReferences.swap(OuterTemplateContext);
 }

 void MicrosoftCXXNameMangler::mangleType(const BlockPointerType *T,


Index: test/CodeGenObjCXX/msabi-protocol-conformance.mm
===
--- /dev/null
+++ test/CodeGenObjCXX/msabi-protocol-conformance.mm
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -triple thumbv7-windows-msvc -fobjc-runtime=ios-6.0 -o - -emit-llvm %s | FileCheck %s
+
+@protocol P;
+@protocol Q;
+
+@class I;
+
+void f(id, id, id, id) {}
+// CHECK-LABEL: "\01?f@@YAXPAUobjc_object@@PAU?$objc_object@YP10@Z"
+
+void f(id, id) {}
+// CHECK-LABEL: "\01?f@@YAXPAU?$objc_object@YP0@Z"
+
+void f(id) {}
+// CHECK-LABEL: "\01?f@@YAXPAU?$objc_object@YP@Z"
+
+void f(id) {}
+// CHECK-LABEL: "\01?f@@YAXPAU?$objc_object@YP@@YQ@Z"
+
+void f(Class) {}
+// CHECK-LABEL: "\01?f@@YAXPAU?$objc_class@YP@Z"
+
+void f(Class) {}
+// CHECK-LABEL: "\01?f@@YAXPAU?$objc_class@YP@@YQ@Z"
+
+void f(I *) {}
+// CHECK-LABEL: "\01?f@@YAXPAU?$I@YP@Z"
+
+void f(I *) {}
+// CHECK-LABEL: "\01?f@@YAXPAU?$I@YP@@YQ@Z"
+
Index: lib/AST/MicrosoftMangle.cpp
===
--- lib/AST/MicrosoftMangle.cpp
+++ lib/AST/MicrosoftMangle.cpp
@@ -2451,9 +2451,36 @@

 void MicrosoftCXXNameMangler::mangleType(const ObjCObjectType *T, Qualifiers,
  SourceRange Range) {
-  // We don't allow overloading by different protocol qualification,
-  // so mangling them isn't necessary.
-  mangleType(T->getBaseType(), Range, QMM_Drop);
+  if (T->qual_empty())
+return mangleType(T->getBaseType(), Range, QMM_Drop);
+
+  ArgBackRefMap OuterArgsContext;
+  BackRefVec OuterTemplateContext;
+
+  TypeBackReferences.swap(OuterArgsContext);
+  NameBackReferences.swap(OuterTemplateContext);
+
+  mangleTagTypeKind(TTK_Struct);
+
+  Out << "?$";
+  if (T->isObjCId())
+mangleSourceName("objc_object");
+  else if (T->isObjCClass())
+mangleSourceName("objc_class");
+  else
+mangleSourceName(T->getInterface()->getName());
+
+  for (const auto  : T->quals()) {
+Out << 'Y'; // cointerface
+mangleSourceName(Q->getName());
+Out << '@';
+  }
+  Out << '@';
+
+  Out << '@';
+
+  TypeBackReferences.swap(OuterArgsContext);
+  NameBackReferences.swap(OuterTemplateContext);
 }

 void MicrosoftCXXNameMangler::mangleType(const BlockPointerType *T,
___

[PATCH] D42248: Always allow "#pragma region".

[David Majnemer via Phabricator via cfe-commits]

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

LGTM


https://reviews.llvm.org/D42248



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

Re: Please subscribe "rUNW libunwind" to cfe-commits.

[Don Hinton via cfe-commits]

Thanks Ben...

On Thu, Jan 25, 2018 at 9:53 AM, Ben Hamilton 
wrote:

> Ah, I see from http://bcain-llvm.readthedocs.io/projects/
> libunwind/en/latest/ that libunwind review discussion does take place on
> cfe-commits.
>
> Added. Sorry about not doing that earlier!
>
> Ben
>
> On Thu, Jan 25, 2018 at 8:37 AM Ben Hamilton 
> wrote:
>
>> Great question. Is there an existing separate list for libunwind?
>>
>> If libunwind discussion already happens on cfe-commits, then I'll make
>> reviews for libunwind subscribe that list.
>>
>> On Tue, Jan 23, 2018, 21:40 Don Hinton  wrote:
>>
>>> Currently, herald doesn't subscribe libunwind to any group.  Should it
>>> be added to cfe-commits?
>>>
>>>   https://reviews.llvm.org/H268
>>>
>>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42499: [clangd] Modify the Span API so that Spans propagate with contexts.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LGTM. And sorry for confusion with https://reviews.llvm.org/D42524, if any.




Comment at: clangd/Function.h:147
 
-  ScopeExitGuard(Func F) : F(std::move(F)) {}
+  ScopeExitGuard(Func F) : F(llvm::make_unique(std::move(F))) {}
   ~ScopeExitGuard() {

sammccall wrote:
> ilya-biryukov wrote:
> > Why do we need `unique_ptr` instead of `Optional` here?
> ScopeExitGuard was broken by the recent Optional changes (when Func is 
> trivially copyable).
> This is a minimal workaround, let's discuss on the other patch.
LG, thanks. Wasn't aware of the breakage when writing the comment.



Comment at: clangd/JSONRPCDispatcher.cpp:31
+  RequestArgs(json::obj *Args) : Args(Args) {}
+  std::mutex Mu;
+  json::obj *Args;

We could make the fields private and expose only the function to modify args 
under lock (similar to `FillRequestInfo`, i.e. the last two line of it).
That would make the class safer to use. But that's work and the class itself is 
an implementation detail, so we could also leave it as is.



Comment at: clangd/Trace.cpp:138
+  Args)
+: Ctx.derive(kSpanArgs, nullptr)) {}
 

Would be nice for `Span`s to have zero cost when no tracer is active. We could 
probably achieve that be not storing the `kSpanArgs` when `T` is null. WDYT?
That's really not a big deal, so I don't think we should block the patch on 
this.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42499



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

Re: Please subscribe "rUNW libunwind" to cfe-commits.

[Ben Hamilton via cfe-commits]

Ah, I see from
http://bcain-llvm.readthedocs.io/projects/libunwind/en/latest/ that
libunwind review discussion does take place on cfe-commits.

Added. Sorry about not doing that earlier!

Ben

On Thu, Jan 25, 2018 at 8:37 AM Ben Hamilton  wrote:

> Great question. Is there an existing separate list for libunwind?
>
> If libunwind discussion already happens on cfe-commits, then I'll make
> reviews for libunwind subscribe that list.
>
> On Tue, Jan 23, 2018, 21:40 Don Hinton  wrote:
>
>> Currently, herald doesn't subscribe libunwind to any group.  Should it be
>> added to cfe-commits?
>>
>>   https://reviews.llvm.org/H268
>>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42248: Always allow "#pragma region".

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

Quuxplusone added a comment.

FWIW, I would also like this patch, because it would mean that I could build 
with -Werror even when the project includes headers written by MSVC-using 
people. Given that we know what "#pragma region" does, it hardly deserves an 
"unknown pragma" diagnostic! So this patch is great IMHO. :)


https://reviews.llvm.org/D42248



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

[PATCH] D42395: [clang-format] Fix bug where -dump-config failed on ObjC header

[Ben Hamilton via Phabricator via cfe-commits]

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



Comment at: test/Format/lit.local.cfg:2-3
+# Suffixes supported by clang-format.
+config.suffixes = ['.cpp', '.h', '.m', '.mm', '.java', '.js', '.ts', '.proto',
+   '.protodevel', '.pb.txt', '.textproto', '.asciipb', '.td']

jolesiak wrote:
> Where does this list come from?
> Shouldn't '.textpb', 'METADATA', '.c', '.cc' be added here?
It's a good question, thanks.

I copied this list from LibFormat's getLanguageByFileName():

https://github.com/llvm-mirror/clang/blob/master/lib/Format/Format.cpp#L2265

I did miss .textpb, added now. I also added .c and .cc since they should 
default to C++ style.

METADATA isn't a suffix understood by clang-format, so I did not add it.



Repository:
  rC Clang

https://reviews.llvm.org/D42395



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

[PATCH] D41454: [clangd] Add ClangdUnit diagnostics tests using annotated code.

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE323448: [clangd] Add ClangdUnit diagnostics tests using 
annotated code. (authored by sammccall, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D41454?vs=127843=131471#toc

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D41454

Files:
  clangd/Protocol.cpp
  clangd/Protocol.h
  test/clangd/diagnostics-preamble.test
  unittests/clangd/CMakeLists.txt
  unittests/clangd/ClangdUnitTests.cpp

Index: unittests/clangd/ClangdUnitTests.cpp
===
--- unittests/clangd/ClangdUnitTests.cpp
+++ unittests/clangd/ClangdUnitTests.cpp
@@ -0,0 +1,130 @@
+//===-- ClangdUnitTests.cpp - ClangdUnit tests --*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "ClangdUnit.h"
+#include "Annotations.h"
+#include "TestFS.h"
+#include "clang/Frontend/CompilerInvocation.h"
+#include "clang/Frontend/PCHContainerOperations.h"
+#include "clang/Frontend/Utils.h"
+#include "llvm/Support/ScopedPrinter.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+using namespace llvm;
+void PrintTo(const DiagWithFixIts , std::ostream *O) {
+  llvm::raw_os_ostream OS(*O);
+  OS << D.Diag;
+  if (!D.FixIts.empty()) {
+OS << " {";
+const char *Sep = "";
+for (const auto  : D.FixIts) {
+  OS << Sep << F;
+  Sep = ", ";
+}
+OS << "}";
+  }
+}
+
+namespace {
+using testing::ElementsAre;
+
+// FIXME: this is duplicated with FileIndexTests. Share it.
+ParsedAST build(StringRef Code, std::vector Flags = {}) {
+  std::vector Cmd = {"clang", "main.cpp"};
+  Cmd.insert(Cmd.begin() + 1, Flags.begin(), Flags.end());
+  auto CI = createInvocationFromCommandLine(Cmd);
+  auto Buf = MemoryBuffer::getMemBuffer(Code);
+  auto AST = ParsedAST::Build(
+  Context::empty(), std::move(CI), nullptr, std::move(Buf),
+  std::make_shared(), vfs::getRealFileSystem());
+  assert(AST.hasValue());
+  return std::move(*AST);
+}
+
+MATCHER_P2(Diag, Range, Message,
+   "Diagnostic at " + llvm::to_string(Range) + " = [" + Message + "]") {
+  return arg.Diag.range == Range && arg.Diag.message == Message &&
+ arg.FixIts.empty();
+}
+
+MATCHER_P3(Fix, Range, Replacement, Message,
+   "Fix " + llvm::to_string(Range) + " => " +
+   testing::PrintToString(Replacement) + " = [" + Message + "]") {
+  return arg.Diag.range == Range && arg.Diag.message == Message &&
+ arg.FixIts.size() == 1 && arg.FixIts[0].range == Range &&
+ arg.FixIts[0].newText == Replacement;
+}
+
+TEST(DiagnosticsTest, DiagnosticRanges) {
+  // Check we report correct ranges, including various edge-cases.
+  Annotations Test(R"cpp(
+void $decl[[foo]]();
+int main() {
+  $typo[[go\
+o]]();
+  foo()$semicolon[[]]
+  $unk[[unknown]]();
+}
+  )cpp");
+  llvm::errs() << Test.code();
+  EXPECT_THAT(
+  build(Test.code()).getDiagnostics(),
+  ElementsAre(
+  // This range spans lines.
+  Fix(Test.range("typo"), "foo",
+  "use of undeclared identifier 'goo'; did you mean 'foo'?"),
+  // This is a pretty normal range.
+  Diag(Test.range("decl"), "'foo' declared here"),
+  // This range is zero-width, and at the end of a line.
+  Fix(Test.range("semicolon"), ";",
+  "expected ';' after expression"),
+  // This range isn't provided by clang, we expand to the token.
+  Diag(Test.range("unk"),
+   "use of undeclared identifier 'unknown'")));
+}
+
+TEST(DiagnosticsTest, FlagsMatter) {
+  Annotations Test("[[void]] main() {}");
+  EXPECT_THAT(
+  build(Test.code()).getDiagnostics(),
+  ElementsAre(Fix(Test.range(), "int", "'main' must return 'int'")));
+  // Same code built as C gets different diagnostics.
+  EXPECT_THAT(
+  build(Test.code(), {"-x", "c"}).getDiagnostics(),
+  ElementsAre(
+  // FIXME: ideally this would be one diagnostic with a named FixIt.
+  Diag(Test.range(), "return type of 'main' is not 'int'"),
+  Fix(Test.range(), "int", "change return type to 'int'")));
+}
+
+TEST(DiagnosticsTest, Preprocessor) {
+  // This looks like a preamble, but there's an #else in the middle!
+  // Check that:
+  //  - the #else doesn't generate diagnostics (we had this bug)
+  //  - we get diagnostics from the taken branch
+  //  - we get no diagnostics from the not taken branch
+  Annotations Test(R"cpp(
+#ifndef FOO
+#define FOO
+  int a = [[b]];
+#else
+  int x = y;
+#endif
+)cpp");
+  EXPECT_THAT(
+  

[clang-tools-extra] r323448 - [clangd] Add ClangdUnit diagnostics tests using annotated code.

[Sam McCall via cfe-commits]

Author: sammccall
Date: Thu Jan 25 09:29:17 2018
New Revision: 323448

URL: http://llvm.org/viewvc/llvm-project?rev=323448=rev
Log:
[clangd] Add ClangdUnit diagnostics tests using annotated code.

Summary:
This adds checks that our diagnostics emit correct ranges in a bunch of cases,
as promised in D41118.

The diagnostics-preamble test is also converted and extended to be a little more
precise.

diagnostics.test stays around as the smoke test for this feature.

Reviewers: ilya-biryukov

Subscribers: klimek, mgorny, cfe-commits

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

Added:
clang-tools-extra/trunk/unittests/clangd/ClangdUnitTests.cpp
Removed:
clang-tools-extra/trunk/test/clangd/diagnostics-preamble.test
Modified:
clang-tools-extra/trunk/clangd/Protocol.cpp
clang-tools-extra/trunk/clangd/Protocol.h
clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt

Modified: clang-tools-extra/trunk/clangd/Protocol.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Protocol.cpp?rev=323448=323447=323448=diff
==
--- clang-tools-extra/trunk/clangd/Protocol.cpp (original)
+++ clang-tools-extra/trunk/clangd/Protocol.cpp Thu Jan 25 09:29:17 2018
@@ -137,6 +137,12 @@ json::Expr toJSON(const TextEdit ) {
   };
 }
 
+llvm::raw_ostream <<(llvm::raw_ostream , const TextEdit ) {
+  OS << TE.range << " => \"";
+  PrintEscapedString(TE.newText, OS);
+  return OS << '"';
+}
+
 bool fromJSON(const json::Expr , TraceLevel ) {
   if (auto S = E.asString()) {
 if (*S == "off") {
@@ -255,6 +261,28 @@ bool fromJSON(const json::Expr ,
   return O && O.map("diagnostics", R.diagnostics);
 }
 
+llvm::raw_ostream <<(llvm::raw_ostream , const Diagnostic ) {
+  OS << D.range << " [";
+  switch (D.severity) {
+case 1:
+  OS << "error";
+  break;
+case 2:
+  OS << "warning";
+  break;
+case 3:
+  OS << "note";
+  break;
+case 4:
+  OS << "remark";
+  break;
+default:
+  OS << "diagnostic";
+  break;
+  }
+  return OS << '(' << D.severity << "): " << D.message << "]";
+}
+
 bool fromJSON(const json::Expr , CodeActionParams ) {
   json::ObjectMapper O(Params);
   return O && O.map("textDocument", R.textDocument) &&

Modified: clang-tools-extra/trunk/clangd/Protocol.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Protocol.h?rev=323448=323447=323448=diff
==
--- clang-tools-extra/trunk/clangd/Protocol.h (original)
+++ clang-tools-extra/trunk/clangd/Protocol.h Thu Jan 25 09:29:17 2018
@@ -150,6 +150,7 @@ struct TextEdit {
 };
 bool fromJSON(const json::Expr &, TextEdit &);
 json::Expr toJSON(const TextEdit &);
+llvm::raw_ostream <<(llvm::raw_ostream &, const TextEdit &);
 
 struct TextDocumentItem {
   /// The text document's URI.
@@ -341,6 +342,7 @@ struct LSPDiagnosticCompare {
   }
 };
 bool fromJSON(const json::Expr &, Diagnostic &);
+llvm::raw_ostream <<(llvm::raw_ostream &, const Diagnostic &);
 
 struct CodeActionContext {
   /// An array of diagnostics.

Removed: clang-tools-extra/trunk/test/clangd/diagnostics-preamble.test
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/diagnostics-preamble.test?rev=323447=auto
==
--- clang-tools-extra/trunk/test/clangd/diagnostics-preamble.test (original)
+++ clang-tools-extra/trunk/test/clangd/diagnostics-preamble.test (removed)
@@ -1,22 +0,0 @@
-# RUN: clangd -pretty -run-synchronously < %s | FileCheck -strict-whitespace %s
-# RUN: clangd -pretty -run-synchronously -pch-storage=memory < %s | FileCheck 
-strict-whitespace %s
-# It is absolutely vital that this file has CRLF line endings.
-#
-Content-Length: 125
-
-{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
-
-Content-Length: 206
-
-{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"file:///main.cpp","languageId":"cpp","version":1,"text":"#ifndef
 FOO\n#define FOO\nint a;\n#else\nint a = b;#endif\n\n\n"}}}
-#  CHECK:  "method": "textDocument/publishDiagnostics",
-# CHECK-NEXT:  "params": {
-# CHECK-NEXT:"diagnostics": [],
-# CHECK-NEXT:"uri": "file:///{{([A-Z]:/)?}}main.cpp"
-# CHECK-NEXT:  }
-Content-Length: 58
-
-{"jsonrpc":"2.0","id":2,"method":"shutdown","params":null}
-Content-Length: 33
-
-{"jsonrpc":"2.0":"method":"exit"}

Modified: clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt?rev=323448=323447=323448=diff
==
--- clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt Thu 

[PATCH] D42395: [clang-format] Fix bug where -dump-config failed on ObjC header

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton updated this revision to Diff 131470.
benhamilton added a comment.

- Add more extensions.


Repository:
  rC Clang

https://reviews.llvm.org/D42395

Files:
  test/Format/dump-config-cxx.h
  test/Format/dump-config-objc.h
  test/Format/lit.local.cfg
  tools/clang-format/ClangFormat.cpp


Index: tools/clang-format/ClangFormat.cpp
===
--- tools/clang-format/ClangFormat.cpp
+++ tools/clang-format/ClangFormat.cpp
@@ -357,10 +357,27 @@
   }
 
   if (DumpConfig) {
+StringRef FileName;
+std::unique_ptr Code;
+if (FileNames.empty()) {
+  // We can't read the code to detect the language if there's no
+  // file name, so leave Code empty here.
+  FileName = AssumeFileName;
+} else {
+  // Read in the code in case the filename alone isn't enough to
+  // detect the language.
+  ErrorOr CodeOrErr =
+  MemoryBuffer::getFileOrSTDIN(FileNames[0]);
+  if (std::error_code EC = CodeOrErr.getError()) {
+llvm::errs() << EC.message() << "\n";
+return 1;
+  }
+  FileName = (FileNames[0] == "-") ? AssumeFileName : FileNames[0];
+  Code = std::move(CodeOrErr.get());
+}
 llvm::Expected FormatStyle =
-clang::format::getStyle(
-Style, FileNames.empty() ? AssumeFileName : FileNames[0],
-FallbackStyle);
+clang::format::getStyle(Style, FileName, FallbackStyle,
+Code ? Code->getBuffer() : "");
 if (!FormatStyle) {
   llvm::errs() << llvm::toString(FormatStyle.takeError()) << "\n";
   return 1;
Index: test/Format/lit.local.cfg
===
--- /dev/null
+++ test/Format/lit.local.cfg
@@ -0,0 +1,4 @@
+# Suffixes supported by clang-format.
+config.suffixes = ['.c', '.cc', '.cpp', '.h', '.m', '.mm', '.java', '.js',
+   '.ts', '.proto', '.protodevel', '.pb.txt', '.textproto',
+   '.textpb', '.asciipb', '.td']
Index: test/Format/dump-config-objc.h
===
--- /dev/null
+++ test/Format/dump-config-objc.h
@@ -0,0 +1,5 @@
+// RUN: clang-format -dump-config %s | FileCheck %s
+
+// CHECK: Language: ObjC
+@interface Foo
+@end
Index: test/Format/dump-config-cxx.h
===
--- /dev/null
+++ test/Format/dump-config-cxx.h
@@ -0,0 +1,3 @@
+// RUN: clang-format -dump-config %s | FileCheck %s
+
+// CHECK: Language: Cpp


Index: tools/clang-format/ClangFormat.cpp
===
--- tools/clang-format/ClangFormat.cpp
+++ tools/clang-format/ClangFormat.cpp
@@ -357,10 +357,27 @@
   }
 
   if (DumpConfig) {
+StringRef FileName;
+std::unique_ptr Code;
+if (FileNames.empty()) {
+  // We can't read the code to detect the language if there's no
+  // file name, so leave Code empty here.
+  FileName = AssumeFileName;
+} else {
+  // Read in the code in case the filename alone isn't enough to
+  // detect the language.
+  ErrorOr CodeOrErr =
+  MemoryBuffer::getFileOrSTDIN(FileNames[0]);
+  if (std::error_code EC = CodeOrErr.getError()) {
+llvm::errs() << EC.message() << "\n";
+return 1;
+  }
+  FileName = (FileNames[0] == "-") ? AssumeFileName : FileNames[0];
+  Code = std::move(CodeOrErr.get());
+}
 llvm::Expected FormatStyle =
-clang::format::getStyle(
-Style, FileNames.empty() ? AssumeFileName : FileNames[0],
-FallbackStyle);
+clang::format::getStyle(Style, FileName, FallbackStyle,
+Code ? Code->getBuffer() : "");
 if (!FormatStyle) {
   llvm::errs() << llvm::toString(FormatStyle.takeError()) << "\n";
   return 1;
Index: test/Format/lit.local.cfg
===
--- /dev/null
+++ test/Format/lit.local.cfg
@@ -0,0 +1,4 @@
+# Suffixes supported by clang-format.
+config.suffixes = ['.c', '.cc', '.cpp', '.h', '.m', '.mm', '.java', '.js',
+   '.ts', '.proto', '.protodevel', '.pb.txt', '.textproto',
+   '.textpb', '.asciipb', '.td']
Index: test/Format/dump-config-objc.h
===
--- /dev/null
+++ test/Format/dump-config-objc.h
@@ -0,0 +1,5 @@
+// RUN: clang-format -dump-config %s | FileCheck %s
+
+// CHECK: Language: ObjC
+@interface Foo
+@end
Index: test/Format/dump-config-cxx.h
===
--- /dev/null
+++ test/Format/dump-config-cxx.h
@@ -0,0 +1,3 @@
+// RUN: clang-format -dump-config %s | FileCheck %s
+
+// CHECK: Language: Cpp
___
cfe-commits mailing list
cfe-commits@lists.llvm.org

[PATCH] D42419: [clangd] Use new URI with scheme support in place of the existing LSP URI

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clangd/ClangdLSPServer.cpp:283
   llvm::Optional Result = Server.switchSourceHeader(Params.uri.file);
-  std::string ResultUri;
-  reply(C, Result ? URI::fromFile(*Result).uri : "");
+  std::string ResultUri = "";
+  if (Result) {

hmm, I think you want to replyerror for unexpected cases.
maybe: 

  if (ResultUri) {
if (auto U = URI::create(*Result))
   reply(C, U->toString());
else
  replyError(C, ErrorCode::InternalError, llvm::toString(U.takeError()));
  } else
reply(C, "");



Comment at: clangd/ClangdLSPServer.cpp:284
+  std::string ResultUri = "";
+  if (Result) {
+auto U = URI::create(*Result);

But basically I think this shows that the API is awkward. We should have a way 
to create a file URI from an absolute path that asserts rather than returning 
expected.
I'd suggest removing the default "file" scheme from create(), and adding 
createFile(abspath) that returns URI



Comment at: clangd/Protocol.h:51
 
-struct URI {
-  std::string uri;
+struct URIWithFile {
+  URI uri;

URIForFile? "withfile" doesn't really capture that they're related



Comment at: clangd/Protocol.h:51
 
-struct URI {
-  std::string uri;
+struct URIWithFile {
+  URI uri;

sammccall wrote:
> URIForFile? "withfile" doesn't really capture that they're related
Hmm actually, what about just `struct URIForFile { std:string AbsPath; }`
fromJSON and toJSON would do the marshalling to URI, but internally we just 
want the path, right?

This also gives us the usual easy null state.



Comment at: clangd/URI.h:32
+  // By default, create a simplest valid file URI.
+  URI() : Scheme("file") {}
+

ioeric wrote:
> same here. There are many default constructions of structures that contain 
> URIs in ClangdLSPServer.cpp...
Does the struct-with-just-an-abspath idea address this?



Comment at: clangd/URI.h:45
+  /// Create a URI from unescaped scheme+authority+body.
+  static llvm::Expected create(llvm::StringRef Scheme,
+llvm::StringRef Authority,

oh, sorry I missed this in the first code review...
It's really confusing to have `create(str, str, str)` do simple initialization 
that can't really fail, and `create(str,str)` do complicated plugin logic that 
can fail in lots of ways.

Can you change the first to be a constructor and just assert on the needed 
invariants?
(Or failing that `createFromComponents`, but I can't imagine a use case where 
you have components but don't know they're correct)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42419



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

[PATCH] D42301: [ASTImporter] Support LambdaExprs and improve template support

[Aleksei Sidorin via Phabricator via cfe-commits]

a.sidorin added a comment.

Thank you, Gabor. I'll change cast<> to cast_or_null<> to enable the assertion 
below.


Repository:
  rC Clang

https://reviews.llvm.org/D42301



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

[PATCH] D42525: [clangd] Replace Optional in ScopeExitGuard (fix after r322838)

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

r323443 removed ScopeExitGuard altogether.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42525



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

[clang-tools-extra] r323443 - [clangd] Replace homegrown make_scope_exit with one from ADT

[Sam McCall via cfe-commits]

Author: sammccall
Date: Thu Jan 25 09:01:39 2018
New Revision: 323443

URL: http://llvm.org/viewvc/llvm-project?rev=323443=rev
Log:
[clangd] Replace homegrown make_scope_exit with one from ADT

Modified:
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/Function.h

Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=323443=323442=323443=diff
==
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Thu Jan 25 09:01:39 2018
@@ -19,6 +19,7 @@
 #include "clang/Tooling/Refactoring/RefactoringResultConsumer.h"
 #include "clang/Tooling/Refactoring/Rename/RenamingAction.h"
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/FormatProviders.h"
@@ -576,7 +577,8 @@ std::future ClangdServer::sched
 const Context &)>
 DeferredRebuild,
 std::promise DonePromise, Context Ctx) -> void {
-auto Guard = onScopeExit([&]() { DonePromise.set_value(std::move(Ctx)); });
+auto Guard =
+llvm::make_scope_exit([&]() { DonePromise.set_value(std::move(Ctx)); 
});
 
 auto CurrentVersion = DraftMgr.getVersion(FileStr);
 if (CurrentVersion != Version)

Modified: clang-tools-extra/trunk/clangd/Function.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Function.h?rev=323443=323442=323443=diff
==
--- clang-tools-extra/trunk/clangd/Function.h (original)
+++ clang-tools-extra/trunk/clangd/Function.h Thu Jan 25 09:01:39 2018
@@ -137,40 +137,6 @@ ForwardBinder BindWithFor
   std::make_tuple(std::forward(F), std::forward(As)...));
 }
 
-namespace detail {
-/// Runs provided callback in destructor. Use onScopeExit helper function to
-/// create this object.
-template  struct ScopeExitGuard {
-  static_assert(std::is_same::type, Func>::value,
-"Func must be decayed");
-
-  ScopeExitGuard(Func F) : F(std::move(F)) {}
-  ~ScopeExitGuard() {
-if (!F)
-  return;
-(*F)();
-  }
-
-  // Move-only.
-  ScopeExitGuard(const ScopeExitGuard &) = delete;
-  ScopeExitGuard =(const ScopeExitGuard &) = delete;
-
-  ScopeExitGuard(ScopeExitGuard &) = default;
-  ScopeExitGuard =(ScopeExitGuard &) = default;
-
-private:
-  llvm::Optional F;
-};
-} // namespace detail
-
-/// Creates a RAII object that will run \p F in its destructor.
-template 
-auto onScopeExit(Func &)
--> detail::ScopeExitGuard::type> {
-  return detail::ScopeExitGuard::type>(
-  std::forward(F));
-}
-
 } // namespace clangd
 } // namespace clang
 


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

[PATCH] D42335: [ASTImporter] Supporting CXXOperatorCallExpr, SizeOfPackExpr, DependentTemplateSpecializationType, DependentSizedArray, CXXTypeidExpr importing.

[Aleksei Sidorin via Phabricator via cfe-commits]

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

LGTM, thank you!


https://reviews.llvm.org/D42335



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

[PATCH] D42248: Always allow "#pragma region".

[Matt Davis via Phabricator via cfe-commits]

mattd added a comment.

Ping.


https://reviews.llvm.org/D42248



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

[PATCH] D42545: [Sema] Classify conversions from enum to float as narrowing

[Mikhail Maltsev via Phabricator via cfe-commits]

miyuki created this revision.
miyuki added reviewers: faisalv, rsmith.

According to [dcl.init.list]p7:

  A narrowing conversion is an implicit conversion
  - ...
  - from an integer type or unscoped enumeration type to a
floating-point type, except where the source is a constant
expression and the actual value after conversion will fit into
the target type and will produce the original value when
converted back to the original type, or
  - ...

Currently clang does not handle the 'unscoped enumeration' case. This
patch fixes the corresponding check.


https://reviews.llvm.org/D42545

Files:
  lib/Sema/SemaOverload.cpp
  test/CXX/dcl.decl/dcl.init/dcl.init.list/p7-0x.cpp


Index: test/CXX/dcl.decl/dcl.init/dcl.init.list/p7-0x.cpp
===
--- test/CXX/dcl.decl/dcl.init/dcl.init.list/p7-0x.cpp
+++ test/CXX/dcl.decl/dcl.init/dcl.init.list/p7-0x.cpp
@@ -24,6 +24,10 @@
 { 2, f(2), f(2.0) };  // OK: the double-to-int conversion is not at the 
top level
 }
 
+enum UnscopedEnum {
+  EnumVal = 300
+};
+
 // Test each rule individually.
 
 template
@@ -115,15 +119,21 @@
 void int_to_float() {
   // Not a constant expression.
   char c = 1;
+  UnscopedEnum e = EnumVal;
 
   // Variables.  Yes, even though all char's will fit into any floating type.
   Agg f1 = {c};  // expected-error {{ cannot be narrowed }} 
expected-note {{silence}}
   Agg f2 = {c};  // expected-error {{ cannot be narrowed }} 
expected-note {{silence}}
   Agg f3 = {c};  // expected-error {{ cannot be narrowed }} 
expected-note {{silence}}
 
+  Agg f4 = {e};  // expected-error {{ cannot be narrowed }} 
expected-note {{silence}}
+  Agg f5 = {e};  // expected-error {{ cannot be narrowed }} 
expected-note {{silence}}
+  Agg f6 = {e};  // expected-error {{ cannot be narrowed }} 
expected-note {{silence}}
+
   // Constants.
-  Agg f4 = {12345678};  // OK (exactly fits in a float)
-  Agg f5 = {123456789};  // expected-error {{ cannot be narrowed }} 
expected-note {{silence}}
+  Agg f7 = {12345678};  // OK (exactly fits in a float)
+  Agg f8 = {EnumVal};  // OK
+  Agg f9 = {123456789};  // expected-error {{ cannot be narrowed }} 
expected-note {{silence}}
 
   Agg ce1 = { Convert(123456789) }; // expected-error {{constant 
expression evaluates to 123456789 which cannot be narrowed to type 'float'}} 
expected-note {{silence}}
   Agg ce2 = { ConvertVar() }; // expected-error 
{{non-constant-expression cannot be narrowed from type 'long long' to 
'double'}} expected-note {{silence}}
@@ -138,7 +148,9 @@
   // Not a constant expression.
   short s = 1;
   unsigned short us = 1;
+  UnscopedEnum e = EnumVal;
   Agg c1 = {s};  // expected-error {{ cannot be narrowed }} 
expected-note {{silence}}
+  Agg c2 = {e};  // expected-error {{ cannot be narrowed }} 
expected-note {{silence}}
   Agg s1 = {s};  // expected-error {{ cannot be narrowed }} 
expected-note {{silence}}
   Agg s2 = {us};  // expected-error {{ cannot be narrowed }} 
expected-note {{silence}}
 
@@ -148,16 +160,19 @@
   // long).
   long l1 = 1;
   Agg i1 = {l1};  // expected-error {{ cannot be narrowed }} 
expected-note {{silence}}
+  Agg i2 = {e};  // OK
   long long ll = 1;
   Agg l2 = {ll};  // OK
 
   // Constants.
-  Agg c2 = {127};  // OK
-  Agg c3 = {300};  // expected-error {{ cannot be narrowed }} 
expected-note {{silence}} expected-warning {{changes value}}
-
-  Agg i2 = {0x7FFFU};  // OK
-  Agg i3 = {0x8000U};  // expected-error {{ cannot be narrowed }} 
expected-note {{silence}}
-  Agg i4 = {-0x8000L};  // expected-error {{ cannot be 
narrowed }} expected-note {{silence}}
+  Agg c3 = {127};  // OK
+  Agg c4 = {300};  // expected-error {{ cannot be narrowed }} 
expected-note {{silence}} expected-warning {{changes value}}
+  Agg c5 = {EnumVal};  // expected-error {{ cannot be narrowed }} 
expected-note {{silence}} expected-warning {{changes value}}
+
+  Agg i3 = {0x7FFFU};  // OK
+  Agg i4 = {EnumVal};  // OK
+  Agg i5 = {0x8000U};  // expected-error {{ cannot be narrowed }} 
expected-note {{silence}}
+  Agg i6 = {-0x8000L};  // expected-error {{ cannot be 
narrowed }} expected-note {{silence}}
 
   // Bool is also an integer type, but conversions to it are a different AST
   // node.
Index: lib/Sema/SemaOverload.cpp
===
--- lib/Sema/SemaOverload.cpp
+++ lib/Sema/SemaOverload.cpp
@@ -327,7 +327,8 @@
   FloatingIntegralConversion:
 if (FromType->isRealFloatingType() && ToType->isIntegralType(Ctx)) {
   return NK_Type_Narrowing;
-} else if (FromType->isIntegralType(Ctx) && ToType->isRealFloatingType()) {
+} else if (FromType->isIntegralOrUnscopedEnumerationType() &&
+   ToType->isRealFloatingType()) {
   llvm::APSInt IntConstantValue;
   const Expr *Initializer = IgnoreNarrowingConversion(Converted);
   assert(Initializer && "Unknown conversion expression");


Index: 

Re: Please subscribe "rUNW libunwind" to cfe-commits.

[Ben Hamilton via cfe-commits]

Great question. Is there an existing separate list for libunwind?

If libunwind discussion already happens on cfe-commits, then I'll make
reviews for libunwind subscribe that list.

On Tue, Jan 23, 2018, 21:40 Don Hinton  wrote:

> Currently, herald doesn't subscribe libunwind to any group.  Should it be
> added to cfe-commits?
>
>   https://reviews.llvm.org/H268
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42499: [clangd] Modify the Span API so that Spans propagate with contexts.

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 131460.
sammccall marked 4 inline comments as done.
sammccall added a comment.

  Address review comments


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42499

Files:
  clangd/ClangdUnit.cpp
  clangd/Context.h
  clangd/Function.h
  clangd/JSONRPCDispatcher.cpp
  clangd/Trace.cpp
  clangd/Trace.h
  unittests/clangd/TraceTests.cpp

Index: unittests/clangd/TraceTests.cpp
===
--- unittests/clangd/TraceTests.cpp
+++ unittests/clangd/TraceTests.cpp
@@ -78,8 +78,8 @@
 auto JSONTracer = trace::createJSONTracer(OS);
 trace::Session Session(*JSONTracer);
 {
-  trace::Span S(Context::empty(), "A");
-  trace::log(Context::empty(), "B");
+  trace::Span Tracer(Context::empty(), "A");
+  trace::log(Tracer.Ctx, "B");
 }
   }
 
Index: clangd/Trace.h
===
--- clangd/Trace.h
+++ clangd/Trace.h
@@ -32,17 +32,15 @@
 /// Implmentations of this interface must be thread-safe.
 class EventTracer {
 public:
-  /// A callback executed when an event with duration ends. Args represent data
-  /// that was attached to the event via SPAN_ATTACH.
-  using EndEventCallback = UniqueFunction;
-
   virtual ~EventTracer() = default;
 
-  /// Called when event that has a duration starts. The returned callback will
-  /// be executed when the event ends. \p Name is a descriptive name
-  /// of the event that was passed to Span constructor.
-  virtual EndEventCallback beginSpan(const Context ,
- llvm::StringRef Name) = 0;
+  /// Called when event that has a duration starts. \p Name describes the event.
+  /// Returns a derived context that will be destroyed when the event ends.
+  /// Usually implementations will store an object in the returned context
+  /// whose destructor records the end of the event.
+  /// The args are *Args, only complete when the event ends.
+  virtual Context beginSpan(const Context , llvm::StringRef Name,
+json::obj *Args) = 0;
 
   /// Called for instant events.
   virtual void instant(const Context , llvm::StringRef Name,
@@ -70,30 +68,35 @@
 void log(const Context , const llvm::Twine );
 
 /// Records an event whose duration is the lifetime of the Span object.
+/// This lifetime is extended when the span's context is reused.
+///
 /// This is the main public interface for producing tracing events.
 ///
 /// Arbitrary JSON metadata can be attached while this span is active:
 ///   SPAN_ATTACH(MySpan, "Payload", SomeJSONExpr);
+///
 /// SomeJSONExpr is evaluated and copied only if actually needed.
 class Span {
-public:
+ public:
   Span(const Context , llvm::StringRef Name);
-  ~Span();
 
-  /// Returns mutable span metadata if this span is interested.
+  /// Mutable metadata, if this span is interested.
   /// Prefer to use SPAN_ATTACH rather than accessing this directly.
-  json::obj *args() { return Args.get(); }
-
-private:
-  std::unique_ptr Args;
-  EventTracer::EndEventCallback Callback;
+  json::obj * const Args;
+  /// Propagating this context will keep the span alive.
+  const Context Ctx;
 };
 
+/// Returns mutable span metadata if this span is interested.
+/// Prefer to use SPAN_ATTACH rather than accessing this directly.
+json::obj *spanArgs(const Context );
+
+/// Attach a key-value pair to a Span event.
+/// This is not threadsafe when used with the same Span.
 #define SPAN_ATTACH(S, Name, Expr) \
   do { \
-if ((S).args() != nullptr) {   \
-  (*((S).args()))[(Name)] = (Expr);\
-}  \
+if (auto *Args = (S).Args) \
+  (*Args)[Name] = Expr;\
   } while (0)
 
 } // namespace trace
Index: clangd/Trace.cpp
===
--- clangd/Trace.cpp
+++ clangd/Trace.cpp
@@ -7,6 +7,8 @@
 //
 //===--===//
 
+#include "Context.h"
+#include "Function.h"
 #include "Trace.h"
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/Support/Chrono.h"
@@ -43,14 +45,12 @@
 Out.flush();
   }
 
-  EndEventCallback beginSpan(const Context ,
- llvm::StringRef Name) override {
+  Context beginSpan(const Context , llvm::StringRef Name,
+json::obj *Args) override {
 jsonEvent("B", json::obj{{"name", Name}});
-
-// The callback that will run when event ends.
-return [this](json::Expr &) {
-  jsonEvent("E", json::obj{{"args", std::move(Args)}});
-};
+return 

[PATCH] D42174: [clangd] Refactored threading in ClangdServer

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Agreed we can defer lots of stuff in order to keep this patch compact.
Generally the things I think we can get right before landing:

- names and file organization for new things
- documentation including where we want to get to




Comment at: clangd/ClangdServer.cpp:246
+
+  ParseInputs Inputs = getInputs(File);
+  std::shared_ptr Preamble =

I think you want to take a reference here, and then capture by value



Comment at: clangd/ClangdServer.cpp:323
+  [](Context Ctx, std::promise DonePromise, llvm::Error Err) {
+// FIXME(ibiryukov): allow to report error to the caller.
+// Discard error, if any.

this fixme doesn't make sense if we're removing the callback



Comment at: clangd/ClangdServer.cpp:345
+  return scheduleReparseAndDiags(std::move(Ctx), File, std::move(FileContents),
+ TaggedFS);
 }

nit: you dropped a move here



Comment at: clangd/ClangdServer.cpp:400
+  CallbackType Callback,
+  llvm::Expected InpPreamble) {
+assert(InpPreamble &&

nit: "Inp" in InpPreamble is pretty awkward. In this context, Read might work? 
or IP.



Comment at: clangd/ClangdServer.cpp:403
+   "error when trying to read preamble for codeComplete");
+auto Preamble = InpPreamble->Preamble;
+auto  = InpPreamble->Inputs.CompileCommand;

InpPreamble->Preamble->Preamble is pretty hard to understand. Can we find 
better names for these things? Or at least unpack it on one line?



Comment at: clangd/ClangdServer.h:164
 
+/// Handles running tasks for ClangdServer and managing the resources (e.g.,
+/// preambles and ASTs) for opened files.

ilya-biryukov wrote:
> sammccall wrote:
> > This is a nice abstraction, so much better than dealing with Cppfile! A 
> > couple of observations:
> > 
> > 1) all methods refer to a single file, neither the contracts nor the 
> > implementation have any interactions between files. An API that reflects 
> > this seems more natural. e.g. it naturally provides operations like "are we 
> > tracking this file", and makes it natural to be able to e.g. lock at the 
> > per-file level. e.g.
> > 
> >   class WorkingSet {
> >  shared_ptr get(Path);
> >  shared_ptr getOrCreate(Path)
> >}
> >class TranslationUnit {
> >  void update(ParseInputs);
> >  void read(Callback);
> >}
> > 
> > This seems like it would make it easier to explain what the semantics of 
> > scheduleRemove are, too.
> > 
> > 2) The callbacks from individual methods seem more powerful than needed, 
> > and encourage deeper coupling. Basically, the inputs (changes) and outputs 
> > (diagnostics, reads) don't seem to want to interact with the same code. 
> > This suggests decoupling them: the changes are a sequence of input events, 
> > diagnostics are a sequence of output events, reads look much as they do now.
> > 
> > One benefit here is that the versioning relationship between inputs and 
> > outputs no longer show up in the function signatures (by coupling an input 
> > to a matching output). Expressing them as data makes it easier to tweak 
> > them.
> > 
> > 3) It's not spelled out how this interacts with drafts: whether 
> > text<->version is maintained here or externally, and what the contracts 
> > around versions are. There are no options offered, so I would guess that 
> > scheduleUpdate delivers new-version-or-nothing, and scheduleASTRead 
> > delivers... current-or-newer? current-or-nothing?
> > 
> > I think it would *probably* be clearer to have versions minted by the 
> > external DraftStore, that way we can decouple "we know about the contents 
> > of this file" from "we're building this file". e.g. we probably want wildly 
> > different policies for discarding resources of old versions, when 
> > "resources" = source code vs "resources" = ASTs and preambles.
> > 
> > 4) Scheduler (or anything it decomposes into) is important and isolated 
> > enough that it deserves its own header.
> 1. Given the nature of the LSP, the users of the interface will probably 
> always call only a single function of `TranslationUnit`, so we won't win much 
> in terms of the code clarity.
> ```
> scheduleUpdate(File, Inputs) --> get(File).update(Inputs)
> ```
> That adds some complexity to the interface, though. I'd opt for not doing 
> that in the initial version.
> 
> 2. One place where I find these callbacks useful are tests where we could 
> wait for latest `addDocument` to complete. A more pressing concern is that 
> the interface of `ClangdServer` does not allow to remove the callbacks 
> (`addDocument` returns a future) and I would really love to keep the public 
> interface of `ClangdServer` the same for the first iteration.
> 
> 3. I would err on the side of saying that 

r323435 - Unused diagnostics can occur in tblgen.

[Benjamin Kramer via cfe-commits]

Author: d0k
Date: Thu Jan 25 07:57:22 2018
New Revision: 323435

URL: http://llvm.org/viewvc/llvm-project?rev=323435=rev
Log:
Unused diagnostics can occur in tblgen.

Modified:
cfe/trunk/utils/find-unused-diagnostics.sh

Modified: cfe/trunk/utils/find-unused-diagnostics.sh
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/find-unused-diagnostics.sh?rev=323435=323434=323435=diff
==
--- cfe/trunk/utils/find-unused-diagnostics.sh (original)
+++ cfe/trunk/utils/find-unused-diagnostics.sh Thu Jan 25 07:57:22 2018
@@ -8,7 +8,7 @@
 ALL_DIAGS=$(grep -E --only-matching --no-filename 
'(err_|warn_|ext_|note_)[a-z_]+' ./include/clang/Basic/Diagnostic*.td)
 
 # Now look for all potential identifiers in the source files.
-ALL_SOURCES=$(find lib include tools -name \*.cpp -or -name \*.h)
+ALL_SOURCES=$(find lib include tools utils -name \*.cpp -or -name \*.h)
 DIAGS_IN_SOURCES=$(grep -E --only-matching --no-filename 
'(err_|warn_|ext_|note_)[a-z_]+' $ALL_SOURCES)
 
 # Print all diags that occur in the .td files but not in the source.


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

[PATCH] D42499: [clangd] Modify the Span API so that Spans propagate with contexts.

[Sam McCall via Phabricator via cfe-commits]

sammccall marked 5 inline comments as done.
sammccall added a comment.

In https://reviews.llvm.org/D42499#987583, @ilya-biryukov wrote:

> I've tried experimenting with that and came up with an alternative that has 
> less changes to the API. Could you take a look, too? 
> https://reviews.llvm.org/D42524


As discussed offline, I've tried to incorporate the big improvements from there:

- moved back to Span objects, that for now expose the derived contexts as in 
your patch (and later will switch the TLS context for their lifetime)
- SPAN_ATTACH doesn't support the weird lookup that JSONRPCDispatcher uses. 
Rather than changing that class's traces, I've added locks to support that 
pattern locally.

Also as discussed offline, The proposed TraceEvent's endSpan/endEvent 
distinction doesn't seem necessary.
The EventTracer needs to be able to:

- store some data until the span ends
- get notified when the span ends

UniqueFunction does provide this, but the interface is slightly confusing: 
operator() and ~UniqueFunction both denote "when the span ends".
An object with a destructor seems a more natural way to express this, so I've 
kept that change - I don't feel really strongly though, happy to change this to 
something else simple.




Comment at: clangd/Context.h:165
+static Key::type> Private;
+return derive(Private, std::forward(Value));
+  }

ilya-biryukov wrote:
> This will call the `const&` version of `derive(Key, Value)`.
> To move we need to do `std::move(*this).derive(Private, 
> std::forward(Value))`
Oops, good catch!



Comment at: clangd/Context.h:172
+  friend bool operator==(const Context , const Context ) {
+return A.DataPtr == B.DataPtr;
+  }

ilya-biryukov wrote:
> I wonder what are the use-cases for this `operator ==`?
Sorry, this was dead code from a previous iteration. Removed



Comment at: clangd/Function.h:147
 
-  ScopeExitGuard(Func F) : F(std::move(F)) {}
+  ScopeExitGuard(Func F) : F(llvm::make_unique(std::move(F))) {}
   ~ScopeExitGuard() {

ilya-biryukov wrote:
> Why do we need `unique_ptr` instead of `Optional` here?
ScopeExitGuard was broken by the recent Optional changes (when Func is 
trivially copyable).
This is a minimal workaround, let's discuss on the other patch.



Comment at: clangd/Trace.h:92
+/// Ctx (or some ancestor) must be created by span().
+/// This macro is not safe for use on the same span from multiple threads.
+#define SPAN_ATTACH(Ctx, Name, Expr)   
\

ilya-biryukov wrote:
> This is too easy to misuse unintentionally. Will this go away if we move to 
> thread-local contexts? I.e. will the API leave out mutations of the context 
> that can be used concurrently
> This is too easy to misuse unintentionally.
Agreed. I've restored the Span object, and restored the requirement that you 
pass the span itself to SPAN_ATTACH.

This disallows "action at a distance" that can make for nice traces e.g. for 
request, but can lead to thread-unsafety.
The only place where we actually use this is JSONRPCDispatcher, so I made that 
do it explicitly using a lock.
It seems mostly reasonable - if you feel we should remove this feature instead 
to avoid the complexity, happy to discuss that, but at least it's not in the 
main tracer interface now.

> Will this go away if we move to thread-local contexts? I.e. will the API 
> leave out mutations of the context that can be used concurrently
You've convinced me that requiring the user to pass SPAN_ATTACH to the span 
makes sense and we should keep it even once we have implicit context passing.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42499



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

[PATCH] D42538: [clang-cl] Add support for /arch:AVX512F and /arch:AVX512

[Nico Weber via Phabricator via cfe-commits]

thakis closed this revision.
thakis added a comment.

r323433, thanks!


https://reviews.llvm.org/D42538



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

r323433 - [clang-cl] Add support for /arch:AVX512F and /arch:AVX512

[Nico Weber via cfe-commits]

Author: nico
Date: Thu Jan 25 07:24:43 2018
New Revision: 323433

URL: http://llvm.org/viewvc/llvm-project?rev=323433=rev
Log:
[clang-cl] Add support for /arch:AVX512F and /arch:AVX512

For /arch:AVX512F:
clang-cl and cl.exe both defines __AVX512F__ __AVX512CD__.
clang-cl also defines __AVX512ER__ __AVX512PF__.
64-bit cl.exe also defines (according to /Bz) _NO_PREFETCHW.

For /arch:AVX512:
clang-cl and cl.exe both define
__AVX512F__ __AVX512CD__ __AVX512BW__ __AVX512DQ__ __AVX512VL__.
64-bit cl.exe also defines _NO_PREFETCHW.

So not 100% identical, but pretty close.

Also refactor the existing AVX / AVX2 code to not repeat itself in both the
32-bit and 64-bit cases.

https://reviews.llvm.org/D42538

Modified:
cfe/trunk/lib/Driver/ToolChains/Arch/X86.cpp
cfe/trunk/test/Driver/cl-x86-flags.c

Modified: cfe/trunk/lib/Driver/ToolChains/Arch/X86.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Arch/X86.cpp?rev=323433=323432=323433=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Arch/X86.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Arch/X86.cpp Thu Jan 25 07:24:43 2018
@@ -43,19 +43,20 @@ const char *x86::getX86TargetCPU(const A
   if (const Arg *A = Args.getLastArgNoClaim(options::OPT__SLASH_arch)) {
 // Mapping built by looking at lib/Basic's X86TargetInfo::initFeatureMap().
 StringRef Arch = A->getValue();
-const char *CPU;
-if (Triple.getArch() == llvm::Triple::x86) {
+const char *CPU = nullptr;
+if (Triple.getArch() == llvm::Triple::x86) {  // 32-bit-only /arch: flags.
   CPU = llvm::StringSwitch(Arch)
 .Case("IA32", "i386")
 .Case("SSE", "pentium3")
 .Case("SSE2", "pentium4")
-.Case("AVX", "sandybridge")
-.Case("AVX2", "haswell")
 .Default(nullptr);
-} else {
+}
+if (CPU == nullptr) {  // 32-bit and 64-bit /arch: flags.
   CPU = llvm::StringSwitch(Arch)
 .Case("AVX", "sandybridge")
 .Case("AVX2", "haswell")
+.Case("AVX512F", "knl")
+.Case("AVX512", "skylake-avx512")
 .Default(nullptr);
 }
 if (CPU) {

Modified: cfe/trunk/test/Driver/cl-x86-flags.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/cl-x86-flags.c?rev=323433=323432=323433=diff
==
--- cfe/trunk/test/Driver/cl-x86-flags.c (original)
+++ cfe/trunk/test/Driver/cl-x86-flags.c Thu Jan 25 07:24:43 2018
@@ -68,6 +68,26 @@
 // RUN: %clang_cl -m32 -arch:avx2 --target=i386-pc-windows -### -- 2>&1 %s | 
FileCheck -check-prefix=avx2 %s
 // avx2: argument unused during compilation
 
+// RUN: %clang_cl -m32 -arch:AVX512F --target=i386-pc-windows /c -Xclang 
-verify -DTEST_32_ARCH_AVX512F -- %s
+#if defined(TEST_32_ARCH_AVX512F)
+#if _M_IX86_FP != 2 || !__AVX__ || !__AVX2__ || !__AVX512F__  || __AVX512BW__
+#error fail
+#endif
+#endif
+
+// RUN: %clang_cl -m32 -arch:avx512f --target=i386-pc-windows -### -- 2>&1 %s 
| FileCheck -check-prefix=avx512f %s
+// avx512f: argument unused during compilation
+
+// RUN: %clang_cl -m32 -arch:AVX512 --target=i386-pc-windows /c -Xclang 
-verify -DTEST_32_ARCH_AVX512 -- %s
+#if defined(TEST_32_ARCH_AVX512)
+#if _M_IX86_FP != 2 || !__AVX__ || !__AVX2__ || !__AVX512F__  || !__AVX512BW__
+#error fail
+#endif
+#endif
+
+// RUN: %clang_cl -m32 -arch:avx512 --target=i386-pc-windows -### -- 2>&1 %s | 
FileCheck -check-prefix=avx512 %s
+// avx512: argument unused during compilation
+
 // RUN: %clang_cl -m64 -arch:AVX --target=x86_64-pc-windows /c -Xclang -verify 
-DTEST_64_ARCH_AVX -- %s
 #if defined(TEST_64_ARCH_AVX)
 #if _M_IX86_FP || !__AVX__ || __AVX2__ || __AVX512F__  || __AVX512BW__
@@ -88,5 +108,25 @@
 // RUN: %clang_cl -m64 -arch:avx2 --target=x86_64-pc-windows -### -- 2>&1 %s | 
FileCheck -check-prefix=avx264 %s
 // avx264: argument unused during compilation
 
+// RUN: %clang_cl -m64 -arch:AVX512F --target=i386-pc-windows /c -Xclang 
-verify -DTEST_64_ARCH_AVX512F -- %s
+#if defined(TEST_64_ARCH_AVX512F)
+#if _M_IX86_FP || !__AVX__ || !__AVX2__ || !__AVX512F__  || __AVX512BW__
+#error fail
+#endif
+#endif
+
+// RUN: %clang_cl -m64 -arch:avx512f --target=i386-pc-windows -### -- 2>&1 %s 
| FileCheck -check-prefix=avx512f64 %s
+// avx512f64: argument unused during compilation
+
+// RUN: %clang_cl -m64 -arch:AVX512 --target=i386-pc-windows /c -Xclang 
-verify -DTEST_64_ARCH_AVX512 -- %s
+#if defined(TEST_64_ARCH_AVX512)
+#if _M_IX86_FP || !__AVX__ || !__AVX2__ || !__AVX512F__  || !__AVX512BW__
+#error fail
+#endif
+#endif
+
+// RUN: %clang_cl -m64 -arch:avx512 --target=i386-pc-windows -### -- 2>&1 %s | 
FileCheck -check-prefix=avx51264 %s
+// avx51264: argument unused during compilation
+
 void f() {
 }


___
cfe-commits mailing list

[PATCH] D42538: [clang-cl] Add support for /arch:AVX512F and /arch:AVX512

[Hans Wennborg via Phabricator via cfe-commits]

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

lgtm


https://reviews.llvm.org/D42538



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

[PATCH] D42538: [clang-cl] Add support for /arch:AVX512F and /arch:AVX512

[Nico Weber via Phabricator via cfe-commits]

thakis created this revision.
thakis added a reviewer: hans.
thakis edited the summary of this revision.

Also refactor the existing AVX / AVX2 code to not repeat itself in both the 
32-bit and 64-bit cases.

For /arch:AVX512F, we define `__AVX512F__` `__AVX512CD__` `__AVX512ER__` 
`__AVX512PF__`. cl.exe defines (according to /Bz) `__AVX512F__` and 
`__AVX512CD__`, and 64-bit cl also defines `_NO_PREFETCHW`.

For /arch:AVX512, we define `__AVX512F__`, `__AVX512CD__`, 
`__AVX512BW__`__AVX512DQ__`, `__AVX512VL__`. cl.exe defines `__AVX512F__`, 
`__AVX512CD__`, `__AVX512BW__`, `__AVX512DQ__`, `__AVX512VL__`, and on 64-bit 
also `_NO_PREFETCHW`.

So not 100% identical, but seems pretty close.


https://reviews.llvm.org/D42538

Files:
  lib/Driver/ToolChains/Arch/X86.cpp
  test/Driver/cl-x86-flags.c


Index: test/Driver/cl-x86-flags.c
===
--- test/Driver/cl-x86-flags.c
+++ test/Driver/cl-x86-flags.c
@@ -68,6 +68,26 @@
 // RUN: %clang_cl -m32 -arch:avx2 --target=i386-pc-windows -### -- 2>&1 %s | 
FileCheck -check-prefix=avx2 %s
 // avx2: argument unused during compilation
 
+// RUN: %clang_cl -m32 -arch:AVX512F --target=i386-pc-windows /c -Xclang 
-verify -DTEST_32_ARCH_AVX512F -- %s
+#if defined(TEST_32_ARCH_AVX512F)
+#if _M_IX86_FP != 2 || !__AVX__ || !__AVX2__ || !__AVX512F__  || __AVX512BW__
+#error fail
+#endif
+#endif
+
+// RUN: %clang_cl -m32 -arch:avx512f --target=i386-pc-windows -### -- 2>&1 %s 
| FileCheck -check-prefix=avx512f %s
+// avx512f: argument unused during compilation
+
+// RUN: %clang_cl -m32 -arch:AVX512 --target=i386-pc-windows /c -Xclang 
-verify -DTEST_32_ARCH_AVX512 -- %s
+#if defined(TEST_32_ARCH_AVX512)
+#if _M_IX86_FP != 2 || !__AVX__ || !__AVX2__ || !__AVX512F__  || !__AVX512BW__
+#error fail
+#endif
+#endif
+
+// RUN: %clang_cl -m32 -arch:avx512 --target=i386-pc-windows -### -- 2>&1 %s | 
FileCheck -check-prefix=avx512 %s
+// avx512: argument unused during compilation
+
 // RUN: %clang_cl -m64 -arch:AVX --target=x86_64-pc-windows /c -Xclang -verify 
-DTEST_64_ARCH_AVX -- %s
 #if defined(TEST_64_ARCH_AVX)
 #if _M_IX86_FP || !__AVX__ || __AVX2__ || __AVX512F__  || __AVX512BW__
@@ -88,5 +108,25 @@
 // RUN: %clang_cl -m64 -arch:avx2 --target=x86_64-pc-windows -### -- 2>&1 %s | 
FileCheck -check-prefix=avx264 %s
 // avx264: argument unused during compilation
 
+// RUN: %clang_cl -m64 -arch:AVX512F --target=i386-pc-windows /c -Xclang 
-verify -DTEST_64_ARCH_AVX512F -- %s
+#if defined(TEST_64_ARCH_AVX512F)
+#if _M_IX86_FP || !__AVX__ || !__AVX2__ || !__AVX512F__  || __AVX512BW__
+#error fail
+#endif
+#endif
+
+// RUN: %clang_cl -m64 -arch:avx512f --target=i386-pc-windows -### -- 2>&1 %s 
| FileCheck -check-prefix=avx512f64 %s
+// avx512f64: argument unused during compilation
+
+// RUN: %clang_cl -m64 -arch:AVX512 --target=i386-pc-windows /c -Xclang 
-verify -DTEST_64_ARCH_AVX512 -- %s
+#if defined(TEST_64_ARCH_AVX512)
+#if _M_IX86_FP || !__AVX__ || !__AVX2__ || !__AVX512F__  || !__AVX512BW__
+#error fail
+#endif
+#endif
+
+// RUN: %clang_cl -m64 -arch:avx512 --target=i386-pc-windows -### -- 2>&1 %s | 
FileCheck -check-prefix=avx51264 %s
+// avx51264: argument unused during compilation
+
 void f() {
 }
Index: lib/Driver/ToolChains/Arch/X86.cpp
===
--- lib/Driver/ToolChains/Arch/X86.cpp
+++ lib/Driver/ToolChains/Arch/X86.cpp
@@ -43,19 +43,20 @@
   if (const Arg *A = Args.getLastArgNoClaim(options::OPT__SLASH_arch)) {
 // Mapping built by looking at lib/Basic's X86TargetInfo::initFeatureMap().
 StringRef Arch = A->getValue();
-const char *CPU;
-if (Triple.getArch() == llvm::Triple::x86) {
+const char *CPU = nullptr;
+if (Triple.getArch() == llvm::Triple::x86) {  // 32-bit-only /arch: flags.
   CPU = llvm::StringSwitch(Arch)
 .Case("IA32", "i386")
 .Case("SSE", "pentium3")
 .Case("SSE2", "pentium4")
-.Case("AVX", "sandybridge")
-.Case("AVX2", "haswell")
 .Default(nullptr);
-} else {
+}
+if (CPU == nullptr) {  // 32-bit and 64-bit /arch: flags.
   CPU = llvm::StringSwitch(Arch)
 .Case("AVX", "sandybridge")
 .Case("AVX2", "haswell")
+.Case("AVX512F", "knl")
+.Case("AVX512", "skylake-avx512")
 .Default(nullptr);
 }
 if (CPU) {


Index: test/Driver/cl-x86-flags.c
===
--- test/Driver/cl-x86-flags.c
+++ test/Driver/cl-x86-flags.c
@@ -68,6 +68,26 @@
 // RUN: %clang_cl -m32 -arch:avx2 --target=i386-pc-windows -### -- 2>&1 %s | FileCheck -check-prefix=avx2 %s
 // avx2: argument unused during compilation
 
+// RUN: %clang_cl -m32 -arch:AVX512F --target=i386-pc-windows /c -Xclang -verify -DTEST_32_ARCH_AVX512F -- %s
+#if defined(TEST_32_ARCH_AVX512F)
+#if _M_IX86_FP != 2 || !__AVX__ || 

[clang-tools-extra] r323431 - Removed Unicode BOM.

[Alexander Kornienko via cfe-commits]

Author: alexfh
Date: Thu Jan 25 07:09:33 2018
New Revision: 323431

URL: http://llvm.org/viewvc/llvm-project?rev=323431=rev
Log:
Removed Unicode BOM.

Modified:
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/NoMallocCheck.cpp

Modified: clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/NoMallocCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/NoMallocCheck.cpp?rev=323431=323430=323431=diff
==
--- clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/NoMallocCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/NoMallocCheck.cpp Thu 
Jan 25 07:09:33 2018
@@ -1,4 +1,4 @@
-//===--- NoMallocCheck.cpp - 
clang-tidy===//
+//===--- NoMallocCheck.cpp - 
clang-tidy===//
 //
 // The LLVM Compiler Infrastructure
 //


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

[PATCH] D39571: [clangd] DidChangeConfiguration Notification

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clangd/ClangdLSPServer.cpp:78
 {"documentHighlightProvider", true},
+{"configurationChangeProvider", true},
 {"renameProvider", true},

simark wrote:
> Nebiroth wrote:
> > simark wrote:
> > > I find `configurationChangeProvider` a bit weird.  It makes it sound like 
> > > clangd can provide configuration changes.  In reality, it can accept 
> > > configuration changes.  So I think this should be named something else.
> > Agreed, perhaps configurationChangeManager would be more appropriate then?
> I'm thinking of removing it for the time being.  Since it's not defined in 
> the protocol what types of configuration changes exist (it's specific to each 
> language server), it not very useful to simply advertise that we support 
> configuration changes.  We would need to advertise that we support 
> compilation database changes in particular.  I think this can be done later.
`"configurationChangeProvider"` is not in the LSP, right?
There's `experimental` field in the specification, let's put it under that 
field if you want to advertise that clangd supports this spec to your clients.



Comment at: clangd/ClangdLSPServer.cpp:302
 
+// FIXME: This function needs to be properly tested.
+void ClangdLSPServer::onChangeConfiguration(

simark wrote:
> ilya-biryukov wrote:
> > simark wrote:
> > > ilya-biryukov wrote:
> > > > Are you planning to to address this FIXME before checking the code in?
> > > Following what you said here:
> > > 
> > > https://reviews.llvm.org/D39571?id=124024#inline-359345
> > > 
> > > I have not really looked into what was wrong with the test, and what is 
> > > missing in the infrastructure to make it work.  But I assumed that the 
> > > situation did not change since then.  Can you enlighten me on what the 
> > > problem was, and what is missing?
> > We usually write unittests for that kind of thing, since they allow to plug 
> > an in-memory filesystem, but we only test `ClangdServer` (examples are in 
> > `unittests/clangd/ClangdTests.cpp`). `ClangdLSPServer` does not allow to 
> > plug in a virtual filesystem (vfs). Even if we add vfs, it's still hard to 
> > unit-test because we'll have to match the json input/output directly.
> > 
> > This leaves us with an option of a lit test that runs `clangd` directly, 
> > similar to tests in `test/clangd`.
> > The lit test would need to create a temporary directory, create proper 
> > `compile_commands.json` there, then send the LSP commands with the path to 
> > the test to clangd.
> > One major complication is that in LSP we have to specify the size of each 
> > message, but in our case the size would change depending on created temp 
> > path. It means we'll have to patch the test input to setup proper paths and 
> > message sizes.
> > If we choose to go down this path, 
> > `clang-tools-extra/test/clang-tidy/vfsoverlay.cpp` does a similar setup 
> > (create temp-dir, patch up some configuration files to point into the temp 
> > directory, etc) and could be used as a starting point.
> > 
> > It's not impossible to write that test, it's just a bit involved. Having a 
> > test would be nice, though, to ensure we don't break this method while 
> > doing other things. Especially given that this functionality is not used 
> > anywhere in clangd.
> > We usually write unittests for that kind of thing, since they allow to plug 
> > an in-memory filesystem, but we only test ClangdServer (examples are in 
> > unittests/clangd/ClangdTests.cpp). ClangdLSPServer does not allow to plug 
> > in a virtual filesystem (vfs). Even if we add vfs, it's still hard to 
> > unit-test because we'll have to match the json input/output directly.
> 
> What do you mean by "we'll have to match the json input/output directly"?  
> That we'll have to match the complete JSON output textually?  Couldn't the 
> test parse the JSON into some data structures, then we could assert specific 
> things, like that this particular field is present and contains a certain 
> substring, for example?
> 
> > This leaves us with an option of a lit test that runs clangd directly, 
> > similar to tests in test/clangd.
> > The lit test would need to create a temporary directory, create proper 
> > compile_commands.json there, then send the LSP commands with the path to 
> > the test to clangd.
> > One major complication is that in LSP we have to specify the size of each 
> > message, but in our case the size would change depending on created temp 
> > path. It means we'll have to patch the test input to setup proper paths and 
> > message sizes.
> > If we choose to go down this path, 
> > clang-tools-extra/test/clang-tidy/vfsoverlay.cpp does a similar setup 
> > (create temp-dir, patch up some configuration files to point into the temp 
> > directory, etc) and could be used as a starting point.
> 
> Ok, I see the complication with the 

[PATCH] D39571: [clangd] DidChangeConfiguration Notification

[Simon Marchi via Phabricator via cfe-commits]

simark added inline comments.



Comment at: clangd/ClangdLSPServer.cpp:302
 
+// FIXME: This function needs to be properly tested.
+void ClangdLSPServer::onChangeConfiguration(

ilya-biryukov wrote:
> simark wrote:
> > ilya-biryukov wrote:
> > > Are you planning to to address this FIXME before checking the code in?
> > Following what you said here:
> > 
> > https://reviews.llvm.org/D39571?id=124024#inline-359345
> > 
> > I have not really looked into what was wrong with the test, and what is 
> > missing in the infrastructure to make it work.  But I assumed that the 
> > situation did not change since then.  Can you enlighten me on what the 
> > problem was, and what is missing?
> We usually write unittests for that kind of thing, since they allow to plug 
> an in-memory filesystem, but we only test `ClangdServer` (examples are in 
> `unittests/clangd/ClangdTests.cpp`). `ClangdLSPServer` does not allow to plug 
> in a virtual filesystem (vfs). Even if we add vfs, it's still hard to 
> unit-test because we'll have to match the json input/output directly.
> 
> This leaves us with an option of a lit test that runs `clangd` directly, 
> similar to tests in `test/clangd`.
> The lit test would need to create a temporary directory, create proper 
> `compile_commands.json` there, then send the LSP commands with the path to 
> the test to clangd.
> One major complication is that in LSP we have to specify the size of each 
> message, but in our case the size would change depending on created temp 
> path. It means we'll have to patch the test input to setup proper paths and 
> message sizes.
> If we choose to go down this path, 
> `clang-tools-extra/test/clang-tidy/vfsoverlay.cpp` does a similar setup 
> (create temp-dir, patch up some configuration files to point into the temp 
> directory, etc) and could be used as a starting point.
> 
> It's not impossible to write that test, it's just a bit involved. Having a 
> test would be nice, though, to ensure we don't break this method while doing 
> other things. Especially given that this functionality is not used anywhere 
> in clangd.
> We usually write unittests for that kind of thing, since they allow to plug 
> an in-memory filesystem, but we only test ClangdServer (examples are in 
> unittests/clangd/ClangdTests.cpp). ClangdLSPServer does not allow to plug in 
> a virtual filesystem (vfs). Even if we add vfs, it's still hard to unit-test 
> because we'll have to match the json input/output directly.

What do you mean by "we'll have to match the json input/output directly"?  That 
we'll have to match the complete JSON output textually?  Couldn't the test 
parse the JSON into some data structures, then we could assert specific things, 
like that this particular field is present and contains a certain substring, 
for example?

> This leaves us with an option of a lit test that runs clangd directly, 
> similar to tests in test/clangd.
> The lit test would need to create a temporary directory, create proper 
> compile_commands.json there, then send the LSP commands with the path to the 
> test to clangd.
> One major complication is that in LSP we have to specify the size of each 
> message, but in our case the size would change depending on created temp 
> path. It means we'll have to patch the test input to setup proper paths and 
> message sizes.
> If we choose to go down this path, 
> clang-tools-extra/test/clang-tidy/vfsoverlay.cpp does a similar setup (create 
> temp-dir, patch up some configuration files to point into the temp directory, 
> etc) and could be used as a starting point.

Ok, I see the complication with the Content-Length.  I am not familiar with lit 
yet, so I don't know what it is capable of.  But being able to craft and send 
arbitrary LSP messages would certainly be helpful in the future for all kinds 
of black box test, so having a framework that allows to do this would be 
helpful, I think.  I'm not familiar enough with the ecosystem to do this right 
now, but I'll keep it in mind.

One question about this particular test.  Would there be some race condition 
here?  If we do:

1. Start clangd with compile_commands.json #1
2. Ask for the definition of a function, expecting a result
3. Change the configuration to compile_commands.json #2
4. Ask for the definition of the same function, expecting a different result

Since clangd is multi-threaded and does work in the background, are we sure 
that we'll get the result we want in #4?

> It's not impossible to write that test, it's just a bit involved. Having a 
> test would be nice, though, to ensure we don't break this method while doing 
> other things. Especially given that this functionality is not used anywhere 
> in clangd.

I agree.  For the time being, is it fine to leave the FIXME there?  We can work 
on improving the test frameworks to get rid of it.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D39571