[PATCH] D44773: [CMake] Use custom command and target to install libc++ headers

2018-04-03 Thread Eric Fiselier via Phabricator via cfe-commits
EricWF accepted this revision.
EricWF added a comment.
This revision is now accepted and ready to land.

This LGTM modulo requested changes.

There's a section in `NOTES.TXT` about the steps required for adding a header. 
Please update that to mention listing it in CMake.




Comment at: libcxx/include/CMakeLists.txt:151
+
+if(LIBCXX_ENABLE_EXPERIMENTAL_LIBRARY)
+  set(files

We still want to install the experimental headers even if the experimental 
library build isn't enabled. Header only components still function. 


Repository:
  rCXX libc++

https://reviews.llvm.org/D44773



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


[PATCH] D44231: [clang-tidy] Check for sizeof that call functions

2018-04-03 Thread Haojian Wu via Phabricator via cfe-commits
hokein added a comment.

Sorry for the delay -- everyone was out because of the long Easter weekend. 
I'll commit for you.


https://reviews.llvm.org/D44231



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


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

2018-04-03 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment.

Sorry for the late response, was on vacation.




Comment at: include/clang/Sema/CodeCompleteConsumer.h:565
+  /// \brief For this completion result correction is required.
+  Optional Corr = None;
+

yvvan wrote:
> yvvan wrote:
> > ilya-biryukov wrote:
> > > Having a string replacement without an actual range to replace still 
> > > moves a lot of responsibility onto the clients. We should probably model 
> > > corrections after the fix-its for diagnostics:
> > > - replacements need to provide a range to be replaced, alongside with a 
> > > text for the replacement,
> > > - we should provide a list of edits to allow corrections that touch two 
> > > separate pieces of code.
> > > 
> > > For the fix-its in the diagnostics, see 
> > > [[https://reviews.llvm.org/source/clang/browse/cfe/trunk/tools/libclang/CXLoadedDiagnostic.h;327861$84
> > >  | CXLoadedDiagnostic.h]] for an interface exported via libclang and 
> > > [[https://reviews.llvm.org/source/clang/browse/cfe/trunk/include/clang/Basic/Diagnostic.h;327861$947|Diagnostic.h]]
> > >  for an interface exported in C++ API.
> > > WDYT?
> > I thought that fixits always exist separately from diagnostics. I will 
> > check this out, thanks.
> I've looked  into diagnostics and realized that i already use them here but 
> the are just not forwarded to completion results which is of source possible.
> 
> I have a draft with Optional instead of Optional
> It allows to use further both string and replaced/removed range and also 
> mostly uses the same code as already provided in this patch since I already 
> generated FixItHint for Diagnostics (line 4117 lib/Sema/SemaCodeComplete.cpp 
> in this patch)
> 
> Is that fine with you?
Sounds good. 
One caveat is that we should probably go with `std::vector` instead 
of `Optional`. This would mirror what `clang::Diagnostic` does.
"Dot to arrow" will only ever add one element there, but more intricate fixes 
might require more than a single edit.


https://reviews.llvm.org/D41537



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


[PATCH] D45202: [X86] Replacing X86-specific floor and ceil vector intrinsics with generic LLVM intrinsics

2018-04-03 Thread Mikhail Dvoretckii via Phabricator via cfe-commits
mike.dvoretsky created this revision.
mike.dvoretsky added reviewers: craig.topper, spatel, RKSimon.
Herald added a subscriber: cfe-commits.

Currently, X86 floor and ceil intrinsics for vectors are implemented as 
target-specific intrinsics that use the generic rounding instruction of the 
corresponding vector processing feature (ROUND* or VRNDSCALE*). This patch 
replaces those specific cases with calls to target-independent @llvm.floor.* 
and @llvm.ceil.* intrinsics. This doesn't affect the resulting machine code, as 
those intrinsics are lowered to the same instructions, but exposes these 
specific rounding cases to generic optimizations.


Repository:
  rC Clang

https://reviews.llvm.org/D45202

Files:
  include/clang/Basic/BuiltinsX86.def
  lib/CodeGen/CGBuiltin.cpp
  lib/Headers/avx512fintrin.h
  lib/Headers/avxintrin.h
  lib/Headers/smmintrin.h
  test/CodeGen/avx-builtins.c
  test/CodeGen/avx512f-builtins.c
  test/CodeGen/sse41-builtins.c

Index: test/CodeGen/sse41-builtins.c
===
--- test/CodeGen/sse41-builtins.c
+++ test/CodeGen/sse41-builtins.c
@@ -44,25 +44,29 @@
 
 __m128d test_mm_ceil_pd(__m128d x) {
   // CHECK-LABEL: test_mm_ceil_pd
-  // CHECK: call <2 x double> @llvm.x86.sse41.round.pd(<2 x double> %{{.*}}, i32 2)
+  // CHECK: @llvm.ceil.v2f64
+  // CHECK-NOT: select
   return _mm_ceil_pd(x);
 }
 
 __m128 test_mm_ceil_ps(__m128 x) {
   // CHECK-LABEL: test_mm_ceil_ps
-  // CHECK: call <4 x float> @llvm.x86.sse41.round.ps(<4 x float> %{{.*}}, i32 2)
+  // CHECK: @llvm.ceil.v4f32
+  // CHECK-NOT: select
   return _mm_ceil_ps(x);
 }
 
 __m128d test_mm_ceil_sd(__m128d x, __m128d y) {
   // CHECK-LABEL: test_mm_ceil_sd
-  // CHECK: call <2 x double> @llvm.x86.sse41.round.sd(<2 x double> %{{.*}}, <2 x double> %{{.*}}, i32 2)
+  // CHECK: @llvm.ceil.v2f64
+  // CHECK: select
   return _mm_ceil_sd(x, y);
 }
 
 __m128 test_mm_ceil_ss(__m128 x, __m128 y) {
   // CHECK-LABEL: test_mm_ceil_ss
-  // CHECK: call <4 x float> @llvm.x86.sse41.round.ss(<4 x float> %{{.*}}, <4 x float> %{{.*}}, i32 2)
+  // CHECK: @llvm.ceil.v4f32
+  // CHECK: select
   return _mm_ceil_ss(x, y);
 }
 
@@ -196,25 +200,29 @@
 
 __m128d test_mm_floor_pd(__m128d x) {
   // CHECK-LABEL: test_mm_floor_pd
-  // CHECK: call <2 x double> @llvm.x86.sse41.round.pd(<2 x double> %{{.*}}, i32 1)
+  // CHECK: @llvm.floor.v2f64
+  // CHECK-NOT: select
   return _mm_floor_pd(x);
 }
 
 __m128 test_mm_floor_ps(__m128 x) {
   // CHECK-LABEL: test_mm_floor_ps
-  // CHECK: call <4 x float> @llvm.x86.sse41.round.ps(<4 x float> %{{.*}}, i32 1)
+  // CHECK: @llvm.floor.v4f32
+  // CHECK-NOT: select
   return _mm_floor_ps(x);
 }
 
 __m128d test_mm_floor_sd(__m128d x, __m128d y) {
   // CHECK-LABEL: test_mm_floor_sd
-  // CHECK: call <2 x double> @llvm.x86.sse41.round.sd(<2 x double> %{{.*}}, <2 x double> %{{.*}}, i32 1)
+  // CHECK: @llvm.floor.v2f64
+  // CHECK: select
   return _mm_floor_sd(x, y);
 }
 
 __m128 test_mm_floor_ss(__m128 x, __m128 y) {
   // CHECK-LABEL: test_mm_floor_ss
-  // CHECK: call <4 x float> @llvm.x86.sse41.round.ss(<4 x float> %{{.*}}, <4 x float> %{{.*}}, i32 1)
+  // CHECK: @llvm.floor.v4f32
+  // CHECK: select
   return _mm_floor_ss(x, y);
 }
 
Index: test/CodeGen/avx512f-builtins.c
===
--- test/CodeGen/avx512f-builtins.c
+++ test/CodeGen/avx512f-builtins.c
@@ -7485,31 +7485,67 @@
   return _mm512_min_round_ps(__A,__B,_MM_FROUND_CUR_DIRECTION);
 }
 
+__m512 test_mm512_floor_ps(__m512 __A)
+{
+  // CHECK-LABEL: @test_mm512_floor_ps
+  // CHECK: @llvm.floor.v16f32
+  // CHECK-NOT: select
+  return _mm512_floor_ps(__A);
+}
+
+__m512d test_mm512_floor_pd(__m512d __A)
+{
+  // CHECK-LABEL: @test_mm512_floor_pd
+  // CHECK: @llvm.floor.v8f64
+  // CHECK-NOT: select
+  return _mm512_floor_pd(__A);
+}
+
 __m512 test_mm512_mask_floor_ps (__m512 __W, __mmask16 __U, __m512 __A)
 {
-  // CHECK-LABEL: @test_mm512_mask_floor_ps 
-  // CHECK: @llvm.x86.avx512.mask.rndscale.ps.512
+  // CHECK-LABEL: @test_mm512_mask_floor_ps
+  // CHECK: @llvm.floor.v16f32
+  // CHECK: select <16 x i1> %{{.*}}, <16 x float> %{{.*}}, <16 x float> %{{.*}}
   return _mm512_mask_floor_ps (__W,__U,__A);
 }
 
 __m512d test_mm512_mask_floor_pd (__m512d __W, __mmask8 __U, __m512d __A)
 {
-  // CHECK-LABEL: @test_mm512_mask_floor_pd 
-  // CHECK: @llvm.x86.avx512.mask.rndscale.pd.512
+  // CHECK-LABEL: @test_mm512_mask_floor_pd
+  // CHECK: @llvm.floor.v8f64
+  // CHECK: select <8 x i1> %{{.*}}, <8 x double> %{{.*}}, <8 x double> %{{.*}}
   return _mm512_mask_floor_pd (__W,__U,__A);
 }
 
+__m512 test_mm512_ceil_ps(__m512 __A)
+{
+  // CHECK-LABEL: @test_mm512_ceil_ps
+  // CHECK: @llvm.ceil.v16f32
+  // CHECK-NOT: select
+  return _mm512_ceil_ps(__A);
+}
+
+__m512d test_mm512_ceil_pd(__m512d __A)
+{
+  // CHECK-LABEL: @test_mm512_ceil_pd
+  // CHECK: @llvm.ceil.v8f64
+  // CHECK-NOT: select
+  return _mm512_ceil_pd(__A);
+}
+
 __m512 test_

[PATCH] D44231: [clang-tidy] Check for sizeof that call functions

2018-04-03 Thread Haojian Wu via Phabricator via cfe-commits
hokein added a comment.

pfultz2@, could you rebase this patch? The check has been moved to bugprone.


https://reviews.llvm.org/D44231



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


r329052 - UsersManual.rst: update text for /GX- to match r328708

2018-04-03 Thread Hans Wennborg via cfe-commits
Author: hans
Date: Tue Apr  3 02:28:21 2018
New Revision: 329052

URL: http://llvm.org/viewvc/llvm-project?rev=329052&view=rev
Log:
UsersManual.rst: update text for /GX- to match r328708

Modified:
cfe/trunk/docs/UsersManual.rst

Modified: cfe/trunk/docs/UsersManual.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/UsersManual.rst?rev=329052&r1=329051&r2=329052&view=diff
==
--- cfe/trunk/docs/UsersManual.rst (original)
+++ cfe/trunk/docs/UsersManual.rst Tue Apr  3 02:28:21 2018
@@ -2791,7 +2791,7 @@ Execute ``clang-cl /?`` to see a list of
   /Gv Set __vectorcall as a default calling convention
   /Gw-Don't put each data item in its own section
   /Gw Put each data item in its own section
-  /GX-Enable exception handling
+  /GX-Disable exception handling
   /GX Enable exception handling
   /Gy-Don't put each function in its own section
   /Gy Put each function in its own section


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


[PATCH] D44248: [clangd][cmake] Provide libatomic when there is no native support for 64bit atomics

2018-04-03 Thread Simon Dardis via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL329053: [clangd][cmake] Provide libatomic when there is no 
native support for 64bit… (authored by sdardis, committed by ).
Herald added subscribers: llvm-commits, MaskRay.

Changed prior to commit:
  https://reviews.llvm.org/D44248?vs=137555&id=140750#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D44248

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


Index: clang-tools-extra/trunk/clangd/CMakeLists.txt
===
--- clang-tools-extra/trunk/clangd/CMakeLists.txt
+++ clang-tools-extra/trunk/clangd/CMakeLists.txt
@@ -2,6 +2,11 @@
   Support
   )
 
+set(CLANGD_ATOMIC_LIB "")
+if(NOT HAVE_CXX_ATOMICS64_WITHOUT_LIB)
+  list(APPEND CLANGD_ATOMIC_LIB "atomic")
+endif()
+
 add_clang_library(clangDaemon
   AST.cpp
   ClangdLSPServer.cpp
@@ -50,6 +55,7 @@
   clangToolingCore
   clangToolingRefactor
   ${LLVM_PTHREAD_LIB}
+  ${CLANGD_ATOMIC_LIB}
   )
 
 if( LLVM_LIB_FUZZING_ENGINE OR LLVM_USE_SANITIZE_COVERAGE )


Index: clang-tools-extra/trunk/clangd/CMakeLists.txt
===
--- clang-tools-extra/trunk/clangd/CMakeLists.txt
+++ clang-tools-extra/trunk/clangd/CMakeLists.txt
@@ -2,6 +2,11 @@
   Support
   )
 
+set(CLANGD_ATOMIC_LIB "")
+if(NOT HAVE_CXX_ATOMICS64_WITHOUT_LIB)
+  list(APPEND CLANGD_ATOMIC_LIB "atomic")
+endif()
+
 add_clang_library(clangDaemon
   AST.cpp
   ClangdLSPServer.cpp
@@ -50,6 +55,7 @@
   clangToolingCore
   clangToolingRefactor
   ${LLVM_PTHREAD_LIB}
+  ${CLANGD_ATOMIC_LIB}
   )
 
 if( LLVM_LIB_FUZZING_ENGINE OR LLVM_USE_SANITIZE_COVERAGE )
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] r329053 - [clangd][cmake] Provide libatomic when there is no native support for 64bit atomics

2018-04-03 Thread Simon Dardis via cfe-commits
Author: sdardis
Date: Tue Apr  3 02:40:07 2018
New Revision: 329053

URL: http://llvm.org/viewvc/llvm-project?rev=329053&view=rev
Log:
[clangd][cmake] Provide libatomic when there is no native support for 64bit 
atomics

This addresses a persistent failure on clang-cmake-mips buildbot.

Reviewers: ioeric

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

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

Modified: clang-tools-extra/trunk/clangd/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CMakeLists.txt?rev=329053&r1=329052&r2=329053&view=diff
==
--- clang-tools-extra/trunk/clangd/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clangd/CMakeLists.txt Tue Apr  3 02:40:07 2018
@@ -2,6 +2,11 @@ set(LLVM_LINK_COMPONENTS
   Support
   )
 
+set(CLANGD_ATOMIC_LIB "")
+if(NOT HAVE_CXX_ATOMICS64_WITHOUT_LIB)
+  list(APPEND CLANGD_ATOMIC_LIB "atomic")
+endif()
+
 add_clang_library(clangDaemon
   AST.cpp
   ClangdLSPServer.cpp
@@ -50,6 +55,7 @@ add_clang_library(clangDaemon
   clangToolingCore
   clangToolingRefactor
   ${LLVM_PTHREAD_LIB}
+  ${CLANGD_ATOMIC_LIB}
   )
 
 if( LLVM_LIB_FUZZING_ENGINE OR LLVM_USE_SANITIZE_COVERAGE )


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


[PATCH] D45163: [Sema] -Wunused-value: diagnose unused std::move() call results.

2018-04-03 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment.

In https://reviews.llvm.org/D45163#1054994, @Quuxplusone wrote:

> `std::move` would definitely be special in this regard if there were a 
> pressing benefit to be gained — i.e., if people were currently getting bitten 
> by accidentally discarded calls of `std::move(x)`. But you haven't shown that 
> people are getting bitten today; in fact I think you said the opposite, 
> right? that there were *no* instances of this happening in the real codebases 
> you tested? So in that case, this diagnostic doesn't have a pressing benefit 
> IMHO, and Clang could safely wait for the library vendors to do the work.


@Quuxplusone Like i said in the differential's description,

> I have seen such a problem when reviewing https://reviews.llvm.org/D43341.

https://reviews.llvm.org/D43341?vs=137245&id=139869&whitespace=ignore-most#toc

Please see `clang-doc/Representation.cpp`, left hand side of that page, line 
`21`:

  static void mergeInfoBase(Info &L, Info &R) {
assert(L.USR == R.USR);
assert(L.Name == R.Name);
if (L.Namespace.empty()) std::move(R.Namespace); // <- here
  ...


Repository:
  rC Clang

https://reviews.llvm.org/D45163



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


[PATCH] D45006: [Tooling] A CompilationDatabase wrapper that infers header commands.

2018-04-03 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added inline comments.



Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:26
+// their compile commands. If getAllFilenames() is empty, no inference occurs.
+//
+//===--===//

Maybe add a comment describing the use-cases we had in mind while designing 
these heuristics? 
  - .cpp and .h files usually have the same (or slightly modified) name, 
usually the prefix match,
  - LLVM (and other) codebases that put .h and .cpp files into different 
directories,
  - even random matches are better than arbitrary defaults,
  - ...



Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:83
+template  int prefixCompare(StringRef S, StringRef Prefix) {
+  if (S.size() >= Prefix.size())
+return Reverse ? rMemCompare(S.end(), Prefix.end(), Prefix.size())

Summarizing the offline discussion, we could exclude suffix matches from the 
initial version. This would make the code much simpler, and it seems most C++ 
projects we know of would actually work with prefix-only matches.



Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:115
+for (auto F : getAllFiles())
+  Paths.emplace_back(Strings.save(F), 0);
+finalizeIndex();

This class seems to do two somewhat orthogonal things: 
  - build and query the index structure for the paths,
  - handle queries to inner CDB and patch the compile commands for other files 
accordingly.

Maybe we could extract the code that handles the index into a separate class?



Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:141
+  void finalizeIndex() {
+llvm::sort(Paths.begin(), Paths.end());
+for (size_t I = 0; I < Paths.size(); ++I) {

Maybe store lower-cased paths in the index and compare case-insensitively when 
querying?
Having slight case mismatches is not uncommon and case-sensitivity shouldn't 
ever be the defining factor for this kind of heuristics.



Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:147
+  auto Dir = ++sys::path::rbegin(Path), DirEnd = sys::path::rend(Path);
+  // Index up to 4 path components.
+  for (int J = 0; J < 4 && Dir != DirEnd; ++J, ++Dir)

Same as prev. comment, maybe comment on why 4 was chosen here? Maybe use a 
named constant?



Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:187
+StringRef Stem = sys::path::stem(Filename);
+llvm::SmallVector Dirs; // Only look up the last 2.
+llvm::StringRef Prefix;

Maybe add a comment why 2 is chosen here? Also, maybe use a named constant?


Repository:
  rC Clang

https://reviews.llvm.org/D45006



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


[PATCH] D45007: [clangd] Use compile-command interpolation to provide commands for header files.

2018-04-03 Thread 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.

Let's just always do it!


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45007



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


[PATCH] D45126: Xray, OpenBSD support

2018-04-03 Thread David CARLIER via Phabricator via cfe-commits
devnexen added a comment.

In https://reviews.llvm.org/D45126#1055266, @dberris wrote:

> LGTM -- at some point, it would be good to refactor all these flag settings 
> to a single place. Maybe file a bug so that we can track that issue on XRay? 
> If you can't do it now, I'd be happy to do it later on.


I do not have access to the bug tracker (yet) you can if you wish (I guess it s 
adding it in the "Common" driver ?


Repository:
  rC Clang

https://reviews.llvm.org/D45126



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


[PATCH] D45126: Xray, OpenBSD support

2018-04-03 Thread Dean Michael Berris via Phabricator via cfe-commits
dberris added a comment.

In https://reviews.llvm.org/D45126#1055451, @devnexen wrote:

> In https://reviews.llvm.org/D45126#1055266, @dberris wrote:
>
> > LGTM -- at some point, it would be good to refactor all these flag settings 
> > to a single place. Maybe file a bug so that we can track that issue on 
> > XRay? If you can't do it now, I'd be happy to do it later on.
>
>
> I do not have access to the bug tracker (yet) you can if you wish (I guess it 
> s adding it in the "Common" driver ?


Fixed: http://llvm.org/PR36985


Repository:
  rC Clang

https://reviews.llvm.org/D45126



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


[PATCH] D45126: Xray, OpenBSD support

2018-04-03 Thread Dean Michael Berris via Phabricator via cfe-commits
dberris added inline comments.



Comment at: lib/Driver/XRayArgs.cpp:53
+} else if (Triple.getOS() == llvm::Triple::FreeBSD ||
+  Triple.getOS() == llvm::Triple::OpenBSD) {
 if (Triple.getArch() != llvm::Triple::x86_64) {

I just noticed this, you probably want to format this part with clang-format 
(or just move the T of the second line one space, to align with the T on the 
previous line.


Repository:
  rC Clang

https://reviews.llvm.org/D45126



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


[PATCH] D45069: [clangd] synthesize fix message when the diagnostic doesn't provide one.

2018-04-03 Thread 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 with a small nit




Comment at: clangd/Diagnostics.cpp:322
+  StringRef Insert = FixIt.CodeToInsert;
+  if (!Invalid && Remove.size() + Insert.size() < 200) {
+llvm::raw_svector_ostream M(Message);

I don't really see a way out of it, but having a limit is a bit frustrating.

It seems weird to have `change 'foo' to 'bar'`, but `couldn't find 'fsdfsdf'. 
did you mean 'verylongidentifierthatsumsuptomorethan200'?` for the same error.
Maybe if the message that clang provides is also very long, use the generated 
message anyway?



Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45069



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


[PATCH] D44226: [clangd] Add -log-lsp-to-stderr option

2018-04-03 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added inline comments.



Comment at: clangd/ClangdLSPServer.cpp:412
+llvm::raw_string_ostream OS(Message);
+OS << "method not found (" << Method << ")";
+replyError(ErrorCode::MethodNotFound, OS.str());

malaperle wrote:
> ilya-biryukov wrote:
> > Could we also `log` (i.e. not `vlog`) names of the methods that were 
> > handled successfully? To have some context when something crashes and we 
> > only have non-verbose logs.
> Wouldn't that mean pretty much logging everything coming in? The idea of this 
> patch it to make it that by default Clangd is not talkative unless there is 
> an error and optionally make it verbose, for troubleshooting.
Logs are also useful to diagnose problems in case something crashes or works 
incorrectly.
Clang will probably log something when processing any request anyway, logging 
the method name first will merely give some more context on which request other 
log messages correspond to.

> The idea of this patch it to make it that by default Clangd is not talkative 
> unless there is an error.
I don't think we use logging this way in clangd. Logs give us a way to make 
sense of what's happening inside clangd even when there's no error. `vlog` lets 
us turn off some extremely noisy output that is not useful most of the time.
We have a whole bunch of log statements that don't correspond to errors (e.g. 
"reusing preamble", "building file with compile command").



Comment at: clangd/JSONRPCDispatcher.h:54
+  /// Whether we should log verbosely.
+  const bool Verbose;
+

This flag should be private.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44226



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


[PATCH] D45126: Xray, OpenBSD support

2018-04-03 Thread David CARLIER via Phabricator via cfe-commits
devnexen updated this revision to Diff 140762.

https://reviews.llvm.org/D45126

Files:
  lib/Driver/ToolChains/OpenBSD.cpp
  lib/Driver/XRayArgs.cpp


Index: lib/Driver/XRayArgs.cpp
===
--- lib/Driver/XRayArgs.cpp
+++ lib/Driver/XRayArgs.cpp
@@ -49,7 +49,8 @@
 D.Diag(diag::err_drv_clang_unsupported)
 << (std::string(XRayInstrumentOption) + " on " + Triple.str());
   }
-} else if (Triple.getOS() == llvm::Triple::FreeBSD) {
+} else if (Triple.getOS() == llvm::Triple::FreeBSD ||
+   Triple.getOS() == llvm::Triple::OpenBSD) {
 if (Triple.getArch() != llvm::Triple::x86_64) {
   D.Diag(diag::err_drv_clang_unsupported)
   << (std::string(XRayInstrumentOption) + " on " + Triple.str());
Index: lib/Driver/ToolChains/OpenBSD.cpp
===
--- lib/Driver/ToolChains/OpenBSD.cpp
+++ lib/Driver/ToolChains/OpenBSD.cpp
@@ -22,6 +22,29 @@
 using namespace clang;
 using namespace llvm::opt;
 
+static bool addXRayRuntime(const ToolChain &TC, const ArgList &Args,
+   ArgStringList &CmdArgs) {
+  if (Args.hasArg(options::OPT_shared))
+return false;
+
+  if (Args.hasFlag(options::OPT_fxray_instrument,
+   options::OPT_fnoxray_instrument, false)) {
+CmdArgs.push_back("-whole-archive");
+CmdArgs.push_back(TC.getCompilerRTArgString(Args, "xray", false));
+CmdArgs.push_back("-no-whole-archive");
+return true;
+  }
+
+  return false;
+}
+
+static void linkXRayRuntimeDeps(const ToolChain &TC, const ArgList &Args,
+ArgStringList &CmdArgs) {
+  CmdArgs.push_back("--no-as-needed");
+  CmdArgs.push_back("-lm");
+  CmdArgs.push_back("-lpthread");
+}
+
 void openbsd::Assembler::ConstructJob(Compilation &C, const JobAction &JA,
   const InputInfo &Output,
   const InputInfoList &Inputs,
@@ -180,6 +203,7 @@
 options::OPT_Z_Flag, options::OPT_r});
 
   bool NeedsSanitizerDeps = addSanitizerRuntimes(ToolChain, Args, CmdArgs);
+  bool NeedsXRayDeps = addXRayRuntime(ToolChain, Args, CmdArgs);
   AddLinkerInputs(getToolChain(), Inputs, Args, CmdArgs, JA);
 
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs)) {
@@ -195,6 +219,10 @@
   CmdArgs.push_back(ToolChain.getCompilerRTArgString(Args, "builtins", 
false));
   linkSanitizerRuntimeDeps(ToolChain, CmdArgs);
 }
+if (NeedsXRayDeps) {
+  CmdArgs.push_back(ToolChain.getCompilerRTArgString(Args, "builtins", 
false));
+  linkXRayRuntimeDeps(ToolChain, Args, CmdArgs);
+}
 // FIXME: For some reason GCC passes -lgcc before adding
 // the default system libraries. Just mimic this for now.
 CmdArgs.push_back("-lgcc");


Index: lib/Driver/XRayArgs.cpp
===
--- lib/Driver/XRayArgs.cpp
+++ lib/Driver/XRayArgs.cpp
@@ -49,7 +49,8 @@
 D.Diag(diag::err_drv_clang_unsupported)
 << (std::string(XRayInstrumentOption) + " on " + Triple.str());
   }
-} else if (Triple.getOS() == llvm::Triple::FreeBSD) {
+} else if (Triple.getOS() == llvm::Triple::FreeBSD ||
+   Triple.getOS() == llvm::Triple::OpenBSD) {
 if (Triple.getArch() != llvm::Triple::x86_64) {
   D.Diag(diag::err_drv_clang_unsupported)
   << (std::string(XRayInstrumentOption) + " on " + Triple.str());
Index: lib/Driver/ToolChains/OpenBSD.cpp
===
--- lib/Driver/ToolChains/OpenBSD.cpp
+++ lib/Driver/ToolChains/OpenBSD.cpp
@@ -22,6 +22,29 @@
 using namespace clang;
 using namespace llvm::opt;
 
+static bool addXRayRuntime(const ToolChain &TC, const ArgList &Args,
+   ArgStringList &CmdArgs) {
+  if (Args.hasArg(options::OPT_shared))
+return false;
+
+  if (Args.hasFlag(options::OPT_fxray_instrument,
+   options::OPT_fnoxray_instrument, false)) {
+CmdArgs.push_back("-whole-archive");
+CmdArgs.push_back(TC.getCompilerRTArgString(Args, "xray", false));
+CmdArgs.push_back("-no-whole-archive");
+return true;
+  }
+
+  return false;
+}
+
+static void linkXRayRuntimeDeps(const ToolChain &TC, const ArgList &Args,
+ArgStringList &CmdArgs) {
+  CmdArgs.push_back("--no-as-needed");
+  CmdArgs.push_back("-lm");
+  CmdArgs.push_back("-lpthread");
+}
+
 void openbsd::Assembler::ConstructJob(Compilation &C, const JobAction &JA,
   const InputInfo &Output,
   const InputInfoList &Inputs,
@@ -180,6 +203,7 @@
 options::OPT_Z_Flag, options::OPT_r});
 
   bool NeedsSanitizerDeps = addSanitizerRuntimes(ToolChain, Args, CmdArgs);
+  bool NeedsXRayDeps = addXRayRun

[PATCH] D45126: Xray, OpenBSD support

2018-04-03 Thread David CARLIER via Phabricator via cfe-commits
devnexen added a comment.



> Fixed: http://llvm.org/PR36985

Nice ! The NetBSD version will benefit it as well (not sure it s really done in 
the frontend side).


https://reviews.llvm.org/D45126



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


[PATCH] D45126: Xray, OpenBSD support

2018-04-03 Thread Dean Michael Berris via Phabricator via cfe-commits
dberris accepted this revision.
dberris added a comment.

LGTM -- I'll defer to @brad on confirming and landing.


https://reviews.llvm.org/D45126



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


[PATCH] D44732: [clangd] Set CLANGD_EDITOR environment variable in vscode extension.

2018-04-03 Thread Haojian Wu via Phabricator via cfe-commits
hokein abandoned this revision.
hokein added a comment.
Herald added a subscriber: MaskRay.

As discussed, we infer the editor from process tree which won't require changes 
in all clients.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44732



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


[PATCH] D44932: [CodeComplete] Fix completion in the middle of ident in ctor lists.

2018-04-03 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a reviewer: arphaman.
ilya-biryukov added a comment.

+Alex, in case he might know someone who can review it.


Repository:
  rC Clang

https://reviews.llvm.org/D44932



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


r329069 - [clang-format/ObjC] Do not insert space after opening brace of ObjC dict literal

2018-04-03 Thread Ben Hamilton via cfe-commits
Author: benhamilton
Date: Tue Apr  3 07:07:09 2018
New Revision: 329069

URL: http://llvm.org/viewvc/llvm-project?rev=329069&view=rev
Log:
[clang-format/ObjC] Do not insert space after opening brace of ObjC dict literal

Summary:
D44816 attempted to fix a few cases where `clang-format` incorrectly
inserted a space before the closing brace of an Objective-C dictionary
literal.

This revealed there were still a few cases where we inserted a space
after the opening brace of an Objective-C dictionary literal.

This fixes the formatting to be consistent and adds more tests.

Test Plan: New tests added. Confirmed tests failed before
  diff and passed after diff.
  Ran tests with:
  % make -j12 FormatTests && ./tools/clang/unittests/Format/FormatTests

Reviewers: djasper, jolesiak, krasimir

Reviewed By: djasper

Subscribers: klimek, cfe-commits

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

Modified:
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/unittests/Format/FormatTestObjC.cpp

Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=329069&r1=329068&r2=329069&view=diff
==
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Tue Apr  3 07:07:09 2018
@@ -2480,6 +2480,9 @@ bool TokenAnnotator::spaceRequiredBetwee
 return false;
   if (Left.is(TT_TemplateCloser) && Right.is(tok::l_square))
 return false;
+  if (Left.is(tok::l_brace) && Left.endsSequence(TT_DictLiteral, tok::at))
+// Objective-C dictionary literal -> no space after opening brace.
+return false;
   if (Right.is(tok::r_brace) && Right.MatchingParen &&
   Right.MatchingParen->endsSequence(TT_DictLiteral, tok::at))
 // Objective-C dictionary literal -> no space before closing brace.

Modified: cfe/trunk/unittests/Format/FormatTestObjC.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestObjC.cpp?rev=329069&r1=329068&r2=329069&view=diff
==
--- cfe/trunk/unittests/Format/FormatTestObjC.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestObjC.cpp Tue Apr  3 07:07:09 2018
@@ -1021,8 +1021,17 @@ TEST_F(FormatTestObjC, ObjCDictLiterals)
"  a12345 = @{a12345 : a12345};\n"
"}");
   verifyFormat("int Foo() {\n"
+   "  a12345 = @{a12345 : @(a12345)};\n"
+   "}");
+  verifyFormat("int Foo() {\n"
"  a12345 = @{(Foo *)a12345 : @(a12345)};\n"
"}");
+  verifyFormat("int Foo() {\n"
+   "  a12345 = @{@(a12345) : a12345};\n"
+   "}");
+  verifyFormat("int Foo() {\n"
+   "  a12345 = @{@(a12345) : @YES};\n"
+   "}");
   Style.SpacesInContainerLiterals = false;
   verifyFormat("int Foo() {\n"
"  b12345 = @{b12345: b12345};\n"


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


r329070 - [clang-format/ObjC] Do not detect "[]" as ObjC method expression

2018-04-03 Thread Ben Hamilton via cfe-commits
Author: benhamilton
Date: Tue Apr  3 07:07:11 2018
New Revision: 329070

URL: http://llvm.org/viewvc/llvm-project?rev=329070&view=rev
Log:
[clang-format/ObjC] Do not detect "[]" as ObjC method expression

Summary:
The following C++ code was being detected by
`guessLanguage()` as Objective-C:

  #define FOO(...) auto bar = [] __VA_ARGS__;

This was because `[] __VA_ARGS__` is not currently detected as a C++
lambda expression (it has no parens or braces), so
`TokenAnnotator::parseSquare()` incorrectly treats the opening square
as an ObjC method expression.

We have two options to fix this:

1. Parse `[] __VA_ARGS__` explicitly as a C++ lambda
2. Make it so `[]` is never parsed as an Objective-C method expression

This diff implements option 2, which causes the `[` to be parsed
as `TT_ArraySubscriptLSquare` instead of `TT_ObjCMethodExpr`.

Note that when I fixed this, it caused one change in formatting
behavior, where the following was implicitly relying on the `[`
being parsed as `TT_ObjCMethodExpr`:

  A a;

becomes:

  A a;

with `Style.PointerAlignment = Middle`.

I don't really know what the desired format is for this syntax; the
test was added by Janusz Sobczak and integrated by @djasper in
https://github.com/llvm-mirror/clang/commit/b511fe9818829d7ece0cc0b2ce1fbe04a1f0739a
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D45168: [clang-format/ObjC] Do not insert space after opening brace of ObjC dict literal

2018-04-03 Thread Ben Hamilton via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL329069: [clang-format/ObjC] Do not insert space after 
opening brace of ObjC dict literal (authored by benhamilton, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D45168?vs=140634&id=140781#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D45168

Files:
  cfe/trunk/lib/Format/TokenAnnotator.cpp
  cfe/trunk/unittests/Format/FormatTestObjC.cpp


Index: cfe/trunk/lib/Format/TokenAnnotator.cpp
===
--- cfe/trunk/lib/Format/TokenAnnotator.cpp
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp
@@ -2480,6 +2480,9 @@
 return false;
   if (Left.is(TT_TemplateCloser) && Right.is(tok::l_square))
 return false;
+  if (Left.is(tok::l_brace) && Left.endsSequence(TT_DictLiteral, tok::at))
+// Objective-C dictionary literal -> no space after opening brace.
+return false;
   if (Right.is(tok::r_brace) && Right.MatchingParen &&
   Right.MatchingParen->endsSequence(TT_DictLiteral, tok::at))
 // Objective-C dictionary literal -> no space before closing brace.
Index: cfe/trunk/unittests/Format/FormatTestObjC.cpp
===
--- cfe/trunk/unittests/Format/FormatTestObjC.cpp
+++ cfe/trunk/unittests/Format/FormatTestObjC.cpp
@@ -1021,8 +1021,17 @@
"  a12345 = @{a12345 : a12345};\n"
"}");
   verifyFormat("int Foo() {\n"
+   "  a12345 = @{a12345 : @(a12345)};\n"
+   "}");
+  verifyFormat("int Foo() {\n"
"  a12345 = @{(Foo *)a12345 : @(a12345)};\n"
"}");
+  verifyFormat("int Foo() {\n"
+   "  a12345 = @{@(a12345) : a12345};\n"
+   "}");
+  verifyFormat("int Foo() {\n"
+   "  a12345 = @{@(a12345) : @YES};\n"
+   "}");
   Style.SpacesInContainerLiterals = false;
   verifyFormat("int Foo() {\n"
"  b12345 = @{b12345: b12345};\n"


Index: cfe/trunk/lib/Format/TokenAnnotator.cpp
===
--- cfe/trunk/lib/Format/TokenAnnotator.cpp
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp
@@ -2480,6 +2480,9 @@
 return false;
   if (Left.is(TT_TemplateCloser) && Right.is(tok::l_square))
 return false;
+  if (Left.is(tok::l_brace) && Left.endsSequence(TT_DictLiteral, tok::at))
+// Objective-C dictionary literal -> no space after opening brace.
+return false;
   if (Right.is(tok::r_brace) && Right.MatchingParen &&
   Right.MatchingParen->endsSequence(TT_DictLiteral, tok::at))
 // Objective-C dictionary literal -> no space before closing brace.
Index: cfe/trunk/unittests/Format/FormatTestObjC.cpp
===
--- cfe/trunk/unittests/Format/FormatTestObjC.cpp
+++ cfe/trunk/unittests/Format/FormatTestObjC.cpp
@@ -1021,8 +1021,17 @@
"  a12345 = @{a12345 : a12345};\n"
"}");
   verifyFormat("int Foo() {\n"
+   "  a12345 = @{a12345 : @(a12345)};\n"
+   "}");
+  verifyFormat("int Foo() {\n"
"  a12345 = @{(Foo *)a12345 : @(a12345)};\n"
"}");
+  verifyFormat("int Foo() {\n"
+   "  a12345 = @{@(a12345) : a12345};\n"
+   "}");
+  verifyFormat("int Foo() {\n"
+   "  a12345 = @{@(a12345) : @YES};\n"
+   "}");
   Style.SpacesInContainerLiterals = false;
   verifyFormat("int Foo() {\n"
"  b12345 = @{b12345: b12345};\n"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D45169: [clang-format/ObjC] Do not detect "[]" as ObjC method expression

2018-04-03 Thread Ben Hamilton via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL329070: [clang-format/ObjC] Do not detect "[]" as 
ObjC method expression (authored by benhamilton, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D45169

Files:
  cfe/trunk/lib/Format/TokenAnnotator.cpp
  cfe/trunk/unittests/Format/FormatTest.cpp


Index: cfe/trunk/lib/Format/TokenAnnotator.cpp
===
--- cfe/trunk/lib/Format/TokenAnnotator.cpp
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp
@@ -386,7 +386,7 @@
 bool StartsObjCMethodExpr =
 !CppArrayTemplates && Style.isCpp() && !IsCpp11AttributeSpecifier &&
 Contexts.back().CanBeExpression && Left->isNot(TT_LambdaLSquare) &&
-CurrentToken->isNot(tok::l_brace) &&
+!CurrentToken->isOneOf(tok::l_brace, tok::r_square) &&
 (!Parent ||
  Parent->isOneOf(tok::colon, tok::l_square, tok::l_paren,
  tok::kw_return, tok::kw_throw) ||
Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -6035,14 +6035,15 @@
   PointerMiddle.PointerAlignment = FormatStyle::PAS_Middle;
   verifyFormat("delete *x;", PointerMiddle);
   verifyFormat("int * x;", PointerMiddle);
+  verifyFormat("int *[] x;", PointerMiddle);
   verifyFormat("template  f() {}", PointerMiddle);
   verifyFormat("int * f(int * a) {}", PointerMiddle);
   verifyFormat("int main(int argc, char ** argv) {}", PointerMiddle);
   verifyFormat("Test::Test(int b) : a(b * b) {}", PointerMiddle);
   verifyFormat("A a;", PointerMiddle);
   verifyFormat("A a;", PointerMiddle);
   verifyFormat("A a;", PointerMiddle);
-  verifyFormat("A a;", PointerMiddle);
+  verifyFormat("A a;", PointerMiddle);
   verifyFormat("A = new SomeType *[Length]();", PointerMiddle);
   verifyFormat("A = new SomeType *[Length];", PointerMiddle);
   verifyFormat("T ** t = new T *;", PointerMiddle);
@@ -12106,6 +12107,9 @@
   FormatStyle::LK_ObjC,
   guessLanguage("foo.h",
 "#define MY_POINT_MAKE(x, y) CGPointMake((x), (y));\n"));
+  EXPECT_EQ(
+  FormatStyle::LK_Cpp,
+  guessLanguage("foo.h", "#define FOO(...) auto bar = [] __VA_ARGS__;"));
 }
 
 TEST_F(FormatTest, GuessLanguageWithCpp11AttributeSpecifiers) {


Index: cfe/trunk/lib/Format/TokenAnnotator.cpp
===
--- cfe/trunk/lib/Format/TokenAnnotator.cpp
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp
@@ -386,7 +386,7 @@
 bool StartsObjCMethodExpr =
 !CppArrayTemplates && Style.isCpp() && !IsCpp11AttributeSpecifier &&
 Contexts.back().CanBeExpression && Left->isNot(TT_LambdaLSquare) &&
-CurrentToken->isNot(tok::l_brace) &&
+!CurrentToken->isOneOf(tok::l_brace, tok::r_square) &&
 (!Parent ||
  Parent->isOneOf(tok::colon, tok::l_square, tok::l_paren,
  tok::kw_return, tok::kw_throw) ||
Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -6035,14 +6035,15 @@
   PointerMiddle.PointerAlignment = FormatStyle::PAS_Middle;
   verifyFormat("delete *x;", PointerMiddle);
   verifyFormat("int * x;", PointerMiddle);
+  verifyFormat("int *[] x;", PointerMiddle);
   verifyFormat("template  f() {}", PointerMiddle);
   verifyFormat("int * f(int * a) {}", PointerMiddle);
   verifyFormat("int main(int argc, char ** argv) {}", PointerMiddle);
   verifyFormat("Test::Test(int b) : a(b * b) {}", PointerMiddle);
   verifyFormat("A a;", PointerMiddle);
   verifyFormat("A a;", PointerMiddle);
   verifyFormat("A a;", PointerMiddle);
-  verifyFormat("A a;", PointerMiddle);
+  verifyFormat("A a;", PointerMiddle);
   verifyFormat("A = new SomeType *[Length]();", PointerMiddle);
   verifyFormat("A = new SomeType *[Length];", PointerMiddle);
   verifyFormat("T ** t = new T *;", PointerMiddle);
@@ -12106,6 +12107,9 @@
   FormatStyle::LK_ObjC,
   guessLanguage("foo.h",
 "#define MY_POINT_MAKE(x, y) CGPointMake((x), (y));\n"));
+  EXPECT_EQ(
+  FormatStyle::LK_Cpp,
+  guessLanguage("foo.h", "#define FOO(...) auto bar = [] __VA_ARGS__;"));
 }
 
 TEST_F(FormatTest, GuessLanguageWithCpp11AttributeSpecifiers) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D44883: [Sema] Extend -Wself-assign and -Wself-assign-field to warn on overloaded self-assignment (classes)

2018-04-03 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 140787.
lebedev.ri added a comment.

- Rebased
- Silence both of the diagnostics in an unevaluated context.
- Fixed `-Wself-assign-field` preexisting problems:
  - False-positives on `volatile` stores.
  - Do not warn in template instantiations.
  - Handle all the macro cases (do not warn)
- +Tests

I'm going to run it on some project next.


Repository:
  rC Clang

https://reviews.llvm.org/D44883

Files:
  docs/ReleaseNotes.rst
  lib/Sema/SemaExpr.cpp
  test/SemaCXX/member-init.cpp
  test/SemaCXX/warn-self-assign-builtin.cpp
  test/SemaCXX/warn-self-assign-field-builtin.cpp
  test/SemaCXX/warn-self-assign-field-overloaded.cpp
  test/SemaCXX/warn-self-assign-overloaded.cpp
  test/SemaCXX/warn-self-assign.cpp

Index: test/SemaCXX/warn-self-assign.cpp
===
--- test/SemaCXX/warn-self-assign.cpp
+++ /dev/null
@@ -1,50 +0,0 @@
-// RUN: %clang_cc1 -fsyntax-only -Wself-assign -verify %s
-
-void f() {
-  int a = 42, b = 42;
-  a = a; // expected-warning{{explicitly assigning}}
-  b = b; // expected-warning{{explicitly assigning}}
-  a = b;
-  b = a = b;
-  a = a = a; // expected-warning{{explicitly assigning}}
-  a = b = b = a;
-  a &= a; // expected-warning{{explicitly assigning}}
-  a |= a; // expected-warning{{explicitly assigning}}
-  a ^= a;
-}
-
-// Dummy type.
-struct S {};
-
-void false_positives() {
-#define OP =
-#define LHS a
-#define RHS a
-  int a = 42;
-  // These shouldn't warn due to the use of the preprocessor.
-  a OP a;
-  LHS = a;
-  a = RHS;
-  LHS OP RHS;
-#undef OP
-#undef LHS
-#undef RHS
-
-  S s;
-  s = s; // Not a builtin assignment operator, no warning.
-
-  // Volatile stores aren't side-effect free.
-  volatile int vol_a;
-  vol_a = vol_a;
-  volatile int &vol_a_ref = vol_a;
-  vol_a_ref = vol_a_ref;
-}
-
-template  void g() {
-  T a;
-  a = a; // May or may not be a builtin assignment operator, no warning.
-}
-void instantiate() {
-  g();
-  g();
-}
Index: test/SemaCXX/warn-self-assign-overloaded.cpp
===
--- /dev/null
+++ test/SemaCXX/warn-self-assign-overloaded.cpp
@@ -0,0 +1,109 @@
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign -DDUMMY -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign -DV0 -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign -DV1 -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign -DV2 -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign -DV3 -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wself-assign -DV4 -verify %s
+
+#ifdef DUMMY
+struct S {};
+#else
+struct S {
+#if defined(V0)
+  S() = default;
+#elif defined(V1)
+  S &operator=(const S &) = default;
+#elif defined(V2)
+  S &operator=(S &) = default;
+#elif defined(V3)
+  S &operator=(const S &);
+#elif defined(V4)
+  S &operator=(S &);
+#else
+#error Define something!
+#endif
+  S &operator*=(const S &);
+  S &operator/=(const S &);
+  S &operator%=(const S &);
+  S &operator+=(const S &);
+  S &operator-=(const S &);
+  S &operator<<=(const S &);
+  S &operator>>=(const S &);
+  S &operator&=(const S &);
+  S &operator|=(const S &);
+  S &operator^=(const S &);
+  S &operator=(const volatile S &) volatile;
+};
+#endif
+
+void f() {
+  S a, b;
+  a = a; // expected-warning{{explicitly assigning}}
+  b = b; // expected-warning{{explicitly assigning}}
+  a = b;
+  b = a = b;
+  a = a = a; // expected-warning{{explicitly assigning}}
+  a = b = b = a;
+
+#ifndef DUMMY
+  a *= a;
+  a /= a; // expected-warning {{explicitly assigning}}
+  a %= a; // expected-warning {{explicitly assigning}}
+  a += a;
+  a -= a; // expected-warning {{explicitly assigning}}
+  a <<= a;
+  a >>= a;
+  a &= a; // expected-warning {{explicitly assigning}}
+  a |= a; // expected-warning {{explicitly assigning}}
+  a ^= a; // expected-warning {{explicitly assigning}}
+#endif
+}
+
+void false_positives() {
+#define OP =
+#define LHS a
+#define RHS a
+  S a;
+  // These shouldn't warn due to the use of the preprocessor.
+  a OP a;
+  LHS = a;
+  a = RHS;
+  LHS OP RHS;
+#undef OP
+#undef LHS
+#undef RHS
+
+  // Ways to silence the warning.
+  a = *&a;
+  a = (S &)a;
+  a = static_cast(a);
+
+#ifndef DUMMY
+  // Volatile stores aren't side-effect free.
+  volatile S vol_a;
+  vol_a = vol_a;
+  volatile S &vol_a_ref = vol_a;
+  vol_a_ref = vol_a_ref;
+#endif
+}
+
+// Do not diagnose self-assigment in an unevaluated context
+struct SNoExcept {
+  SNoExcept() = default;
+  SNoExcept &operator=(const SNoExcept &) noexcept;
+};
+void false_positives_unevaluated_ctx(SNoExcept a) noexcept(noexcept(a = a)) {
+  decltype(a = a) b = a;
+  static_assert(noexcept(a = a), "");
+  static_assert(sizeof(a = a), "");
+}
+
+template 
+void g() {
+  T a;
+  a = a; // expected-warning{{explicitly assigning}}
+}
+void instantiate() {
+  g();
+  g();
+}
Index: test/SemaCXX/warn-self-assign-field-overloaded.cpp
=

[PATCH] D45128: [libcxx][test] Silence -Wself-assign diagnostics

2018-04-03 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 140788.
lebedev.ri added a comment.

The diagnostic was adjusted not to warn in an unevaluated context.
The remaining diff will have to remain, unless you want to disable 
`-Wself-assign` altogether for tests...

If there are remaining problems with this diff, please let me know; otherwise 
it would be **really** great to get this accepted!


Repository:
  rCXX libc++

https://reviews.llvm.org/D45128

Files:
  test/std/utilities/any/any.class/any.assign/copy.pass.cpp
  
test/std/utilities/function.objects/func.wrap/func.wrap.func/func.wrap.func.con/copy_assign.pass.cpp


Index: 
test/std/utilities/function.objects/func.wrap/func.wrap.func/func.wrap.func.con/copy_assign.pass.cpp
===
--- 
test/std/utilities/function.objects/func.wrap/func.wrap.func/func.wrap.func.con/copy_assign.pass.cpp
+++ 
test/std/utilities/function.objects/func.wrap/func.wrap.func/func.wrap.func.con/copy_assign.pass.cpp
@@ -92,28 +92,28 @@
   {
 typedef std::function Func;
 Func f = g0;
-Func& fr = (f = f);
+Func& fr = (f = (Func &)f);
 assert(&fr == &f);
 assert(*f.target() == g0);
   }
   {
 typedef std::function Func;
 Func f = g;
-Func& fr = (f = f);
+Func& fr = (f = (Func &)f);
 assert(&fr == &f);
 assert(*f.target() == g);
   }
   {
 typedef std::function Func;
 Func f = g2;
-Func& fr = (f = f);
+Func& fr = (f = (Func &)f);
 assert(&fr == &f);
 assert(*f.target() == g2);
   }
   {
 typedef std::function Func;
 Func f = g3;
-Func& fr = (f = f);
+Func& fr = (f = (Func &)f);
 assert(&fr == &f);
 assert(*f.target() == g3);
   }
Index: test/std/utilities/any/any.class/any.assign/copy.pass.cpp
===
--- test/std/utilities/any/any.class/any.assign/copy.pass.cpp
+++ test/std/utilities/any/any.class/any.assign/copy.pass.cpp
@@ -102,7 +102,7 @@
 // empty
 {
 any a;
-a = a;
+a = (any &)a;
 assertEmpty(a);
 assert(globalMemCounter.checkOutstandingNewEq(0));
 }
@@ -112,7 +112,7 @@
 any a((small(1)));
 assert(small::count == 1);
 
-a = a;
+a = (any &)a;
 
 assert(small::count == 1);
 assertContains(a, 1);
@@ -125,7 +125,7 @@
 any a(large(1));
 assert(large::count == 1);
 
-a = a;
+a = (any &)a;
 
 assert(large::count == 1);
 assertContains(a, 1);


Index: test/std/utilities/function.objects/func.wrap/func.wrap.func/func.wrap.func.con/copy_assign.pass.cpp
===
--- test/std/utilities/function.objects/func.wrap/func.wrap.func/func.wrap.func.con/copy_assign.pass.cpp
+++ test/std/utilities/function.objects/func.wrap/func.wrap.func/func.wrap.func.con/copy_assign.pass.cpp
@@ -92,28 +92,28 @@
   {
 typedef std::function Func;
 Func f = g0;
-Func& fr = (f = f);
+Func& fr = (f = (Func &)f);
 assert(&fr == &f);
 assert(*f.target() == g0);
   }
   {
 typedef std::function Func;
 Func f = g;
-Func& fr = (f = f);
+Func& fr = (f = (Func &)f);
 assert(&fr == &f);
 assert(*f.target() == g);
   }
   {
 typedef std::function Func;
 Func f = g2;
-Func& fr = (f = f);
+Func& fr = (f = (Func &)f);
 assert(&fr == &f);
 assert(*f.target() == g2);
   }
   {
 typedef std::function Func;
 Func f = g3;
-Func& fr = (f = f);
+Func& fr = (f = (Func &)f);
 assert(&fr == &f);
 assert(*f.target() == g3);
   }
Index: test/std/utilities/any/any.class/any.assign/copy.pass.cpp
===
--- test/std/utilities/any/any.class/any.assign/copy.pass.cpp
+++ test/std/utilities/any/any.class/any.assign/copy.pass.cpp
@@ -102,7 +102,7 @@
 // empty
 {
 any a;
-a = a;
+a = (any &)a;
 assertEmpty(a);
 assert(globalMemCounter.checkOutstandingNewEq(0));
 }
@@ -112,7 +112,7 @@
 any a((small(1)));
 assert(small::count == 1);
 
-a = a;
+a = (any &)a;
 
 assert(small::count == 1);
 assertContains(a, 1);
@@ -125,7 +125,7 @@
 any a(large(1));
 assert(large::count == 1);
 
-a = a;
+a = (any &)a;
 
 assert(large::count == 1);
 assertContains(a, 1);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D45006: [Tooling] A CompilationDatabase wrapper that infers header commands.

2018-04-03 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments.



Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:115
+for (auto F : getAllFiles())
+  Paths.emplace_back(Strings.save(F), 0);
+finalizeIndex();

ilya-biryukov wrote:
> This class seems to do two somewhat orthogonal things: 
>   - build and query the index structure for the paths,
>   - handle queries to inner CDB and patch the compile commands for other 
> files accordingly.
> 
> Maybe we could extract the code that handles the index into a separate class?
I split out FilenameIndex, which just does the filename->filename mapping. The 
CompilationDatabase implementation now only contains the interface methods and 
adjust().


Repository:
  rC Clang

https://reviews.llvm.org/D45006



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


[PATCH] D44231: [clang-tidy] Check for sizeof that call functions

2018-04-03 Thread Paul Fultz II via Phabricator via cfe-commits
pfultz2 updated this revision to Diff 140790.
pfultz2 added a comment.

I have rebased.


https://reviews.llvm.org/D44231

Files:
  clang-tidy/bugprone/SizeofExpressionCheck.cpp
  clang-tidy/bugprone/SizeofExpressionCheck.h
  docs/clang-tidy/checks/bugprone-sizeof-expression.rst
  test/clang-tidy/bugprone-sizeof-expression.cpp

Index: test/clang-tidy/bugprone-sizeof-expression.cpp
===
--- test/clang-tidy/bugprone-sizeof-expression.cpp
+++ test/clang-tidy/bugprone-sizeof-expression.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s bugprone-sizeof-expression %t
+// RUN: %check_clang_tidy %s bugprone-sizeof-expression %t -- -config="{CheckOptions: [{key: bugprone-sizeof-expression.WarnOnSizeOfIntegerExpression, value: 1}]}" --
 
 class C {
   int size() { return sizeof(this); }
@@ -14,14 +14,82 @@
 #pragma pack(1)
 struct  S { char a, b, c; };
 
+enum E { E_VALUE = 0 };
+enum class EC { VALUE = 0 };
+
+bool AsBool() { return false; }
+int AsInt() { return 0; }
+E AsEnum() { return E_VALUE; }
+EC AsEnumClass() { return EC::VALUE; }
+S AsStruct() { return {}; }
+
+struct M {
+  int AsInt() { return 0; }
+  E AsEnum() { return E_VALUE; }
+  S AsStruct() { return {}; }
+};
+
+int ReturnOverload(int) { return {}; }
+S ReturnOverload(S) { return {}; }
+
+template 
+T ReturnTemplate(T) { return {}; }
+
+template 
+bool TestTrait1() {
+  return sizeof(ReturnOverload(T{})) == sizeof(A);
+}
+
+template 
+bool TestTrait2() {
+  return sizeof(ReturnTemplate(T{})) == sizeof(A);
+}
+
+template 
+bool TestTrait3() {
+  return sizeof(ReturnOverload(0)) == sizeof(T{});
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in an integer
+}
+
+template 
+bool TestTrait4() {
+  return sizeof(ReturnTemplate(0)) == sizeof(T{});
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in an integer
+}
+
+bool TestTemplates() {
+  bool b = true;
+  b &= TestTrait1();
+  b &= TestTrait1();
+  b &= TestTrait2();
+  b &= TestTrait2();
+  b &= TestTrait3();
+  b &= TestTrait3();
+  b &= TestTrait4();
+  b &= TestTrait4();
+  return b;
+}
+
 int Test1(const char* ptr) {
   int sum = 0;
   sum += sizeof(LEN);
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(K)'
   sum += sizeof(LEN + 1);
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(K)'
   sum += sizeof(sum, LEN);
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(..., ...)'
+  sum += sizeof(AsBool());
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in an integer
+  sum += sizeof(AsInt());
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in an integer
+  sum += sizeof(AsEnum());
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in an integer
+  sum += sizeof(AsEnumClass());
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in an integer
+  sum += sizeof(M{}.AsInt());
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in an integer
+  sum += sizeof(M{}.AsEnum());
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof()' on an expression that results in an integer
   sum += sizeof(sizeof(X));
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(sizeof(...))'
   sum += sizeof(LEN + sizeof(X));
@@ -171,6 +239,8 @@
   if (sizeof(A) < 10)
 sum += sizeof(A);
   sum += sizeof(int);
+  sum += sizeof(AsStruct());
+  sum += sizeof(M{}.AsStruct());
   sum += sizeof(A[sizeof(A) / sizeof(int)]);
   sum += sizeof(&A[sizeof(A) / sizeof(int)]);
   sum += sizeof(sizeof(0));  // Special case: sizeof size_t.
Index: docs/clang-tidy/checks/bugprone-sizeof-expression.rst
===
--- docs/clang-tidy/checks/bugprone-sizeof-expression.rst
+++ docs/clang-tidy/checks/bugprone-sizeof-expression.rst
@@ -22,6 +22,36 @@
   char buf[BUFLEN];
   memset(buf, 0, sizeof(BUFLEN));  // sizeof(42) ==> sizeof(int)
 
+Suspicious usage of 'sizeof(expr)'
+--
+
+In cases, where there is an enum or integer to represent a type, a common
+mistake is to query the ``sizeof`` on the integer or enum that represents the
+type that should be used by ``sizeof``. This results in the size of the integer
+and not of the type the integer represents:
+
+.. code-block:: c++
+
+  enum data_type {
+FLOAT_TYPE,
+DOUBLE_TYPE
+  };
+
+  struct data {
+data_type type;
+void* buffer;
+data_type get_type() {
+  return type;
+}
+  };
+
+  void f(data d, int numElements) {
+// should be sizeof(float) or sizeof(double), depending on d.get_type()
+

[PATCH] D45006: [Tooling] A CompilationDatabase wrapper that infers header commands.

2018-04-03 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 140791.
sammccall marked 6 inline comments as done.
sammccall added a comment.

Address review comments.


Repository:
  rC Clang

https://reviews.llvm.org/D45006

Files:
  include/clang/Tooling/CompilationDatabase.h
  lib/Tooling/CMakeLists.txt
  lib/Tooling/InterpolatingCompilationDatabase.cpp
  unittests/Tooling/CompilationDatabaseTest.cpp

Index: unittests/Tooling/CompilationDatabaseTest.cpp
===
--- unittests/Tooling/CompilationDatabaseTest.cpp
+++ unittests/Tooling/CompilationDatabaseTest.cpp
@@ -626,5 +626,99 @@
   EXPECT_EQ(2, Argc);
 }
 
+struct MemCDB : public CompilationDatabase {
+  using EntryMap = llvm::StringMap>;
+  EntryMap Entries;
+  MemCDB(const EntryMap &E) : Entries(E) {}
+
+  std::vector getCompileCommands(StringRef F) const override {
+auto Ret = Entries.lookup(F);
+return {Ret.begin(), Ret.end()};
+  }
+
+  std::vector getAllFiles() const override {
+std::vector Result;
+for (const auto &Entry : Entries)
+  Result.push_back(Entry.first());
+return Result;
+  }
+};
+
+class InterpolateTest : public ::testing::Test {
+protected:
+  // Adds an entry to the underlying compilation database.
+  // A flag is injected: -D , so the command used can be identified.
+  void add(llvm::StringRef File, llvm::StringRef Flags = "") {
+llvm::SmallVector Argv = {"clang", File, "-D", File};
+llvm::SplitString(Flags, Argv);
+llvm::SmallString<32> Dir;
+llvm::sys::path::system_temp_directory(false, Dir);
+Entries[path(File)].push_back(
+{Dir, File, {Argv.begin(), Argv.end()}, "foo.o"});
+  }
+
+  // Turn a unix path fragment (foo/bar.h) into a native path (C:\tmp\foo\bar.h)
+  std::string path(llvm::SmallString<32> File) {
+llvm::SmallString<32> Dir;
+llvm::sys::path::system_temp_directory(false, Dir);
+llvm::sys::path::native(File);
+llvm::SmallString<64> Result;
+llvm::sys::path::append(Result, Dir, File);
+return Result.str();
+  }
+
+  // Look up the command from a relative path, and return it in string form.
+  // The input file is not included in the returned command.
+  std::string getCommand(llvm::StringRef F) {
+auto Results =
+inferMissingCompileCommands(llvm::make_unique(Entries))
+->getCompileCommands(path(F));
+if (Results.empty())
+  return "none";
+// drop the input file argument, so tests don't have to deal with path().
+EXPECT_EQ(Results[0].CommandLine.back(), path(F))
+<< "Last arg should be the file";
+Results[0].CommandLine.pop_back();
+return llvm::join(Results[0].CommandLine, " ");
+  }
+
+  MemCDB::EntryMap Entries;
+};
+
+TEST_F(InterpolateTest, Nearby) {
+  add("dir/foo.cpp");
+  add("dir/bar.cpp");
+  add("an/other/foo.cpp");
+
+  // great: dir and name both match (prefix or full, case insensitive)
+  EXPECT_EQ(getCommand("dir/f.cpp"), "clang -D dir/foo.cpp");
+  EXPECT_EQ(getCommand("dir/FOO.cpp"), "clang -D dir/foo.cpp");
+  // no name match. prefer matching dir, break ties by alpha
+  EXPECT_EQ(getCommand("dir/a.cpp"), "clang -D dir/bar.cpp");
+  // an exact name match beats one segment of directory match
+  EXPECT_EQ(getCommand("some/other/bar.h"), "clang -x c++ -D dir/bar.cpp");
+  // two segments of directory match beat a prefix name match
+  EXPECT_EQ(getCommand("an/other/b.cpp"), "clang -D an/other/foo.cpp");
+  // if nothing matches at all, we still get the closest alpha match
+  EXPECT_EQ(getCommand("below/some/obscure/path.cpp"),
+"clang -D an/other/foo.cpp");
+}
+
+TEST_F(InterpolateTest, Language) {
+  add("dir/foo.cpp");
+  add("dir/baz.cee", "-x c");
+
+  // extension changed, so we add explicit language flags
+  EXPECT_EQ(getCommand("foo.h"), "clang -x c++ -D dir/foo.cpp");
+  // but we don't add -x if it's already there
+  EXPECT_EQ(getCommand("baz.h"), "clang -D dir/baz.cee -x c");
+}
+
+TEST_F(InterpolateTest, Strip) {
+  add("dir/foo.cpp", "-o foo.o -Wall");
+  // the -o option and the input file are removed, but -Wall is preserved.
+  EXPECT_EQ(getCommand("dir/bar.cpp"), "clang -D dir/foo.cpp -Wall");
+}
+
 } // end namespace tooling
 } // end namespace clang
Index: lib/Tooling/InterpolatingCompilationDatabase.cpp
===
--- /dev/null
+++ lib/Tooling/InterpolatingCompilationDatabase.cpp
@@ -0,0 +1,329 @@
+//===- InterpolatingCompilationDatabase.cpp -*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// InterpolatingCompilationDatabase wraps another CompilationDatabase and
+// attempts to heuristically determine appropriate compile commands for files
+// that are not included, such as headers or newly cr

[PATCH] D45006: [Tooling] A CompilationDatabase wrapper that infers header commands.

2018-04-03 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 140792.
sammccall added a comment.

clang-format


Repository:
  rC Clang

https://reviews.llvm.org/D45006

Files:
  include/clang/Tooling/CompilationDatabase.h
  lib/Tooling/CMakeLists.txt
  lib/Tooling/InterpolatingCompilationDatabase.cpp
  unittests/Tooling/CompilationDatabaseTest.cpp

Index: unittests/Tooling/CompilationDatabaseTest.cpp
===
--- unittests/Tooling/CompilationDatabaseTest.cpp
+++ unittests/Tooling/CompilationDatabaseTest.cpp
@@ -626,5 +626,99 @@
   EXPECT_EQ(2, Argc);
 }
 
+struct MemCDB : public CompilationDatabase {
+  using EntryMap = llvm::StringMap>;
+  EntryMap Entries;
+  MemCDB(const EntryMap &E) : Entries(E) {}
+
+  std::vector getCompileCommands(StringRef F) const override {
+auto Ret = Entries.lookup(F);
+return {Ret.begin(), Ret.end()};
+  }
+
+  std::vector getAllFiles() const override {
+std::vector Result;
+for (const auto &Entry : Entries)
+  Result.push_back(Entry.first());
+return Result;
+  }
+};
+
+class InterpolateTest : public ::testing::Test {
+protected:
+  // Adds an entry to the underlying compilation database.
+  // A flag is injected: -D , so the command used can be identified.
+  void add(llvm::StringRef File, llvm::StringRef Flags = "") {
+llvm::SmallVector Argv = {"clang", File, "-D", File};
+llvm::SplitString(Flags, Argv);
+llvm::SmallString<32> Dir;
+llvm::sys::path::system_temp_directory(false, Dir);
+Entries[path(File)].push_back(
+{Dir, File, {Argv.begin(), Argv.end()}, "foo.o"});
+  }
+
+  // Turn a unix path fragment (foo/bar.h) into a native path (C:\tmp\foo\bar.h)
+  std::string path(llvm::SmallString<32> File) {
+llvm::SmallString<32> Dir;
+llvm::sys::path::system_temp_directory(false, Dir);
+llvm::sys::path::native(File);
+llvm::SmallString<64> Result;
+llvm::sys::path::append(Result, Dir, File);
+return Result.str();
+  }
+
+  // Look up the command from a relative path, and return it in string form.
+  // The input file is not included in the returned command.
+  std::string getCommand(llvm::StringRef F) {
+auto Results =
+inferMissingCompileCommands(llvm::make_unique(Entries))
+->getCompileCommands(path(F));
+if (Results.empty())
+  return "none";
+// drop the input file argument, so tests don't have to deal with path().
+EXPECT_EQ(Results[0].CommandLine.back(), path(F))
+<< "Last arg should be the file";
+Results[0].CommandLine.pop_back();
+return llvm::join(Results[0].CommandLine, " ");
+  }
+
+  MemCDB::EntryMap Entries;
+};
+
+TEST_F(InterpolateTest, Nearby) {
+  add("dir/foo.cpp");
+  add("dir/bar.cpp");
+  add("an/other/foo.cpp");
+
+  // great: dir and name both match (prefix or full, case insensitive)
+  EXPECT_EQ(getCommand("dir/f.cpp"), "clang -D dir/foo.cpp");
+  EXPECT_EQ(getCommand("dir/FOO.cpp"), "clang -D dir/foo.cpp");
+  // no name match. prefer matching dir, break ties by alpha
+  EXPECT_EQ(getCommand("dir/a.cpp"), "clang -D dir/bar.cpp");
+  // an exact name match beats one segment of directory match
+  EXPECT_EQ(getCommand("some/other/bar.h"), "clang -x c++ -D dir/bar.cpp");
+  // two segments of directory match beat a prefix name match
+  EXPECT_EQ(getCommand("an/other/b.cpp"), "clang -D an/other/foo.cpp");
+  // if nothing matches at all, we still get the closest alpha match
+  EXPECT_EQ(getCommand("below/some/obscure/path.cpp"),
+"clang -D an/other/foo.cpp");
+}
+
+TEST_F(InterpolateTest, Language) {
+  add("dir/foo.cpp");
+  add("dir/baz.cee", "-x c");
+
+  // extension changed, so we add explicit language flags
+  EXPECT_EQ(getCommand("foo.h"), "clang -x c++ -D dir/foo.cpp");
+  // but we don't add -x if it's already there
+  EXPECT_EQ(getCommand("baz.h"), "clang -D dir/baz.cee -x c");
+}
+
+TEST_F(InterpolateTest, Strip) {
+  add("dir/foo.cpp", "-o foo.o -Wall");
+  // the -o option and the input file are removed, but -Wall is preserved.
+  EXPECT_EQ(getCommand("dir/bar.cpp"), "clang -D dir/foo.cpp -Wall");
+}
+
 } // end namespace tooling
 } // end namespace clang
Index: lib/Tooling/InterpolatingCompilationDatabase.cpp
===
--- /dev/null
+++ lib/Tooling/InterpolatingCompilationDatabase.cpp
@@ -0,0 +1,329 @@
+//===- InterpolatingCompilationDatabase.cpp -*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// InterpolatingCompilationDatabase wraps another CompilationDatabase and
+// attempts to heuristically determine appropriate compile commands for files
+// that are not included, such as headers or newly created files.
+//
+// Motivating cases include:
+//   Hea

[PATCH] D44842: Add Parameters to DW_AT_name Attribute of Template Variables

2018-04-03 Thread Jonas Devlieghere via Phabricator via cfe-commits
JDevlieghere added a comment.

@jingham is this safe to land?




Comment at: lib/CodeGen/CGDebugInfo.cpp:2992
+  std::string NameString = Name.str();
+  llvm::raw_string_ostream ParameterizedName(NameString);
+  ParameterizedName << "<";

ormris wrote:
> JDevlieghere wrote:
> > Why not use Name.str() directly?
> I got "error: invalid initialization of non-const reference ... from rvalue 
> ...". Using NameString fixed the issue.
Right, makes sense, you're using the string as the buffer :-) 


Repository:
  rC Clang

https://reviews.llvm.org/D44842



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


[PATCH] D43281: [AMDGPU] fixes for lds f32 builtins

2018-04-03 Thread Daniil Fukalov via Phabricator via cfe-commits
dfukalov added a comment.

ping...


Repository:
  rC Clang

https://reviews.llvm.org/D43281



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


[PATCH] D44882: [clangd] Implementation of workspace/symbol request

2018-04-03 Thread Haojian Wu via Phabricator via cfe-commits
hokein added inline comments.



Comment at: clangd/ClangdLSPServer.cpp:258
+if (!Items)
+  return replyError(ErrorCode::InvalidParams,
+llvm::toString(Items.takeError()));

`InvalidParams` doesn't match all cases where internal errors occur. Maybe use 
`ErrorCode::InternalError`?



Comment at: clangd/FindSymbols.cpp:145
+if (!Uri) {
+  log(llvm::formatv(
+  "Workspace symbol: Could not parse URI '{0}' for symbol '{1}'.",

I think we may want to report the error to client instead of just logging them.



Comment at: clangd/SourceCode.cpp:143
+/// From "a::b::c", return {"a::b::", "c"}. Scope is empty if there's no
+/// qualifier.
+std::pair

nit: this comment is duplicated with the one in header.



Comment at: clangd/SourceCode.h:59
+/// Turn a pair of offsets into a Location.
+llvm::Optional
+offsetRangeToLocation(const DraftStore &DS, SourceManager &SourceMgr,

We should use `llvm::Expected`?

The function needs a comment documenting its behavior, especially on the 
unsaved file content. 



Comment at: clangd/SourceCode.h:61
+offsetRangeToLocation(const DraftStore &DS, SourceManager &SourceMgr,
+  StringRef File, size_t OffsetStart, size_t OffsetEnd);
+

nit: name `FilePath` or `FileName`, `File` seems to be a bit confusing, does it 
mean `FileContent` or `FileName`?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44882



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


[clang-tools-extra] r329073 - [clang-tidy] Check for sizeof that call functions

2018-04-03 Thread Haojian Wu via cfe-commits
Author: hokein
Date: Tue Apr  3 08:10:24 2018
New Revision: 329073

URL: http://llvm.org/viewvc/llvm-project?rev=329073&view=rev
Log:
[clang-tidy] Check for sizeof that call functions

Summary:
A common mistake that I have found in our codebase is calling a function to get 
an integer or enum that represents the type such as:

```
int numBytes = numElements * sizeof(x.GetType());
```

So this extends the `sizeof` check to check for these cases. There is also a 
`WarnOnSizeOfCall` option so it can be disabled.

Patch by Paul Fultz II!

Reviewers: hokein, alexfh, aaron.ballman, ilya-biryukov

Reviewed By: alexfh

Subscribers: lebedev.ri, xazax.hun, jkorous-apple, cfe-commits

Tags: #clang-tools-extra

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

Modified:
clang-tools-extra/trunk/clang-tidy/bugprone/SizeofExpressionCheck.cpp
clang-tools-extra/trunk/clang-tidy/bugprone/SizeofExpressionCheck.h

clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-sizeof-expression.rst
clang-tools-extra/trunk/test/clang-tidy/bugprone-sizeof-expression.cpp

Modified: clang-tools-extra/trunk/clang-tidy/bugprone/SizeofExpressionCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/SizeofExpressionCheck.cpp?rev=329073&r1=329072&r2=329073&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/bugprone/SizeofExpressionCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/bugprone/SizeofExpressionCheck.cpp Tue 
Apr  3 08:10:24 2018
@@ -62,12 +62,16 @@ SizeofExpressionCheck::SizeofExpressionC
  ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context),
   WarnOnSizeOfConstant(Options.get("WarnOnSizeOfConstant", 1) != 0),
+  WarnOnSizeOfIntegerExpression(
+  Options.get("WarnOnSizeOfIntegerExpression", 0) != 0),
   WarnOnSizeOfThis(Options.get("WarnOnSizeOfThis", 1) != 0),
   WarnOnSizeOfCompareToConstant(
   Options.get("WarnOnSizeOfCompareToConstant", 1) != 0) {}
 
 void SizeofExpressionCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "WarnOnSizeOfConstant", WarnOnSizeOfConstant);
+  Options.store(Opts, "WarnOnSizeOfIntegerExpression",
+WarnOnSizeOfIntegerExpression);
   Options.store(Opts, "WarnOnSizeOfThis", WarnOnSizeOfThis);
   Options.store(Opts, "WarnOnSizeOfCompareToConstant",
 WarnOnSizeOfCompareToConstant);
@@ -78,6 +82,9 @@ void SizeofExpressionCheck::registerMatc
   const auto ConstantExpr = expr(ignoringParenImpCasts(
   anyOf(integerLiteral(), unaryOperator(hasUnaryOperand(IntegerExpr)),
 binaryOperator(hasLHS(IntegerExpr), hasRHS(IntegerExpr);
+  const auto IntegerCallExpr = expr(ignoringParenImpCasts(
+  callExpr(anyOf(hasType(isInteger()), hasType(enumType())),
+   unless(isInTemplateInstantiation();
   const auto SizeOfExpr =
   expr(anyOf(sizeOfExpr(has(type())), sizeOfExpr(has(expr();
   const auto SizeOfZero = expr(
@@ -94,6 +101,14 @@ void SizeofExpressionCheck::registerMatc
 this);
   }
 
+  // Detect sizeof(f())
+  if (WarnOnSizeOfIntegerExpression) {
+Finder->addMatcher(
+expr(sizeOfExpr(ignoringParenImpCasts(has(IntegerCallExpr
+.bind("sizeof-integer-call"),
+this);
+  }
+
   // Detect expression like: sizeof(this);
   if (WarnOnSizeOfThis) {
 Finder->addMatcher(
@@ -203,6 +218,10 @@ void SizeofExpressionCheck::check(const
   if (const auto *E = Result.Nodes.getNodeAs("sizeof-constant")) {
 diag(E->getLocStart(),
  "suspicious usage of 'sizeof(K)'; did you mean 'K'?");
+  } else if (const auto *E =
+ Result.Nodes.getNodeAs("sizeof-integer-call")) {
+diag(E->getLocStart(), "suspicious usage of 'sizeof()' on an expression "
+   "that results in an integer");
   } else if (const auto *E = Result.Nodes.getNodeAs("sizeof-this")) {
 diag(E->getLocStart(),
  "suspicious usage of 'sizeof(this)'; did you mean 'sizeof(*this)'");

Modified: clang-tools-extra/trunk/clang-tidy/bugprone/SizeofExpressionCheck.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/SizeofExpressionCheck.h?rev=329073&r1=329072&r2=329073&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/bugprone/SizeofExpressionCheck.h 
(original)
+++ clang-tools-extra/trunk/clang-tidy/bugprone/SizeofExpressionCheck.h Tue Apr 
 3 08:10:24 2018
@@ -29,6 +29,7 @@ public:
 
 private:
   const bool WarnOnSizeOfConstant;
+  const bool WarnOnSizeOfIntegerExpression;
   const bool WarnOnSizeOfThis;
   const bool WarnOnSizeOfCompareToConstant;
 };

Modified: 
clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-sizeof-expression.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clan

[PATCH] D44231: [clang-tidy] Check for sizeof that call functions

2018-04-03 Thread Haojian Wu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL329073: [clang-tidy] Check for sizeof that call functions 
(authored by hokein, committed by ).
Herald added subscribers: llvm-commits, klimek.

Changed prior to commit:
  https://reviews.llvm.org/D44231?vs=140790&id=140799#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D44231

Files:
  clang-tools-extra/trunk/clang-tidy/bugprone/SizeofExpressionCheck.cpp
  clang-tools-extra/trunk/clang-tidy/bugprone/SizeofExpressionCheck.h
  clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-sizeof-expression.rst
  clang-tools-extra/trunk/test/clang-tidy/bugprone-sizeof-expression.cpp

Index: clang-tools-extra/trunk/clang-tidy/bugprone/SizeofExpressionCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/bugprone/SizeofExpressionCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/bugprone/SizeofExpressionCheck.cpp
@@ -62,12 +62,16 @@
  ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context),
   WarnOnSizeOfConstant(Options.get("WarnOnSizeOfConstant", 1) != 0),
+  WarnOnSizeOfIntegerExpression(
+  Options.get("WarnOnSizeOfIntegerExpression", 0) != 0),
   WarnOnSizeOfThis(Options.get("WarnOnSizeOfThis", 1) != 0),
   WarnOnSizeOfCompareToConstant(
   Options.get("WarnOnSizeOfCompareToConstant", 1) != 0) {}
 
 void SizeofExpressionCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "WarnOnSizeOfConstant", WarnOnSizeOfConstant);
+  Options.store(Opts, "WarnOnSizeOfIntegerExpression",
+WarnOnSizeOfIntegerExpression);
   Options.store(Opts, "WarnOnSizeOfThis", WarnOnSizeOfThis);
   Options.store(Opts, "WarnOnSizeOfCompareToConstant",
 WarnOnSizeOfCompareToConstant);
@@ -78,6 +82,9 @@
   const auto ConstantExpr = expr(ignoringParenImpCasts(
   anyOf(integerLiteral(), unaryOperator(hasUnaryOperand(IntegerExpr)),
 binaryOperator(hasLHS(IntegerExpr), hasRHS(IntegerExpr);
+  const auto IntegerCallExpr = expr(ignoringParenImpCasts(
+  callExpr(anyOf(hasType(isInteger()), hasType(enumType())),
+   unless(isInTemplateInstantiation();
   const auto SizeOfExpr =
   expr(anyOf(sizeOfExpr(has(type())), sizeOfExpr(has(expr();
   const auto SizeOfZero = expr(
@@ -94,6 +101,14 @@
 this);
   }
 
+  // Detect sizeof(f())
+  if (WarnOnSizeOfIntegerExpression) {
+Finder->addMatcher(
+expr(sizeOfExpr(ignoringParenImpCasts(has(IntegerCallExpr
+.bind("sizeof-integer-call"),
+this);
+  }
+
   // Detect expression like: sizeof(this);
   if (WarnOnSizeOfThis) {
 Finder->addMatcher(
@@ -203,6 +218,10 @@
   if (const auto *E = Result.Nodes.getNodeAs("sizeof-constant")) {
 diag(E->getLocStart(),
  "suspicious usage of 'sizeof(K)'; did you mean 'K'?");
+  } else if (const auto *E =
+ Result.Nodes.getNodeAs("sizeof-integer-call")) {
+diag(E->getLocStart(), "suspicious usage of 'sizeof()' on an expression "
+   "that results in an integer");
   } else if (const auto *E = Result.Nodes.getNodeAs("sizeof-this")) {
 diag(E->getLocStart(),
  "suspicious usage of 'sizeof(this)'; did you mean 'sizeof(*this)'");
Index: clang-tools-extra/trunk/clang-tidy/bugprone/SizeofExpressionCheck.h
===
--- clang-tools-extra/trunk/clang-tidy/bugprone/SizeofExpressionCheck.h
+++ clang-tools-extra/trunk/clang-tidy/bugprone/SizeofExpressionCheck.h
@@ -29,6 +29,7 @@
 
 private:
   const bool WarnOnSizeOfConstant;
+  const bool WarnOnSizeOfIntegerExpression;
   const bool WarnOnSizeOfThis;
   const bool WarnOnSizeOfCompareToConstant;
 };
Index: clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-sizeof-expression.rst
===
--- clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-sizeof-expression.rst
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-sizeof-expression.rst
@@ -22,6 +22,36 @@
   char buf[BUFLEN];
   memset(buf, 0, sizeof(BUFLEN));  // sizeof(42) ==> sizeof(int)
 
+Suspicious usage of 'sizeof(expr)'
+--
+
+In cases, where there is an enum or integer to represent a type, a common
+mistake is to query the ``sizeof`` on the integer or enum that represents the
+type that should be used by ``sizeof``. This results in the size of the integer
+and not of the type the integer represents:
+
+.. code-block:: c++
+
+  enum data_type {
+FLOAT_TYPE,
+DOUBLE_TYPE
+  };
+
+  struct data {
+data_type type;
+void* buffer;
+data_type get_type() {
+  return type;
+}
+  };
+
+  void f(data d, int numElements) {
+// should be sizeof(float) or sizeof(double), depending on d.get_type()
+int numBytes = numElements * sizeof(d.g

[PATCH] D44231: [clang-tidy] Check for sizeof that call functions

2018-04-03 Thread Haojian Wu via Phabricator via cfe-commits
hokein added a comment.

Thanks, I have committed for you.


Repository:
  rL LLVM

https://reviews.llvm.org/D44231



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


[PATCH] D45006: [Tooling] A CompilationDatabase wrapper that infers header commands.

2018-04-03 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 140802.
sammccall added a comment.

Add more tests, and only add -x when actually needed.


Repository:
  rC Clang

https://reviews.llvm.org/D45006

Files:
  include/clang/Tooling/CompilationDatabase.h
  lib/Tooling/CMakeLists.txt
  lib/Tooling/InterpolatingCompilationDatabase.cpp
  unittests/Tooling/CompilationDatabaseTest.cpp

Index: unittests/Tooling/CompilationDatabaseTest.cpp
===
--- unittests/Tooling/CompilationDatabaseTest.cpp
+++ unittests/Tooling/CompilationDatabaseTest.cpp
@@ -626,5 +626,108 @@
   EXPECT_EQ(2, Argc);
 }
 
+struct MemCDB : public CompilationDatabase {
+  using EntryMap = llvm::StringMap>;
+  EntryMap Entries;
+  MemCDB(const EntryMap &E) : Entries(E) {}
+
+  std::vector getCompileCommands(StringRef F) const override {
+auto Ret = Entries.lookup(F);
+return {Ret.begin(), Ret.end()};
+  }
+
+  std::vector getAllFiles() const override {
+std::vector Result;
+for (const auto &Entry : Entries)
+  Result.push_back(Entry.first());
+return Result;
+  }
+};
+
+class InterpolateTest : public ::testing::Test {
+protected:
+  // Adds an entry to the underlying compilation database.
+  // A flag is injected: -D , so the command used can be identified.
+  void add(llvm::StringRef File, llvm::StringRef Flags = "") {
+llvm::SmallVector Argv = {"clang", File, "-D", File};
+llvm::SplitString(Flags, Argv);
+llvm::SmallString<32> Dir;
+llvm::sys::path::system_temp_directory(false, Dir);
+Entries[path(File)].push_back(
+{Dir, File, {Argv.begin(), Argv.end()}, "foo.o"});
+  }
+
+  // Turn a unix path fragment (foo/bar.h) into a native path (C:\tmp\foo\bar.h)
+  std::string path(llvm::SmallString<32> File) {
+llvm::SmallString<32> Dir;
+llvm::sys::path::system_temp_directory(false, Dir);
+llvm::sys::path::native(File);
+llvm::SmallString<64> Result;
+llvm::sys::path::append(Result, Dir, File);
+return Result.str();
+  }
+
+  // Look up the command from a relative path, and return it in string form.
+  // The input file is not included in the returned command.
+  std::string getCommand(llvm::StringRef F) {
+auto Results =
+inferMissingCompileCommands(llvm::make_unique(Entries))
+->getCompileCommands(path(F));
+if (Results.empty())
+  return "none";
+// drop the input file argument, so tests don't have to deal with path().
+EXPECT_EQ(Results[0].CommandLine.back(), path(F))
+<< "Last arg should be the file";
+Results[0].CommandLine.pop_back();
+return llvm::join(Results[0].CommandLine, " ");
+  }
+
+  MemCDB::EntryMap Entries;
+};
+
+TEST_F(InterpolateTest, Nearby) {
+  add("dir/foo.cpp");
+  add("dir/bar.cpp");
+  add("an/other/foo.cpp");
+
+  // great: dir and name both match (prefix or full, case insensitive)
+  EXPECT_EQ(getCommand("dir/f.cpp"), "clang -D dir/foo.cpp");
+  EXPECT_EQ(getCommand("dir/FOO.cpp"), "clang -D dir/foo.cpp");
+  // no name match. prefer matching dir, break ties by alpha
+  EXPECT_EQ(getCommand("dir/a.cpp"), "clang -D dir/bar.cpp");
+  // an exact name match beats one segment of directory match
+  EXPECT_EQ(getCommand("some/other/bar.h"), "clang -x c++ -D dir/bar.cpp");
+  // two segments of directory match beat a prefix name match
+  EXPECT_EQ(getCommand("an/other/b.cpp"), "clang -D an/other/foo.cpp");
+  // if nothing matches at all, we still get the closest alpha match
+  EXPECT_EQ(getCommand("below/some/obscure/path.cpp"),
+"clang -D an/other/foo.cpp");
+}
+
+TEST_F(InterpolateTest, Language) {
+  add("dir/foo.cpp");
+  add("dir/baz.cee", "-x c");
+
+  // extension changed, so we add explicit language flags
+  EXPECT_EQ(getCommand("foo.h"), "clang -x c++ -D dir/foo.cpp");
+  // but we don't add -x if it's already there
+  EXPECT_EQ(getCommand("baz.h"), "clang -D dir/baz.cee -x c");
+  // and don't add -x if the inferred language didn't change
+  EXPECT_EQ(getCommand("foo.cc"), "clang -D dir/foo.cpp");
+}
+
+TEST_F(InterpolateTest, Strip) {
+  add("dir/foo.cpp", "-o foo.o -Wall");
+  // the -o option and the input file are removed, but -Wall is preserved.
+  EXPECT_EQ(getCommand("dir/bar.cpp"), "clang -D dir/foo.cpp -Wall");
+}
+
+TEST_F(InterpolateTest, Case) {
+  add("FOO/BAR/BAZ/SHOUT.cc");
+  add("foo/bar/baz/quiet.cc");
+  // Case mismatches are completely ignored, so we choose the name match.
+  EXPECT_EQ(getCommand("foo/bar/baz/shout.C"), "clang -D FOO/BAR/BAZ/SHOUT.cc");
+}
+
 } // end namespace tooling
 } // end namespace clang
Index: lib/Tooling/InterpolatingCompilationDatabase.cpp
===
--- /dev/null
+++ lib/Tooling/InterpolatingCompilationDatabase.cpp
@@ -0,0 +1,332 @@
+//===- InterpolatingCompilationDatabase.cpp -*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the Universi

[libcxx] r329075 - Implement P0754R2: The header.

2018-04-03 Thread Marshall Clow via cfe-commits
Author: marshall
Date: Tue Apr  3 08:48:24 2018
New Revision: 329075

URL: http://llvm.org/viewvc/llvm-project?rev=329075&view=rev
Log:
Implement P0754R2: The  header.

Added:
libcxx/trunk/include/version
libcxx/trunk/test/libcxx/language.support/support.limits/version.pass.cpp
libcxx/trunk/test/std/language.support/support.limits/version.pass.cpp
Modified:
libcxx/trunk/www/cxx2a_status.html

Added: libcxx/trunk/include/version
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/version?rev=329075&view=auto
==
--- libcxx/trunk/include/version (added)
+++ libcxx/trunk/include/version Tue Apr  3 08:48:24 2018
@@ -0,0 +1,25 @@
+// -*- C++ -*-
+//===--- version 
--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef _LIBCPP_VERSIONH
+#define _LIBCPP_VERSIONH
+
+/*
+version synopsis
+
+*/
+
+#include <__config>
+
+#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
+#pragma GCC system_header
+#endif
+
+#endif  // _LIBCPP_VERSIONH

Added: libcxx/trunk/test/libcxx/language.support/support.limits/version.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/libcxx/language.support/support.limits/version.pass.cpp?rev=329075&view=auto
==
--- libcxx/trunk/test/libcxx/language.support/support.limits/version.pass.cpp 
(added)
+++ libcxx/trunk/test/libcxx/language.support/support.limits/version.pass.cpp 
Tue Apr  3 08:48:24 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.
+//
+//===--===//
+
+// 
+// UNSUPPORTED: c++98, c++03, c++11, c++14, c++17
+// UNSUPPORTED: clang-3.3, clang-3.4, clang-3.5, clang-3.6, clang-3.7
+// UNSUPPORTED: clang-3.8, clang-3.9, clang-4.0, clang-5.0, clang-6.0
+
+#include 
+
+#if !defined(_LIBCPP_VERSION)
+#error "_LIBCPP_VERSION must be defined after including "
+#endif
+
+int main()
+{
+}

Added: libcxx/trunk/test/std/language.support/support.limits/version.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/language.support/support.limits/version.pass.cpp?rev=329075&view=auto
==
--- libcxx/trunk/test/std/language.support/support.limits/version.pass.cpp 
(added)
+++ libcxx/trunk/test/std/language.support/support.limits/version.pass.cpp Tue 
Apr  3 08:48:24 2018
@@ -0,0 +1,17 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// 
+// UNSUPPORTED: c++98, c++03, c++11, c++14, c++17
+
+#include 
+
+int main()
+{
+}

Modified: libcxx/trunk/www/cxx2a_status.html
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/www/cxx2a_status.html?rev=329075&r1=329074&r2=329075&view=diff
==
--- libcxx/trunk/www/cxx2a_status.html (original)
+++ libcxx/trunk/www/cxx2a_status.html Tue Apr  3 08:48:24 2018
@@ -76,7 +76,7 @@
https://wg21.link/P0355R7";>P0355R7LWGExtending 
chrono to Calendars and Time 
ZonesJacksonville
https://wg21.link/P0551R3";>P0551R3LWGThou Shalt Not 
Specialize std Function 
Templates!Jacksonville
https://wg21.link/P0753R2";>P0753R2LWGManipulators 
for C++ Synchronized Buffered 
OstreamJacksonville
-   https://wg21.link/P0754R2";>P0754R2LWGJacksonville
+   https://wg21.link/P0754R2";>P0754R2LWGJacksonvilleComplete7.0
https://wg21.link/P0809R0";>P0809R0LWGComparing 
Unordered ContainersJacksonville
https://wg21.link/P0858R0";>P0858R0LWGConstexpr 
iterator requirementsJacksonville
https://wg21.link/P0905R1";>P0905R1CWGSymmetry for 
spaceshipJacksonville
@@ -180,7 +180,7 @@
 
   
 
-  Last Updated: 20-Mar-2018
+  Last Updated: 3-Apr-2018
 
 
 


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


[PATCH] D44842: Add Parameters to DW_AT_name Attribute of Template Variables

2018-04-03 Thread Jim Ingham via Phabricator via cfe-commits
jingham added a comment.

I don't know off the top of my head.  Did anybody working on this patch try 
printing variables in lldb with this name change, particularly global 
variables?  Global lookups are particularly tricky, but it shouldn't be hard to 
check.  You'd want to check that you can print them with frame var and expr.


Repository:
  rC Clang

https://reviews.llvm.org/D44842



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


r329077 - [Hexagon] Remove -mhvx-double and the corresponding subtarget feature

2018-04-03 Thread Krzysztof Parzyszek via cfe-commits
Author: kparzysz
Date: Tue Apr  3 08:59:10 2018
New Revision: 329077

URL: http://llvm.org/viewvc/llvm-project?rev=329077&view=rev
Log:
[Hexagon] Remove -mhvx-double and the corresponding subtarget feature

Specifying the HVX vector length should be done via the -mhvx-length
option.

Modified:
cfe/trunk/docs/ClangCommandLineReference.rst
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/lib/Basic/Targets/Hexagon.cpp
cfe/trunk/lib/Driver/ToolChains/Hexagon.cpp
cfe/trunk/test/Driver/hexagon-hvx.c

Modified: cfe/trunk/docs/ClangCommandLineReference.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ClangCommandLineReference.rst?rev=329077&r1=329076&r2=329077&view=diff
==
--- cfe/trunk/docs/ClangCommandLineReference.rst (original)
+++ cfe/trunk/docs/ClangCommandLineReference.rst Tue Apr  3 08:59:10 2018
@@ -2294,10 +2294,6 @@ Hexagon
 
 Enable Hexagon Vector eXtensions
 
-.. option:: -mhvx-double, -mno-hvx-double
-
-Enable Hexagon Double Vector eXtensions
-
 .. option:: -mhvx-length=
 
 Set Hexagon Vector Length

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=329077&r1=329076&r2=329077&view=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Tue Apr  3 08:59:10 2018
@@ -2504,13 +2504,6 @@ def mpackets : Flag<["-"], "mpackets">,
   Flags<[CC1Option]>, HelpText<"Enable generation of instruction packets">;
 def mno_packets : Flag<["-"], "mno-packets">, Group,
   Flags<[CC1Option]>, HelpText<"Disable generation of instruction packets">;
-// hvx-double deprecrated flag.
-def mhexagon_hvx_double : Flag<[ "-" ], "mhvx-double">,
-  Group,
-  HelpText<"Enable Hexagon Double Vector eXtensions">;
-def mno_hexagon_hvx_double : Flag<[ "-" ], "mno-hvx-double">,
-  Group,
-  HelpText<"Disable Hexagon Double Vector eXtensions">;
 
 
 // X86 feature flags

Modified: cfe/trunk/lib/Basic/Targets/Hexagon.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/Hexagon.cpp?rev=329077&r1=329076&r2=329077&view=diff
==
--- cfe/trunk/lib/Basic/Targets/Hexagon.cpp (original)
+++ cfe/trunk/lib/Basic/Targets/Hexagon.cpp Tue Apr  3 08:59:10 2018
@@ -75,7 +75,6 @@ void HexagonTargetInfo::getTargetDefines
 bool HexagonTargetInfo::initFeatureMap(
 llvm::StringMap &Features, DiagnosticsEngine &Diags, StringRef CPU,
 const std::vector &FeaturesVec) const {
-  Features["hvx-double"] = false;
   Features["long-calls"] = false;
 
   return TargetInfo::initFeatureMap(Features, Diags, CPU, FeaturesVec);

Modified: cfe/trunk/lib/Driver/ToolChains/Hexagon.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Hexagon.cpp?rev=329077&r1=329076&r2=329077&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Hexagon.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Hexagon.cpp Tue Apr  3 08:59:10 2018
@@ -37,12 +37,6 @@ static StringRef getDefaultHvxLength(Str
 }
 
 static void handleHVXWarnings(const Driver &D, const ArgList &Args) {
-  // Handle deprecated HVX double warnings.
-  if (Arg *A = Args.getLastArg(options::OPT_mhexagon_hvx_double))
-D.Diag(diag::warn_drv_deprecated_arg)
-<< A->getAsString(Args) << "-mhvx-length=128B";
-  if (Arg *A = Args.getLastArg(options::OPT_mno_hexagon_hvx_double))
-D.Diag(diag::warn_drv_deprecated_arg) << A->getAsString(Args) << 
"-mno-hvx";
   // Handle the unsupported values passed to mhvx-length.
   if (Arg *A = Args.getLastArg(options::OPT_mhexagon_hvx_length_EQ)) {
 StringRef Val = A->getValue();
@@ -63,14 +57,13 @@ static void handleHVXTargetFeatures(cons
   StringRef HVXFeature, HVXLength;
   StringRef Cpu(toolchains::HexagonToolChain::GetTargetCPUVersion(Args));
 
-  // Handle -mhvx, -mhvx=, -mno-hvx, -mno-hvx-double.
-  if (Arg *A = Args.getLastArg(
-  options::OPT_mno_hexagon_hvx, options::OPT_mno_hexagon_hvx_double,
-  options::OPT_mhexagon_hvx, options::OPT_mhexagon_hvx_EQ)) {
-if (A->getOption().matches(options::OPT_mno_hexagon_hvx) ||
-A->getOption().matches(options::OPT_mno_hexagon_hvx_double)) {
+  // Handle -mhvx, -mhvx=, -mno-hvx.
+  if (Arg *A = Args.getLastArg(options::OPT_mno_hexagon_hvx,
+   options::OPT_mhexagon_hvx,
+   options::OPT_mhexagon_hvx_EQ)) {
+if (A->getOption().matches(options::OPT_mno_hexagon_hvx))
   return;
-} else if (A->getOption().matches(options::OPT_mhexagon_hvx_EQ)) {
+if (A->getOption().matches(options::OPT_mhexagon_hvx_EQ)) {
   HasHVX = true;
   HVXFeature = Cpu = A->getValue();
   HVXFeature = Args.MakeArgString(llvm::Tw

[PATCH] D44842: Add Parameters to DW_AT_name Attribute of Template Variables

2018-04-03 Thread Matthew Voss via Phabricator via cfe-commits
ormris added a comment.

@jingham Will do.


Repository:
  rC Clang

https://reviews.llvm.org/D44842



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


[PATCH] D45212: [WIP][HIP] Let CUDA toolchain support HIP language mode

2018-04-03 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl created this revision.
yaxunl added reviewers: rjmccall, tra.
Herald added subscribers: tpr, nhaehnle, jholewinski.

This patch let CUDA toolchain support HIP language mode. It includes:

Add offloading archs for amdgpu for CUDA/HIP.

Add file type hip for toolchain.

Create specific compiler jobs for HIP.

Choose specific header/libraries for HIP.

Use clang-offload-bindler to create binary for device ISA.

This is work in progress. There are still a few lit test failures to clean up.

Patch by Greg Rodgers.
Revised and lit test added by Yaxun Liu.


https://reviews.llvm.org/D45212

Files:
  include/clang/Basic/Cuda.h
  include/clang/Basic/DiagnosticDriverKinds.td
  include/clang/Driver/Options.td
  include/clang/Driver/ToolChain.h
  include/clang/Driver/Types.def
  lib/Basic/Cuda.cpp
  lib/Basic/Targets.cpp
  lib/Basic/Targets.h
  lib/Basic/Targets/AMDGPU.cpp
  lib/Basic/Targets/AMDGPU.h
  lib/Basic/Targets/NVPTX.cpp
  lib/Driver/Driver.cpp
  lib/Driver/SanitizerArgs.cpp
  lib/Driver/ToolChain.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Driver/ToolChains/Cuda.cpp
  lib/Driver/ToolChains/Cuda.h
  lib/Driver/ToolChains/Gnu.cpp
  lib/Driver/Types.cpp
  lib/Frontend/CompilerInstance.cpp
  test/Driver/cuda-phases.cu

Index: test/Driver/cuda-phases.cu
===
--- test/Driver/cuda-phases.cu
+++ test/Driver/cuda-phases.cu
@@ -7,22 +7,26 @@
 // REQUIRES: clang-driver
 // REQUIRES: powerpc-registered-target
 // REQUIRES: nvptx-registered-target
-
+// REQUIRES: amdgpu-registered-target
 //
 // Test single gpu architecture with complete compilation.
 //
 // RUN: %clang -target powerpc64le-ibm-linux-gnu -ccc-print-phases --cuda-gpu-arch=sm_30 %s 2>&1 \
-// RUN: | FileCheck -check-prefix=BIN %s
-// BIN-DAG: [[P0:[0-9]+]]: input, "{{.*}}cuda-phases.cu", cuda, (host-cuda)
-// BIN-DAG: [[P1:[0-9]+]]: preprocessor, {[[P0]]}, cuda-cpp-output, (host-cuda)
+// RUN: | FileCheck -check-prefixes=BIN,BIN_NV %s
+// RUN: %clang -x hip -target powerpc64le-ibm-linux-gnu -ccc-print-phases --cuda-gpu-arch=gfx803 %s 2>&1 \
+// RUN: | FileCheck -check-prefixes=BIN,BIN_AMD %s
+// BIN_NV-DAG: [[P0:[0-9]+]]: input, "{{.*}}cuda-phases.cu", [[T:cuda]], (host-cuda)
+// BIN_AMD-DAG: [[P0:[0-9]+]]: input, "{{.*}}cuda-phases.cu", [[T:hip]], (host-cuda)
+// BIN-DAG: [[P1:[0-9]+]]: preprocessor, {[[P0]]}, [[T]]-cpp-output, (host-cuda)
 // BIN-DAG: [[P2:[0-9]+]]: compiler, {[[P1]]}, ir, (host-cuda)
-// BIN-DAG: [[P3:[0-9]+]]: input, "{{.*}}cuda-phases.cu", cuda, (device-cuda, sm_30)
-// BIN-DAG: [[P4:[0-9]+]]: preprocessor, {[[P3]]}, cuda-cpp-output, (device-cuda, sm_30)
-// BIN-DAG: [[P5:[0-9]+]]: compiler, {[[P4]]}, ir, (device-cuda, sm_30)
-// BIN-DAG: [[P6:[0-9]+]]: backend, {[[P5]]}, assembler, (device-cuda, sm_30)
-// BIN-DAG: [[P7:[0-9]+]]: assembler, {[[P6]]}, object, (device-cuda, sm_30)
-// BIN-DAG: [[P8:[0-9]+]]: offload, "device-cuda (nvptx64-nvidia-cuda:sm_30)" {[[P7]]}, object
-// BIN-DAG: [[P9:[0-9]+]]: offload, "device-cuda (nvptx64-nvidia-cuda:sm_30)" {[[P6]]}, assembler
+// BIN_NV-DAG: [[P3:[0-9]+]]: input, "{{.*}}cuda-phases.cu", [[T]], (device-cuda, [[ARCH:sm_30]])
+// BIN_AMD-DAG: [[P3:[0-9]+]]: input, "{{.*}}cuda-phases.cu", [[T]], (device-cuda, [[ARCH:gfx803]])
+// BIN-DAG: [[P4:[0-9]+]]: preprocessor, {[[P3]]}, [[T]]-cpp-output, (device-cuda, [[ARCH]])
+// BIN-DAG: [[P5:[0-9]+]]: compiler, {[[P4]]}, ir, (device-cuda, [[ARCH]])
+// BIN-DAG: [[P6:[0-9]+]]: backend, {[[P5]]}, assembler, (device-cuda, [[ARCH]])
+// BIN-DAG: [[P7:[0-9]+]]: assembler, {[[P6]]}, object, (device-cuda, [[ARCH]])
+// BIN-DAG: [[P8:[0-9]+]]: offload, "device-cuda (nvptx64-nvidia-cuda:[[ARCH]])" {[[P7]]}, object
+// BIN-DAG: [[P9:[0-9]+]]: offload, "device-cuda (nvptx64-nvidia-cuda:[[ARCH]])" {[[P6]]}, assembler
 // BIN-DAG: [[P10:[0-9]+]]: linker, {[[P8]], [[P9]]}, cuda-fatbin, (device-cuda)
 // BIN-DAG: [[P11:[0-9]+]]: offload, "host-cuda (powerpc64le-ibm-linux-gnu)" {[[P2]]}, "device-cuda (nvptx64-nvidia-cuda)" {[[P10]]}, ir
 // BIN-DAG: [[P12:[0-9]+]]: backend, {[[P11]]}, assembler, (host-cuda)
@@ -34,38 +38,42 @@
 //
 // RUN: %clang -target powerpc64le-ibm-linux-gnu -ccc-print-phases --cuda-gpu-arch=sm_30 %s -S 2>&1 \
 // RUN: | FileCheck -check-prefix=ASM %s
-// ASM-DAG: [[P0:[0-9]+]]: input, "{{.*}}cuda-phases.cu", cuda, (device-cuda, sm_30)
-// ASM-DAG: [[P1:[0-9]+]]: preprocessor, {[[P0]]}, cuda-cpp-output, (device-cuda, sm_30)
-// ASM-DAG: [[P2:[0-9]+]]: compiler, {[[P1]]}, ir, (device-cuda, sm_30)
-// ASM-DAG: [[P3:[0-9]+]]: backend, {[[P2]]}, assembler, (device-cuda, sm_30)
-// ASM-DAG: [[P4:[0-9]+]]: offload, "device-cuda (nvptx64-nvidia-cuda:sm_30)" {[[P3]]}, assembler
-// ASM-DAG: [[P5:[0-9]+]]: input, "{{.*}}cuda-phases.cu", cuda, (host-cuda)
-// ASM-DAG: [[P6:[0-9]+]]: preprocessor, {[[P5]]}, cuda-cpp-output, (host-cuda)
+// RUN: %clang -x hip -target powerpc64le-ibm-linux-gnu -ccc-print-phases --cuda-gpu-arch=gfx803 %s -S 2>&1 \
+// RUN: | FileCheck -check-prefix=ASM %s

[PATCH] D44842: Add Parameters to DW_AT_name Attribute of Template Variables

2018-04-03 Thread Jim Ingham via Phabricator via cfe-commits
jingham added a comment.

@ormris Thanks!  I you come up with some examples that test this, I can whip up 
an lldb test case (or you can if you are feeling adventurous.)  If you want to 
try I'd be happy to give you a hand.


Repository:
  rC Clang

https://reviews.llvm.org/D44842



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


[PATCH] D44226: [clangd] Add -log-lsp-to-stderr option

2018-04-03 Thread Simon Marchi via Phabricator via cfe-commits
simark added inline comments.



Comment at: clangd/ClangdLSPServer.cpp:412
+llvm::raw_string_ostream OS(Message);
+OS << "method not found (" << Method << ")";
+replyError(ErrorCode::MethodNotFound, OS.str());

ilya-biryukov wrote:
> Could we also `log` (i.e. not `vlog`) names of the methods that were handled 
> successfully? To have some context when something crashes and we only have 
> non-verbose logs.
That would be against the purpose of this patch, which is to output nothing if 
everything goes right (so it's easier to notice when something goes wrong).  
Just outputting the names of the methods that are handled successfully would 
still be very verbose I think.



Comment at: clangd/ClangdLSPServer.cpp:412
+llvm::raw_string_ostream OS(Message);
+OS << "method not found (" << Method << ")";
+replyError(ErrorCode::MethodNotFound, OS.str());

ilya-biryukov wrote:
> malaperle wrote:
> > simark wrote:
> > > ilya-biryukov wrote:
> > > > Could we also `log` (i.e. not `vlog`) names of the methods that were 
> > > > handled successfully? To have some context when something crashes and 
> > > > we only have non-verbose logs.
> > > That would be against the purpose of this patch, which is to output 
> > > nothing if everything goes right (so it's easier to notice when something 
> > > goes wrong).  Just outputting the names of the methods that are handled 
> > > successfully would still be very verbose I think.
> > Wouldn't that mean pretty much logging everything coming in? The idea of 
> > this patch it to make it that by default Clangd is not talkative unless 
> > there is an error and optionally make it verbose, for troubleshooting.
> Logs are also useful to diagnose problems in case something crashes or works 
> incorrectly.
> Clang will probably log something when processing any request anyway, logging 
> the method name first will merely give some more context on which request 
> other log messages correspond to.
> 
> > The idea of this patch it to make it that by default Clangd is not 
> > talkative unless there is an error.
> I don't think we use logging this way in clangd. Logs give us a way to make 
> sense of what's happening inside clangd even when there's no error. `vlog` 
> lets us turn off some extremely noisy output that is not useful most of the 
> time.
> We have a whole bunch of log statements that don't correspond to errors (e.g. 
> "reusing preamble", "building file with compile command").
> Logs are also useful to diagnose problems in case something crashes or works 
> incorrectly.
Clang will probably log something when processing any request anyway, logging 
the method name first will merely give some more context on which request other 
log messages correspond to.

I think it's fine if clangd logs warning/errors by default, that a user might 
want to look at and address.  But logging things that happen recurrently and 
are part of clangd's normal operation just flood the terminal (and makes it 
harder to spot actual errors).

I agree that having some context helps to make sense of an error.  A 
reasonnable way would be to include the method name only when printing an 
error.  For example, `While handling method 'workspace/symbol': Could not open 
file foo.`.  That would require us to carry around some more context 
information, but I think it would be better than printing all the handled 
methods. 

> I don't think we use logging this way in clangd. Logs give us a way to make 
> sense of what's happening inside clangd even when there's no error. 

In that case you turn on a more verbose log level.

> vlog lets us turn off some extremely noisy output that is not useful most of 
> the time. We have a whole bunch of log statements that don't correspond to 
> errors (e.g. "reusing preamble", "building file with compile command").

I would consider everything that happens at each json-rpc request to be 
"noisy".  When using clangd from an IDE, there's quite a lot of requests being 
done.  I was also thinking of modyfing the patch to also use vlog for the 
"reusing preamble" and "building file with compile command" messages.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44226



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


[PATCH] D44842: Add Parameters to DW_AT_name Attribute of Template Variables

2018-04-03 Thread Matthew Voss via Phabricator via cfe-commits
ormris added a comment.

@jingham 
Doesn't look like it works.

  $ cat repro.cpp
  template  T crazy = T();
  
  int main(void) {
crazy = 5;
return crazy;
  }
  $ clang++ -g repro.cpp
  $ lldb a.out
  ...
  (lldb) frame variable
  (lldb) expr crazy
  error: use of undeclared identifier 'crazy'
  error: 1 errors parsing expression
  (lldb) expr crazy
  error: use of undeclared identifier 'crazy'
  error: expected '(' for function-style cast or type construction
  error: expected expression
  error: 3 errors parsing expression
  (lldb)


Repository:
  rC Clang

https://reviews.llvm.org/D44842



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


[PATCH] D45061: [NVPTX, CUDA] Use custom feature detection to handle NVPTX target builtins.

2018-04-03 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment.

In https://reviews.llvm.org/D45061#1053795, @echristo wrote:

> Let's talk about the rest of it more. I'm not sure I'm seeing the need here 
> rather than the annotations that are already here. Can you elaborate more 
> here on why we need an additional method when you've already got subtarget 
> features for each of the ptx versions anyhow?


The patch intends to address two issues:

- mismatch on constraints between llvm and clang. E.g. hasPTX60() on LLVM side 
means "ptx60 or newer". "ptx60" in TARGET_BUILTIN on clang side means "ptx60" 
*only*. It is possible to address this within existing implementation by 
enumerating all PTX versions that are newer. It works OK for ptx60 as we only 
need to write "ptx60|ptx61". It gets more interesting for older GPUs E.g 
"ptx31+" would have to become 
"ptx31|ptx32|ptx40|ptx41|ptx42|ptx43|ptx50|ptx60|ptx61". Similar enumeration 
will need to happen for GPU version, which brings us to the next point
- NVIDIA keeps growing PTX versions and GPU variants. Recently they've changed 
CUDA release frequency to ~1/quarter and they tend to add minor variants of PTX 
and GPU versions fairly frequently. It's going to be an unnecessary maintenance 
headache as after every new introduced variant I'll need to go and update all 
NVPTX builtins that have nothing to do with the CUDA changes. Granted, it's not 
a showstopper, but it is an annoyance that is guaranteed to stay.

With this patch, TARGET_BUILTIN constraints become semantically identical to 
LLVM and we no longer need to chase every bump in PTX version or GPU variant.


https://reviews.llvm.org/D45061



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


[PATCH] D44226: [clangd] Add -log-lsp-to-stderr option

2018-04-03 Thread Simon Marchi via Phabricator via cfe-commits
simark added inline comments.



Comment at: clangd/ClangdLSPServer.cpp:412
+llvm::raw_string_ostream OS(Message);
+OS << "method not found (" << Method << ")";
+replyError(ErrorCode::MethodNotFound, OS.str());

simark wrote:
> ilya-biryukov wrote:
> > malaperle wrote:
> > > simark wrote:
> > > > ilya-biryukov wrote:
> > > > > Could we also `log` (i.e. not `vlog`) names of the methods that were 
> > > > > handled successfully? To have some context when something crashes and 
> > > > > we only have non-verbose logs.
> > > > That would be against the purpose of this patch, which is to output 
> > > > nothing if everything goes right (so it's easier to notice when 
> > > > something goes wrong).  Just outputting the names of the methods that 
> > > > are handled successfully would still be very verbose I think.
> > > Wouldn't that mean pretty much logging everything coming in? The idea of 
> > > this patch it to make it that by default Clangd is not talkative unless 
> > > there is an error and optionally make it verbose, for troubleshooting.
> > Logs are also useful to diagnose problems in case something crashes or 
> > works incorrectly.
> > Clang will probably log something when processing any request anyway, 
> > logging the method name first will merely give some more context on which 
> > request other log messages correspond to.
> > 
> > > The idea of this patch it to make it that by default Clangd is not 
> > > talkative unless there is an error.
> > I don't think we use logging this way in clangd. Logs give us a way to make 
> > sense of what's happening inside clangd even when there's no error. `vlog` 
> > lets us turn off some extremely noisy output that is not useful most of the 
> > time.
> > We have a whole bunch of log statements that don't correspond to errors 
> > (e.g. "reusing preamble", "building file with compile command").
> > Logs are also useful to diagnose problems in case something crashes or 
> > works incorrectly.
> Clang will probably log something when processing any request anyway, logging 
> the method name first will merely give some more context on which request 
> other log messages correspond to.
> 
> I think it's fine if clangd logs warning/errors by default, that a user might 
> want to look at and address.  But logging things that happen recurrently and 
> are part of clangd's normal operation just flood the terminal (and makes it 
> harder to spot actual errors).
> 
> I agree that having some context helps to make sense of an error.  A 
> reasonnable way would be to include the method name only when printing an 
> error.  For example, `While handling method 'workspace/symbol': Could not 
> open file foo.`.  That would require us to carry around some more context 
> information, but I think it would be better than printing all the handled 
> methods. 
> 
> > I don't think we use logging this way in clangd. Logs give us a way to make 
> > sense of what's happening inside clangd even when there's no error. 
> 
> In that case you turn on a more verbose log level.
> 
> > vlog lets us turn off some extremely noisy output that is not useful most 
> > of the time. We have a whole bunch of log statements that don't correspond 
> > to errors (e.g. "reusing preamble", "building file with compile command").
> 
> I would consider everything that happens at each json-rpc request to be 
> "noisy".  When using clangd from an IDE, there's quite a lot of requests 
> being done.  I was also thinking of modyfing the patch to also use vlog for 
> the "reusing preamble" and "building file with compile command" messages.
One more thing: when using multiple workers and the asynchronous model, the 
error may not be related to the last printed method name:

```
Handling json-rpc method A
Handling json-rpc method B
Some error related to the handling of request A
```

If you just see the error like this, you would think it's cause by request B, 
even though it's when handling request A.  Passing some context to the callback 
would allow us to print the request which is really at the origin of the error, 
avoiding any confusion.  And it would allow us to not print it if everything 
goes fine.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44226



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


[PATCH] D45121: [coroutines] Add noop_coroutine to

2018-04-03 Thread Gor Nishanov via Phabricator via cfe-commits
GorNishanov added a comment.

@EricWF , gentle ping. Super tini-tiny change. Last piece missing to be 
post-Jax 2018 compilant


https://reviews.llvm.org/D45121



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


[PATCH] D45069: [clangd] synthesize fix message when the diagnostic doesn't provide one.

2018-04-03 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments.



Comment at: clangd/Diagnostics.cpp:322
+  StringRef Insert = FixIt.CodeToInsert;
+  if (!Invalid && Remove.size() + Insert.size() < 200) {
+llvm::raw_svector_ostream M(Message);

ilya-biryukov wrote:
> I don't really see a way out of it, but having a limit is a bit frustrating.
> 
> It seems weird to have `change 'foo' to 'bar'`, but `couldn't find 'fsdfsdf'. 
> did you mean 'verylongidentifierthatsumsuptomorethan200'?` for the same error.
> Maybe if the message that clang provides is also very long, use the generated 
> message anyway?
> 
Hard to know exactly what to do here without motivating cases. I just removed 
it.

My reasoning: these diagnostics get printed by clang, if they need a shortened 
message then the diagnostic should/will provide one as note text.

(An alternative that I tried out was replacing the middle of the text with 
"..." if it was too long, this worked well but probably isn't worth the extra 
lines of code)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45069



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


[clang-tools-extra] r329090 - [clangd] synthesize fix message when the diagnostic doesn't provide one.

2018-04-03 Thread Sam McCall via cfe-commits
Author: sammccall
Date: Tue Apr  3 10:35:57 2018
New Revision: 329090

URL: http://llvm.org/viewvc/llvm-project?rev=329090&view=rev
Log:
[clangd] synthesize fix message when the diagnostic doesn't provide one.

Summary:
Currently if a fix is attached directly to a diagnostic, we repeat the
diagnostic message as the fix message. From eyeballing the top diagnostics,
it seems describing the textual replacement would be much clearer.

e.g.
error: use of undeclared identifier 'goo'; did you mean 'foo'?
action before: use of undeclared identifier 'goo'; did you mean 'foo'?
action after: change 'goo' to 'foo'

Reviewers: ilya-biryukov

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

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

Modified:
clang-tools-extra/trunk/clangd/Diagnostics.cpp
clang-tools-extra/trunk/unittests/clangd/ClangdUnitTests.cpp

Modified: clang-tools-extra/trunk/clangd/Diagnostics.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Diagnostics.cpp?rev=329090&r1=329089&r2=329090&view=diff
==
--- clang-tools-extra/trunk/clangd/Diagnostics.cpp (original)
+++ clang-tools-extra/trunk/clangd/Diagnostics.cpp Tue Apr  3 10:35:57 2018
@@ -297,7 +297,7 @@ void StoreDiags::HandleDiagnostic(Diagno
 return D;
   };
 
-  auto AddFix = [&]() -> bool {
+  auto AddFix = [&](bool SyntheticMessage) -> bool {
 assert(!Info.getFixItHints().empty() &&
"diagnostic does not have attached fix-its");
 if (!InsideMainFile)
@@ -312,7 +312,27 @@ void StoreDiags::HandleDiagnostic(Diagno
 }
 
 llvm::SmallString<64> Message;
-Info.FormatDiagnostic(Message);
+// If requested and possible, create a message like "change 'foo' to 
'bar'".
+if (SyntheticMessage && Info.getNumFixItHints() == 1) {
+  const auto &FixIt = Info.getFixItHint(0);
+  bool Invalid = false;
+  StringRef Remove = Lexer::getSourceText(
+  FixIt.RemoveRange, Info.getSourceManager(), *LangOpts, &Invalid);
+  StringRef Insert = FixIt.CodeToInsert;
+  if (!Invalid) {
+llvm::raw_svector_ostream M(Message);
+if (!Remove.empty() && !Insert.empty())
+  M << "change '" << Remove << "' to '" << Insert << "'";
+else if (!Remove.empty())
+  M << "remove '" << Remove << "'";
+else if (!Insert.empty())
+  M << "insert '" << Insert << "'";
+// Don't allow source code to inject newlines into diagnostics.
+std::replace(Message.begin(), Message.end(), '\n', ' ');
+  }
+}
+if (Message.empty()) // either !SytheticMessage, or we failed to make one.
+  Info.FormatDiagnostic(Message);
 LastDiag->Fixes.push_back(Fix{Message.str(), std::move(Edits)});
 return true;
   };
@@ -325,7 +345,7 @@ void StoreDiags::HandleDiagnostic(Diagno
 FillDiagBase(*LastDiag);
 
 if (!Info.getFixItHints().empty())
-  AddFix();
+  AddFix(true /* try to invent a message instead of repeating the diag */);
   } else {
 // Handle a note to an existing diagnostic.
 if (!LastDiag) {
@@ -337,7 +357,7 @@ void StoreDiags::HandleDiagnostic(Diagno
 if (!Info.getFixItHints().empty()) {
   // A clang note with fix-it is not a separate diagnostic in clangd. We
   // attach it as a Fix to the main diagnostic instead.
-  if (!AddFix())
+  if (!AddFix(false /* use the note as the message */))
 IgnoreDiagnostics::log(DiagLevel, Info);
 } else {
   // A clang note without fix-its corresponds to clangd::Note.

Modified: clang-tools-extra/trunk/unittests/clangd/ClangdUnitTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/ClangdUnitTests.cpp?rev=329090&r1=329089&r2=329090&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/ClangdUnitTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/ClangdUnitTests.cpp Tue Apr  3 
10:35:57 2018
@@ -92,14 +92,6 @@ Position pos(int line, int character) {
   return Res;
 }
 
-/// Matches diagnostic that has exactly one fix with the same range and message
-/// as the diagnostic itself.
-testing::Matcher DiagWithEqualFix(clangd::Range Range,
-std::string 
Replacement,
-std::string Message) {
-  return AllOf(Diag(Range, Message), WithFix(Fix(Range, Replacement, 
Message)));
-}
-
 TEST(DiagnosticsTest, DiagnosticRanges) {
   // Check we report correct ranges, including various edge-cases.
   Annotations Test(R"cpp(
@@ -111,19 +103,19 @@ o]]();
   $unk[[unknown]]();
 }
   )cpp");
-  llvm::errs() << Test.code();
   EXPECT_THAT(
   buildDiags(Test.code()),
   ElementsAre(
   // This range spans lines.
-  AllOf(DiagWithEqualFix(
-Test.range("typo"), "f

[PATCH] D45069: [clangd] synthesize fix message when the diagnostic doesn't provide one.

2018-04-03 Thread Sam McCall via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL329090: [clangd] synthesize fix message when the diagnostic 
doesn't provide one. (authored by sammccall, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D45069?vs=140357&id=140824#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D45069

Files:
  clang-tools-extra/trunk/clangd/Diagnostics.cpp
  clang-tools-extra/trunk/unittests/clangd/ClangdUnitTests.cpp

Index: clang-tools-extra/trunk/clangd/Diagnostics.cpp
===
--- clang-tools-extra/trunk/clangd/Diagnostics.cpp
+++ clang-tools-extra/trunk/clangd/Diagnostics.cpp
@@ -297,7 +297,7 @@
 return D;
   };
 
-  auto AddFix = [&]() -> bool {
+  auto AddFix = [&](bool SyntheticMessage) -> bool {
 assert(!Info.getFixItHints().empty() &&
"diagnostic does not have attached fix-its");
 if (!InsideMainFile)
@@ -312,7 +312,27 @@
 }
 
 llvm::SmallString<64> Message;
-Info.FormatDiagnostic(Message);
+// If requested and possible, create a message like "change 'foo' to 'bar'".
+if (SyntheticMessage && Info.getNumFixItHints() == 1) {
+  const auto &FixIt = Info.getFixItHint(0);
+  bool Invalid = false;
+  StringRef Remove = Lexer::getSourceText(
+  FixIt.RemoveRange, Info.getSourceManager(), *LangOpts, &Invalid);
+  StringRef Insert = FixIt.CodeToInsert;
+  if (!Invalid) {
+llvm::raw_svector_ostream M(Message);
+if (!Remove.empty() && !Insert.empty())
+  M << "change '" << Remove << "' to '" << Insert << "'";
+else if (!Remove.empty())
+  M << "remove '" << Remove << "'";
+else if (!Insert.empty())
+  M << "insert '" << Insert << "'";
+// Don't allow source code to inject newlines into diagnostics.
+std::replace(Message.begin(), Message.end(), '\n', ' ');
+  }
+}
+if (Message.empty()) // either !SytheticMessage, or we failed to make one.
+  Info.FormatDiagnostic(Message);
 LastDiag->Fixes.push_back(Fix{Message.str(), std::move(Edits)});
 return true;
   };
@@ -325,7 +345,7 @@
 FillDiagBase(*LastDiag);
 
 if (!Info.getFixItHints().empty())
-  AddFix();
+  AddFix(true /* try to invent a message instead of repeating the diag */);
   } else {
 // Handle a note to an existing diagnostic.
 if (!LastDiag) {
@@ -337,7 +357,7 @@
 if (!Info.getFixItHints().empty()) {
   // A clang note with fix-it is not a separate diagnostic in clangd. We
   // attach it as a Fix to the main diagnostic instead.
-  if (!AddFix())
+  if (!AddFix(false /* use the note as the message */))
 IgnoreDiagnostics::log(DiagLevel, Info);
 } else {
   // A clang note without fix-its corresponds to clangd::Note.
Index: clang-tools-extra/trunk/unittests/clangd/ClangdUnitTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/ClangdUnitTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/ClangdUnitTests.cpp
@@ -92,14 +92,6 @@
   return Res;
 }
 
-/// Matches diagnostic that has exactly one fix with the same range and message
-/// as the diagnostic itself.
-testing::Matcher DiagWithEqualFix(clangd::Range Range,
-std::string Replacement,
-std::string Message) {
-  return AllOf(Diag(Range, Message), WithFix(Fix(Range, Replacement, Message)));
-}
-
 TEST(DiagnosticsTest, DiagnosticRanges) {
   // Check we report correct ranges, including various edge-cases.
   Annotations Test(R"cpp(
@@ -111,28 +103,29 @@
   $unk[[unknown]]();
 }
   )cpp");
-  llvm::errs() << Test.code();
   EXPECT_THAT(
   buildDiags(Test.code()),
   ElementsAre(
   // This range spans lines.
-  AllOf(DiagWithEqualFix(
-Test.range("typo"), "foo",
-"use of undeclared identifier 'goo'; did you mean 'foo'?"),
+  AllOf(Diag(Test.range("typo"),
+ "use of undeclared identifier 'goo'; did you mean 'foo'?"),
+WithFix(
+Fix(Test.range("typo"), "foo", "change 'go\\ o' to 'foo'")),
 // This is a pretty normal range.
 WithNote(Diag(Test.range("decl"), "'foo' declared here"))),
   // This range is zero-width, and at the end of a line.
-  DiagWithEqualFix(Test.range("semicolon"), ";",
-   "expected ';' after expression"),
+  AllOf(Diag(Test.range("semicolon"), "expected ';' after expression"),
+WithFix(Fix(Test.range("semicolon"), ";", "insert ';'"))),
   // This range isn't provided by clang, we expand to the token.
   Diag(Test.range("unk"), "use of undeclared identifier 'unknown'")));
 }
 
 TEST(Diagn

[PATCH] D44747: Set calling convention for CUDA kernel

2018-04-03 Thread Artem Belevich via Phabricator via cfe-commits
tra added inline comments.



Comment at: lib/Sema/SemaType.cpp:3319-3330
+  // Attribute AT_CUDAGlobal affects the calling convention for AMDGPU targets.
+  // This is the simplest place to infer calling convention for CUDA kernels.
+  if (S.getLangOpts().CUDA && S.getLangOpts().CUDAIsDevice) {
+for (const AttributeList *Attr = D.getDeclSpec().getAttributes().getList();
+ Attr; Attr = Attr->getNext()) {
+  if (Attr->getKind() == AttributeList::AT_CUDAGlobal) {
+CC = CC_CUDAKernel;

This apparently breaks compilation of some CUDA code in our internal tests. I'm 
working on minimizing a reproduction case. Should this code be enabled for AMD 
GPUs only?


Repository:
  rL LLVM

https://reviews.llvm.org/D44747



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


[PATCH] D44842: Add Parameters to DW_AT_name Attribute of Template Variables

2018-04-03 Thread Jim Ingham via Phabricator via cfe-commits
jingham added a comment.

Apparently this makes a global variable, so "frame var" wouldn't show it by 
default.  What does "frame var -g" show?

We would also need to get:

  (lldb) frame var -g crazy

or something like it to work.  "frame var" has its own parser to support "->" 
and ".".  That doesn't currently work with:

  (lldb) frame var -g crazy
  error: unexpected char '<' encountered after "crazy" in ""

so for that part to work, either the name lookup must work w/o the  or 
frame var's parser must be updated to cope with this (and of course with any 
arbitrarily complex type that could get substituted in there).  That's likely a 
non-trivial bit of work.

I wonder if expr is failing because this is a C++14 extension, lldb sets 
CPlusPlus11 to true in the LangOpts for the compiler it makes, but not 
CPlusPlus14.


Repository:
  rC Clang

https://reviews.llvm.org/D44842



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


[PATCH] D45163: [Sema] -Wunused-value: diagnose unused std::move() call results.

2018-04-03 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

Well, we should feel welcome to submit patches to the C++ library 
implementations, I think.  I can understand why Marshall is waiting to just do 
this until he gets a comprehensive committee paper, but he might consider 
taking a patch in the meantime.  But I'm not sure what the rush is if it 
doesn't change what goes into any concrete release of the compiler + library.


Repository:
  rC Clang

https://reviews.llvm.org/D45163



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


[PATCH] D44747: Set calling convention for CUDA kernel

2018-04-03 Thread Artem Belevich via Phabricator via cfe-commits
tra added inline comments.



Comment at: lib/Sema/SemaType.cpp:3319-3330
+  // Attribute AT_CUDAGlobal affects the calling convention for AMDGPU targets.
+  // This is the simplest place to infer calling convention for CUDA kernels.
+  if (S.getLangOpts().CUDA && S.getLangOpts().CUDAIsDevice) {
+for (const AttributeList *Attr = D.getDeclSpec().getAttributes().getList();
+ Attr; Attr = Attr->getNext()) {
+  if (Attr->getKind() == AttributeList::AT_CUDAGlobal) {
+CC = CC_CUDAKernel;

tra wrote:
> This apparently breaks compilation of some CUDA code in our internal tests. 
> I'm working on minimizing a reproduction case. Should this code be enabled 
> for AMD GPUs only?
Here's a small snippet of code that previously used to compile and work:

```
template 
__global__ void EmptyKernel(void) { }

struct Dummy {
  /// Type definition of the EmptyKernel kernel entry point
  typedef void (*EmptyKernelPtr)();
  EmptyKernelPtr Empty() { return EmptyKernel; }
};
```
AFAICT,  it's currently impossible to apply __global__ to pointers, so there's 
no way to make the code above work with this patch applied.


Repository:
  rL LLVM

https://reviews.llvm.org/D44747



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


[PATCH] D45212: [HIP] Let CUDA toolchain support HIP language mode

2018-04-03 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl updated this revision to Diff 140827.
yaxunl retitled this revision from "[WIP][HIP] Let CUDA toolchain support HIP 
language mode" to "[HIP] Let CUDA toolchain support HIP language mode".
yaxunl edited the summary of this revision.
yaxunl added a comment.

Fixed typo which causes lit test failures. Now all lit tests pass.


https://reviews.llvm.org/D45212

Files:
  include/clang/Basic/Cuda.h
  include/clang/Basic/DiagnosticDriverKinds.td
  include/clang/Driver/Options.td
  include/clang/Driver/ToolChain.h
  include/clang/Driver/Types.def
  lib/Basic/Cuda.cpp
  lib/Basic/Targets.cpp
  lib/Basic/Targets.h
  lib/Basic/Targets/AMDGPU.cpp
  lib/Basic/Targets/AMDGPU.h
  lib/Basic/Targets/NVPTX.cpp
  lib/Driver/Driver.cpp
  lib/Driver/SanitizerArgs.cpp
  lib/Driver/ToolChain.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Driver/ToolChains/Cuda.cpp
  lib/Driver/ToolChains/Cuda.h
  lib/Driver/ToolChains/Gnu.cpp
  lib/Driver/Types.cpp
  lib/Frontend/CompilerInstance.cpp
  test/Driver/cuda-phases.cu

Index: test/Driver/cuda-phases.cu
===
--- test/Driver/cuda-phases.cu
+++ test/Driver/cuda-phases.cu
@@ -7,22 +7,26 @@
 // REQUIRES: clang-driver
 // REQUIRES: powerpc-registered-target
 // REQUIRES: nvptx-registered-target
-
+// REQUIRES: amdgpu-registered-target
 //
 // Test single gpu architecture with complete compilation.
 //
 // RUN: %clang -target powerpc64le-ibm-linux-gnu -ccc-print-phases --cuda-gpu-arch=sm_30 %s 2>&1 \
-// RUN: | FileCheck -check-prefix=BIN %s
-// BIN-DAG: [[P0:[0-9]+]]: input, "{{.*}}cuda-phases.cu", cuda, (host-cuda)
-// BIN-DAG: [[P1:[0-9]+]]: preprocessor, {[[P0]]}, cuda-cpp-output, (host-cuda)
+// RUN: | FileCheck -check-prefixes=BIN,BIN_NV %s
+// RUN: %clang -x hip -target powerpc64le-ibm-linux-gnu -ccc-print-phases --cuda-gpu-arch=gfx803 %s 2>&1 \
+// RUN: | FileCheck -check-prefixes=BIN,BIN_AMD %s
+// BIN_NV-DAG: [[P0:[0-9]+]]: input, "{{.*}}cuda-phases.cu", [[T:cuda]], (host-cuda)
+// BIN_AMD-DAG: [[P0:[0-9]+]]: input, "{{.*}}cuda-phases.cu", [[T:hip]], (host-cuda)
+// BIN-DAG: [[P1:[0-9]+]]: preprocessor, {[[P0]]}, [[T]]-cpp-output, (host-cuda)
 // BIN-DAG: [[P2:[0-9]+]]: compiler, {[[P1]]}, ir, (host-cuda)
-// BIN-DAG: [[P3:[0-9]+]]: input, "{{.*}}cuda-phases.cu", cuda, (device-cuda, sm_30)
-// BIN-DAG: [[P4:[0-9]+]]: preprocessor, {[[P3]]}, cuda-cpp-output, (device-cuda, sm_30)
-// BIN-DAG: [[P5:[0-9]+]]: compiler, {[[P4]]}, ir, (device-cuda, sm_30)
-// BIN-DAG: [[P6:[0-9]+]]: backend, {[[P5]]}, assembler, (device-cuda, sm_30)
-// BIN-DAG: [[P7:[0-9]+]]: assembler, {[[P6]]}, object, (device-cuda, sm_30)
-// BIN-DAG: [[P8:[0-9]+]]: offload, "device-cuda (nvptx64-nvidia-cuda:sm_30)" {[[P7]]}, object
-// BIN-DAG: [[P9:[0-9]+]]: offload, "device-cuda (nvptx64-nvidia-cuda:sm_30)" {[[P6]]}, assembler
+// BIN_NV-DAG: [[P3:[0-9]+]]: input, "{{.*}}cuda-phases.cu", [[T]], (device-cuda, [[ARCH:sm_30]])
+// BIN_AMD-DAG: [[P3:[0-9]+]]: input, "{{.*}}cuda-phases.cu", [[T]], (device-cuda, [[ARCH:gfx803]])
+// BIN-DAG: [[P4:[0-9]+]]: preprocessor, {[[P3]]}, [[T]]-cpp-output, (device-cuda, [[ARCH]])
+// BIN-DAG: [[P5:[0-9]+]]: compiler, {[[P4]]}, ir, (device-cuda, [[ARCH]])
+// BIN-DAG: [[P6:[0-9]+]]: backend, {[[P5]]}, assembler, (device-cuda, [[ARCH]])
+// BIN-DAG: [[P7:[0-9]+]]: assembler, {[[P6]]}, object, (device-cuda, [[ARCH]])
+// BIN-DAG: [[P8:[0-9]+]]: offload, "device-cuda (nvptx64-nvidia-cuda:[[ARCH]])" {[[P7]]}, object
+// BIN-DAG: [[P9:[0-9]+]]: offload, "device-cuda (nvptx64-nvidia-cuda:[[ARCH]])" {[[P6]]}, assembler
 // BIN-DAG: [[P10:[0-9]+]]: linker, {[[P8]], [[P9]]}, cuda-fatbin, (device-cuda)
 // BIN-DAG: [[P11:[0-9]+]]: offload, "host-cuda (powerpc64le-ibm-linux-gnu)" {[[P2]]}, "device-cuda (nvptx64-nvidia-cuda)" {[[P10]]}, ir
 // BIN-DAG: [[P12:[0-9]+]]: backend, {[[P11]]}, assembler, (host-cuda)
@@ -34,38 +38,42 @@
 //
 // RUN: %clang -target powerpc64le-ibm-linux-gnu -ccc-print-phases --cuda-gpu-arch=sm_30 %s -S 2>&1 \
 // RUN: | FileCheck -check-prefix=ASM %s
-// ASM-DAG: [[P0:[0-9]+]]: input, "{{.*}}cuda-phases.cu", cuda, (device-cuda, sm_30)
-// ASM-DAG: [[P1:[0-9]+]]: preprocessor, {[[P0]]}, cuda-cpp-output, (device-cuda, sm_30)
-// ASM-DAG: [[P2:[0-9]+]]: compiler, {[[P1]]}, ir, (device-cuda, sm_30)
-// ASM-DAG: [[P3:[0-9]+]]: backend, {[[P2]]}, assembler, (device-cuda, sm_30)
-// ASM-DAG: [[P4:[0-9]+]]: offload, "device-cuda (nvptx64-nvidia-cuda:sm_30)" {[[P3]]}, assembler
-// ASM-DAG: [[P5:[0-9]+]]: input, "{{.*}}cuda-phases.cu", cuda, (host-cuda)
-// ASM-DAG: [[P6:[0-9]+]]: preprocessor, {[[P5]]}, cuda-cpp-output, (host-cuda)
+// RUN: %clang -x hip -target powerpc64le-ibm-linux-gnu -ccc-print-phases --cuda-gpu-arch=gfx803 %s -S 2>&1 \
+// RUN: | FileCheck -check-prefix=ASM %s
+// ASM-DAG: [[P0:[0-9]+]]: input, "{{.*}}cuda-phases.cu", [[T:cuda|hip]], (device-cuda, [[ARCH:sm_30|gfx803]])
+// ASM-DAG: [[P1:[0-9]+]]: preprocessor, {[[P0]]}, [[T]]-cpp-output, (device-cuda, [[ARCH]])
+// ASM-DAG: [[P2:[0-9]+

[PATCH] D45152: [MinGW] Add option for disabling looking for a mingw gcc in PATH

2018-04-03 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

Seems reasonable, looks good.


Repository:
  rC Clang

https://reviews.llvm.org/D45152



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


[PATCH] D44842: Add Parameters to DW_AT_name Attribute of Template Variables

2018-04-03 Thread Matthew Voss via Phabricator via cfe-commits
ormris added a comment.

About to test with CPlusPlus14 enabled

Here's the output from those two commands.

  (lldb) frame var -g crazy
  error: no variable named 'crazy' found in this frame
  (lldb) frame var -g crazy
  error: no variable named 'crazy' found in this frame
  (lldb)


Repository:
  rC Clang

https://reviews.llvm.org/D44842



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


RE: [PATCH] D44747: Set calling convention for CUDA kernel

2018-04-03 Thread Liu, Yaxun (Sam) via cfe-commits
I will try fixing that.

The CUDA kernel calling convention should be dropped in all DRE's since it is 
invisible to the user.

Sam

-Original Message-
From: Artem Belevich via Phabricator [mailto:revi...@reviews.llvm.org] 
Sent: Tuesday, April 03, 2018 1:51 PM
To: Liu, Yaxun (Sam) ; rjmcc...@gmail.com; Arsenault, 
Matthew 
Cc: llvm-comm...@lists.llvm.org; t...@google.com; Zhuravlyov, Konstantin 
; wei.di...@amd.com; Stuttard, David 
; tpr.l...@botech.co.uk; Tye, Tony ; 
cfe-commits@lists.llvm.org; jle...@google.com
Subject: [PATCH] D44747: Set calling convention for CUDA kernel

tra added inline comments.



Comment at: lib/Sema/SemaType.cpp:3319-3330
+  // Attribute AT_CUDAGlobal affects the calling convention for AMDGPU targets.
+  // This is the simplest place to infer calling convention for CUDA kernels.
+  if (S.getLangOpts().CUDA && S.getLangOpts().CUDAIsDevice) {
+for (const AttributeList *Attr = D.getDeclSpec().getAttributes().getList();
+ Attr; Attr = Attr->getNext()) {
+  if (Attr->getKind() == AttributeList::AT_CUDAGlobal) {
+CC = CC_CUDAKernel;

tra wrote:
> This apparently breaks compilation of some CUDA code in our internal tests. 
> I'm working on minimizing a reproduction case. Should this code be enabled 
> for AMD GPUs only?
Here's a small snippet of code that previously used to compile and work:

```
template 
__global__ void EmptyKernel(void) { }

struct Dummy {
  /// Type definition of the EmptyKernel kernel entry point
  typedef void (*EmptyKernelPtr)();
  EmptyKernelPtr Empty() { return EmptyKernel; } }; ``` AFAICT,  it's 
currently impossible to apply __global__ to pointers, so there's no way to make 
the code above work with this patch applied.


Repository:
  rL LLVM

https://reviews.llvm.org/D44747



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


[PATCH] D44842: Add Parameters to DW_AT_name Attribute of Template Variables

2018-04-03 Thread Matthew Voss via Phabricator via cfe-commits
ormris added a comment.

No such luck...
Patch:

  Index: source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
  ===
  --- source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp 
(revision 329079)
  +++ source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp 
(working copy)
  @@ -400,8 +400,9 @@
   break;
 case lldb::eLanguageTypeC_plus_plus:
 case lldb::eLanguageTypeC_plus_plus_11:
  +m_compiler->getLangOpts().CPlusPlus11 = true;
 case lldb::eLanguageTypeC_plus_plus_14:
  -m_compiler->getLangOpts().CPlusPlus11 = true;
  +m_compiler->getLangOpts().CPlusPlus14 = true;
   m_compiler->getHeaderSearchOpts().UseLibcxx = true;
   LLVM_FALLTHROUGH;
 case lldb::eLanguageTypeC_plus_plus_03:

Output:

  (lldb) e crazy
  error: use of undeclared identifier 'crazy'
  error: expected '(' for function-style cast or type construction
  error: expected expression


Repository:
  rC Clang

https://reviews.llvm.org/D44842



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


[PATCH] D45161: [AST] Add new printing policy to suppress printing template arguments

2018-04-03 Thread Kalle Huttunen via Phabricator via cfe-commits
khuttun abandoned this revision.
khuttun added a comment.

Figured out a better way to do this by using 
`FunctionDecl::getInstantiatedFromMemberFunction`


Repository:
  rC Clang

https://reviews.llvm.org/D45161



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


[PATCH] D44747: Set calling convention for CUDA kernel

2018-04-03 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment.

In https://reviews.llvm.org/D44747#1055877, @yaxunl wrote:

> I will try fixing that.
>
> The CUDA kernel calling convention should be dropped in all DRE's since it is 
> invisible to the user.
>
> Sam


Apparently it's not always the case.
Here's a bit more elaborate example demonstrating inconsistency in this 
behavior. Calling convention is ignored for regular functions, but not for 
function templates.

  __global__ void EmptyKernel(void) { }
  
  template 
  __global__ void EmptyKernelT(void) { }
  
  struct Dummy {
/// Type definition of the EmptyKernel kernel entry point
typedef void (*EmptyKernelPtr)();
EmptyKernelPtr Empty() { return EmptyKernel; } // this one works
EmptyKernelPtr EmptyT() { return EmptyKernelT; } // this one errors 
out.
  };

Do you think this is something you can fix quickly or do you want to unroll the 
change while you figure out what's going on?


Repository:
  rL LLVM

https://reviews.llvm.org/D44747



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


RE: [PATCH] D44747: Set calling convention for CUDA kernel

2018-04-03 Thread Liu, Yaxun (Sam) via cfe-commits
Let's revert it for now. I will create a review after fixing it and commit it 
again with the fix.

Thanks.

Sam 

-Original Message-
From: Artem Belevich via Phabricator [mailto:revi...@reviews.llvm.org] 
Sent: Tuesday, April 03, 2018 2:09 PM
To: Liu, Yaxun (Sam) ; rjmcc...@gmail.com; Arsenault, 
Matthew 
Cc: jle...@google.com; llvm-comm...@lists.llvm.org; t...@google.com; 
Zhuravlyov, Konstantin ; wei.di...@amd.com; 
Stuttard, David ; tpr.l...@botech.co.uk; Tye, Tony 
; cfe-commits@lists.llvm.org
Subject: [PATCH] D44747: Set calling convention for CUDA kernel

tra added a comment.

In https://reviews.llvm.org/D44747#1055877, @yaxunl wrote:

> I will try fixing that.
>
> The CUDA kernel calling convention should be dropped in all DRE's since it is 
> invisible to the user.
>
> Sam


Apparently it's not always the case.
Here's a bit more elaborate example demonstrating inconsistency in this 
behavior. Calling convention is ignored for regular functions, but not for 
function templates.

  __global__ void EmptyKernel(void) { }
  
  template 
  __global__ void EmptyKernelT(void) { }
  
  struct Dummy {
/// Type definition of the EmptyKernel kernel entry point
typedef void (*EmptyKernelPtr)();
EmptyKernelPtr Empty() { return EmptyKernel; } // this one works
EmptyKernelPtr EmptyT() { return EmptyKernelT; } // this one errors 
out.
  };

Do you think this is something you can fix quickly or do you want to unroll the 
change while you figure out what's going on?


Repository:
  rL LLVM

https://reviews.llvm.org/D44747



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


[PATCH] D45217: [ThinLTO] Pass -save-temps to LTO backend for distributed ThinLTO builds

2018-04-03 Thread Teresa Johnson via Phabricator via cfe-commits
tejohnson created this revision.
tejohnson added a reviewer: pcc.
Herald added subscribers: eraman, inglorion, mehdi_amini.

The clang driver option -save-temps was not passed to the LTO config,
so when invoking the ThinLTO backends via clang during distributed
builds there was no way to get LTO to save temp files.

Getting this to work with ThinLTO distributed builds also required
changing the driver to avoid a separate compile step to emit unoptimized
bitcode when the input was already bitcode under -save-temps. Not only is
this unnecessary in general, it is problematic for ThinLTO backends since
the temporary bitcode file to the backend would not match the module path
in the combined index, leading to incorrect ThinLTO backend index-based
optimizations.


Repository:
  rC Clang

https://reviews.llvm.org/D45217

Files:
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.h
  lib/CodeGen/BackendUtil.cpp
  lib/Driver/Driver.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/thinlto_backend.ll
  test/Driver/thinlto_backend.c

Index: test/Driver/thinlto_backend.c
===
--- test/Driver/thinlto_backend.c
+++ test/Driver/thinlto_backend.c
@@ -5,6 +5,15 @@
 // RUN: %clang -O2 -o %t1.o -x ir %t.o -c -fthinlto-index=%t.thinlto.bc -### 2>&1 | FileCheck %s -check-prefix=CHECK-THINLTOBE-ACTION
 // CHECK-THINLTOBE-ACTION: -fthinlto-index=
 
+// -save-temps should be passed to cc1
+// RUN: %clang -O2 -o %t1.o -x ir %t.o -c -fthinlto-index=%t.thinlto.bc -save-temps -### 2>&1 | FileCheck %s -check-prefix=CHECK-SAVE-TEMPS -check-prefix=CHECK-SAVE-TEMPS-CWD
+// RUN: %clang -O2 -o %t1.o -x ir %t.o -c -fthinlto-index=%t.thinlto.bc -save-temps=cwd -### 2>&1 | FileCheck %s -check-prefix=CHECK-SAVE-TEMPS -check-prefix=CHECK-SAVE-TEMPS-CWD
+// RUN: %clang -O2 -o %t1.o -x ir %t.o -c -fthinlto-index=%t.thinlto.bc -save-temps=obj -### 2>&1 | FileCheck %s -check-prefix=CHECK-SAVE-TEMPS -check-prefix=CHECK-SAVE-TEMPS-OBJ
+// CHECK-SAVE-TEMPS-NOT: -emit-llvm-bc
+// CHECK-SAVE-TEMPS-CWD: -save-temps=cwd
+// CHECK-SAVE-TEMPS-OBJ: -save-temps=obj
+// CHECK-SAVE-TEMPS-NOT: -emit-llvm-bc
+
 // Ensure clang driver gives the expected error for incorrect input type
 // RUN: not %clang -O2 -o %t1.o %s -c -fthinlto-index=%t.thinlto.bc 2>&1 | FileCheck %s -check-prefix=CHECK-WARNING
 // CHECK-WARNING: error: invalid argument '-fthinlto-index={{.*}}' only allowed with '-x ir'
Index: test/CodeGen/thinlto_backend.ll
===
--- test/CodeGen/thinlto_backend.ll
+++ test/CodeGen/thinlto_backend.ll
@@ -27,10 +27,12 @@
 ; RUN: llvm-nm %t4.o | count 0
 
 ; Ensure f2 was imported
-; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t3.o -x ir %t1.o -c -fthinlto-index=%t.thinlto.bc
+; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t3.o -x ir %t1.o -c -fthinlto-index=%t.thinlto.bc -save-temps=obj
 ; RUN: llvm-nm %t3.o | FileCheck --check-prefix=CHECK-OBJ %s
 ; CHECK-OBJ: T f1
 ; CHECK-OBJ-NOT: U f2
+; RUN: llvm-dis %t1.s.0.3.import.bc -o - | FileCheck --check-prefix=CHECK-IMPORT %s
+; CHECK-IMPORT: define available_externally void @f2()
 
 ; Ensure we get expected error for input files without summaries
 ; RUN: opt -o %t2.o %s
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -487,7 +487,8 @@
 
 static bool ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args, InputKind IK,
  DiagnosticsEngine &Diags,
- const TargetOptions &TargetOpts) {
+ const TargetOptions &TargetOpts,
+ const FrontendOptions &FrontendOpts) {
   bool Success = true;
   llvm::Triple Triple = llvm::Triple(TargetOpts.Triple);
 
@@ -739,6 +740,13 @@
   << A->getAsString(Args) << "-x ir";
 Opts.ThinLTOIndexFile = Args.getLastArgValue(OPT_fthinlto_index_EQ);
   }
+  if (Arg *A = Args.getLastArg(OPT_save_temps_EQ))
+Opts.SaveTempsFilePrefix =
+llvm::StringSwitch(A->getValue())
+.Case("obj", FrontendOpts.OutputFile)
+.Default("./" +
+ llvm::sys::path::filename(FrontendOpts.OutputFile).str());
+
   Opts.ThinLinkBitcodeFile = Args.getLastArgValue(OPT_fthin_link_bitcode_EQ);
 
   Opts.MSVolatile = Args.hasArg(OPT_fms_volatile);
@@ -2890,7 +2898,7 @@
   LangOpts.IsHeaderFile);
   ParseTargetArgs(Res.getTargetOpts(), Args, Diags);
   Success &= ParseCodeGenArgs(Res.getCodeGenOpts(), Args, DashX, Diags,
-  Res.getTargetOpts());
+  Res.getTargetOpts(), Res.getFrontendOpts());
   ParseHeaderSearchArgs(Res.getHeaderSearchOpts(), Args,
 Res.getFileSystemOpts().WorkingDir);
   if (DashX

r329097 - Restrict a test using named file descriptors to using the system shell

2018-04-03 Thread David Blaikie via cfe-commits
Author: dblaikie
Date: Tue Apr  3 11:22:14 2018
New Revision: 329097

URL: http://llvm.org/viewvc/llvm-project?rev=329097&view=rev
Log:
Restrict a test using named file descriptors to using the system shell

Modified:
cfe/trunk/test/Misc/dev-fd-fs.c

Modified: cfe/trunk/test/Misc/dev-fd-fs.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Misc/dev-fd-fs.c?rev=329097&r1=329096&r2=329097&view=diff
==
--- cfe/trunk/test/Misc/dev-fd-fs.c (original)
+++ cfe/trunk/test/Misc/dev-fd-fs.c Tue Apr  3 11:22:14 2018
@@ -1,5 +1,6 @@
 // Check that we can operate on files from /dev/fd.
 // REQUIRES: dev-fd-fs
+// REQUIRES: shell
 
 // Check reading from named pipes. We cat the input here instead of redirecting
 // it to ensure that /dev/fd/0 is a named pipe, not just a redirected file.


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


r329098 - Use Clang when referring to the project and clang when referring to the binary.

2018-04-03 Thread Richard Smith via cfe-commits
Author: rsmith
Date: Tue Apr  3 11:28:13 2018
New Revision: 329098

URL: http://llvm.org/viewvc/llvm-project?rev=329098&view=rev
Log:
Use Clang when referring to the project and clang when referring to the binary.

Modified:
cfe/trunk/docs/ReleaseNotes.rst

Modified: cfe/trunk/docs/ReleaseNotes.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ReleaseNotes.rst?rev=329098&r1=329097&r2=329098&view=diff
==
--- cfe/trunk/docs/ReleaseNotes.rst (original)
+++ cfe/trunk/docs/ReleaseNotes.rst Tue Apr  3 11:28:13 2018
@@ -62,8 +62,9 @@ Improvements to Clang's diagnostics
 Non-comprehensive list of changes in this release
 -
 
-- clang binary and libraries have been renamed from 7.0 to 7.
-  For example, the clang binary will be called clang-7 instead of clang-7.0.
+- Clang binary and libraries have been renamed from 7.0 to 7.
+  For example, the ``clang`` binary will be called ``clang-7``
+  instead of ``clang-7.0``.
 
 - ...
 


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


[PATCH] D44878: libFuzzer OpenBSD support

2018-04-03 Thread David CARLIER via Phabricator via cfe-commits
devnexen added a comment.

ping


Repository:
  rC Clang

https://reviews.llvm.org/D44878



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


[PATCH] D45174: non-zero-length bit-fields should make a class non-empty

2018-04-03 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith marked an inline comment as done.
rsmith added inline comments.



Comment at: ReleaseNotes.rst:68-72
+- Clang implements the proposed resolution of LWG issue 2358, along with the
+  `corresponding change to the Itanium C++ ABI
+  `_, which make classes
+  containing only unnamed non-zero-length bit-fields be considered non-empty.
+  This is an ABI break compared to prior Clang releases, but makes Clang

majnemer wrote:
> The "Clang" in the above paragraph has a lowercase "clang". I think we should 
> choose a consistent capitalization.
Fixed in r329098.


Repository:
  rC Clang

https://reviews.llvm.org/D45174



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


r329099 - Revert "Set calling convention for CUDA kernel"

2018-04-03 Thread Artem Belevich via cfe-commits
Author: tra
Date: Tue Apr  3 11:29:31 2018
New Revision: 329099

URL: http://llvm.org/viewvc/llvm-project?rev=329099&view=rev
Log:
Revert "Set calling convention for CUDA kernel"

This reverts r328795 which introduced an issue with referencing __global__
function templates. More details in the original review D44747.

Removed:
cfe/trunk/test/CodeGenCUDA/kernel-amdgcn.cu
Modified:
cfe/trunk/include/clang/Basic/Specifiers.h
cfe/trunk/lib/AST/ItaniumMangle.cpp
cfe/trunk/lib/AST/Type.cpp
cfe/trunk/lib/AST/TypePrinter.cpp
cfe/trunk/lib/CodeGen/CGCall.cpp
cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
cfe/trunk/lib/CodeGen/TargetInfo.cpp
cfe/trunk/lib/CodeGen/TargetInfo.h
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/lib/Sema/SemaOverload.cpp
cfe/trunk/lib/Sema/SemaType.cpp
cfe/trunk/tools/libclang/CXType.cpp

Modified: cfe/trunk/include/clang/Basic/Specifiers.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Specifiers.h?rev=329099&r1=329098&r2=329099&view=diff
==
--- cfe/trunk/include/clang/Basic/Specifiers.h (original)
+++ cfe/trunk/include/clang/Basic/Specifiers.h Tue Apr  3 11:29:31 2018
@@ -231,24 +231,23 @@ namespace clang {
 
   /// \brief CallingConv - Specifies the calling convention that a function 
uses.
   enum CallingConv {
-CC_C, // __attribute__((cdecl))
-CC_X86StdCall,// __attribute__((stdcall))
-CC_X86FastCall,   // __attribute__((fastcall))
-CC_X86ThisCall,   // __attribute__((thiscall))
+CC_C,   // __attribute__((cdecl))
+CC_X86StdCall,  // __attribute__((stdcall))
+CC_X86FastCall, // __attribute__((fastcall))
+CC_X86ThisCall, // __attribute__((thiscall))
 CC_X86VectorCall, // __attribute__((vectorcall))
-CC_X86Pascal, // __attribute__((pascal))
-CC_Win64, // __attribute__((ms_abi))
-CC_X86_64SysV,// __attribute__((sysv_abi))
-CC_X86RegCall,// __attribute__((regcall))
-CC_AAPCS, // __attribute__((pcs("aapcs")))
-CC_AAPCS_VFP, // __attribute__((pcs("aapcs-vfp")))
-CC_IntelOclBicc,  // __attribute__((intel_ocl_bicc))
-CC_SpirFunction,  // default for OpenCL functions on SPIR target
-CC_OpenCLKernel,  // inferred for OpenCL kernels
-CC_Swift, // __attribute__((swiftcall))
-CC_PreserveMost,  // __attribute__((preserve_most))
-CC_PreserveAll,   // __attribute__((preserve_all))
-CC_CUDAKernel,// inferred for CUDA kernels
+CC_X86Pascal,   // __attribute__((pascal))
+CC_Win64,   // __attribute__((ms_abi))
+CC_X86_64SysV,  // __attribute__((sysv_abi))
+CC_X86RegCall, // __attribute__((regcall))
+CC_AAPCS,   // __attribute__((pcs("aapcs")))
+CC_AAPCS_VFP,   // __attribute__((pcs("aapcs-vfp")))
+CC_IntelOclBicc, // __attribute__((intel_ocl_bicc))
+CC_SpirFunction, // default for OpenCL functions on SPIR target
+CC_OpenCLKernel, // inferred for OpenCL kernels
+CC_Swift,// __attribute__((swiftcall))
+CC_PreserveMost, // __attribute__((preserve_most))
+CC_PreserveAll,  // __attribute__((preserve_all))
   };
 
   /// \brief Checks whether the given calling convention supports variadic

Modified: cfe/trunk/lib/AST/ItaniumMangle.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ItaniumMangle.cpp?rev=329099&r1=329098&r2=329099&view=diff
==
--- cfe/trunk/lib/AST/ItaniumMangle.cpp (original)
+++ cfe/trunk/lib/AST/ItaniumMangle.cpp Tue Apr  3 11:29:31 2018
@@ -2628,7 +2628,6 @@ StringRef CXXNameMangler::getCallingConv
   case CC_OpenCLKernel:
   case CC_PreserveMost:
   case CC_PreserveAll:
-  case CC_CUDAKernel:
 // FIXME: we should be mangling all of the above.
 return "";
 

Modified: cfe/trunk/lib/AST/Type.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Type.cpp?rev=329099&r1=329098&r2=329099&view=diff
==
--- cfe/trunk/lib/AST/Type.cpp (original)
+++ cfe/trunk/lib/AST/Type.cpp Tue Apr  3 11:29:31 2018
@@ -2748,7 +2748,6 @@ StringRef FunctionType::getNameForCallCo
   case CC_Swift: return "swiftcall";
   case CC_PreserveMost: return "preserve_most";
   case CC_PreserveAll: return "preserve_all";
-  case CC_CUDAKernel: return "cuda_kernel";
   }
 
   llvm_unreachable("Invalid calling convention.");

Modified: cfe/trunk/lib/AST/TypePrinter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/TypePrinter.cpp?rev=329099&r1=329098&r2=329099&view=diff
==
--- cfe/trunk/lib/AST/TypePrinter.cpp (original)
+++ cfe/trunk/lib/AST/TypePrinter.cpp Tue Apr  3 11:29:31 2018
@@ -780,10 +780,6 @@ void TypePrinter::printFunctionAfter(con
 case CC_OpenCLKernel:
   // Do nothing. These CCs are not a

[PATCH] D44747: Set calling convention for CUDA kernel

2018-04-03 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment.

In https://reviews.llvm.org/D44747#1055894, @yaxunl wrote:

> Let's revert it for now. I will create a review after fixing it and commit it 
> again with the fix.
>
> Thanks.
>
> Sam


reverted in r329099.


Repository:
  rL LLVM

https://reviews.llvm.org/D44747



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


[PATCH] D45163: [Sema] -Wunused-value: diagnose unused std::move() call results.

2018-04-03 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment.

In https://reviews.llvm.org/D45163#1055856, @rjmccall wrote:

> Well, we should feel welcome to submit patches to the C++ library 
> implementations, I think.  I can understand why Marshall is waiting to just 
> do this until he gets a comprehensive committee paper,




> but he might consider taking a patch in the meantime.

That has been discussed, and it's not going to happen. Ask him if you want more 
info.

> But I'm not sure what the rush is if it doesn't change what goes into any 
> concrete release of the compiler + library.

TLDW: `std::move()` is "The Assign Operator (`a = b;`) of RAII". It would be 
good to diagnose such a problem, and not just rely that some day the std libs 
will mark it with the attribute.
BTW, even when they do, even for libcxx, it won't be of any immediate use to 
LLVM, we will need to provide a define (see https://reviews.llvm.org/D45179) to 
actually enable that attribute, because LLVM is using C++11, not 
C++17/something...



Repository:
  rC Clang

https://reviews.llvm.org/D45163



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


r329102 - [analyzer] Fix diagnostics in callees of interesting callees.

2018-04-03 Thread Artem Dergachev via cfe-commits
Author: dergachev
Date: Tue Apr  3 11:52:30 2018
New Revision: 329102

URL: http://llvm.org/viewvc/llvm-project?rev=329102&view=rev
Log:
[analyzer] Fix diagnostics in callees of interesting callees.

removeUnneededCalls() is responsible for removing path diagnostic pieces within
functions that don't contain "interesting" events. It makes bug reports
much tidier.

When a stack frame is known to be interesting, the function doesn't descend
into it to prune anything within it, even other callees that are totally boring.

Fix the function to prune boring callees in interesting stack frames.

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

Modified:
cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
cfe/trunk/test/Analysis/inlining/path-notes.c

Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp?rev=329102&r1=329101&r2=329102&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp Tue Apr  3 11:52:30 2018
@@ -192,8 +192,9 @@ using LocationContextMap =
 /// that aren't needed.  Return true if afterwards the path contains
 /// "interesting stuff" which means it shouldn't be pruned from the parent 
path.
 static bool removeUnneededCalls(PathPieces &pieces, BugReport *R,
-LocationContextMap &LCM) {
-  bool containsSomethingInteresting = false;
+LocationContextMap &LCM,
+bool IsInteresting = false) {
+  bool containsSomethingInteresting = IsInteresting;
   const unsigned N = pieces.size();
 
   for (unsigned i = 0 ; i < N ; ++i) {
@@ -207,12 +208,8 @@ static bool removeUnneededCalls(PathPiec
 auto &call = cast(*piece);
 // Check if the location context is interesting.
 assert(LCM.count(&call.path));
-if (R->isInteresting(LCM[&call.path])) {
-  containsSomethingInteresting = true;
-  break;
-}
-
-if (!removeUnneededCalls(call.path, R, LCM))
+if (!removeUnneededCalls(call.path, R, LCM,
+ R->isInteresting(LCM[&call.path])))
   continue;
 
 containsSomethingInteresting = true;
@@ -220,7 +217,7 @@ static bool removeUnneededCalls(PathPiec
   }
   case PathDiagnosticPiece::Macro: {
 auto ¯o = cast(*piece);
-if (!removeUnneededCalls(macro.subPieces, R, LCM))
+if (!removeUnneededCalls(macro.subPieces, R, LCM, IsInteresting))
   continue;
 containsSomethingInteresting = true;
 break;

Modified: cfe/trunk/test/Analysis/inlining/path-notes.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/inlining/path-notes.c?rev=329102&r1=329101&r2=329102&view=diff
==
--- cfe/trunk/test/Analysis/inlining/path-notes.c (original)
+++ cfe/trunk/test/Analysis/inlining/path-notes.c Tue Apr  3 11:52:30 2018
@@ -140,6 +140,22 @@ void test4(int **p) {
// expected-note@-1 {{Dereference of null pointer}}
 }
 
+void boringCallee() {
+}
+
+void interestingCallee(int *x) {
+  *x = 0; // expected-note{{The value 0 is assigned to 'x'}}
+  boringCallee(); // no-note
+}
+
+int testBoringCalleeOfInterestingCallee() {
+  int x;
+  interestingCallee(&x); // expected-note{{Calling 'interestingCallee'}}
+ // expected-note@-1{{Returning from 
'interestingCallee'}}
+  return 1 / x; // expected-warning{{Division by zero}}
+// expected-note@-1{{Division by zero}}
+}
+
 // CHECK:  diagnostics
 // CHECK-NEXT:  
 // CHECK-NEXT:   
@@ -3377,4 +3393,290 @@ void test4(int **p) {
 // CHECK-NEXT:file0
 // CHECK-NEXT:   
 // CHECK-NEXT:   
+// CHECK-NEXT:   
+// CHECK-NEXT:path
+// CHECK-NEXT:
+// CHECK-NEXT: 
+// CHECK-NEXT:  kindcontrol
+// CHECK-NEXT:  edges
+// CHECK-NEXT:   
+// CHECK-NEXT:
+// CHECK-NEXT: start
+// CHECK-NEXT:  
+// CHECK-NEXT:   
+// CHECK-NEXT:line152
+// CHECK-NEXT:col3
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:   
+// CHECK-NEXT:line152
+// CHECK-NEXT:col5
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:  
+// CHECK-NEXT: end
+// CHECK-NEXT:  
+// CHECK-NEXT:   
+// CHECK-NEXT:line153
+// CHECK-NEXT:col3
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:   
+// CHECK-NEXT:line153
+// CHECK-NEXT:col19
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:  
+// CHECK-NEXT:
+// CHECK-NEXT:   
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:  k

[PATCH] D45117: [analyzer] Fix diagnostics in callees of interesting callees.

2018-04-03 Thread 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 rL329102: [analyzer] Fix diagnostics in callees of interesting 
callees. (authored by dergachev, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D45117?vs=140513&id=140840#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D45117

Files:
  cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
  cfe/trunk/test/Analysis/inlining/path-notes.c

Index: cfe/trunk/test/Analysis/inlining/path-notes.c
===
--- cfe/trunk/test/Analysis/inlining/path-notes.c
+++ cfe/trunk/test/Analysis/inlining/path-notes.c
@@ -140,6 +140,22 @@
// expected-note@-1 {{Dereference of null pointer}}
 }
 
+void boringCallee() {
+}
+
+void interestingCallee(int *x) {
+  *x = 0; // expected-note{{The value 0 is assigned to 'x'}}
+  boringCallee(); // no-note
+}
+
+int testBoringCalleeOfInterestingCallee() {
+  int x;
+  interestingCallee(&x); // expected-note{{Calling 'interestingCallee'}}
+ // expected-note@-1{{Returning from 'interestingCallee'}}
+  return 1 / x; // expected-warning{{Division by zero}}
+// expected-note@-1{{Division by zero}}
+}
+
 // CHECK:  diagnostics
 // CHECK-NEXT:  
 // CHECK-NEXT:   
@@ -3377,4 +3393,290 @@
 // CHECK-NEXT:file0
 // CHECK-NEXT:   
 // CHECK-NEXT:   
+// CHECK-NEXT:   
+// CHECK-NEXT:path
+// CHECK-NEXT:
+// CHECK-NEXT: 
+// CHECK-NEXT:  kindcontrol
+// CHECK-NEXT:  edges
+// CHECK-NEXT:   
+// CHECK-NEXT:
+// CHECK-NEXT: start
+// CHECK-NEXT:  
+// CHECK-NEXT:   
+// CHECK-NEXT:line152
+// CHECK-NEXT:col3
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:   
+// CHECK-NEXT:line152
+// CHECK-NEXT:col5
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:  
+// CHECK-NEXT: end
+// CHECK-NEXT:  
+// CHECK-NEXT:   
+// CHECK-NEXT:line153
+// CHECK-NEXT:col3
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:   
+// CHECK-NEXT:line153
+// CHECK-NEXT:col19
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:  
+// CHECK-NEXT:
+// CHECK-NEXT:   
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:  kindevent
+// CHECK-NEXT:  location
+// CHECK-NEXT:  
+// CHECK-NEXT:   line153
+// CHECK-NEXT:   col3
+// CHECK-NEXT:   file0
+// CHECK-NEXT:  
+// CHECK-NEXT:  ranges
+// CHECK-NEXT:  
+// CHECK-NEXT:
+// CHECK-NEXT: 
+// CHECK-NEXT:  line153
+// CHECK-NEXT:  col3
+// CHECK-NEXT:  file0
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:  line153
+// CHECK-NEXT:  col23
+// CHECK-NEXT:  file0
+// CHECK-NEXT: 
+// CHECK-NEXT:
+// CHECK-NEXT:  
+// CHECK-NEXT:  depth0
+// CHECK-NEXT:  extended_message
+// CHECK-NEXT:  Calling 'interestingCallee'
+// CHECK-NEXT:  message
+// CHECK-NEXT:  Calling 'interestingCallee'
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:  kindevent
+// CHECK-NEXT:  location
+// CHECK-NEXT:  
+// CHECK-NEXT:   line146
+// CHECK-NEXT:   col1
+// CHECK-NEXT:   file0
+// CHECK-NEXT:  
+// CHECK-NEXT:  depth1
+// CHECK-NEXT:  extended_message
+// CHECK-NEXT:  Entered call from 'testBoringCalleeOfInterestingCallee'
+// CHECK-NEXT:  message
+// CHECK-NEXT:  Entered call from 'testBoringCalleeOfInterestingCallee'
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:  kindcontrol
+// CHECK-NEXT:  edges
+// CHECK-NEXT:   
+// CHECK-NEXT:
+// CHECK-NEXT: start
+// CHECK-NEXT:  
+// CHECK-NEXT:   
+// CHECK-NEXT:line146
+// CHECK-NEXT:col1
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:   
+// CHECK-NEXT:line146
+// CHECK-NEXT:col4
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:  
+// CHECK-NEXT: end
+// CHECK-NEXT:  
+// CHECK-NEXT:   
+// CHECK-NEXT:line147
+// CHECK-NEXT:col3
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:   
+// CHECK-NEXT:line147
+// CHECK-NEXT:col3
+// CHECK-NEXT:file0
+// CHECK-NEXT:   
+// CHECK-NEXT:  
+// CHECK-NEXT:
+// CHECK-NEXT:   
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT:  kindevent
+// CHECK-NEXT:  location
+// CHECK-NEXT:  
+// CHECK-NEXT:   line147
+// CHECK-N

[PATCH] D44720: [clangd] Simplify fuzzy matcher (sequence alignment) by removing some condition checks.

2018-04-03 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

Thanks for your work on this patch!
I think there a several useful improvements here, which I'd love to see landed. 
Particularly the logic around matches that end early is much better.

There are also places that change existing design decisions in ways that don't 
seem to be clear improvements: doing extra work (albeit minor) at `match()` 
time, and using different naming conventions for indexes in `dumpLast`. I see 
you have strong feelings about these and I do think I understand your 
arguments, but don't agree as discussed above.

Where should we go from here? One option is to land the pieces we agree on. But 
it's your patch, and if you'd like an opinion from another clangd maintainer 
that's fine too; happy to help with that.




Comment at: clangd/FuzzyMatch.cpp:230
 void FuzzyMatcher::buildGraph() {
+  Scores[0][0][Miss] = Scores[0][0][Match] = {0, Miss};
   for (int W = 0; W < WordN; ++W) {

MaskRay wrote:
> MaskRay wrote:
> > sammccall wrote:
> > > MaskRay wrote:
> > > > sammccall wrote:
> > > > > why this change?
> > > > > this has also been moved from the cheaper constructor to the more 
> > > > > expensive per-match call. (also the diagonal assignment added in the 
> > > > > next loop)
> > > > > 
> > > > > Also, shouldn't [0][0][Match] be AwfulScore?
> > > > > 
> > > > "The more expensive per-match call" is just two value assignments.
> > > > 
> > > > I have removed the expensive table initialization in the constructor.
> > > > 
> > > > [0][0][*] can be any value.
> > > > "The more expensive per-match call" is just two value assignments.
> > > Oops, sorry - by "more expensive" I mean "called thousands of times more 
> > > often".
> > > 
> > > > I have removed the expensive table initialization in the constructor.
> > > I don't want to be rude, but I asked why you changed this, and you didn't 
> > > answer. Unless there's a strong reason, I'd prefer to revert this change, 
> > > as I find this harder to reason about.
> > > (Roughly: in the old version of the code, any data that didn't need to 
> > > change for the life of the object was initialized in the constructor. 
> > > That way I didn't need to worry what was performance-critical and what 
> > > wasn't - match() only did what was strictly necessary).
> > > 
> > > > [0][0][*] can be any value.
> > > Can you please explain why?
> > > Oops, sorry - by "more expensive" I mean "called thousands of times more 
> > > often".
> > 
> > It is true that `Scores[0][0][Miss] = Scores[0][0][Match] = {0, Miss};` is 
> > the cost incurred for each word.
> > 
> > But **it is not full table initialization**, it is just two variable 
> > assignments. And we will assign to other values of the first row 
> > `Scores[0][*][*]` in the following loop. The old scatters the table 
> > construction to **two places**, the constructor and this dynamic 
> > programming site.
> > [0][0][*] can be any value.
> 
> Can you please explain why?
> 
> `Scores[0][0][*]` is the initial value which will be propagated to all other 
> values in the table.
> The relative difference of pairwise values in the table is a constant 
> whatever initial value is chosen.
> 
> If you ignore the max clamp you used later, the initial value does not matter.
> 
> 
Is it not possible that we'll choose a best path starting at 
Scores[0][0][Match]?
This is invalid, and previously that was signaled by giving that cell 
AwfulScore, which ensures any path that emerges from it isAwful.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44720



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


[PATCH] D25001: [Module] Merge function prototype with a non-prototype function declaration

2018-04-03 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak abandoned this revision.
ahatanak added a comment.

This appears to have been fixed.


https://reviews.llvm.org/D25001



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


[PATCH] D44720: [clangd] Simplify fuzzy matcher (sequence alignment) by removing some condition checks.

2018-04-03 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment.

Glad you took another look. I don't want to yield, let's find another reviewer 
:)




Comment at: clangd/FuzzyMatch.cpp:230
 void FuzzyMatcher::buildGraph() {
+  Scores[0][0][Miss] = Scores[0][0][Match] = {0, Miss};
   for (int W = 0; W < WordN; ++W) {

sammccall wrote:
> MaskRay wrote:
> > MaskRay wrote:
> > > sammccall wrote:
> > > > MaskRay wrote:
> > > > > sammccall wrote:
> > > > > > why this change?
> > > > > > this has also been moved from the cheaper constructor to the more 
> > > > > > expensive per-match call. (also the diagonal assignment added in 
> > > > > > the next loop)
> > > > > > 
> > > > > > Also, shouldn't [0][0][Match] be AwfulScore?
> > > > > > 
> > > > > "The more expensive per-match call" is just two value assignments.
> > > > > 
> > > > > I have removed the expensive table initialization in the constructor.
> > > > > 
> > > > > [0][0][*] can be any value.
> > > > > "The more expensive per-match call" is just two value assignments.
> > > > Oops, sorry - by "more expensive" I mean "called thousands of times 
> > > > more often".
> > > > 
> > > > > I have removed the expensive table initialization in the constructor.
> > > > I don't want to be rude, but I asked why you changed this, and you 
> > > > didn't answer. Unless there's a strong reason, I'd prefer to revert 
> > > > this change, as I find this harder to reason about.
> > > > (Roughly: in the old version of the code, any data that didn't need to 
> > > > change for the life of the object was initialized in the constructor. 
> > > > That way I didn't need to worry what was performance-critical and what 
> > > > wasn't - match() only did what was strictly necessary).
> > > > 
> > > > > [0][0][*] can be any value.
> > > > Can you please explain why?
> > > > Oops, sorry - by "more expensive" I mean "called thousands of times 
> > > > more often".
> > > 
> > > It is true that `Scores[0][0][Miss] = Scores[0][0][Match] = {0, Miss};` 
> > > is the cost incurred for each word.
> > > 
> > > But **it is not full table initialization**, it is just two variable 
> > > assignments. And we will assign to other values of the first row 
> > > `Scores[0][*][*]` in the following loop. The old scatters the table 
> > > construction to **two places**, the constructor and this dynamic 
> > > programming site.
> > > [0][0][*] can be any value.
> > 
> > Can you please explain why?
> > 
> > `Scores[0][0][*]` is the initial value which will be propagated to all 
> > other values in the table.
> > The relative difference of pairwise values in the table is a constant 
> > whatever initial value is chosen.
> > 
> > If you ignore the max clamp you used later, the initial value does not 
> > matter.
> > 
> > 
> Is it not possible that we'll choose a best path starting at 
> Scores[0][0][Match]?
> This is invalid, and previously that was signaled by giving that cell 
> AwfulScore, which ensures any path that emerges from it isAwful.
I think `Scores[0][0][Match]` is as valid as `Scores[0][0][Miss]`. The argument 
is that a `Miss` state should ensure a skipped position in Text. When there is 
zero position, it cannot be `Miss`ed. A dynamic programming algorithm does not 
necessarily have only one valid initial state (thinking about the constant term 
in an indefinite integral). I will choose the one which makes more sense if 
such initial state exists or if there is one that simplifies the case. In this 
case treating both of them as valid simplifies the code.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44720



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


[PATCH] D45223: [CUDA] Fix overloading resolution failure due to kernel calling convention

2018-04-03 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl created this revision.
yaxunl added reviewers: rjmccall, tra.

The following test causes overloading resolution failure in clang due to
missing kernel calling convention in the function pointer type.

  template 
  __global__ void EmptyKernel(void) {}
  
  struct Dummy {
/// Type definition of the EmptyKernel kernel entry point
typedef void (*EmptyKernelPtr)();
EmptyKernelPtr Empty() { return EmptyKernel; } 
  };

This happens before DRE is created. The fix is to drop the kernel
calling convention when converting function types.


https://reviews.llvm.org/D45223

Files:
  lib/Sema/SemaOverload.cpp
  test/CodeGenCUDA/kernel-amdgcn.cu


Index: test/CodeGenCUDA/kernel-amdgcn.cu
===
--- test/CodeGenCUDA/kernel-amdgcn.cu
+++ test/CodeGenCUDA/kernel-amdgcn.cu
@@ -15,15 +15,27 @@
   non_kernel();
 }
 
+// CHECK: define amdgpu_kernel void @_Z11EmptyKernelIvEvv
+template 
+__global__ void EmptyKernel(void) {}
+
+struct Dummy {
+  /// Type definition of the EmptyKernel kernel entry point
+  typedef void (*EmptyKernelPtr)();
+  EmptyKernelPtr Empty() { return EmptyKernel; } 
+};
+
 // CHECK: define amdgpu_kernel void @_Z15template_kernelI1AEvT_
 template
 __global__ void template_kernel(T x) {}
 
 void launch(void *f);
 
 int main() {
+  Dummy D;
   launch((void*)A::kernel);
   launch((void*)kernel);
   launch((void*)template_kernel);
+  launch((void*)D.Empty());
   return 0;
 }
Index: lib/Sema/SemaOverload.cpp
===
--- lib/Sema/SemaOverload.cpp
+++ lib/Sema/SemaOverload.cpp
@@ -1471,16 +1471,26 @@
 Changed = true;
   }
 
-  // Drop 'noexcept' if not present in target type.
   if (const auto *FromFPT = dyn_cast(FromFn)) {
 const auto *ToFPT = cast(ToFn);
+
+// Drop 'noexcept' if not present in target type.
 if (FromFPT->isNothrow(Context) && !ToFPT->isNothrow(Context)) {
   FromFn = cast(
   Context.getFunctionTypeWithExceptionSpec(QualType(FromFPT, 0),
EST_None)
  .getTypePtr());
   Changed = true;
 }
+
+// Drop cuda_kernel calling convention since it is invisible in AST.
+if (FromFPT->getCallConv() == CC_CUDAKernel &&
+FromFPT->getCallConv() != ToFPT->getCallConv()) {
+  FromFn = Context.adjustFunctionType(
+  FromFn, FromEInfo.withCallingConv(ToFPT->getCallConv()));
+  Changed = true;
+}
+
 // Convert FromFPT's ExtParameterInfo if necessary. The conversion is valid
 // only if the ExtParameterInfo lists of the two function prototypes can be
 // merged and the merged list is identical to ToFPT's ExtParameterInfo 
list.


Index: test/CodeGenCUDA/kernel-amdgcn.cu
===
--- test/CodeGenCUDA/kernel-amdgcn.cu
+++ test/CodeGenCUDA/kernel-amdgcn.cu
@@ -15,15 +15,27 @@
   non_kernel();
 }
 
+// CHECK: define amdgpu_kernel void @_Z11EmptyKernelIvEvv
+template 
+__global__ void EmptyKernel(void) {}
+
+struct Dummy {
+  /// Type definition of the EmptyKernel kernel entry point
+  typedef void (*EmptyKernelPtr)();
+  EmptyKernelPtr Empty() { return EmptyKernel; } 
+};
+
 // CHECK: define amdgpu_kernel void @_Z15template_kernelI1AEvT_
 template
 __global__ void template_kernel(T x) {}
 
 void launch(void *f);
 
 int main() {
+  Dummy D;
   launch((void*)A::kernel);
   launch((void*)kernel);
   launch((void*)template_kernel);
+  launch((void*)D.Empty());
   return 0;
 }
Index: lib/Sema/SemaOverload.cpp
===
--- lib/Sema/SemaOverload.cpp
+++ lib/Sema/SemaOverload.cpp
@@ -1471,16 +1471,26 @@
 Changed = true;
   }
 
-  // Drop 'noexcept' if not present in target type.
   if (const auto *FromFPT = dyn_cast(FromFn)) {
 const auto *ToFPT = cast(ToFn);
+
+// Drop 'noexcept' if not present in target type.
 if (FromFPT->isNothrow(Context) && !ToFPT->isNothrow(Context)) {
   FromFn = cast(
   Context.getFunctionTypeWithExceptionSpec(QualType(FromFPT, 0),
EST_None)
  .getTypePtr());
   Changed = true;
 }
+
+// Drop cuda_kernel calling convention since it is invisible in AST.
+if (FromFPT->getCallConv() == CC_CUDAKernel &&
+FromFPT->getCallConv() != ToFPT->getCallConv()) {
+  FromFn = Context.adjustFunctionType(
+  FromFn, FromEInfo.withCallingConv(ToFPT->getCallConv()));
+  Changed = true;
+}
+
 // Convert FromFPT's ExtParameterInfo if necessary. The conversion is valid
 // only if the ExtParameterInfo lists of the two function prototypes can be
 // merged and the merged list is identical to ToFPT's ExtParameterInfo list.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commi

[PATCH] D45163: [Sema] -Wunused-value: diagnose unused std::move() call results.

2018-04-03 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

Trust me, I understand that this is an important function, but a lot of 
functions are important, and we're not going to hardcode knowledge about all of 
them in the compiler.

It seems reasonable to ask libc++ to use `__attribute__((warn_unused_result))` 
if `[[nodiscard]]` is unavailable.


Repository:
  rC Clang

https://reviews.llvm.org/D45163



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


[PATCH] D45179: [libc++] Add _LIBCPP_FORCE_NODISCARD define to force-enable nodiscard in pre-C++17

2018-04-03 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added inline comments.



Comment at: test/libcxx/diagnostics/force_nodiscard.fail.cpp:15
+// MODULES_DEFINES: _LIBCPP_FORCE_NODISCARD
+#define _LIBCPP_FORCE_NODISCARD
+#include <__config>

What is the purpose of `_LIBCPP_FORCE_NODISCARD`?
On one of your other nodiscard-related reviews, @EricWF suggested that we 
should warn all the time, even e.g. on a discarded `std::move(x)` in C++11, or 
a discarded `vec.empty()` in C++03. And *maybe* we could provide an opt-out 
mechanism, but honestly *I* don't see why anyone would want to opt out.
`_LIBCPP_FORCE_NODISCARD` smells like an opt-in mechanism, which I would think 
is the opposite of what we want.



Comment at: test/libcxx/diagnostics/force_nodiscard.pass.cpp:16
+// MODULES_DEFINES: _LIBCPP_FORCE_NODISCARD
+#define _LIBCPP_DISABLE_NODISCARD_AFTER_CXX17
+#define _LIBCPP_FORCE_NODISCARD

What is the purpose of `_LIBCPP_DISABLE_NODISCARD_AFTER_CXX17`? I guess I could 
understand a blanket opt-in "please don't warn me about discarded [[nodiscard]] 
results"; but that should be (and is) spelled `-Wno-unused-result`, and it has 
nothing to do with C++17.

I like how this patch defines `_LIBCPP_NODISCARD` in non-C++17 modes; that's 
going to be very useful. But I think all these opt-in mechanisms are confusing 
and not-helpful.

If we must have an opt-in/out mechanism (which I don't believe we do), please 
consider adding the following two lines to `<__config>` and removing the rest:

#ifdef _LIBCPP_NODISCARD
// the user has given us their preferred spelling; use it 
unconditionally
#elif __has_cpp_attribute(nodiscard) && _LIBCPP_STD_VER > 17
[... etc etc ...]



Repository:
  rCXX libc++

https://reviews.llvm.org/D45179



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


[PATCH] D45163: [Sema] -Wunused-value: diagnose unused std::move() call results.

2018-04-03 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added a comment.

In https://reviews.llvm.org/D45163#1055957, @lebedev.ri wrote:

> In https://reviews.llvm.org/D45163#1055856, @rjmccall wrote:
>
> > Well, we should feel welcome to submit patches to the C++ library 
> > implementations, I think.  I can understand why Marshall is waiting to just 
> > do this until he gets a comprehensive committee paper, but he might 
> > consider taking a patch in the meantime.
>
>
> That has been discussed, and it's not going to happen. Ask him if you want 
> more info.


You say a library patch is not going to happen, but isn't that library patch 
literally https://reviews.llvm.org/D45179, which has been accepted (although I 
just left a comment explaining why the current macro-soup is suboptimal)?

> TLDW: `std::move()` is "The Assign Operator (`a = b;`) of RAII". It would be 
> good to diagnose such a problem, and not just rely that some day the std libs 
> will mark it with the attribute.
>  BTW, even when they do, even for libcxx, it won't be of any immediate use to 
> LLVM, we will need to provide a define (see https://reviews.llvm.org/D45179) 
> to actually enable that attribute, because LLVM is using C++11, not 
> C++17/something...

Yes, but that patch (which you've written in https://reviews.llvm.org/D45179) 
is essentially just "if C++17, `[[nodiscard]]`, else 
`__attribute__((warn_unused_result))`." The library vendor is completely free 
to mark nodiscard-ish function with [[nodiscard]] or any other attribute they 
want to. And if I'm not mistaken, https://reviews.llvm.org/D45179 is a step in 
that direction.


Repository:
  rC Clang

https://reviews.llvm.org/D45163



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


[PATCH] D45202: [X86] Replacing X86-specific floor and ceil vector intrinsics with generic LLVM intrinsics

2018-04-03 Thread Craig Topper via Phabricator via cfe-commits
craig.topper added inline comments.



Comment at: include/clang/Basic/BuiltinsX86.def:951
 TARGET_BUILTIN(__builtin_ia32_rndscalepd_mask, "V8dV8dIiV8dUcIi", "", 
"avx512f")
+TARGET_BUILTIN(__builtin_ia32_floorps_mask, "V16fV16fV16fUs", "", "avx512f")
+TARGET_BUILTIN(__builtin_ia32_floorpd_mask, "V8dV8dV8dUc", "", "avx512f")

I'd prefer CGBuiltin to detect the specific immediates on the rndscale value. 
Primarily because we should be able to optimize _mm512_roundscale_pd when the 
ceil/floor immediate is used.


Repository:
  rC Clang

https://reviews.llvm.org/D45202



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


[PATCH] D45179: [libc++] Add _LIBCPP_FORCE_NODISCARD define to force-enable nodiscard in pre-C++17

2018-04-03 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments.



Comment at: test/libcxx/diagnostics/force_nodiscard.fail.cpp:15
+// MODULES_DEFINES: _LIBCPP_FORCE_NODISCARD
+#define _LIBCPP_FORCE_NODISCARD
+#include <__config>

Quuxplusone wrote:
> What is the purpose of `_LIBCPP_FORCE_NODISCARD`?
> On one of your other nodiscard-related reviews, @EricWF suggested that we 
> should warn all the time, even e.g. on a discarded `std::move(x)` in C++11, 
> or a discarded `vec.empty()` in C++03. And *maybe* we could provide an 
> opt-out mechanism, but honestly *I* don't see why anyone would want to opt 
> out.
> `_LIBCPP_FORCE_NODISCARD` smells like an opt-in mechanism, which I would 
> think is the opposite of what we want.
> _LIBCPP_FORCE_NODISCARD smells like an opt-in mechanism
Correct.

> And *maybe* we could provide an opt-out mechanism, but honestly *I* don't see 
> why anyone would want to opt out.
I agree.

> which I would think is the opposite of what we want.
Also correct.



Comment at: test/libcxx/diagnostics/force_nodiscard.pass.cpp:16
+// MODULES_DEFINES: _LIBCPP_FORCE_NODISCARD
+#define _LIBCPP_DISABLE_NODISCARD_AFTER_CXX17
+#define _LIBCPP_FORCE_NODISCARD

Quuxplusone wrote:
> What is the purpose of `_LIBCPP_DISABLE_NODISCARD_AFTER_CXX17`? I guess I 
> could understand a blanket opt-in "please don't warn me about discarded 
> [[nodiscard]] results"; but that should be (and is) spelled 
> `-Wno-unused-result`, and it has nothing to do with C++17.
> 
> I like how this patch defines `_LIBCPP_NODISCARD` in non-C++17 modes; that's 
> going to be very useful. But I think all these opt-in mechanisms are 
> confusing and not-helpful.
> 
> If we must have an opt-in/out mechanism (which I don't believe we do), please 
> consider adding the following two lines to `<__config>` and removing the rest:
> 
> #ifdef _LIBCPP_NODISCARD
> // the user has given us their preferred spelling; use it 
> unconditionally
> #elif __has_cpp_attribute(nodiscard) && _LIBCPP_STD_VER > 17
> [... etc etc ...]
> 
> If we must have an opt-in/out mechanism (which I don't believe we do)

Yes, we do.
Opt-out is pre-existing, and removing it would be an [unacceptable] regression.
Opt-in is an enhancement. Of course, it would be nice to always default it to 
on,
but as it was disscussed with @mclow.lists, this is simply not going to happen.
This is the best we'll get.

```
#ifdef _LIBCPP_NODISCARD
// the user has given us their preferred spelling; use it unconditionally
```
So you propose to shift the burden of picking which define to use to each and 
every
libc++ user (who wants to enable nodiscard attribute for pre-C++17/whatever) 
out there?
I really don't see how that would be better.


Repository:
  rCXX libc++

https://reviews.llvm.org/D45179



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


[PATCH] D45163: [Sema] -Wunused-value: diagnose unused std::move() call results.

2018-04-03 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment.

In https://reviews.llvm.org/D45163#1056044, @rjmccall wrote:

> It seems reasonable to ask libc++ to use 
> `__attribute__((warn_unused_result))` if `[[nodiscard]]` is unavailable.


https://reviews.llvm.org/D45179 should hopefully add an **opt-in** define to 
allow doing just that. But it will then have to be enabled in the LLVM 
buildsystem.

In https://reviews.llvm.org/D45163#1056074, @Quuxplusone wrote:

> In https://reviews.llvm.org/D45163#1055957, @lebedev.ri wrote:
>
> > In https://reviews.llvm.org/D45163#1055856, @rjmccall wrote:
> >
> > > Well, we should feel welcome to submit patches to the C++ library 
> > > implementations, I think.  I can understand why Marshall is waiting to 
> > > just do this until he gets a comprehensive committee paper, but he might 
> > > consider taking a patch in the meantime.
> >
> >
> > That has been discussed, and it's not going to happen. Ask him if you want 
> > more info.
>
>
> You say a library patch is not going to happen, but isn't that library patch 
> literally https://reviews.llvm.org/D45179, which has been accepted (although 
> I just left a comment explaining why the current macro-soup is suboptimal)?


We may be talking about different things here.
I'm simply pointing out that libc++ will not be adding `nodiscard` attribute to 
functions unless it is mandated by the standard,
and that it *has* to be opt-in for earlier standards. I'm not saying that just 
to argue, i'm literally retranslating what @mclow.lists has said, it's his 
words.


Repository:
  rC Clang

https://reviews.llvm.org/D45163



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


[PATCH] D45179: [libc++] Add _LIBCPP_FORCE_NODISCARD define to force-enable nodiscard in pre-C++17

2018-04-03 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added a comment.

>> If we must have an opt-in/out mechanism (which I don't believe we do)
> 
> Yes, we do. Opt-out is pre-existing, and removing it would be an 
> [unacceptable] regression.

Ah, I see now that you weren't the originator of 
`_LIBCPP_DISABLE_NODISCARD_AFTER_CXX17`; that existed on the left-hand side of 
this diff already. Okay, I don't object to keeping it around for historical 
interest.

> Opt-in is an enhancement. Of course, it would be nice to always default it to 
> on, but as it was disscussed with @mclow.lists, this is simply not going to 
> happen. This is the best we'll get.

Is @mclow.lists on record anywhere as saying that, that you could link to? I 
ask because I haven't seen him commenting on these patches. At the moment, it 
seems like you're alone in both pushing these patches and simultaneously 
arguing that any effort is futile. :)

>>   #ifdef _LIBCPP_NODISCARD
>>  // the user has given us their preferred spelling; use it 
>> unconditionally
> 
> So you propose to shift the burden of picking which define to use to each and 
> every libc++ user

Not at all!  I merely suggested that rather than having three different macros 
concerned with the spelling of `nodiscard`, you reduce it to *one* macro: 
`_LIBCPP_NODISCARD`. There would be a "sensible default" (namely, whatever 
spelling Does The Right Thing on the detected compiler), and then users who 
didn't want that default would be free to override it by passing `-D` on the 
command line. The `#ifdef` I suggested is a clean, concise way of allowing the 
user's explicit choice to override the libc++-provided "sensible default", no 
matter whether the user is trying to opt-in on an unsupported compiler 
(`-D_LIBCPP_NODISCARD='[[foobar::nodiscard]]'`) or opt-out on a supported 
compiler (`-D_LIBCPP_NODISCARD=''`). It would be a way to eliminate the soup of 
`ENABLE_` and `DISABLE_` macros.

Of course the benefit of this idea decreases if we must preserve 
`_LIBCPP_DISABLE_NODISCARD_AFTER_CXX17` for historical reasons anyway.


Repository:
  rCXX libc++

https://reviews.llvm.org/D45179



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


[PATCH] D45179: [libc++] Add _LIBCPP_FORCE_NODISCARD define to force-enable nodiscard in pre-C++17

2018-04-03 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment.

In https://reviews.llvm.org/D45179#1056108, @Quuxplusone wrote:

> > Opt-in is an enhancement. Of course, it would be nice to always default it 
> > to on, but as it was disscussed with @mclow.lists, this is simply not going 
> > to happen. This is the best we'll get.
>
> Is @mclow.lists on record anywhere as saying that, that you could link to?


That particular disscussion was in IRC, so no, i can not link/paste.

> I ask because I haven't seen him commenting on these patches.



> At the moment, it seems like you're alone in both pushing these patches and 
> simultaneously arguing that any effort is futile. :)

:)


Repository:
  rCXX libc++

https://reviews.llvm.org/D45179



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


[PATCH] D44720: [clangd] Simplify fuzzy matcher (sequence alignment) by removing some condition checks.

2018-04-03 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

In https://reviews.llvm.org/D44720#1055997, @MaskRay wrote:

> Glad you took another look. I don't want to yield, let's find another 
> reviewer :)


OK - the people with the most context on this particular code are ilya-biryukov 
and klimek (but klimek is out this week).
Others who write/review clangd stuff include hokein, malaperle, simark, 
bkramer. (ioeric is out for a few weeks).

I can give ilya or someone context on the specific changes we're looking at if 
that's useful.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44720



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


[PATCH] D45152: [MinGW] Add option for disabling looking for a mingw gcc in PATH

2018-04-03 Thread Martin Storsjö via Phabricator via cfe-commits
mstorsjo added a comment.

In https://reviews.llvm.org/D45152#1055871, @rnk wrote:

> Seems reasonable, looks good.


Any opinion on the wording of the option name?


Repository:
  rC Clang

https://reviews.llvm.org/D45152



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


[PATCH] D45152: [MinGW] Add option for disabling looking for a mingw gcc in PATH

2018-04-03 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment.

In https://reviews.llvm.org/D45152#1056122, @mstorsjo wrote:

> In https://reviews.llvm.org/D45152#1055871, @rnk wrote:
>
> > Seems reasonable, looks good.
>
>
> Any opinion on the wording of the option name?


Maybe what we're trying to do is find the sysroot relative to clang, so an 
option name phrased along those lines makes more sense? --clang-in-sysroot-bin? 
--from-sysroot-bin? --assume-sysroot-bin? --in-sysroot-bin?


Repository:
  rC Clang

https://reviews.llvm.org/D45152



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


[PATCH] D44788: Add an option to support debug fission on implicit ThinLTO.

2018-04-03 Thread Yunlian Jiang via Phabricator via cfe-commits
yunlian updated this revision to Diff 140857.

https://reviews.llvm.org/D44788

Files:
  docs/ClangCommandLineReference.rst
  include/clang/Driver/Options.td
  lib/Driver/ToolChains/CommonArgs.cpp
  test/Driver/lto-dwo.c


Index: test/Driver/lto-dwo.c
===
--- /dev/null
+++ test/Driver/lto-dwo.c
@@ -0,0 +1,8 @@
+// Confirm that -glto-dwo-dir=DIR is passed to linker
+
+// RUN: %clang -target x86_64-unknown-linux -### %s -flto=thin 
-glto-dwo-dir=DIR 2> %t
+// RUN: FileCheck -check-prefix=CHECK-LINK-DWO-DIR < %t %s
+// RUN: FileCheck -check-prefix=CHECK-LINK-OBJCOPY < %t %s
+//
+// CHECK-LINK-DWO-DIR: "-plugin-opt=dwo_dir=DIR"
+// CHECK-LINK-OBJCOPY: "-plugin-opt=objcopy={{.*}}objcopy"
Index: lib/Driver/ToolChains/CommonArgs.cpp
===
--- lib/Driver/ToolChains/CommonArgs.cpp
+++ lib/Driver/ToolChains/CommonArgs.cpp
@@ -415,6 +415,16 @@
   if (IsThinLTO)
 CmdArgs.push_back("-plugin-opt=thinlto");
 
+  if (Arg *A = Args.getLastArg(options::OPT_glto_dwo_dir_EQ)) {
+const char *Objcopy =
+Args.MakeArgString(ToolChain.GetProgramPath(CLANG_DEFAULT_OBJCOPY));
+CmdArgs.push_back(
+Args.MakeArgString(Twine("-plugin-opt=objcopy=") + Objcopy));
+StringRef DWO_Dir = A->getValue();
+CmdArgs.push_back(
+Args.MakeArgString(Twine("-plugin-opt=dwo_dir=") + DWO_Dir));
+  }
+
   if (unsigned Parallelism = getLTOParallelism(Args, D))
 CmdArgs.push_back(
 Args.MakeArgString("-plugin-opt=jobs=" + Twine(Parallelism)));
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -1700,6 +1700,7 @@
 def gcolumn_info : Flag<["-"], "gcolumn-info">, Group, 
Flags<[CoreOption]>;
 def gno_column_info : Flag<["-"], "gno-column-info">, Group, 
Flags<[CoreOption]>;
 def gsplit_dwarf : Flag<["-"], "gsplit-dwarf">, Group;
+def glto_dwo_dir_EQ : Joined<["-"], "glto-dwo-dir=">, Group;
 def ggnu_pubnames : Flag<["-"], "ggnu-pubnames">, Group, 
Flags<[CC1Option]>;
 def gdwarf_aranges : Flag<["-"], "gdwarf-aranges">, Group;
 def gmodules : Flag <["-"], "gmodules">, Group,
Index: docs/ClangCommandLineReference.rst
===
--- docs/ClangCommandLineReference.rst
+++ docs/ClangCommandLineReference.rst
@@ -2587,6 +2587,8 @@
 
 .. option:: -gstrict-dwarf, -gno-strict-dwarf
 
+.. option:: -glto-dwo-dir=
+
 .. option:: -gz
 
 DWARF debug sections compression type


Index: test/Driver/lto-dwo.c
===
--- /dev/null
+++ test/Driver/lto-dwo.c
@@ -0,0 +1,8 @@
+// Confirm that -glto-dwo-dir=DIR is passed to linker
+
+// RUN: %clang -target x86_64-unknown-linux -### %s -flto=thin -glto-dwo-dir=DIR 2> %t
+// RUN: FileCheck -check-prefix=CHECK-LINK-DWO-DIR < %t %s
+// RUN: FileCheck -check-prefix=CHECK-LINK-OBJCOPY < %t %s
+//
+// CHECK-LINK-DWO-DIR: "-plugin-opt=dwo_dir=DIR"
+// CHECK-LINK-OBJCOPY: "-plugin-opt=objcopy={{.*}}objcopy"
Index: lib/Driver/ToolChains/CommonArgs.cpp
===
--- lib/Driver/ToolChains/CommonArgs.cpp
+++ lib/Driver/ToolChains/CommonArgs.cpp
@@ -415,6 +415,16 @@
   if (IsThinLTO)
 CmdArgs.push_back("-plugin-opt=thinlto");
 
+  if (Arg *A = Args.getLastArg(options::OPT_glto_dwo_dir_EQ)) {
+const char *Objcopy =
+Args.MakeArgString(ToolChain.GetProgramPath(CLANG_DEFAULT_OBJCOPY));
+CmdArgs.push_back(
+Args.MakeArgString(Twine("-plugin-opt=objcopy=") + Objcopy));
+StringRef DWO_Dir = A->getValue();
+CmdArgs.push_back(
+Args.MakeArgString(Twine("-plugin-opt=dwo_dir=") + DWO_Dir));
+  }
+
   if (unsigned Parallelism = getLTOParallelism(Args, D))
 CmdArgs.push_back(
 Args.MakeArgString("-plugin-opt=jobs=" + Twine(Parallelism)));
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -1700,6 +1700,7 @@
 def gcolumn_info : Flag<["-"], "gcolumn-info">, Group, Flags<[CoreOption]>;
 def gno_column_info : Flag<["-"], "gno-column-info">, Group, Flags<[CoreOption]>;
 def gsplit_dwarf : Flag<["-"], "gsplit-dwarf">, Group;
+def glto_dwo_dir_EQ : Joined<["-"], "glto-dwo-dir=">, Group;
 def ggnu_pubnames : Flag<["-"], "ggnu-pubnames">, Group, Flags<[CC1Option]>;
 def gdwarf_aranges : Flag<["-"], "gdwarf-aranges">, Group;
 def gmodules : Flag <["-"], "gmodules">, Group,
Index: docs/ClangCommandLineReference.rst
===
--- docs/ClangCommandLineReference.rst
+++ docs/ClangCommandLineReference.rst
@@ -2587,6 +2587,8 @@
 
 .. option:: -gstrict-dwarf, -gno-strict-dwarf
 
+.. option:: -glto-dwo-dir=
+
 .. option:: -gz
 
 DWARF debug sections compres

r329110 - [driver][darwin] Do not infer -simulator environment for non-simulator SDKs

2018-04-03 Thread Alex Lorenz via cfe-commits
Author: arphaman
Date: Tue Apr  3 13:50:05 2018
New Revision: 329110

URL: http://llvm.org/viewvc/llvm-project?rev=329110&view=rev
Log:
[driver][darwin] Do not infer -simulator environment for non-simulator SDKs

rdar://36369832

Modified:
cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
cfe/trunk/test/Driver/darwin-sdkroot.c

Modified: cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Darwin.cpp?rev=329110&r1=329109&r2=329110&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Darwin.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Darwin.cpp Tue Apr  3 13:50:05 2018
@@ -1211,6 +1211,9 @@ struct DarwinPlatform {
   /// Returns true if the target OS was explicitly specified.
   bool isExplicitlySpecified() const { return Kind <= DeploymentTargetEnv; }
 
+  /// Returns true if the simulator environment can be inferred from the arch.
+  bool canInferSimulatorFromArch() const { return Kind != InferredFromSDK; }
+
   /// Adds the -m-version-min argument to the compiler invocation.
   void addOSVersionMinArgument(DerivedArgList &Args, const OptTable &Opts) {
 if (Argument)
@@ -1280,8 +1283,12 @@ struct DarwinPlatform {
 return Result;
   }
   static DarwinPlatform createFromSDK(DarwinPlatformKind Platform,
-  StringRef Value) {
-return DarwinPlatform(InferredFromSDK, Platform, Value);
+  StringRef Value,
+  bool IsSimulator = false) {
+DarwinPlatform Result(InferredFromSDK, Platform, Value);
+if (IsSimulator)
+  Result.Environment = DarwinEnvironmentKind::Simulator;
+return Result;
   }
   static DarwinPlatform createFromArch(llvm::Triple::OSType OS,
StringRef Value) {
@@ -1437,14 +1444,20 @@ Optional inferDeployment
   if (StartVer != StringRef::npos && EndVer > StartVer) {
 StringRef Version = SDK.slice(StartVer, EndVer + 1);
 if (SDK.startswith("iPhoneOS") || SDK.startswith("iPhoneSimulator"))
-  return DarwinPlatform::createFromSDK(Darwin::IPhoneOS, Version);
+  return DarwinPlatform::createFromSDK(
+  Darwin::IPhoneOS, Version,
+  /*IsSimulator=*/SDK.startswith("iPhoneSimulator"));
 else if (SDK.startswith("MacOSX"))
   return DarwinPlatform::createFromSDK(Darwin::MacOS,

getSystemOrSDKMacOSVersion(Version));
 else if (SDK.startswith("WatchOS") || SDK.startswith("WatchSimulator"))
-  return DarwinPlatform::createFromSDK(Darwin::WatchOS, Version);
+  return DarwinPlatform::createFromSDK(
+  Darwin::WatchOS, Version,
+  /*IsSimulator=*/SDK.startswith("WatchSimulator"));
 else if (SDK.startswith("AppleTVOS") || SDK.startswith("AppleTVSimulator"))
-  return DarwinPlatform::createFromSDK(Darwin::TvOS, Version);
+  return DarwinPlatform::createFromSDK(
+  Darwin::TvOS, Version,
+  /*IsSimulator=*/SDK.startswith("AppleTVSimulator"));
   }
   return None;
 }
@@ -1645,6 +1658,7 @@ void Darwin::AddDeploymentTarget(Derived
   DarwinEnvironmentKind Environment = OSTarget->getEnvironment();
   // Recognize iOS targets with an x86 architecture as the iOS simulator.
   if (Environment == NativeEnvironment && Platform != MacOS &&
+  OSTarget->canInferSimulatorFromArch() &&
   (getTriple().getArch() == llvm::Triple::x86 ||
getTriple().getArch() == llvm::Triple::x86_64))
 Environment = Simulator;

Modified: cfe/trunk/test/Driver/darwin-sdkroot.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/darwin-sdkroot.c?rev=329110&r1=329109&r2=329110&view=diff
==
--- cfe/trunk/test/Driver/darwin-sdkroot.c (original)
+++ cfe/trunk/test/Driver/darwin-sdkroot.c Tue Apr  3 13:50:05 2018
@@ -51,12 +51,21 @@
 // CHECK-IPHONE: "-triple" "arm64-apple-ios8.0.0"
 // CHECK-IPHONE: ld
 // CHECK-IPHONE: "-iphoneos_version_min" "8.0.0"
+// RUN: env SDKROOT=%t/SDKs/iPhoneOS8.0.0.sdk %clang %s -### 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-IPHONE-X86 %s
+// CHECK-IPHONE-X86: clang
+// CHECK-IPHONE-X86: "-cc1"
+// CHECK-IPHONE-X86: -apple-ios8.0.0"
+// CHECK-IPHONE-X86: ld
+// CHECK-IPHONE-X86: "-iphoneos_version_min" "8.0.0"
 //
 //
 // RUN: rm -rf %t/SDKs/iPhoneSimulator8.0.sdk
 // RUN: mkdir -p %t/SDKs/iPhoneSimulator8.0.sdk
 // RUN: env SDKROOT=%t/SDKs/iPhoneSimulator8.0.sdk %clang -target 
x86_64-apple-darwin %s -### 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-SIMULATOR %s
+// RUN: env SDKROOT=%t/SDKs/iPhoneSimulator8.0.sdk %clang -arch x86_64 %s -### 
2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-SIMULATOR %s
 //
 // CHECK-SIMULATOR: clang
 // CHECK-SIMULATOR: "-cc1"
@@ -74,3 +83,49 @@
 // CHECK-MACOSX: "-triple" "x86_64-apple-macosx10.10.0"
 // CHECK-MACOSX: ld
 

[PATCH] D45152: [MinGW] Add option for disabling looking for a mingw gcc in PATH

2018-04-03 Thread Martin Storsjö via Phabricator via cfe-commits
mstorsjo added a comment.

In https://reviews.llvm.org/D45152#1056134, @rnk wrote:

> In https://reviews.llvm.org/D45152#1056122, @mstorsjo wrote:
>
> > In https://reviews.llvm.org/D45152#1055871, @rnk wrote:
> >
> > > Seems reasonable, looks good.
> >
> >
> > Any opinion on the wording of the option name?
>
>
> Maybe what we're trying to do is find the sysroot relative to clang, so an 
> option name phrased along those lines makes more sense? 
> --clang-in-sysroot-bin? --from-sysroot-bin? --assume-sysroot-bin? 
> --in-sysroot-bin?


That sounds sensible - and positive options is often better than negated/ignore 
etc. I think I prefer `--assume-sysroot-bin` out of those.


Repository:
  rC Clang

https://reviews.llvm.org/D45152



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


r329113 - Add REQUIRES: darwin-system to test/Driver/darwin-sdkroot.c

2018-04-03 Thread Alex Lorenz via cfe-commits
Author: arphaman
Date: Tue Apr  3 14:10:26 2018
New Revision: 329113

URL: http://llvm.org/viewvc/llvm-project?rev=329113&view=rev
Log:
Add REQUIRES: darwin-system to test/Driver/darwin-sdkroot.c

The test from r329110 is for Darwin only

Modified:
cfe/trunk/test/Driver/darwin-sdkroot.c

Modified: cfe/trunk/test/Driver/darwin-sdkroot.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/darwin-sdkroot.c?rev=329113&r1=329112&r2=329113&view=diff
==
--- cfe/trunk/test/Driver/darwin-sdkroot.c (original)
+++ cfe/trunk/test/Driver/darwin-sdkroot.c Tue Apr  3 14:10:26 2018
@@ -1,4 +1,5 @@
 // Check that SDKROOT is used to define the default for -isysroot on Darwin.
+// REQUIRES: system-darwin
 //
 // RUN: rm -rf %t.tmpdir
 // RUN: mkdir -p %t.tmpdir


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


Re: r329113 - Add REQUIRES: darwin-system to test/Driver/darwin-sdkroot.c

2018-04-03 Thread Chandler Carruth via cfe-commits
See my reply to the original commit, but I think this is the wrong fix.

On Tue, Apr 3, 2018 at 2:13 PM Alex Lorenz via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: arphaman
> Date: Tue Apr  3 14:10:26 2018
> New Revision: 329113
>
> URL: http://llvm.org/viewvc/llvm-project?rev=329113&view=rev
> Log:
> Add REQUIRES: darwin-system to test/Driver/darwin-sdkroot.c
>
> The test from r329110 is for Darwin only
>
> Modified:
> cfe/trunk/test/Driver/darwin-sdkroot.c
>
> Modified: cfe/trunk/test/Driver/darwin-sdkroot.c
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/darwin-sdkroot.c?rev=329113&r1=329112&r2=329113&view=diff
>
> ==
> --- cfe/trunk/test/Driver/darwin-sdkroot.c (original)
> +++ cfe/trunk/test/Driver/darwin-sdkroot.c Tue Apr  3 14:10:26 2018
> @@ -1,4 +1,5 @@
>  // Check that SDKROOT is used to define the default for -isysroot on
> Darwin.
> +// REQUIRES: system-darwin
>  //
>  // RUN: rm -rf %t.tmpdir
>  // RUN: mkdir -p %t.tmpdir
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: r329110 - [driver][darwin] Do not infer -simulator environment for non-simulator SDKs

2018-04-03 Thread Chandler Carruth via cfe-commits
On Tue, Apr 3, 2018 at 1:52 PM Alex Lorenz via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: arphaman
> Date: Tue Apr  3 13:50:05 2018
> New Revision: 329110
>
> URL: http://llvm.org/viewvc/llvm-project?rev=329110&view=rev
> Log:
> [driver][darwin] Do not infer -simulator environment for non-simulator SDKs
>

I know you added a REQUIRES line to these tests, but I think there is a
much better way:


> --- cfe/trunk/test/Driver/darwin-sdkroot.c (original)
> +++ cfe/trunk/test/Driver/darwin-sdkroot.c Tue Apr  3 13:50:05 2018
> @@ -51,12 +51,21 @@
>  // CHECK-IPHONE: "-triple" "arm64-apple-ios8.0.0"
>  // CHECK-IPHONE: ld
>  // CHECK-IPHONE: "-iphoneos_version_min" "8.0.0"
> +// RUN: env SDKROOT=%t/SDKs/iPhoneOS8.0.0.sdk %clang %s -### 2>&1 \
>

Instead of just running %clang, actually pass the `-target` you want to it
like we do in the below invocation and the other invocations in this file.

We shouldn't lose driver testing on other systems as long as you can
specify the desired target.


> +// RUN:   | FileCheck --check-prefix=CHECK-IPHONE-X86 %s
> +// CHECK-IPHONE-X86: clang
> +// CHECK-IPHONE-X86: "-cc1"
> +// CHECK-IPHONE-X86: -apple-ios8.0.0"
> +// CHECK-IPHONE-X86: ld
> +// CHECK-IPHONE-X86: "-iphoneos_version_min" "8.0.0"
>  //
>  //
>  // RUN: rm -rf %t/SDKs/iPhoneSimulator8.0.sdk
>  // RUN: mkdir -p %t/SDKs/iPhoneSimulator8.0.sdk
>  // RUN: env SDKROOT=%t/SDKs/iPhoneSimulator8.0.sdk %clang -target
> x86_64-apple-darwin %s -### 2>&1 \
>  // RUN:   | FileCheck --check-prefix=CHECK-SIMULATOR %s
> +// RUN: env SDKROOT=%t/SDKs/iPhoneSimulator8.0.sdk %clang -arch x86_64 %s
> -### 2>&1 \
> +// RUN:   | FileCheck --check-prefix=CHECK-SIMULATOR %s
>  //
>  // CHECK-SIMULATOR: clang
>  // CHECK-SIMULATOR: "-cc1"
> @@ -74,3 +83,49 @@
>  // CHECK-MACOSX: "-triple" "x86_64-apple-macosx10.10.0"
>  // CHECK-MACOSX: ld
>  // CHECK-MACOSX: "-macosx_version_min" "10.10.0"
> +
> +// RUN: rm -rf %t/SDKs/WatchOS3.0.sdk
> +// RUN: mkdir -p %t/SDKs/WatchOS3.0.sdk
> +// RUN: env SDKROOT=%t/SDKs/WatchOS3.0.sdk %clang %s -### 2>&1 \
> +// RUN:   | FileCheck --check-prefix=CHECK-WATCH %s
> +//
> +// CHECK-WATCH: clang
> +// CHECK-WATCH: "-cc1"
> +// CHECK-WATCH: -apple-watchos3.0.0"
> +// CHECK-WATCH: ld
> +// CHECK-WATCH: "-watchos_version_min" "3.0.0"
> +//
> +//
> +// RUN: rm -rf %t/SDKs/WatchSimulator3.0.sdk
> +// RUN: mkdir -p %t/SDKs/WatchSimulator3.0.sdk
> +// RUN: env SDKROOT=%t/SDKs/WatchSimulator3.0.sdk %clang %s -### 2>&1 \
> +// RUN:   | FileCheck --check-prefix=CHECK-WATCH-SIMULATOR %s
> +//
> +// CHECK-WATCH-SIMULATOR: clang
> +// CHECK-WATCH-SIMULATOR: "-cc1"
> +// CHECK-WATCH-SIMULATOR: -apple-watchos3.0.0-simulator"
> +// CHECK-WATCH-SIMULATOR: ld
> +// CHECK-WATCH-SIMULATOR: "-watchos_simulator_version_min" "3.0.0"
> +
> +// RUN: rm -rf %t/SDKs/AppleTVOS10.0.sdk
> +// RUN: mkdir -p %t/SDKs/AppleTVOS10.0.sdk
> +// RUN: env SDKROOT=%t/SDKs/AppleTVOS10.0.sdk %clang %s -### 2>&1 \
> +// RUN:   | FileCheck --check-prefix=CHECK-TV %s
> +//
> +// CHECK-TV: clang
> +// CHECK-TV: "-cc1"
> +// CHECK-TV: -apple-tvos10.0.0"
> +// CHECK-TV: ld
> +// CHECK-TV: "-tvos_version_min" "10.0.0"
> +//
> +//
> +// RUN: rm -rf %t/SDKs/AppleTVSimulator10.0.sdk
> +// RUN: mkdir -p %t/SDKs/AppleTVSimulator10.0.sdk
> +// RUN: env SDKROOT=%t/SDKs/AppleTVSimulator10.0.sdk %clang %s -### 2>&1 \
> +// RUN:   | FileCheck --check-prefix=CHECK-TV-SIMULATOR %s
> +//
> +// CHECK-TV-SIMULATOR: clang
> +// CHECK-TV-SIMULATOR: "-cc1"
> +// CHECK-TV-SIMULATOR: -apple-tvos10.0.0-simulator"
> +// CHECK-TV-SIMULATOR: ld
> +// CHECK-TV-SIMULATOR: "-tvos_simulator_version_min" "10.0.0"
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D45056: [X86] Split up -march=icelake to -client & -server

2018-04-03 Thread Craig Topper via Phabricator via cfe-commits
craig.topper added inline comments.



Comment at: test/Preprocessor/predefined-arch-macros.c:1164
+// CHECK_ICX_M32: #define __corei7 1
+// CHECK_ICX_M32: #define __corei7__ 1
+// CHECK_ICX_M32: #define __i386 1

The ICX_M32 and ICX_M64 should be contiguous in the file. The file is grouped 
by CPUs with the 32 and 64 bit together.


Repository:
  rC Clang

https://reviews.llvm.org/D45056



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


[PATCH] D45179: [libc++] Add _LIBCPP_FORCE_NODISCARD define to force-enable nodiscard in pre-C++17

2018-04-03 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

Is Marshall arguing that the standard doesn't allow compilers to warn about 
failing to use these function results prior to C++17?  Because I don't think 
that's true; warnings are thoroughly non-normative.

If libc++ wants to allow its users to opt out of these warnings just for libc++ 
functions, that's fine (although unusual), but it seems to me that that ought 
to be an explicit opt-out that's independent of language version.


Repository:
  rCXX libc++

https://reviews.llvm.org/D45179



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


[PATCH] D45223: [CUDA] Fix overloading resolution failure due to kernel calling convention

2018-04-03 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

I think the appropriate place to do this is in IsStandardConversion, 
immediately after the call to ResolveAddressOfOverloadedFunction.  You might 
want to add a general utility for getting the type-of-reference of a function 
decl.


https://reviews.llvm.org/D45223



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


r329115 - [StaticAnalyzer] Fix some Clang-tidy modernize and Include What You Use warnings; other minor fixes (NFC).

2018-04-03 Thread Eugene Zelenko via cfe-commits
Author: eugenezelenko
Date: Tue Apr  3 14:31:50 2018
New Revision: 329115

URL: http://llvm.org/viewvc/llvm-project?rev=329115&view=rev
Log:
[StaticAnalyzer] Fix some Clang-tidy modernize and Include What You Use 
warnings; other minor fixes (NFC).

Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/CheckerRegistry.h
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/TaintManager.h
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/TaintTag.h
cfe/trunk/lib/StaticAnalyzer/Core/CheckerRegistry.cpp
cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/CheckerRegistry.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/CheckerRegistry.h?rev=329115&r1=329114&r2=329115&view=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/CheckerRegistry.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/CheckerRegistry.h Tue Apr  3 
14:31:50 2018
@@ -1,4 +1,4 @@
-//===--- CheckerRegistry.h - Maintains all available checkers ---*- C++ 
-*-===//
+//===- CheckerRegistry.h - Maintains all available checkers -*- C++ 
-*-===//
 //
 // The LLVM Compiler Infrastructure
 //
@@ -12,6 +12,9 @@
 
 #include "clang/Basic/LLVM.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "llvm/ADT/StringMap.h"
+#include "llvm/ADT/StringRef.h"
+#include 
 #include 
 
 // FIXME: move this information to an HTML file in docs/.
@@ -64,8 +67,9 @@
 #endif
 
 namespace clang {
-class DiagnosticsEngine;
+
 class AnalyzerOptions;
+class DiagnosticsEngine;
 
 namespace ento {
 
@@ -81,17 +85,18 @@ class CheckerRegistry {
 public:
   /// Initialization functions perform any necessary setup for a checker.
   /// They should include a call to CheckerManager::registerChecker.
-  typedef void (*InitializationFunction)(CheckerManager &);
+  using InitializationFunction = void (*)(CheckerManager &);
+
   struct CheckerInfo {
 InitializationFunction Initialize;
 StringRef FullName;
 StringRef Desc;
 
 CheckerInfo(InitializationFunction fn, StringRef name, StringRef desc)
-: Initialize(fn), FullName(name), Desc(desc) {}
+: Initialize(fn), FullName(name), Desc(desc) {}
   };
 
-  typedef std::vector CheckerInfoList;
+  using CheckerInfoList = std::vector;
 
 private:
   template 
@@ -136,7 +141,8 @@ private:
   mutable llvm::StringMap Packages;
 };
 
-} // end namespace ento
-} // end namespace clang
+} // namespace ento
 
-#endif
+} // namespace clang
+
+#endif // LLVM_CLANG_STATICANALYZER_CORE_CHECKERREGISTRY_H

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h?rev=329115&r1=329114&r2=329115&view=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h 
(original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h Tue 
Apr  3 14:31:50 2018
@@ -1,4 +1,4 @@
-//== MemRegion.h - Abstract memory regions for static analysis --*- C++ 
-*--==//
+//==- MemRegion.h - Abstract memory regions for static analysis -*- C++ 
-*--==//
 //
 // The LLVM Compiler Infrastructure
 //
@@ -19,24 +19,39 @@
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/CharUnits.h"
 #include "clang/AST/Decl.h"
-#include "clang/AST/DeclCXX.h"
+#include "clang/AST/DeclObjC.h"
+#include "clang/AST/DeclarationName.h"
+#include "clang/AST/Expr.h"
 #include "clang/AST/ExprObjC.h"
-#include "clang/Analysis/AnalysisDeclContext.h"
+#include "clang/AST/Type.h"
 #include "clang/Basic/LLVM.h"
+#include "clang/Basic/SourceLocation.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/SymExpr.h"
+#include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/FoldingSet.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/PointerIntPair.h"
 #include "llvm/Support/Allocator.h"
-#include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/Casting.h"
+#include 
+#include 
+#include 
 #include 
+#include 
 
 namespace clang {
 
+class AnalysisDeclContext;
+class CXXRecordDecl;
+class Decl;
 class LocationContext;
 class StackFrameContext;
 
 namespace ento {
 
 class CodeTextRegion;
+class MemRegion;
 class MemRegionManager;
 class MemSpaceRegion;
 class SValBuilder;
@@ -46,7 +61,7 @@ class VarRegion;
 /// Represent a region's offset within the top level base region.
 class RegionOffset {
   /// The base region.
-  const MemRegion *R;
+  const MemRegion *R = nullptr;
 
   /// The bit offset within the base region. Can be negative.
   int64_t Offset;
@@ -54,9 +69,9 @@ class RegionOffset {
 public:
   // We're using a const instead of an en

[PATCH] D45179: [libc++] Add _LIBCPP_FORCE_NODISCARD define to force-enable nodiscard in pre-C++17

2018-04-03 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment.

In https://reviews.llvm.org/D45179#1056183, @rjmccall wrote:

> Is Marshall arguing that the standard doesn't allow compilers to warn about 
> failing to use these function results prior to C++17?


No. He was simply saying that people are opposed to having nodiscard attribute 
specified when the standard does not require that yet.
For same reason, this has to be an opt-in (`_LIBCPP_FORCE_NODISCARD`), it can't 
be just an opt-out.
But, you might as well just ask him that.


Repository:
  rCXX libc++

https://reviews.llvm.org/D45179



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


  1   2   >