Re: r269100 - [VFS] Reconstruct the VFS overlay tree for more accurate lookup

2016-05-10 Thread Sean Silva via cfe-commits
On Tue, May 10, 2016 at 9:49 PM, Bruno Cardoso Lopes <
bruno.card...@gmail.com> wrote:

> Thanks Sean,
>
> I was about to ask for help and try a 4th attempt, as you noted after
> 3 attempts I was not yet able to make this work on windows though...
>

So sorry! It had gone quiet for a few hours and I wanted to avoid leaving
it red any longer.


> The new tests I added first failed on windows because looks like a
> missing "/" on the dir name forces "\\" to be generated while
> appending the filename instead of "/". After fixing that, now seems
> like the directory iteration on windows has a different order from
> what we are getting on the other platforms, do you know anything about
> this?


Does any platform define a guaranteed directory iteration order? I feel
like depending on it in the first place is unwise (or maybe I'm
misunderstanding? e.g. I'm thinking that we should sort the results of
directory iteration to guarantee a particular order if needed for a
comparison in a test)


> Or can you help me debug it once I land it again?


Sure, but like I mentioned the nature of this error is very troubling to me
(high-level API use running into low-level system issues). It suggests that
there is an incorrect layering or something. I hate these annoying windows
path issues as much as everyone else, so I'd rather we focus energy on
building an appropriate layering to consolidate all the special handling in
one place than to continually band-aid issues as they creep into all parts
of the codebase. VirtualFileSystem.cpp already has quite a bit of technical
debt that violates our policies in this regard (platform-specific #ifdef's
outside libSupport; I count 3 in VirtualFileSystem.cpp which is a huge
number considering how recent that code is).


> I might
> check-in the new tests separately from the rest to make it easier to
> revert.
>

I don't think there's precedent for doing this in LLVM (or at least it
would be considered as bad practice). We typically check in tests alongside
the code being tested specifically to avoid the temptation to leave
untested/broken code in tree.

-- Sean Silva


>
>
>  TEST 'Clang-Unit ::
> Basic/BasicTests.exe/VFSFromYAMLTest.DirectoryIteration' FAILED
> 
> Note: Google Test filter = VFSFromYAMLTest.DirectoryIteration
> [==] Running 1 test from 1 test case.
> [--] Global test environment set-up.
> [--] 1 test from VFSFromYAMLTest
> [ RUN  ] VFSFromYAMLTest.DirectoryIteration
>
>
> C:\Buildbot\Slave\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\unittests\Basic\VirtualFileSystemTest.cpp(379):
> error: Value of: I->getName()
>
>   Actual: { '/' (47, 0x2F), '/' (47, 0x2F), 'r' (114, 0x72), 'o' (111,
> 0x6F), 'o' (111, 0x6F), 't' (116, 0x74), '/' (47, 0x2F), 'b' (98,
> 0x62), 'a' (97, 0x61), 'z' (122, 0x7A), '/' (47, 0x2F), 'c' (99, 0x63)
> }
>
> Expected: *ExpectedIter
>
> Which is: { '/' (47, 0x2F), '/' (47, 0x2F), 'r' (114, 0x72), 'o' (111,
> 0x6F), 'o' (111, 0x6F), 't' (116, 0x74), '/' (47, 0x2F), 'b' (98,
> 0x62), 'a' (97, 0x61), 'z' (122, 0x7A), '/' (47, 0x2F), 'x' (120,
> 0x78) }
>
>
> C:\Buildbot\Slave\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\unittests\Basic\VirtualFileSystemTest.cpp(381):
> error: Value of: ExpectedIter
>
>   Actual: 00C72A87F7E0
>
> Expected: ExpectedEnd
> Which is: 00C72A87F800
> [  FAILED  ] VFSFromYAMLTest.DirectoryIteration (1 ms)
>
>
>
> On Tue, May 10, 2016 at 9:27 PM, Sean Silva  wrote:
> > Hi Bruno,
> >
> > I had to revert this in r269100 because it was looking like the bot was
> > going to be left red overnight.
> >
> > Changes to this VFS code seem to have a trend of breaking on windows. Any
> > idea why that is? I can understand things breaking on windows when
> writing
> > low-level parts of an FS abstraction, but this patch seems fairly
> > high-level. Is there a missing layering or something?
> >
> > -- Sean Silva
> >
> > On Tue, May 10, 2016 at 11:43 AM, Bruno Cardoso Lopes via cfe-commits
> >  wrote:
> >>
> >> Author: bruno
> >> Date: Tue May 10 13:43:00 2016
> >> New Revision: 269100
> >>
> >> URL: http://llvm.org/viewvc/llvm-project?rev=269100=rev
> >> Log:
> >> [VFS] Reconstruct the VFS overlay tree for more accurate lookup
> >>
> >> The way we currently build the internal VFS overlay representation leads
> >> to inefficient path search and might yield wrong answers when asked for
> >> recursive or regular directory iteration.
> >>
> >> Currently, when reading an YAML file, each YAML root entry is placed
> >> inside a new root in the filesystem overlay. In the crash reproducer, a
> >> simple "@import Foundation" currently maps to 43 roots, and when looking
> >> up paths, we traverse a directory tree for each of these different
> >> roots, until we find a match (or don't). This has two consequences:
> >>
> >> - It's slow.
> >> - Directory iteration gives 

Re: r269100 - [VFS] Reconstruct the VFS overlay tree for more accurate lookup

2016-05-10 Thread Bruno Cardoso Lopes via cfe-commits
Thanks Sean,

I was about to ask for help and try a 4th attempt, as you noted after
3 attempts I was not yet able to make this work on windows though...

The new tests I added first failed on windows because looks like a
missing "/" on the dir name forces "\\" to be generated while
appending the filename instead of "/". After fixing that, now seems
like the directory iteration on windows has a different order from
what we are getting on the other platforms, do you know anything about
this? Or can you help me debug it once I land it again? I might
check-in the new tests separately from the rest to make it easier to
revert.


 TEST 'Clang-Unit ::
Basic/BasicTests.exe/VFSFromYAMLTest.DirectoryIteration' FAILED

Note: Google Test filter = VFSFromYAMLTest.DirectoryIteration
[==] Running 1 test from 1 test case.
[--] Global test environment set-up.
[--] 1 test from VFSFromYAMLTest
[ RUN  ] VFSFromYAMLTest.DirectoryIteration

C:\Buildbot\Slave\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\unittests\Basic\VirtualFileSystemTest.cpp(379):
error: Value of: I->getName()

  Actual: { '/' (47, 0x2F), '/' (47, 0x2F), 'r' (114, 0x72), 'o' (111,
0x6F), 'o' (111, 0x6F), 't' (116, 0x74), '/' (47, 0x2F), 'b' (98,
0x62), 'a' (97, 0x61), 'z' (122, 0x7A), '/' (47, 0x2F), 'c' (99, 0x63)
}

Expected: *ExpectedIter

Which is: { '/' (47, 0x2F), '/' (47, 0x2F), 'r' (114, 0x72), 'o' (111,
0x6F), 'o' (111, 0x6F), 't' (116, 0x74), '/' (47, 0x2F), 'b' (98,
0x62), 'a' (97, 0x61), 'z' (122, 0x7A), '/' (47, 0x2F), 'x' (120,
0x78) }

C:\Buildbot\Slave\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\unittests\Basic\VirtualFileSystemTest.cpp(381):
error: Value of: ExpectedIter

  Actual: 00C72A87F7E0

Expected: ExpectedEnd
Which is: 00C72A87F800
[  FAILED  ] VFSFromYAMLTest.DirectoryIteration (1 ms)



On Tue, May 10, 2016 at 9:27 PM, Sean Silva  wrote:
> Hi Bruno,
>
> I had to revert this in r269100 because it was looking like the bot was
> going to be left red overnight.
>
> Changes to this VFS code seem to have a trend of breaking on windows. Any
> idea why that is? I can understand things breaking on windows when writing
> low-level parts of an FS abstraction, but this patch seems fairly
> high-level. Is there a missing layering or something?
>
> -- Sean Silva
>
> On Tue, May 10, 2016 at 11:43 AM, Bruno Cardoso Lopes via cfe-commits
>  wrote:
>>
>> Author: bruno
>> Date: Tue May 10 13:43:00 2016
>> New Revision: 269100
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=269100=rev
>> Log:
>> [VFS] Reconstruct the VFS overlay tree for more accurate lookup
>>
>> The way we currently build the internal VFS overlay representation leads
>> to inefficient path search and might yield wrong answers when asked for
>> recursive or regular directory iteration.
>>
>> Currently, when reading an YAML file, each YAML root entry is placed
>> inside a new root in the filesystem overlay. In the crash reproducer, a
>> simple "@import Foundation" currently maps to 43 roots, and when looking
>> up paths, we traverse a directory tree for each of these different
>> roots, until we find a match (or don't). This has two consequences:
>>
>> - It's slow.
>> - Directory iteration gives incomplete results since it only return
>> results within one root - since contents of the same directory can be
>> declared inside different roots, the result isn't accurate.
>>
>> This is in part fault of the way we currently write out the YAML file
>> when emitting the crash reproducer - we could generate only one root and
>> that would make it fast and correct again. However, we should not rely
>> on how the client writes the YAML, but provide a good internal
>> representation regardless.
>>
>> This patch builds a proper virtual directory tree out of the YAML
>> representation, allowing faster search and proper iteration. Besides the
>> crash reproducer, this potentially benefits other VFS clients.
>>
>> Modified:
>> cfe/trunk/lib/Basic/VirtualFileSystem.cpp
>> cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp
>>
>> Modified: cfe/trunk/lib/Basic/VirtualFileSystem.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/VirtualFileSystem.cpp?rev=269100=269099=269100=diff
>>
>> ==
>> --- cfe/trunk/lib/Basic/VirtualFileSystem.cpp (original)
>> +++ cfe/trunk/lib/Basic/VirtualFileSystem.cpp Tue May 10 13:43:00 2016
>> @@ -719,7 +719,13 @@ public:
>>  Status S)
>>: Entry(EK_Directory, Name), Contents(std::move(Contents)),
>>  S(std::move(S)) {}
>> +  RedirectingDirectoryEntry(StringRef Name, Status S)
>> +  : Entry(EK_Directory, Name), S(std::move(S)) {}
>>Status getStatus() { return S; }
>> +  void addContent(std::unique_ptr Content) {
>> +

Re: r269100 - [VFS] Reconstruct the VFS overlay tree for more accurate lookup

2016-05-10 Thread Sean Silva via cfe-commits
Hi Bruno,

I had to revert this in r269100 because it was looking like the bot was
going to be left red overnight.

Changes to this VFS code seem to have a trend of breaking on windows. Any
idea why that is? I can understand things breaking on windows when writing
low-level parts of an FS abstraction, but this patch seems fairly
high-level. Is there a missing layering or something?

-- Sean Silva

On Tue, May 10, 2016 at 11:43 AM, Bruno Cardoso Lopes via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: bruno
> Date: Tue May 10 13:43:00 2016
> New Revision: 269100
>
> URL: http://llvm.org/viewvc/llvm-project?rev=269100=rev
> Log:
> [VFS] Reconstruct the VFS overlay tree for more accurate lookup
>
> The way we currently build the internal VFS overlay representation leads
> to inefficient path search and might yield wrong answers when asked for
> recursive or regular directory iteration.
>
> Currently, when reading an YAML file, each YAML root entry is placed
> inside a new root in the filesystem overlay. In the crash reproducer, a
> simple "@import Foundation" currently maps to 43 roots, and when looking
> up paths, we traverse a directory tree for each of these different
> roots, until we find a match (or don't). This has two consequences:
>
> - It's slow.
> - Directory iteration gives incomplete results since it only return
> results within one root - since contents of the same directory can be
> declared inside different roots, the result isn't accurate.
>
> This is in part fault of the way we currently write out the YAML file
> when emitting the crash reproducer - we could generate only one root and
> that would make it fast and correct again. However, we should not rely
> on how the client writes the YAML, but provide a good internal
> representation regardless.
>
> This patch builds a proper virtual directory tree out of the YAML
> representation, allowing faster search and proper iteration. Besides the
> crash reproducer, this potentially benefits other VFS clients.
>
> Modified:
> cfe/trunk/lib/Basic/VirtualFileSystem.cpp
> cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp
>
> Modified: cfe/trunk/lib/Basic/VirtualFileSystem.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/VirtualFileSystem.cpp?rev=269100=269099=269100=diff
>
> ==
> --- cfe/trunk/lib/Basic/VirtualFileSystem.cpp (original)
> +++ cfe/trunk/lib/Basic/VirtualFileSystem.cpp Tue May 10 13:43:00 2016
> @@ -719,7 +719,13 @@ public:
>  Status S)
>: Entry(EK_Directory, Name), Contents(std::move(Contents)),
>  S(std::move(S)) {}
> +  RedirectingDirectoryEntry(StringRef Name, Status S)
> +  : Entry(EK_Directory, Name), S(std::move(S)) {}
>Status getStatus() { return S; }
> +  void addContent(std::unique_ptr Content) {
> +Contents.push_back(std::move(Content));
> +  }
> +  Entry *getLastContent() const { return Contents.back().get(); }
>typedef decltype(Contents)::iterator iterator;
>iterator contents_begin() { return Contents.begin(); }
>iterator contents_end() { return Contents.end(); }
> @@ -747,6 +753,7 @@ public:
>  return UseName == NK_NotSet ? GlobalUseExternalName
>  : (UseName == NK_External);
>}
> +  NameKind getUseName() const { return UseName; }
>static bool classof(const Entry *E) { return E->getKind() == EK_File; }
>  };
>
> @@ -1023,6 +1030,70 @@ class RedirectingFileSystemParser {
>  return true;
>}
>
> +  Entry *lookupOrCreateEntry(RedirectingFileSystem *FS, StringRef Name,
> + Entry *ParentEntry = nullptr) {
> +if (!ParentEntry) { // Look for a existent root
> +  for (const std::unique_ptr  : FS->Roots) {
> +if (Name.equals(Root->getName())) {
> +  ParentEntry = Root.get();
> +  return ParentEntry;
> +}
> +  }
> +} else { // Advance to the next component
> +  auto *DE = dyn_cast(ParentEntry);
> +  for (std::unique_ptr  :
> +   llvm::make_range(DE->contents_begin(), DE->contents_end())) {
> +auto *DirContent =
> dyn_cast(Content.get());
> +if (DirContent && Name.equals(Content->getName()))
> +  return DirContent;
> +  }
> +}
> +
> +// ... or create a new one
> +std::unique_ptr E =
> llvm::make_unique(
> +Name, Status("", getNextVirtualUniqueID(), sys::TimeValue::now(),
> 0, 0,
> + 0, file_type::directory_file, sys::fs::all_all));
> +
> +if (!ParentEntry) { // Add a new root to the overlay
> +  FS->Roots.push_back(std::move(E));
> +  ParentEntry = FS->Roots.back().get();
> +  return ParentEntry;
> +}
> +
> +auto *DE = dyn_cast(ParentEntry);
> +DE->addContent(std::move(E));
> +return DE->getLastContent();
> +  }
> +
> +  void uniqueOverlayTree(RedirectingFileSystem *FS, Entry *SrcE,
> + 

Re: [PATCH] D20112: [OpenMP] Add support for the 'private pointer' flag to signal variables captured in target regions and used in first-private clauses.

2016-05-10 Thread Alexey Bataev via cfe-commits
ABataev added inline comments.


Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:5452
@@ +5451,3 @@
+  // in there.
+  for (const auto *C : D.getClausesOfKind()) {
+for (const auto *D : C->varlists()) {

I think this is too greedy. You're rescanning list of firstprivates 
clauses/variables for each variable.


Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:5458
@@ +5457,3 @@
+  // 'private ptr' and 'map to' flag.
+  if (CurVD == VD)
+return MappableExprsHandler::OMP_MAP_PRIVATE_PTR |

What if the variable is also referenced in lastprivate clause?


http://reviews.llvm.org/D20112



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


r269160 - Hopefully bring llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast back to life

2016-05-10 Thread Sean Silva via cfe-commits
Author: silvas
Date: Tue May 10 23:04:59 2016
New Revision: 269160

URL: http://llvm.org/viewvc/llvm-project?rev=269160=rev
Log:
Hopefully bring llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast back to life

Bruno made a couple valiant attempts but the bot is still red.

This reverts r269100 (primary commit), r269108 (fix attempt), r269133
(fix attempt).

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

Modified: cfe/trunk/lib/Basic/VirtualFileSystem.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/VirtualFileSystem.cpp?rev=269160=269159=269160=diff
==
--- cfe/trunk/lib/Basic/VirtualFileSystem.cpp (original)
+++ cfe/trunk/lib/Basic/VirtualFileSystem.cpp Tue May 10 23:04:59 2016
@@ -719,13 +719,7 @@ public:
 Status S)
   : Entry(EK_Directory, Name), Contents(std::move(Contents)),
 S(std::move(S)) {}
-  RedirectingDirectoryEntry(StringRef Name, Status S)
-  : Entry(EK_Directory, Name), S(std::move(S)) {}
   Status getStatus() { return S; }
-  void addContent(std::unique_ptr Content) {
-Contents.push_back(std::move(Content));
-  }
-  Entry *getLastContent() const { return Contents.back().get(); }
   typedef decltype(Contents)::iterator iterator;
   iterator contents_begin() { return Contents.begin(); }
   iterator contents_end() { return Contents.end(); }
@@ -753,7 +747,6 @@ public:
 return UseName == NK_NotSet ? GlobalUseExternalName
 : (UseName == NK_External);
   }
-  NameKind getUseName() const { return UseName; }
   static bool classof(const Entry *E) { return E->getKind() == EK_File; }
 };
 
@@ -1030,70 +1023,6 @@ class RedirectingFileSystemParser {
 return true;
   }
 
-  Entry *lookupOrCreateEntry(RedirectingFileSystem *FS, StringRef Name,
- Entry *ParentEntry = nullptr) {
-if (!ParentEntry) { // Look for a existent root
-  for (const std::unique_ptr  : FS->Roots) {
-if (Name.equals(Root->getName())) {
-  ParentEntry = Root.get();
-  return ParentEntry;
-}
-  }
-} else { // Advance to the next component
-  auto *DE = dyn_cast(ParentEntry);
-  for (std::unique_ptr  :
-   llvm::make_range(DE->contents_begin(), DE->contents_end())) {
-auto *DirContent = dyn_cast(Content.get());
-if (DirContent && Name.equals(Content->getName()))
-  return DirContent;
-  }
-}
-
-// ... or create a new one
-std::unique_ptr E = llvm::make_unique(
-Name, Status("", getNextVirtualUniqueID(), sys::TimeValue::now(), 0, 0,
- 0, file_type::directory_file, sys::fs::all_all));
-
-if (!ParentEntry) { // Add a new root to the overlay
-  FS->Roots.push_back(std::move(E));
-  ParentEntry = FS->Roots.back().get();
-  return ParentEntry;
-}
-
-auto *DE = dyn_cast(ParentEntry);
-DE->addContent(std::move(E));
-return DE->getLastContent();
-  }
-
-  void uniqueOverlayTree(RedirectingFileSystem *FS, Entry *SrcE,
- Entry *NewParentE = nullptr) {
-StringRef Name = SrcE->getName();
-switch (SrcE->getKind()) {
-case EK_Directory: {
-  auto *DE = dyn_cast(SrcE);
-  assert(DE && "Must be a directory");
-  // Empty directories could be present in the YAML as a way to
-  // describe a file for a current directory after some of its subdir
-  // is parsed. This only leads to redundant walks, ignore it.
-  if (!Name.empty())
-NewParentE = lookupOrCreateEntry(FS, Name, NewParentE);
-  for (std::unique_ptr  :
-   llvm::make_range(DE->contents_begin(), DE->contents_end()))
-uniqueOverlayTree(FS, SubEntry.get(), NewParentE);
-  break;
-}
-case EK_File: {
-  auto *FE = dyn_cast(SrcE);
-  assert(FE && "Must be a file");
-  assert(NewParentE && "Parent entry must exist");
-  auto *DE = dyn_cast(NewParentE);
-  DE->addContent(llvm::make_unique(
-  Name, FE->getExternalContentsPath(), FE->getUseName()));
-  break;
-}
-}
-  }
-
   std::unique_ptr parseEntry(yaml::Node *N, RedirectingFileSystem *FS) {
 yaml::MappingNode *M = dyn_cast(N);
 if (!M) {
@@ -1296,7 +1225,6 @@ public:
 };
 
 DenseMap Keys(std::begin(Fields), std::end(Fields));
-std::vector RootEntries;
 
 // Parse configuration and 'roots'
 for (yaml::MappingNode::iterator I = Top->begin(), E = Top->end(); I != E;
@@ -1319,7 +1247,7 @@ public:
 for (yaml::SequenceNode::iterator I = Roots->begin(), E = Roots->end();
  I != E; ++I) {
   if (std::unique_ptr E = parseEntry(&*I, FS))
-RootEntries.push_back(std::move(E));
+FS->Roots.push_back(std::move(E));
   else
 return false;
 }
@@ -1360,13 

Re: [PATCH] D20131: Fixed crash during code completion in file included within declaration

2016-05-10 Thread Richard Smith via cfe-commits
rsmith added a comment.

Please add a test to test/CodeCompletion.



Comment at: lib/Lex/PPLexerChange.cpp:380-382
@@ -379,3 +379,5 @@
 CurLexer->FormTokenWithChars(Result, CurLexer->BufferEnd, tok::eof);
 CurLexer.reset();
+if (CurLexerKind == CLK_Lexer)
+  CurLexerKind = CLK_CachingLexer;
   } else {

Can you use `CurLexer->cutOffLexing()` instead?


http://reviews.llvm.org/D20131



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


Re: [PATCH] D15421: [Feature] Add a builtin for indexing into parameter packs

2016-05-10 Thread Louis Dionne via cfe-commits
ldionne added a comment.

ping


http://reviews.llvm.org/D15421



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


Re: [PATCH] D20132: [libclang] Add clang_getAllSkippedRanges function

2016-05-10 Thread Richard Smith via cfe-commits
rsmith added a comment.

This looks reasonable, but please add a test.


http://reviews.llvm.org/D20132



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


Re: [PATCH] D20134: [libclang] Fixed bug where ranges in spelling locations (in macro expansions) would get mangled

2016-05-10 Thread Richard Smith via cfe-commits
rsmith added a comment.

I agree that this looks wrong.

Does test/Index/cindex-on-invalid.m still pass with this change? (See 
http://reviews.llvm.org/rL129872 in which it was added.) It looks like the 
problem is that we lose the information about whether we're supposed to be 
pointing to the start or the end of the token when we later map to the 
expansion location, which means that libclang's attempt to pretend that source 
ranges are half-open intervals rather than closed intervals breaks.


http://reviews.llvm.org/D20134



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


Re: [PATCH] D20090: [OPENCL] Fix wrongly vla error for OpenCL array.

2016-05-10 Thread Xiuli PAN via cfe-commits
pxli168 added inline comments.


Comment at: lib/Sema/SemaType.cpp:2055
@@ -2054,3 +2054,3 @@
 
-  return S.VerifyIntegerConstantExpression(ArraySize, , Diagnoser,
-   S.LangOpts.GNUMode).isInvalid();
+  return S
+  .VerifyIntegerConstantExpression(ArraySize, , Diagnoser,

Anastasia wrote:
> Formatting looks weird though... may be better to leave the first line 
> unmodified?
I felt very strange, too. But this is what clang-format gives me.


Comment at: test/CodeGenOpenCL/vla.cl:1
@@ +1,2 @@
+// RUN: %clang_cc1 -emit-llvm -O0 -o - %s | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm -O0 -cl-std=CL2.0 -DCL20 -o - %s | FileCheck %s 
--check-prefix=CL20

Anastasia wrote:
> Could we have a Sema test instead where we accept the VLA if constant AS 
> object is used and give an error otherwise?
> 
> Also do we need to test CL2.0? We don't seem to be doing anything different 
> for that version. 
In earier than OpenCL1.2  program scope variable must reside in constant 
address space and no
```
const global int
```
can be here.
But const global value should also not be seen as VLA either.


http://reviews.llvm.org/D20090



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


Re: [PATCH] D20125: [libclang] Added clang_getRealSpellingLocation to compensate for clang_getSpellingLocation returning the expansion location

2016-05-10 Thread Richard Smith via cfe-commits
rsmith added a comment.

Ugh. Yes. I suppose it's too late to make this actually do the right thing. 
@akyrtzi, what do you think?

Please add a test for this (see unittests/libclang/LibclangTest.cpp).


http://reviews.llvm.org/D20125



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


Re: [PATCH] D19932: [OpenCL] Add to_{global|local|private} builtin functions.

2016-05-10 Thread Xiuli PAN via cfe-commits
pxli168 added inline comments.


Comment at: test/SemaOpenCL/to_addr_builtin.cl:25
@@ +24,3 @@
+  glob = to_global(con);
+#if __OPENCL_C_VERSION__ < CL_VERSION_2_0
+  // expected-error@-2{{'to_global' needs OpenCL version 2.0 or above}}

yaxunl wrote:
> pxli168 wrote:
> > But in Spec OpenCL V2.0 s6.13.9 the description for to_global is:
> > 
> > > Returns a pointer that points to a region in the global address space if 
> > > to_global can cast ptr to the global address space. Otherwise it returns 
> > > NULL.
> > 
> I think this is only for valid call expression. For constant pointer, it is 
> an invalid call expression due to s6.5.5, so an error of invalid argument 
> should be emitted.
> 
> If we allow passing constant pointer to this function, we violates s6.5.5.
>A pointer that points to the constant address space cannot be cast or 
>implicitly converted to the generic address space.
 
So if we can not cast, we maybe should return NULL?
@Anastasia What do you think about this?




http://reviews.llvm.org/D19932



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


r269154 - [Sema] Fix value-dependent enable_if bug.

2016-05-10 Thread George Burgess IV via cfe-commits
Author: gbiv
Date: Tue May 10 20:38:27 2016
New Revision: 269154

URL: http://llvm.org/viewvc/llvm-project?rev=269154=rev
Log:
[Sema] Fix value-dependent enable_if bug.

This patch fixes a bug where we would assume all value-dependent
enable_if conditions give successful results.

Instead, we consider value-dependent enable_if conditions to always
fail. While this isn't ideal, this is the best we can realistically do
without changing both enable_if's semantics and large parts of Sema
(specifically, all of the parts that don't expect type dependence to
come out of nowhere, and that may interact with overload resolution).

Differential Revision: http://reviews.llvm.org/D20130

Modified:
cfe/trunk/include/clang/Basic/AttrDocs.td
cfe/trunk/lib/Sema/SemaOverload.cpp
cfe/trunk/test/SemaCXX/enable_if.cpp

Modified: cfe/trunk/include/clang/Basic/AttrDocs.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/AttrDocs.td?rev=269154=269153=269154=diff
==
--- cfe/trunk/include/clang/Basic/AttrDocs.td (original)
+++ cfe/trunk/include/clang/Basic/AttrDocs.td Tue May 10 20:38:27 2016
@@ -283,6 +283,32 @@ their address taken, unless all of the c
 ptr =  // OK: 'TrueConstant' is a truthy constant
 ptr =  // error: 'FalseConstant' is a constant, but not truthy
   }
+
+Because ``enable_if`` evaluation happens during overload resolution,
+``enable_if`` may give unintuitive results when used with templates, depending
+on when overloads are resolved. In the example below, clang will emit a
+diagnostic about no viable overloads for ``foo`` in ``bar``, but not in 
``baz``:
+
+.. code-block:: c++
+
+  double foo(int i) __attribute__((enable_if(i > 0, "")));
+  void *foo(int i) __attribute__((enable_if(i <= 0, "")));
+  template 
+  auto bar() { return foo(I); }
+
+  template 
+  auto baz() { return foo(T::number); }
+
+  struct WithNumber { constexpr static int number = 1; };
+  void callThem() {
+bar();
+baz();
+  }
+
+This is because, in ``bar``, ``foo`` is resolved prior to template
+instantiation, so the value for ``I`` isn't known (thus, both ``enable_if``
+conditions for ``foo`` fail). However, in ``baz``, ``foo`` is resolved during
+template instantiation, so the value for ``T::number`` is known.
   }];
 }
 

Modified: cfe/trunk/lib/Sema/SemaOverload.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOverload.cpp?rev=269154=269153=269154=diff
==
--- cfe/trunk/lib/Sema/SemaOverload.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOverload.cpp Tue May 10 20:38:27 2016
@@ -5984,7 +5984,6 @@ EnableIfAttr *Sema::CheckEnableIf(Functi
   SFINAETrap Trap(*this);
   SmallVector ConvertedArgs;
   bool InitializationFailed = false;
-  bool ContainsValueDependentExpr = false;
 
   // Convert the arguments.
   for (unsigned I = 0, E = Args.size(); I != E; ++I) {
@@ -6006,7 +6005,6 @@ EnableIfAttr *Sema::CheckEnableIf(Functi
   break;
 }
 
-ContainsValueDependentExpr |= R.get()->isValueDependent();
 ConvertedArgs.push_back(R.get());
   }
 
@@ -6027,7 +6025,6 @@ EnableIfAttr *Sema::CheckEnableIf(Functi
 InitializationFailed = true;
 break;
   }
-  ContainsValueDependentExpr |= R.get()->isValueDependent();
   ConvertedArgs.push_back(R.get());
 }
 
@@ -6037,18 +6034,14 @@ EnableIfAttr *Sema::CheckEnableIf(Functi
 
   for (auto *EIA : EnableIfAttrs) {
 APValue Result;
-if (EIA->getCond()->isValueDependent()) {
-  // Don't even try now, we'll examine it after instantiation.
-  continue;
-}
-
+// FIXME: This doesn't consider value-dependent cases, because doing so is
+// very difficult. Ideally, we should handle them more gracefully.
 if (!EIA->getCond()->EvaluateWithSubstitution(
-Result, Context, Function, llvm::makeArrayRef(ConvertedArgs))) {
-  if (!ContainsValueDependentExpr)
-return EIA;
-} else if (!Result.isInt() || !Result.getInt().getBoolValue()) {
+Result, Context, Function, llvm::makeArrayRef(ConvertedArgs)))
+  return EIA;
+
+if (!Result.isInt() || !Result.getInt().getBoolValue())
   return EIA;
-}
   }
   return nullptr;
 }

Modified: cfe/trunk/test/SemaCXX/enable_if.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/enable_if.cpp?rev=269154=269153=269154=diff
==
--- cfe/trunk/test/SemaCXX/enable_if.cpp (original)
+++ cfe/trunk/test/SemaCXX/enable_if.cpp Tue May 10 20:38:27 2016
@@ -116,9 +116,9 @@ template  class C {
   void g() { f(); }
 };
 
-int fn3(bool b) __attribute__((enable_if(b, "")));
+int fn3(bool b) __attribute__((enable_if(b, ""))); // FIXME: This test should 
net 0 error messages.
 template  void test3() {
-  fn3(sizeof(T) == 1);
+  fn3(sizeof(T) == 1); // 

Re: [PATCH] D20130: Fix enable_if evaluation in value-dependent contexts.

2016-05-10 Thread George Burgess IV via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL269154: [Sema] Fix value-dependent enable_if bug. (authored 
by gbiv).

Changed prior to commit:
  http://reviews.llvm.org/D20130?vs=56786=56849#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D20130

Files:
  cfe/trunk/include/clang/Basic/AttrDocs.td
  cfe/trunk/lib/Sema/SemaOverload.cpp
  cfe/trunk/test/SemaCXX/enable_if.cpp

Index: cfe/trunk/include/clang/Basic/AttrDocs.td
===
--- cfe/trunk/include/clang/Basic/AttrDocs.td
+++ cfe/trunk/include/clang/Basic/AttrDocs.td
@@ -283,6 +283,32 @@
 ptr =  // OK: 'TrueConstant' is a truthy constant
 ptr =  // error: 'FalseConstant' is a constant, but not truthy
   }
+
+Because ``enable_if`` evaluation happens during overload resolution,
+``enable_if`` may give unintuitive results when used with templates, depending
+on when overloads are resolved. In the example below, clang will emit a
+diagnostic about no viable overloads for ``foo`` in ``bar``, but not in ``baz``:
+
+.. code-block:: c++
+
+  double foo(int i) __attribute__((enable_if(i > 0, "")));
+  void *foo(int i) __attribute__((enable_if(i <= 0, "")));
+  template 
+  auto bar() { return foo(I); }
+
+  template 
+  auto baz() { return foo(T::number); }
+
+  struct WithNumber { constexpr static int number = 1; };
+  void callThem() {
+bar();
+baz();
+  }
+
+This is because, in ``bar``, ``foo`` is resolved prior to template
+instantiation, so the value for ``I`` isn't known (thus, both ``enable_if``
+conditions for ``foo`` fail). However, in ``baz``, ``foo`` is resolved during
+template instantiation, so the value for ``T::number`` is known.
   }];
 }
 
Index: cfe/trunk/test/SemaCXX/enable_if.cpp
===
--- cfe/trunk/test/SemaCXX/enable_if.cpp
+++ cfe/trunk/test/SemaCXX/enable_if.cpp
@@ -116,9 +116,9 @@
   void g() { f(); }
 };
 
-int fn3(bool b) __attribute__((enable_if(b, "")));
+int fn3(bool b) __attribute__((enable_if(b, ""))); // FIXME: This test should net 0 error messages.
 template  void test3() {
-  fn3(sizeof(T) == 1);
+  fn3(sizeof(T) == 1); // expected-error{{no matching function for call to 'fn3'}} expected-note@-2{{candidate disabled}}
 }
 
 template 
@@ -138,7 +138,7 @@
 void h(int);
 template  void outer() {
   void local_function() __attribute__((enable_if(::h(T()), "")));
-  local_function();
+  local_function(); // expected-error{{no matching function for call to 'local_function'}} expected-note@-1{{candidate disabled}}
 };
 
 namespace PR20988 {
@@ -158,9 +158,9 @@
 fn2(expr);  // expected-error{{no matching function for call to 'fn2'}}
   }
 
-  int fn3(bool b) __attribute__((enable_if(b, "")));
+  int fn3(bool b) __attribute__((enable_if(b, ""))); // FIXME: This test should net 0 error messages.
   template  void test3() {
-fn3(sizeof(T) == 1);
+fn3(sizeof(T) == 1); // expected-error{{no matching function for call to 'fn3'}} expected-note@-2{{candidate disabled}}
   }
 }
 
@@ -386,3 +386,34 @@
   f.bar(1, 2); // expected-error{{too many arguments}}
 }
 }
+
+// Ideally, we should be able to handle value-dependent expressions sanely.
+// Sadly, that isn't the case at the moment.
+namespace dependent {
+int error(int N) __attribute__((enable_if(N, ""))); // expected-note{{candidate disabled}}
+int error(int N) __attribute__((enable_if(!N, ""))); // expected-note{{candidate disabled}}
+template  int callUnavailable() {
+  return error(N); // expected-error{{no matching function for call to 'error'}}
+}
+
+constexpr int noError(int N) __attribute__((enable_if(N, ""))) { return -1; }
+constexpr int noError(int N) __attribute__((enable_if(!N, ""))) { return -1; }
+constexpr int noError(int N) { return 0; }
+
+template 
+constexpr int callNoError() { return noError(N); }
+static_assert(callNoError<0>() == 0, "");
+static_assert(callNoError<1>() == 0, "");
+
+template  constexpr int templated() __attribute__((enable_if(N, ""))) {
+  return 1;
+}
+
+constexpr int A = templated<0>(); // expected-error{{no matching function for call to 'templated'}} expected-note@-4{{candidate disabled}}
+static_assert(templated<1>() == 1, "");
+
+template  constexpr int callTemplated() { return templated(); }
+
+constexpr int B = callTemplated<0>(); // expected-error{{initialized by a constant expression}} expected-error@-2{{no matching function for call to 'templated'}} expected-note{{in instantiation of function template}} expected-note@-9{{candidate disabled}}
+static_assert(callTemplated<1>() == 1, "");
+}
Index: cfe/trunk/lib/Sema/SemaOverload.cpp
===
--- cfe/trunk/lib/Sema/SemaOverload.cpp
+++ cfe/trunk/lib/Sema/SemaOverload.cpp
@@ -5984,7 +5984,6 @@
   SFINAETrap Trap(*this);
   SmallVector ConvertedArgs;
   bool InitializationFailed = false;
-  bool ContainsValueDependentExpr = 

Re: r269148 - Disable -Wcast-calling-convention by default (follow-up to r269116)

2016-05-10 Thread Hans Wennborg via cfe-commits
The review and commit message of r269116 both stated this was disabled
by default because Reid hadn't evaluated the impact yet. I just fixed
the glitch. :-)

On Tue, May 10, 2016 at 6:00 PM, David Majnemer via cfe-commits
 wrote:
> Er, why? Why not just disable this warning in chromium?
> Clangs diagnostics wouldn't have developed as well as they have if we took
> this approach to all warnings.
>
>
> On Tuesday, May 10, 2016, Hans Wennborg via cfe-commits
>  wrote:
>>
>> Author: hans
>> Date: Tue May 10 19:49:20 2016
>> New Revision: 269148
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=269148=rev
>> Log:
>> Disable -Wcast-calling-convention by default (follow-up to r269116)
>>
>> Modified:
>> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>> cfe/trunk/test/Sema/callingconv-cast.c
>>
>> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=269148=269147=269148=diff
>>
>> ==
>> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
>> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue May 10
>> 19:49:20 2016
>> @@ -6625,7 +6625,7 @@ def warn_function_def_in_objc_container
>>  def warn_cast_calling_conv : Warning<
>>"cast between incompatible calling conventions '%0' and '%1'; "
>>"calls through this pointer may abort at runtime">,
>> -  InGroup>;
>> +  InGroup>, DefaultIgnore;
>>  def note_change_calling_conv_fixit : Note<
>>"consider defining %0 with the '%1' calling convention">;
>>  def warn_bad_function_cast : Warning<
>>
>> Modified: cfe/trunk/test/Sema/callingconv-cast.c
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/callingconv-cast.c?rev=269148=269147=269148=diff
>>
>> ==
>> --- cfe/trunk/test/Sema/callingconv-cast.c (original)
>> +++ cfe/trunk/test/Sema/callingconv-cast.c Tue May 10 19:49:20 2016
>> @@ -3,6 +3,9 @@
>>  // RUN: %clang_cc1 -fms-extensions -triple i686-pc-windows-msvc
>> -Wcast-calling-convention -DMSVC -fdiagnostics-parseable-fixits %s 2>&1 |
>> FileCheck %s --check-prefix=MSFIXIT
>>  // RUN: %clang_cc1 -triple i686-pc-windows-gnu -Wcast-calling-convention
>> -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
>> --check-prefix=GNUFIXIT
>>
>> +// Check that the warning is disabled by default:
>> +// RUN: %clang_cc1 -fms-extensions -triple i686-pc-windows-msvc -DMSVC
>> -Werror -Wno-pointer-bool-conversion -x c %s
>> +
>>  // expected-note@+1 {{consider defining 'mismatched_before_winapi' with
>> the 'stdcall' calling convention}}
>>  void mismatched_before_winapi(int x) {}
>>
>> @@ -43,12 +46,12 @@ int main() {
>>take_callback((callback_t)(void*)mismatched);
>>  }
>>
>> -// MSFIXIT: fix-it:"{{.*}}callingconv-cast.c":{16:6-16:6}:"WINAPI "
>> -// MSFIXIT: fix-it:"{{.*}}callingconv-cast.c":{16:6-16:6}:"WINAPI "
>> -// MSFIXIT: fix-it:"{{.*}}callingconv-cast.c":{16:6-16:6}:"WINAPI "
>> -// MSFIXIT: fix-it:"{{.*}}callingconv-cast.c":{7:6-7:6}:"__stdcall "
>> -
>> -// GNUFIXIT: fix-it:"{{.*}}callingconv-cast.c":{16:6-16:6}:"WINAPI "
>> -// GNUFIXIT: fix-it:"{{.*}}callingconv-cast.c":{16:6-16:6}:"WINAPI "
>> -// GNUFIXIT: fix-it:"{{.*}}callingconv-cast.c":{16:6-16:6}:"WINAPI "
>> -// GNUFIXIT:
>> fix-it:"{{.*}}callingconv-cast.c":{7:6-7:6}:"__attribute__((stdcall)) "
>> +// MSFIXIT: fix-it:"{{.*}}callingconv-cast.c":{19:6-19:6}:"WINAPI "
>> +// MSFIXIT: fix-it:"{{.*}}callingconv-cast.c":{19:6-19:6}:"WINAPI "
>> +// MSFIXIT: fix-it:"{{.*}}callingconv-cast.c":{19:6-19:6}:"WINAPI "
>> +// MSFIXIT: fix-it:"{{.*}}callingconv-cast.c":{10:6-10:6}:"__stdcall "
>> +
>> +// GNUFIXIT: fix-it:"{{.*}}callingconv-cast.c":{19:6-19:6}:"WINAPI "
>> +// GNUFIXIT: fix-it:"{{.*}}callingconv-cast.c":{19:6-19:6}:"WINAPI "
>> +// GNUFIXIT: fix-it:"{{.*}}callingconv-cast.c":{19:6-19:6}:"WINAPI "
>> +// GNUFIXIT:
>> fix-it:"{{.*}}callingconv-cast.c":{10:6-10:6}:"__attribute__((stdcall)) "
>>
>>
>> ___
>> 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
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: r269148 - Disable -Wcast-calling-convention by default (follow-up to r269116)

2016-05-10 Thread Nico Weber via cfe-commits
I asked Reid the same thing on the review. He said he wanted to try the
warning in practice first and maybe polish it a bit before enabling it. (It
was supposed to land in disabled state.)

On Tue, May 10, 2016 at 9:00 PM, David Majnemer via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Er, why? Why not just disable this warning in chromium?
> Clangs diagnostics wouldn't have developed as well as they have if we took
> this approach to all warnings.
>
>
> On Tuesday, May 10, 2016, Hans Wennborg via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: hans
>> Date: Tue May 10 19:49:20 2016
>> New Revision: 269148
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=269148=rev
>> Log:
>> Disable -Wcast-calling-convention by default (follow-up to r269116)
>>
>> Modified:
>> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>> cfe/trunk/test/Sema/callingconv-cast.c
>>
>> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=269148=269147=269148=diff
>>
>> ==
>> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
>> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue May 10
>> 19:49:20 2016
>> @@ -6625,7 +6625,7 @@ def warn_function_def_in_objc_container
>>  def warn_cast_calling_conv : Warning<
>>"cast between incompatible calling conventions '%0' and '%1'; "
>>"calls through this pointer may abort at runtime">,
>> -  InGroup>;
>> +  InGroup>, DefaultIgnore;
>>  def note_change_calling_conv_fixit : Note<
>>"consider defining %0 with the '%1' calling convention">;
>>  def warn_bad_function_cast : Warning<
>>
>> Modified: cfe/trunk/test/Sema/callingconv-cast.c
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/callingconv-cast.c?rev=269148=269147=269148=diff
>>
>> ==
>> --- cfe/trunk/test/Sema/callingconv-cast.c (original)
>> +++ cfe/trunk/test/Sema/callingconv-cast.c Tue May 10 19:49:20 2016
>> @@ -3,6 +3,9 @@
>>  // RUN: %clang_cc1 -fms-extensions -triple i686-pc-windows-msvc
>> -Wcast-calling-convention -DMSVC -fdiagnostics-parseable-fixits %s 2>&1 |
>> FileCheck %s --check-prefix=MSFIXIT
>>  // RUN: %clang_cc1 -triple i686-pc-windows-gnu -Wcast-calling-convention
>> -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
>> --check-prefix=GNUFIXIT
>>
>> +// Check that the warning is disabled by default:
>> +// RUN: %clang_cc1 -fms-extensions -triple i686-pc-windows-msvc -DMSVC
>> -Werror -Wno-pointer-bool-conversion -x c %s
>> +
>>  // expected-note@+1 {{consider defining 'mismatched_before_winapi' with
>> the 'stdcall' calling convention}}
>>  void mismatched_before_winapi(int x) {}
>>
>> @@ -43,12 +46,12 @@ int main() {
>>take_callback((callback_t)(void*)mismatched);
>>  }
>>
>> -// MSFIXIT: fix-it:"{{.*}}callingconv-cast.c":{16:6-16:6}:"WINAPI "
>> -// MSFIXIT: fix-it:"{{.*}}callingconv-cast.c":{16:6-16:6}:"WINAPI "
>> -// MSFIXIT: fix-it:"{{.*}}callingconv-cast.c":{16:6-16:6}:"WINAPI "
>> -// MSFIXIT: fix-it:"{{.*}}callingconv-cast.c":{7:6-7:6}:"__stdcall "
>> -
>> -// GNUFIXIT: fix-it:"{{.*}}callingconv-cast.c":{16:6-16:6}:"WINAPI "
>> -// GNUFIXIT: fix-it:"{{.*}}callingconv-cast.c":{16:6-16:6}:"WINAPI "
>> -// GNUFIXIT: fix-it:"{{.*}}callingconv-cast.c":{16:6-16:6}:"WINAPI "
>> -// GNUFIXIT:
>> fix-it:"{{.*}}callingconv-cast.c":{7:6-7:6}:"__attribute__((stdcall)) "
>> +// MSFIXIT: fix-it:"{{.*}}callingconv-cast.c":{19:6-19:6}:"WINAPI "
>> +// MSFIXIT: fix-it:"{{.*}}callingconv-cast.c":{19:6-19:6}:"WINAPI "
>> +// MSFIXIT: fix-it:"{{.*}}callingconv-cast.c":{19:6-19:6}:"WINAPI "
>> +// MSFIXIT: fix-it:"{{.*}}callingconv-cast.c":{10:6-10:6}:"__stdcall "
>> +
>> +// GNUFIXIT: fix-it:"{{.*}}callingconv-cast.c":{19:6-19:6}:"WINAPI "
>> +// GNUFIXIT: fix-it:"{{.*}}callingconv-cast.c":{19:6-19:6}:"WINAPI "
>> +// GNUFIXIT: fix-it:"{{.*}}callingconv-cast.c":{19:6-19:6}:"WINAPI "
>> +// GNUFIXIT:
>> fix-it:"{{.*}}callingconv-cast.c":{10:6-10:6}:"__attribute__((stdcall)) "
>>
>>
>> ___
>> 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
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: r269148 - Disable -Wcast-calling-convention by default (follow-up to r269116)

2016-05-10 Thread David Majnemer via cfe-commits
Er, why? Why not just disable this warning in chromium?
Clangs diagnostics wouldn't have developed as well as they have if we took
this approach to all warnings.

On Tuesday, May 10, 2016, Hans Wennborg via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: hans
> Date: Tue May 10 19:49:20 2016
> New Revision: 269148
>
> URL: http://llvm.org/viewvc/llvm-project?rev=269148=rev
> Log:
> Disable -Wcast-calling-convention by default (follow-up to r269116)
>
> Modified:
> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> cfe/trunk/test/Sema/callingconv-cast.c
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=269148=269147=269148=diff
>
> ==
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue May 10
> 19:49:20 2016
> @@ -6625,7 +6625,7 @@ def warn_function_def_in_objc_container
>  def warn_cast_calling_conv : Warning<
>"cast between incompatible calling conventions '%0' and '%1'; "
>"calls through this pointer may abort at runtime">,
> -  InGroup>;
> +  InGroup>, DefaultIgnore;
>  def note_change_calling_conv_fixit : Note<
>"consider defining %0 with the '%1' calling convention">;
>  def warn_bad_function_cast : Warning<
>
> Modified: cfe/trunk/test/Sema/callingconv-cast.c
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/callingconv-cast.c?rev=269148=269147=269148=diff
>
> ==
> --- cfe/trunk/test/Sema/callingconv-cast.c (original)
> +++ cfe/trunk/test/Sema/callingconv-cast.c Tue May 10 19:49:20 2016
> @@ -3,6 +3,9 @@
>  // RUN: %clang_cc1 -fms-extensions -triple i686-pc-windows-msvc
> -Wcast-calling-convention -DMSVC -fdiagnostics-parseable-fixits %s 2>&1 |
> FileCheck %s --check-prefix=MSFIXIT
>  // RUN: %clang_cc1 -triple i686-pc-windows-gnu -Wcast-calling-convention
> -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
> --check-prefix=GNUFIXIT
>
> +// Check that the warning is disabled by default:
> +// RUN: %clang_cc1 -fms-extensions -triple i686-pc-windows-msvc -DMSVC
> -Werror -Wno-pointer-bool-conversion -x c %s
> +
>  // expected-note@+1 {{consider defining 'mismatched_before_winapi' with
> the 'stdcall' calling convention}}
>  void mismatched_before_winapi(int x) {}
>
> @@ -43,12 +46,12 @@ int main() {
>take_callback((callback_t)(void*)mismatched);
>  }
>
> -// MSFIXIT: fix-it:"{{.*}}callingconv-cast.c":{16:6-16:6}:"WINAPI "
> -// MSFIXIT: fix-it:"{{.*}}callingconv-cast.c":{16:6-16:6}:"WINAPI "
> -// MSFIXIT: fix-it:"{{.*}}callingconv-cast.c":{16:6-16:6}:"WINAPI "
> -// MSFIXIT: fix-it:"{{.*}}callingconv-cast.c":{7:6-7:6}:"__stdcall "
> -
> -// GNUFIXIT: fix-it:"{{.*}}callingconv-cast.c":{16:6-16:6}:"WINAPI "
> -// GNUFIXIT: fix-it:"{{.*}}callingconv-cast.c":{16:6-16:6}:"WINAPI "
> -// GNUFIXIT: fix-it:"{{.*}}callingconv-cast.c":{16:6-16:6}:"WINAPI "
> -// GNUFIXIT:
> fix-it:"{{.*}}callingconv-cast.c":{7:6-7:6}:"__attribute__((stdcall)) "
> +// MSFIXIT: fix-it:"{{.*}}callingconv-cast.c":{19:6-19:6}:"WINAPI "
> +// MSFIXIT: fix-it:"{{.*}}callingconv-cast.c":{19:6-19:6}:"WINAPI "
> +// MSFIXIT: fix-it:"{{.*}}callingconv-cast.c":{19:6-19:6}:"WINAPI "
> +// MSFIXIT: fix-it:"{{.*}}callingconv-cast.c":{10:6-10:6}:"__stdcall "
> +
> +// GNUFIXIT: fix-it:"{{.*}}callingconv-cast.c":{19:6-19:6}:"WINAPI "
> +// GNUFIXIT: fix-it:"{{.*}}callingconv-cast.c":{19:6-19:6}:"WINAPI "
> +// GNUFIXIT: fix-it:"{{.*}}callingconv-cast.c":{19:6-19:6}:"WINAPI "
> +// GNUFIXIT:
> fix-it:"{{.*}}callingconv-cast.c":{10:6-10:6}:"__attribute__((stdcall)) "
>
>
> ___
> 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: r269116 - Add -Wcast-calling-convention to warn when casting away calling conventions

2016-05-10 Thread Hans Wennborg via cfe-commits
On Tue, May 10, 2016 at 2:00 PM, Reid Kleckner via cfe-commits
 wrote:
> Author: rnk
> Date: Tue May 10 16:00:03 2016
> New Revision: 269116
>
> URL: http://llvm.org/viewvc/llvm-project?rev=269116=rev
> Log:
> Add -Wcast-calling-convention to warn when casting away calling conventions
>
> Summary:
> This only warns on casts of the address of a function defined in the
> current TU. In this case, the fix is likely to be local and the warning
> useful.
>
> Here are some things we could experiment with in the future:
> - Fire on declarations as well as definitions
> - Limit the warning to non-void function prototypes
> - Limit the warning to mismatches of caller and callee cleanup CCs
>
> This warning is currently off by default while we study its usefulness.

Turns out this wasn't true :-) For example, it's firing here:
https://build.chromium.org/p/chromium.fyi/builders/ClangToTWin%28dbg%29/builds/5569/steps/compile/logs/stdio

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


r269148 - Disable -Wcast-calling-convention by default (follow-up to r269116)

2016-05-10 Thread Hans Wennborg via cfe-commits
Author: hans
Date: Tue May 10 19:49:20 2016
New Revision: 269148

URL: http://llvm.org/viewvc/llvm-project?rev=269148=rev
Log:
Disable -Wcast-calling-convention by default (follow-up to r269116)

Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/test/Sema/callingconv-cast.c

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=269148=269147=269148=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue May 10 19:49:20 
2016
@@ -6625,7 +6625,7 @@ def warn_function_def_in_objc_container
 def warn_cast_calling_conv : Warning<
   "cast between incompatible calling conventions '%0' and '%1'; "
   "calls through this pointer may abort at runtime">,
-  InGroup>;
+  InGroup>, DefaultIgnore;
 def note_change_calling_conv_fixit : Note<
   "consider defining %0 with the '%1' calling convention">;
 def warn_bad_function_cast : Warning<

Modified: cfe/trunk/test/Sema/callingconv-cast.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/callingconv-cast.c?rev=269148=269147=269148=diff
==
--- cfe/trunk/test/Sema/callingconv-cast.c (original)
+++ cfe/trunk/test/Sema/callingconv-cast.c Tue May 10 19:49:20 2016
@@ -3,6 +3,9 @@
 // RUN: %clang_cc1 -fms-extensions -triple i686-pc-windows-msvc 
-Wcast-calling-convention -DMSVC -fdiagnostics-parseable-fixits %s 2>&1 | 
FileCheck %s --check-prefix=MSFIXIT
 // RUN: %clang_cc1 -triple i686-pc-windows-gnu -Wcast-calling-convention 
-fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s --check-prefix=GNUFIXIT
 
+// Check that the warning is disabled by default:
+// RUN: %clang_cc1 -fms-extensions -triple i686-pc-windows-msvc -DMSVC -Werror 
-Wno-pointer-bool-conversion -x c %s
+
 // expected-note@+1 {{consider defining 'mismatched_before_winapi' with the 
'stdcall' calling convention}}
 void mismatched_before_winapi(int x) {}
 
@@ -43,12 +46,12 @@ int main() {
   take_callback((callback_t)(void*)mismatched);
 }
 
-// MSFIXIT: fix-it:"{{.*}}callingconv-cast.c":{16:6-16:6}:"WINAPI "
-// MSFIXIT: fix-it:"{{.*}}callingconv-cast.c":{16:6-16:6}:"WINAPI "
-// MSFIXIT: fix-it:"{{.*}}callingconv-cast.c":{16:6-16:6}:"WINAPI "
-// MSFIXIT: fix-it:"{{.*}}callingconv-cast.c":{7:6-7:6}:"__stdcall "
-
-// GNUFIXIT: fix-it:"{{.*}}callingconv-cast.c":{16:6-16:6}:"WINAPI "
-// GNUFIXIT: fix-it:"{{.*}}callingconv-cast.c":{16:6-16:6}:"WINAPI "
-// GNUFIXIT: fix-it:"{{.*}}callingconv-cast.c":{16:6-16:6}:"WINAPI "
-// GNUFIXIT: 
fix-it:"{{.*}}callingconv-cast.c":{7:6-7:6}:"__attribute__((stdcall)) "
+// MSFIXIT: fix-it:"{{.*}}callingconv-cast.c":{19:6-19:6}:"WINAPI "
+// MSFIXIT: fix-it:"{{.*}}callingconv-cast.c":{19:6-19:6}:"WINAPI "
+// MSFIXIT: fix-it:"{{.*}}callingconv-cast.c":{19:6-19:6}:"WINAPI "
+// MSFIXIT: fix-it:"{{.*}}callingconv-cast.c":{10:6-10:6}:"__stdcall "
+
+// GNUFIXIT: fix-it:"{{.*}}callingconv-cast.c":{19:6-19:6}:"WINAPI "
+// GNUFIXIT: fix-it:"{{.*}}callingconv-cast.c":{19:6-19:6}:"WINAPI "
+// GNUFIXIT: fix-it:"{{.*}}callingconv-cast.c":{19:6-19:6}:"WINAPI "
+// GNUFIXIT: 
fix-it:"{{.*}}callingconv-cast.c":{10:6-10:6}:"__attribute__((stdcall)) "


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


Re: [PATCH] D20034: [CUDA] Only __shared__ variables can be static local on device side.

2016-05-10 Thread Justin Lebar via cfe-commits
jlebar added a comment.

This patch regresses Eigen, because it raises an error even on host+device 
functions.


Repository:
  rL LLVM

http://reviews.llvm.org/D20034



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


Re: [PATCH] D19708: [CGDebugInfo] Generate debug info for member calls in the context of the callee expression

2016-05-10 Thread Hal Finkel via cfe-commits
Ping.

Thanks again,
Hal

- Original Message -
> From: "Hal Finkel via cfe-commits" 
> To: "David Blaikie" 
> Cc: "Jun Bum Lim" , "Richard Smith" 
> , "cfe-commits"
> , 
> reviews+d19708+public+e9ddc42503732...@reviews.llvm.org
> Sent: Monday, May 2, 2016 3:17:22 PM
> Subject: Re: [PATCH] D19708: [CGDebugInfo] Generate debug info for member 
> calls in the context of the callee
> expression
> 
> - Original Message -
> > From: "David Blaikie" 
> > To: reviews+d19708+public+e9ddc42503732...@reviews.llvm.org, "Hal
> > Finkel" 
> > Cc: "Richard Smith" , "Adrian Prantl"
> > , "Duncan P. N. Exon Smith"
> > , "Eric Christopher" ,
> > "Jun Bum Lim" ,
> > "cfe-commits" 
> > Sent: Friday, April 29, 2016 4:52:26 PM
> > Subject: Re: [PATCH] D19708: [CGDebugInfo] Generate debug info for
> > member calls in the context of the callee
> > expression
> > 
> > 
> > You could simplify the test case further, down to just:
> > 
> > struct foo { void bar(); };
> > void f(foo *f) {
> > f->bar();
> > }
> > 
> > and check that the call instruction has the desired column (or if
> > you
> > want a test that doesn't depend on column info (you can force it on
> > with a flag, but we might vary whether it's on by default based on
> > target, etc, I'm not sure) you could put 'bar();' on a separate
> > line
> > from 'f->' and check the call was on the second line and not the
> > first).
> 
> Certainly. I'm not sure much we want to reduce the test case,
> however, because I particularly want to cover the case of two calls
> on the same line with column info. Given that this is still pretty
> simple, I think that covering it directly seems reasonable.
> 
> > 
> > Richard might be able to tell us whether there's a preferred place
> > for a test for a change like this - should it be a debug info test,
> > a diagnostic test,
> 
> At least for the test you suggested in a previous e-mail, this patch
> has no effect on the output diagnostics (although it certainly does
> have the desired effect on the debug info). Specifically, even with
> this patch, we still have:
> 
>   $ cat /tmp/loc.cpp
>   struct foo {
> const foo *x() const;
> void y();
>   };
>   
>   void f(const foo *g) {
> g->x()->y();
> g->x()->x()->y();
>   }
> 
> $ clang /tmp/loc.cpp -fsyntax-only
>   /tmp/loc.cpp:7:3: error: member function 'y' not viable: 'this'
>   argument has type 'const foo', but function is not marked const
> g->x()->y();
> ^~
>   /tmp/loc.cpp:3:8: note: 'y' declared here
> void y();
>  ^
>   /tmp/loc.cpp:8:3: error: member function 'y' not viable: 'this'
>   argument has type 'const foo', but function is not marked const
> g->x()->x()->y();
> ^~~
>   /tmp/loc.cpp:3:8: note: 'y' declared here
> void y();
>  ^
>   2 errors generated.
> 
> Thanks again,
> Hal
> 
> > or perhaps just an ast dump test?
> > 
> > Perhaps a test for the case where there is no valid callee would be
> > good? Where does that come up - call through a member function
> > pointer?
> > 
> > 
> > On Fri, Apr 29, 2016 at 9:19 AM, Hal Finkel via cfe-commits <
> > cfe-commits@lists.llvm.org > wrote:
> > 
> > 
> > hfinkel updated this revision to Diff 55610.
> > hfinkel added a comment.
> > 
> > Use David's suggested approach: Modify the preferred expression
> > location for member calls. If the callee has a valid location (not
> > all do), then use that. Otherwise, fall back to the starting
> > location. This seems to cleanly fix the debug-info problem.
> > 
> > 
> > http://reviews.llvm.org/D19708
> > 
> > Files:
> > include/clang/AST/ExprCXX.h
> > 
> > 
> > test/CodeGenCXX/debug-info-member-call.cpp
> > 
> > Index: test/CodeGenCXX/debug-info-member-call.cpp
> > ===
> > --- /dev/null
> > +++ test/CodeGenCXX/debug-info-member-call.cpp
> > @@ -0,0 +1,24 @@
> > +// RUN: %clang_cc1 -triple x86_64-unknown_unknown -emit-llvm
> > -debug-info-kind=standalone -dwarf-column-info %s -o - | FileCheck
> > %s
> > +void ext();
> > +
> > +struct Bar {
> > + void bar() { ext(); }
> > +};
> > +
> > +struct Foo {
> > + Bar *b;
> > +
> > + Bar *foo() { return b; }
> > +};
> > +
> > +void test(Foo *f) {
> > + f->foo()->bar();
> > +}
> > +
> > +// CHECK-LABEL: @_Z4testP3Foo
> > +// CHECK: call {{.*}} @_ZN3Foo3fooEv{{.*}}, !dbg ![[CALL1LOC:.*]]
> > +// CHECK: call void @_ZN3Bar3barEv{{.*}}, !dbg ![[CALL2LOC:.*]]
> > +
> > +// CHECK: ![[CALL1LOC]] = !DILocation(line: [[LINE:[0-9]+]],
> > column:
> > 6,
> > +// CHECK: ![[CALL2LOC]] = !DILocation(line: [[LINE]], column: 13,
> > +
> > Index: include/clang/AST/ExprCXX.h
> > 

Re: [PATCH] D20141: Check for nullptr argument.

2016-05-10 Thread Justin Lebar via cfe-commits
jlebar accepted this revision.
jlebar added a comment.
This revision is now accepted and ready to land.

OK, if the function explicitly says it accepts null values and if we check 
elsewhere in the function, I'm personally OK adding the checks.


http://reviews.llvm.org/D20141



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


Re: [PATCH] D20141: Check for nullptr argument.

2016-05-10 Thread David Blaikie via cfe-commits
If you have a check that doesn't have a test/is never triggered - simple
thing to do is just make it an assert & run some testing of your own
(selfhost, etc) then if there's insufficient evidence that it's needed,
ship it and wait until it fails on a buildbot, etc. Then reduce the test
case and check that in.

- Dave

On Tue, May 10, 2016 at 4:25 PM, Artem Belevich via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> tra added a comment.
>
> I've never seen it triggered. Fix is based on the comment above the
> function that D==nullptr is acceptable and the fact that we are checking D
> in other places in this function.
>
> Two cases where nullptr D is passed explicitly has something to do with
> -fblocks, but that does not seem to be used neither with CUDA nor with
> 'IsForDefinition' which was brought in to deal with a strange case of
> redefining C++ symbols in C. This is probably why it's not triggered.
>
>
> http://reviews.llvm.org/D20141
>
>
>
> ___
> 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: [PATCH] D20136: Get default -fms-compatibility-version from cl.exe's version

2016-05-10 Thread Paul Robinson via cfe-commits
probinson added a subscriber: probinson.
probinson added a comment.

In http://reviews.llvm.org/D20136#426586, @amccarth wrote:

> now using wide-chars for WinAPI calls,


Dōmo arigatō gozaimashita!


http://reviews.llvm.org/D20136



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


Re: [PATCH] D20136: Get default -fms-compatibility-version from cl.exe's version

2016-05-10 Thread Adrian McCarthy via cfe-commits
amccarth marked 4 inline comments as done.


Comment at: lib/Driver/MSVCToolChain.cpp:478
@@ +477,3 @@
+
+  const DWORD VersionSize = ::GetFileVersionInfoSizeW(ClExeWide.c_str(),
+  nullptr);

Yes, it looks in the executable (which I tried to emphasize with the method 
name).

I don't think this is very expensive given that Explorer often makes zillions 
of such calls, but I'm open to other suggestions.

I know that you can't use a library that's newer than the compiler (because it 
may use new language features), but I don't know if that applies in the other 
direction or how we would safely and reliably map directory names to library 
versions and therefore to compiler versions.


http://reviews.llvm.org/D20136



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


Re: [PATCH] D20136: Get default -fms-compatibility-version from cl.exe's version

2016-05-10 Thread Adrian McCarthy via cfe-commits
amccarth updated this revision to Diff 56833.
amccarth marked an inline comment as done.
amccarth added a comment.

Addressed most comments: now using wide-chars for WinAPI calls, made 
getMSVCVersionFromExe a virtual method, removed extraneous typo-correction.


http://reviews.llvm.org/D20136

Files:
  include/clang/Driver/ToolChain.h
  lib/Driver/MSVCToolChain.cpp
  lib/Driver/ToolChains.h
  lib/Driver/Tools.cpp
  lib/Driver/Tools.h
  test/Driver/cl-options.c
  test/Driver/msc-version.c
  test/Driver/msvc-triple.c
  test/Misc/diag-format.c

Index: test/Misc/diag-format.c
===
--- test/Misc/diag-format.c
+++ test/Misc/diag-format.c
@@ -37,7 +37,7 @@
 // DEFAULT: {{.*}}:36:8: warning: extra tokens at end of #endif directive [-Wextra-tokens]
 // MSVC2010: {{.*}}(36,7) : warning: extra tokens at end of #endif directive [-Wextra-tokens]
 // MSVC2013: {{.*}}(36,8) : warning: extra tokens at end of #endif directive [-Wextra-tokens]
-// MSVC: {{.*}}(36,8) : warning: extra tokens at end of #endif directive [-Wextra-tokens]
+// MSVC: {{.*}}(36,8){{ ?}}: warning: extra tokens at end of #endif directive [-Wextra-tokens]
 // MSVC2015: {{.*}}(36,8): warning: extra tokens at end of #endif directive [-Wextra-tokens]
 // VI: {{.*}} +36:8: warning: extra tokens at end of #endif directive [-Wextra-tokens]
 // MSVC2015_ORIG: {{.*}}(36): warning: extra tokens at end of #endif directive [-Wextra-tokens]
Index: test/Driver/msvc-triple.c
===
--- test/Driver/msvc-triple.c
+++ test/Driver/msvc-triple.c
@@ -1,9 +1,7 @@
-// RUN: %clang -target i686-pc-windows-msvc   -S -emit-llvm %s -o - | FileCheck %s --check-prefix=DEFAULT
 // RUN: %clang -target i686-pc-windows-msvc19 -S -emit-llvm %s -o - | FileCheck %s --check-prefix=TARGET-19
 // RUN: %clang -target i686-pc-windows-msvc   -S -emit-llvm %s -o - -fms-compatibility-version=19 | FileCheck %s --check-prefix=OVERRIDE-19
 // RUN: %clang -target i686-pc-windows-msvc-elf -S -emit-llvm %s -o - | FileCheck %s --check-prefix=ELF-DEFAULT
 
-// DEFAULT: target triple = "i686-pc-windows-msvc18.0.0"
 // TARGET-19:   target triple = "i686-pc-windows-msvc19.0.0"
 // OVERRIDE-19: target triple = "i686-pc-windows-msvc19.0.0"
-// ELF-DEFAULT: target triple = "i686-pc-windows-msvc18.0.0-elf"
+// ELF-DEFAULT: target triple = "i686-pc-windows-msvc{{.*}}-elf"
Index: test/Driver/msc-version.c
===
--- test/Driver/msc-version.c
+++ test/Driver/msc-version.c
@@ -1,15 +1,4 @@
 //
-// Verify defaults
-//
-
-// RUN: %clang -target i686-windows -fms-compatibility -dM -E - &1 | FileCheck -check-prefix=NoThreadSafeStatics %s
+// RUN: %clang_cl /c -### -fms-compatibility-version=18 -- %s 2>&1 | FileCheck -check-prefix=NoThreadSafeStatics %s
 // RUN: %clang_cl /Zc:threadSafeInit /Zc:threadSafeInit- /c -### -- %s 2>&1 | FileCheck -check-prefix=NoThreadSafeStatics %s
 // NoThreadSafeStatics: "-fno-threadsafe-statics"
 
Index: lib/Driver/Tools.h
===
--- lib/Driver/Tools.h
+++ lib/Driver/Tools.h
@@ -682,7 +682,8 @@
 
 /// Visual studio tools.
 namespace visualstudio {
-VersionTuple getMSVCVersion(const Driver *D, const llvm::Triple ,
+VersionTuple getMSVCVersion(const Driver *D, const ToolChain ,
+const llvm::Triple ,
 const llvm::opt::ArgList , bool IsWindowsMSVC);
 
 class LLVM_LIBRARY_VISIBILITY Linker : public Tool {
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -3291,7 +3291,7 @@
   Result.append(UID.begin(), UID.end());
 }
 
-VersionTuple visualstudio::getMSVCVersion(const Driver *D,
+VersionTuple visualstudio::getMSVCVersion(const Driver *D, const ToolChain ,
   const llvm::Triple ,
   const llvm::opt::ArgList ,
   bool IsWindowsMSVC) {
@@ -,8 +,14 @@
 if (Major || Minor || Micro)
   return VersionTuple(Major, Minor, Micro);
 
-// FIXME: Consider bumping this to 19 (MSVC2015) soon.
-return VersionTuple(18);
+if (IsWindowsMSVC) {
+  VersionTuple MSVT = TC.getMSVCVersionFromExe();
+  if (!MSVT.empty())
+return MSVT;
+
+  // FIXME: Consider bumping this to 19 (MSVC2015) soon.
+  return VersionTuple(18);
+}
   }
   return VersionTuple();
 }
@@ -5224,7 +5230,7 @@
 
   // -fms-compatibility-version=18.00 is default.
   VersionTuple MSVT = visualstudio::getMSVCVersion(
-  , getToolChain().getTriple(), Args, IsWindowsMSVC);
+  , getToolChain(), getToolChain().getTriple(), Args, IsWindowsMSVC);
   if (!MSVT.empty())
 CmdArgs.push_back(
 Args.MakeArgString("-fms-compatibility-version=" + MSVT.getAsString()));
Index: 

Re: [PATCH] D20141: Check for nullptr argument.

2016-05-10 Thread Artem Belevich via cfe-commits
tra added a comment.

I've never seen it triggered. Fix is based on the comment above the function 
that D==nullptr is acceptable and the fact that we are checking D in other 
places in this function.

Two cases where nullptr D is passed explicitly has something to do with 
-fblocks, but that does not seem to be used neither with CUDA nor with 
'IsForDefinition' which was brought in to deal with a strange case of 
redefining C++ symbols in C. This is probably why it's not triggered.


http://reviews.llvm.org/D20141



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


Re: [PATCH] D20141: Check for nullptr argument.

2016-05-10 Thread Justin Lebar via cfe-commits
jlebar added a comment.

Can we have a test?


http://reviews.llvm.org/D20141



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


[PATCH] D20141: Check for nullptr argument.

2016-05-10 Thread Artem Belevich via cfe-commits
tra created this revision.
tra added reviewers: jlebar, jordan_rose.
tra added a subscriber: cfe-commits.

GetOrCreateLLVMGlobal() accepts nullptr D, but in some cases we end up 
dereferencing it without checking if it's non-null.

Fixes PR15492.

http://reviews.llvm.org/D20141

Files:
  lib/CodeGen/CodeGenModule.cpp

Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -2080,7 +2080,7 @@
 
   // Check that D is not yet in DiagnosedConflictingDefinitions is required
   // to make sure that we issue an error only once.
-  if (lookupRepresentativeDecl(MangledName, OtherGD) &&
+  if (D && lookupRepresentativeDecl(MangledName, OtherGD) &&
   (D->getCanonicalDecl() != OtherGD.getCanonicalDecl().getDecl()) &&
   (OtherD = dyn_cast(OtherGD.getDecl())) &&
   OtherD->hasInit() &&
@@ -2299,7 +2299,7 @@
 
 unsigned CodeGenModule::GetGlobalVarAddressSpace(const VarDecl *D,
  unsigned AddrSpace) {
-  if (LangOpts.CUDA && LangOpts.CUDAIsDevice) {
+  if (D && LangOpts.CUDA && LangOpts.CUDAIsDevice) {
 if (D->hasAttr())
   AddrSpace = getContext().getTargetAddressSpace(LangAS::cuda_constant);
 else if (D->hasAttr())


Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -2080,7 +2080,7 @@
 
   // Check that D is not yet in DiagnosedConflictingDefinitions is required
   // to make sure that we issue an error only once.
-  if (lookupRepresentativeDecl(MangledName, OtherGD) &&
+  if (D && lookupRepresentativeDecl(MangledName, OtherGD) &&
   (D->getCanonicalDecl() != OtherGD.getCanonicalDecl().getDecl()) &&
   (OtherD = dyn_cast(OtherGD.getDecl())) &&
   OtherD->hasInit() &&
@@ -2299,7 +2299,7 @@
 
 unsigned CodeGenModule::GetGlobalVarAddressSpace(const VarDecl *D,
  unsigned AddrSpace) {
-  if (LangOpts.CUDA && LangOpts.CUDAIsDevice) {
+  if (D && LangOpts.CUDA && LangOpts.CUDAIsDevice) {
 if (D->hasAttr())
   AddrSpace = getContext().getTargetAddressSpace(LangAS::cuda_constant);
 else if (D->hasAttr())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D20136: Get default -fms-compatibility-version from cl.exe's version

2016-05-10 Thread Reid Kleckner via cfe-commits
rnk added inline comments.


Comment at: lib/Driver/MSVCToolChain.cpp:41
@@ -40,1 +40,3 @@
+
+  #pragma comment(lib, "version.lib")
 #endif

Personally, I think this is OK but I know Aaron Ballman and other people don't 
like using pragma comment lib. The alternative is hacking CMake goo, which is 
always best avoided when possible.


Comment at: lib/Driver/MSVCToolChain.cpp:472
@@ +471,3 @@
+
+  const DWORD VersionSize = ::GetFileVersionInfoSizeA(ClExe.c_str(), nullptr);
+  if (VersionSize == 0) {

majnemer wrote:
> amccarth wrote:
> > majnemer wrote:
> > > Why not use the `GetFileVersionInfoSizeW` variant?
> > I started down that road, but it seemed overkill to convert the path to a 
> > wide string.  I'm happy to do it if you think it worthwhile.
> I think it's worth it, we get bug reports whenever we break this sort of 
> thing...
+1, you can use ConvertUTF8toWide to make this easy.


Comment at: lib/Driver/MSVCToolChain.cpp:477
@@ +476,3 @@
+  std::vector VersionBlock(VersionSize);
+  if (!::GetFileVersionInfoA(ClExe.c_str(), 0, VersionSize,
+ VersionBlock.data())) {

majnemer wrote:
> amccarth wrote:
> > thakis wrote:
> > > We already stat a bunch of directories to find the sdk include path. Can 
> > > we use the result of that instead of looking at cl.exe? Then we wouldn't 
> > > have to do additional stats.
> > I'm just a simple CPM/VMS/Windows developer.  Your Linux terms (stat) 
> > frighten and confuse me.
> > 
> > Seriously, though, this API isn't a file system check.  It's digging out 
> > the version record from the file's resources.
> > 
> > We _could_ guess at the version from the names of the directories in the 
> > path, but that would require mapping names to versions, and if it's 
> > installed in a non-standard place it wouldn't help at all.
> > 
> > Also, `-fms-compatibility-version` is really about the version of the 
> > compiler (cl.exe), not that of the standard library nor of the SDK, so 
> > trying to check something else as a proxy for the version seems prone to 
> > obscure failures.
> > 
> > I share your concern about speed, especially since getting the version 
> > happens twice (once for the triple and once for the compatibility version), 
> > but invoking clang and having it choose the wrong default costs a lot of 
> > time, too.
> > 
> > The bug report correctly says we shouldn't spin up a process to run `cl 
> > /version`--that would be far more expensive.  And if you put 
> > `-fms-compatibility-version` on the command line, then this function won't 
> > be called as it won't need to figure out the default.
> > Seriously, though, this API isn't a file system check. It's digging out the 
> > version record from the file's resources.
> 
> Isn't the content stored as a resource in the PE?  If so, that means that 
> getting the version information requires reading bytes inside of cl.exe
> 
> With regard to `-fms-compatibility-version`, it shouldn't have anything to do 
> with the platform SDK.  However, it is fundamentally the case that the CRT 
> and the compiler have the same version.  Otherwise, really terrible things 
> happen (MSVC19 assumes char16_t is a keyword which it provides but the MSVC18 
> STL assumes it is responsible for providing the keyword).
I think one extra file read is probably worth the convenience it buys for our 
users. It's easy to win back by having the user pass an explicit 
-fms-compatibility-version flag.


Comment at: lib/Driver/Tools.cpp:3337-3338
@@ +3336,4 @@
+if (IsWindowsMSVC) {
+  const auto  = static_cast(TC);
+  VersionTuple MSVT = MSVC.getMSVCVersionFromExe();
+  if (!MSVT.empty())

IMO you should make this a virtual method on Toolchain that does nothing and is 
only overridden in MSVCToolChain. You can also cache it if you do that.


Comment at: tools/driver/driver.cpp:504
@@ -503,3 +503,3 @@
   // Exit status should not be negative on Win32, unless abnormal termination.
-  // Once abnormal termiation was caught, negative status should not be
+  // Once abnormal termination was caught, negative status should not be
   // propagated.

Yeah, it's a typo, but you don't have any other changes in this file, so I 
wouldn't touch it as part of this change.


http://reviews.llvm.org/D20136



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


[clang-tools-extra] r269135 - Disable IncludeFixerTests/IncludeFixer.NestedName for now. It doesn't pass for targeting win32. Investigating.

2016-05-10 Thread NAKAMURA Takumi via cfe-commits
Author: chapuni
Date: Tue May 10 17:46:32 2016
New Revision: 269135

URL: http://llvm.org/viewvc/llvm-project?rev=269135=rev
Log:
Disable IncludeFixerTests/IncludeFixer.NestedName for now. It doesn't pass for 
targeting win32. Investigating.

Modified:
clang-tools-extra/trunk/unittests/include-fixer/IncludeFixerTest.cpp

Modified: clang-tools-extra/trunk/unittests/include-fixer/IncludeFixerTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/include-fixer/IncludeFixerTest.cpp?rev=269135=269134=269135=diff
==
--- clang-tools-extra/trunk/unittests/include-fixer/IncludeFixerTest.cpp 
(original)
+++ clang-tools-extra/trunk/unittests/include-fixer/IncludeFixerTest.cpp Tue 
May 10 17:46:32 2016
@@ -115,11 +115,14 @@ TEST(IncludeFixer, MinimizeInclude) {
 runIncludeFixer("a::b::foo bar;\n", IncludePath));
 }
 
+#if 0
+// It doesn't pass for targeting win32. Investigating.
 TEST(IncludeFixer, NestedName) {
   EXPECT_EQ("#include \"dir/otherdir/qux.h\"\n"
 "namespace a {}\nint a = a::b::foo(0);\n",
 runIncludeFixer("namespace a {}\nint a = a::b::foo(0);\n"));
 }
+#endif
 
 TEST(IncludeFixer, MultipleMissingSymbols) {
   EXPECT_EQ("#include \nstd::string bar;\nstd::sting foo;\n",


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


Re: [PATCH] D20139: [CUDA] Split device-var-init.cu tests into separate Sema and CodeGen parts.

2016-05-10 Thread Artem Belevich via cfe-commits
tra added inline comments.


Comment at: test/SemaCUDA/device-var-init.cu:7-11
@@ -6,9 +6,7 @@
 // RUN: %clang_cc1 -triple nvptx64-nvidia-cuda -fcuda-is-device -std=c++11 \
-// RUN: -fno-threadsafe-statics -emit-llvm -o - %s | FileCheck %s
-// RUN: %clang_cc1 -triple nvptx64-nvidia-cuda -fcuda-is-device -std=c++11 \
-// RUN: -emit-llvm -DERROR_CASE -verify -o /dev/null %s
+// RUN: -I %S/.. -fsyntax-only -verify -o /dev/null %s
 
-#ifdef __clang__
-#include "Inputs/cuda.h"
-#endif
+// Counterpart in CodeGen covers valid cases that pass Sema
+// checks. Here we'll only cover cases that trigger errors.
+#include "CodeGenCUDA/device-var-init.cu"
 

I wonder if it's OK for me to #include a file from another test directory?
Please let me know if you have concerns or suggestions.



http://reviews.llvm.org/D20139



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


Re: [PATCH] D20140: [CUDA] Do not allow non-empty destructors for global device-side variables.

2016-05-10 Thread Justin Lebar via cfe-commits
jlebar accepted this revision.
jlebar added a comment.
This revision is now accepted and ready to land.

lgtm, but I'd like Richard to sign off on this too.



Comment at: lib/Sema/SemaDecl.cpp:10438
@@ -10437,1 +10437,3 @@
 
+  // Also make sure that destructor, ifthere is one, is empty.
+  if (AllowedInit)

if there


http://reviews.llvm.org/D20140



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


[PATCH] D20140: [CUDA] Do not allow non-empty destructors for global device-side variables.

2016-05-10 Thread Artem Belevich via cfe-commits
tra created this revision.
tra added reviewers: jlebar, rsmith, jingyue.
tra added a subscriber: cfe-commits.

According to Cuda Programming guide (v7.5, E2.3.1):
> __device__, __constant__ and __shared__ variables defined in namespace
> scope, that are of class type, cannot have a non-empty constructor or a
> non-empty destructor.
> 
> 
> A destructor for a class is considered empty at a point in the translation 
> unit, if it is either a trivial destructor or it satisfies all of the 
> following conditions:
> 
> * The destructor function has been defined.
> * The destructor function body is an empty compound statement.
> * Its class has no virtual functions and no virtual base classes.
> * The destructors of all base classes of its class can be considered empty.
>  * For all the nonstatic data members of its class that are of class type (or 
> array thereof), the destructor can be considered empty.
> 

Clang already deals with device-side constructors (see D15305).
This patch enforces similar rules for destructors.

http://reviews.llvm.org/D20140

Files:
  include/clang/Sema/Sema.h
  lib/Sema/SemaCUDA.cpp
  lib/Sema/SemaDecl.cpp
  test/CodeGenCUDA/device-var-init.cu
  test/SemaCUDA/device-var-init.cu

Index: test/SemaCUDA/device-var-init.cu
===
--- test/SemaCUDA/device-var-init.cu
+++ test/SemaCUDA/device-var-init.cu
@@ -58,6 +58,13 @@
 __constant__ UC c_uc;
 // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
 
+__device__ UD d_ud;
+// expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
+__shared__ UD s_ud;
+// expected-error@-1 {{initialization is not supported for __shared__ variables.}}
+__constant__ UD c_ud;
+// expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
+
 __device__ ECI d_eci;
 // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
 __shared__ ECI s_eci;
@@ -72,13 +79,27 @@
 __constant__ NEC c_nec;
 // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
 
+__device__ NED d_ned;
+// expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
+__shared__ NED s_ned;
+// expected-error@-1 {{initialization is not supported for __shared__ variables.}}
+__constant__ NED c_ned;
+// expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
+
 __device__ NCV d_ncv;
 // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
 __shared__ NCV s_ncv;
 // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
 __constant__ NCV c_ncv;
 // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
 
+__device__ VD d_vd;
+// expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
+__shared__ VD s_vd;
+// expected-error@-1 {{initialization is not supported for __shared__ variables.}}
+__constant__ VD c_vd;
+// expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
+
 __device__ NCF d_ncf;
 // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
 __shared__ NCF s_ncf;
@@ -152,13 +173,37 @@
 __constant__ T_FA_NEC c_t_fa_nec;
 // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
 
-// Make sure that initialization restrictions do not apply to local
-// variables.
+__device__ T_B_NED d_t_b_ned;
+// expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
+__shared__ T_B_NED s_t_b_ned;
+// expected-error@-1 {{initialization is not supported for __shared__ variables.}}
+__constant__ T_B_NED c_t_b_ned;
+// expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
+
+__device__ T_F_NED d_t_f_ned;
+// expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
+__shared__ T_F_NED s_t_f_ned;
+// expected-error@-1 {{initialization is not supported for __shared__ variables.}}
+__constant__ T_F_NED c_t_f_ned;
+// expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
+
+__device__ T_FA_NED d_t_fa_ned;
+// expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
+__shared__ T_FA_NED s_t_fa_ned;
+// expected-error@-1 

r269133 - [VFS] One more unittest change to fix win10 buildbot

2016-05-10 Thread Bruno Cardoso Lopes via cfe-commits
Author: bruno
Date: Tue May 10 17:30:01 2016
New Revision: 269133

URL: http://llvm.org/viewvc/llvm-project?rev=269133=rev
Log:
[VFS] One more unittest change to fix win10 buildbot

http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/5110
Follow up from r269100.

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

Modified: cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp?rev=269133=269132=269133=diff
==
--- cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp (original)
+++ cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp Tue May 10 17:30:01 2016
@@ -1085,6 +1085,6 @@ TEST_F(VFSFromYAMLTest, DirectoryIterati
   checkContents(O->dir_begin("//root/foo/bar", EC),
 {"//root/foo/bar/a", "//root/foo/bar/b"});
 
-  checkContents(O->dir_begin("//root/baz", EC),
+  checkContents(O->dir_begin("//root/baz/", EC),
 {"//root/baz/x", "//root/baz/y", "//root/baz/c"});
 }


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


Re: [PATCH] D20139: [CUDA] Split device-var-init.cu tests into separate Sema and CodeGen parts.

2016-05-10 Thread Justin Lebar via cfe-commits
jlebar accepted this revision.
jlebar added a comment.
This revision is now accepted and ready to land.

Rubber stamp.


http://reviews.llvm.org/D20139



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


[PATCH] D20139: [CUDA] Split device-var-init.cu tests into separate Sema and CodeGen parts.

2016-05-10 Thread Artem Belevich via cfe-commits
tra created this revision.
tra added a reviewer: jlebar.
tra added a subscriber: cfe-commits.

Codegen tests for device-side variable initialization are subset of test 
cases used to verify Sema's part of the job. 
Including CodeGenCUDA/device-var-init.cu from SemaCUDA makes it easier to
keep both sides in sync.


http://reviews.llvm.org/D20139

Files:
  test/CodeGenCUDA/device-var-init.cu
  test/SemaCUDA/device-var-init.cu

Index: test/SemaCUDA/device-var-init.cu
===
--- test/SemaCUDA/device-var-init.cu
+++ test/SemaCUDA/device-var-init.cu
@@ -4,131 +4,25 @@
 // variables, but accept empty constructors allowed by CUDA.
 
 // RUN: %clang_cc1 -triple nvptx64-nvidia-cuda -fcuda-is-device -std=c++11 \
-// RUN: -fno-threadsafe-statics -emit-llvm -o - %s | FileCheck %s
-// RUN: %clang_cc1 -triple nvptx64-nvidia-cuda -fcuda-is-device -std=c++11 \
-// RUN: -emit-llvm -DERROR_CASE -verify -o /dev/null %s
+// RUN: -I %S/.. -fsyntax-only -verify -o /dev/null %s
 
-#ifdef __clang__
-#include "Inputs/cuda.h"
-#endif
+// Counterpart in CodeGen covers valid cases that pass Sema
+// checks. Here we'll only cover cases that trigger errors.
+#include "CodeGenCUDA/device-var-init.cu"
 
-// Base classes with different initializer variants.
-
-// trivial constructor -- allowed
-struct T {
-  int t;
-};
-
-// empty constructor
-struct EC {
-  int ec;
-  __device__ EC() {} // -- allowed
-  __device__ EC(int) {}  // -- not allowed
-};
-
-// empty templated constructor -- allowed with no arguments
-struct ETC {
-  template  __device__ ETC(T...) {}
-};
-
-// undefined constructor -- not allowed
-struct UC {
-  int uc;
-  __device__ UC();
-};
-
-// empty constructor w/ initializer list -- not allowed
-struct ECI {
-  int eci;
-  __device__ ECI() : eci(1) {}
-};
-
-// non-empty constructor -- not allowed
-struct NEC {
-  int nec;
-  __device__ NEC() { nec = 1; }
-};
-
-// no-constructor,  virtual method -- not allowed
-struct NCV {
-  int ncv;
-  __device__ virtual void vm() {}
-};
-
-// dynamic in-class field initializer -- not allowed
-__device__ int f();
-struct NCF {
-  int ncf = f();
-};
-
-// static in-class field initializer.  NVCC does not allow it, but
-// clang generates static initializer for this, so we'll accept it.
-// We still can't use it on __shared__ vars as they don't allow *any*
-// initializers.
-struct NCFS {
-  int ncfs = 3;
-};
-
-// undefined templated constructor -- not allowed
-struct UTC {
-  template  __device__ UTC(T...);
-};
-
-// non-empty templated constructor -- not allowed
-struct NETC {
-  int netc;
-  template  __device__ NETC(T...) { netc = 1; }
-};
-
-__device__ int d_v;
-// CHECK: @d_v = addrspace(1) externally_initialized global i32 0,
-__shared__ int s_v;
-// CHECK: @s_v = addrspace(3) global i32 undef,
-__constant__ int c_v;
-// CHECK: addrspace(4) externally_initialized global i32 0,
-
-__device__ int d_v_i = 1;
-// CHECK: @d_v_i = addrspace(1) externally_initialized global i32 1,
-#ifdef ERROR_CASE
 __shared__ int s_v_i = 1;
 // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
-#endif
-__constant__ int c_v_i = 1;
-// CHECK: @c_v_i = addrspace(4) externally_initialized global i32 1,
 
-#ifdef ERROR_CASE
 __device__ int d_v_f = f();
 // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
 __shared__ int s_v_f = f();
 // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
 __constant__ int c_v_f = f();
 // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
-#endif
 
-__device__ T d_t;
-// CHECK: @d_t = addrspace(1) externally_initialized global %struct.T zeroinitializer
-__shared__ T s_t;
-// CHECK: @s_t = addrspace(3) global %struct.T undef,
-__constant__ T c_t;
-// CHECK: @c_t = addrspace(4) externally_initialized global %struct.T zeroinitializer,
-
-__device__ T d_t_i = {2};
-// CHECKL @d_t_i = addrspace(1) externally_initialized global %struct.T { i32 2 },
-#ifdef ERROR_CASE
 __shared__ T s_t_i = {2};
 // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
-#endif
-__constant__ T c_t_i = {2};
-// CHECK: @c_t_i = addrspace(4) externally_initialized global %struct.T { i32 2 },
 
-__device__ EC d_ec;
-// CHECK: @d_ec = addrspace(1) externally_initialized global %struct.EC zeroinitializer,
-__shared__ EC s_ec;
-// CHECK: @s_ec = addrspace(3) global %struct.EC undef,
-__constant__ EC c_ec;
-// CHECK: @c_ec = addrspace(4) externally_initialized global %struct.EC zeroinitializer,
-
-#if ERROR_CASE
 __device__ EC d_ec_i(3);
 // expected-error@-1 {{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables.}}
 __shared__ EC s_ec_i(3);
@@ -142,16 +36,7 @@
 // expected-error@-1 {{initialization is not supported for __shared__ variables.}}
 __constant__ EC c_ec_i2 = 

Re: [PATCH] D19941: [tooling] FixItHint Tooling refactoring

2016-05-10 Thread Etienne Bergeron via cfe-commits
etienneb updated this revision to Diff 56826.
etienneb added a comment.

nits


http://reviews.llvm.org/D19941

Files:
  include/clang/Tooling/FixIt.h
  lib/Tooling/CMakeLists.txt
  lib/Tooling/FixIt.cpp
  unittests/Tooling/CMakeLists.txt
  unittests/Tooling/FixItTest.cpp

Index: unittests/Tooling/FixItTest.cpp
===
--- /dev/null
+++ unittests/Tooling/FixItTest.cpp
@@ -0,0 +1,238 @@
+//===- unittest/Tooling/FixitTest.cpp ===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "TestVisitor.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Tooling/FixIt.h"
+
+using namespace clang;
+
+namespace {
+
+struct CallsVisitor : TestVisitor {
+  bool VisitCallExpr(CallExpr *Expr) {
+OnCall(Expr, Context);
+return true;
+  }
+
+  std::function OnCall;
+};
+
+std::string LocationToString(SourceLocation Loc, ASTContext *Context) {
+  return Loc.printToString(Context->getSourceManager());
+}
+
+TEST(FixItTest, getText) {
+  CallsVisitor Visitor;
+
+  Visitor.OnCall = [](CallExpr *CE, ASTContext *Context) {
+EXPECT_EQ("foo(x, y)", tooling::fixit::getText(*CE, *Context));
+EXPECT_EQ("foo(x, y)",
+  tooling::fixit::getText(CE->getSourceRange(), *Context));
+
+Expr *P0 = CE->getArg(0);
+Expr *P1 = CE->getArg(1);
+EXPECT_EQ("x", tooling::fixit::getText(*P0, *Context));
+EXPECT_EQ("y", tooling::fixit::getText(*P1, *Context));
+  };
+  Visitor.runOver("void foo(int x, int y) { foo(x, y); }");
+
+  Visitor.OnCall = [](CallExpr *CE, ASTContext *Context) {
+EXPECT_EQ("APPLY(foo, x, y)", tooling::fixit::getText(*CE, *Context));
+  };
+  Visitor.runOver("#define APPLY(f, x, y) f(x, y)\n"
+  "void foo(int x, int y) { APPLY(foo, x, y); }");
+}
+
+TEST(FixItTest, getTextWithMacro) {
+  CallsVisitor Visitor;
+
+  Visitor.OnCall = [](CallExpr *CE, ASTContext *Context) {
+EXPECT_EQ("F OO", tooling::fixit::getText(*CE, *Context));
+Expr *P0 = CE->getArg(0);
+Expr *P1 = CE->getArg(1);
+EXPECT_EQ("", tooling::fixit::getText(*P0, *Context));
+EXPECT_EQ("", tooling::fixit::getText(*P1, *Context));
+  };
+  Visitor.runOver("#define F foo(\n"
+  "#define OO x, y)\n"
+  "void foo(int x, int y) { F OO ; }");
+
+  Visitor.OnCall = [](CallExpr *CE, ASTContext *Context) {
+EXPECT_EQ("", tooling::fixit::getText(*CE, *Context));
+Expr *P0 = CE->getArg(0);
+Expr *P1 = CE->getArg(1);
+EXPECT_EQ("x", tooling::fixit::getText(*P0, *Context));
+EXPECT_EQ("y", tooling::fixit::getText(*P1, *Context));
+  };
+  Visitor.runOver("#define FOO(x, y) (void)x; (void)y; foo(x, y);\n"
+  "void foo(int x, int y) { FOO(x,y) }");
+}
+
+TEST(FixItTest, createRemoval) {
+  CallsVisitor Visitor;
+
+  Visitor.OnCall = [](CallExpr *CE, ASTContext *Context) {
+FixItHint Hint = tooling::fixit::createRemoval(*CE);
+EXPECT_EQ("foo(x, y)",
+  tooling::fixit::getText(Hint.RemoveRange.getAsRange(), *Context));
+EXPECT_TRUE(Hint.InsertFromRange.isInvalid());
+EXPECT_TRUE(Hint.CodeToInsert.empty());
+
+Expr *P0 = CE->getArg(0);
+FixItHint Hint0 = tooling::fixit::createRemoval(*P0);
+EXPECT_EQ(
+"x", tooling::fixit::getText(Hint0.RemoveRange.getAsRange(), *Context));
+EXPECT_TRUE(Hint0.InsertFromRange.isInvalid());
+EXPECT_TRUE(Hint0.CodeToInsert.empty());
+
+Expr *P1 = CE->getArg(1);
+FixItHint Hint1 = tooling::fixit::createRemoval(*P1);
+EXPECT_EQ(
+"y", tooling::fixit::getText(Hint1.RemoveRange.getAsRange(), *Context));
+EXPECT_TRUE(Hint1.InsertFromRange.isInvalid());
+EXPECT_TRUE(Hint1.CodeToInsert.empty());
+  };
+  Visitor.runOver("void foo(int x, int y) { foo(x, y); }");
+
+  Visitor.OnCall = [](CallExpr *CE, ASTContext *Context) {
+Expr *P0 = CE->getArg(0);
+FixItHint Hint0 = tooling::fixit::createRemoval(*P0);
+EXPECT_EQ("x + y", tooling::fixit::getText(Hint0.RemoveRange.getAsRange(),
+   *Context));
+
+Expr *P1 = CE->getArg(1);
+FixItHint Hint1 = tooling::fixit::createRemoval(*P1);
+EXPECT_EQ("y + x", tooling::fixit::getText(Hint1.RemoveRange.getAsRange(),
+   *Context));
+  };
+  Visitor.runOver("void foo(int x, int y) { foo(x + y, y + x); }");
+}
+
+TEST(FixItTest, createRemovalWithMacro) {
+  CallsVisitor Visitor;
+
+  Visitor.OnCall = [](CallExpr *CE, ASTContext *Context) {
+FixItHint Hint = tooling::fixit::createRemoval(*CE);
+EXPECT_EQ("FOO",
+  tooling::fixit::getText(Hint.RemoveRange.getAsRange(), *Context));
+

Re: [PATCH] D19941: [tooling] FixItHint Tooling refactoring

2016-05-10 Thread Etienne Bergeron via cfe-commits
etienneb updated this revision to Diff 56825.
etienneb marked 5 inline comments as done.
etienneb added a comment.

address alexfh comments


http://reviews.llvm.org/D19941

Files:
  include/clang/Tooling/FixIt.h
  lib/Tooling/CMakeLists.txt
  lib/Tooling/FixIt.cpp
  unittests/Tooling/CMakeLists.txt
  unittests/Tooling/FixItTest.cpp

Index: unittests/Tooling/FixItTest.cpp
===
--- /dev/null
+++ unittests/Tooling/FixItTest.cpp
@@ -0,0 +1,238 @@
+//===- unittest/Tooling/FixitTest.cpp ===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "TestVisitor.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Tooling/FixIt.h"
+
+using namespace clang;
+
+namespace {
+
+struct CallsVisitor : TestVisitor {
+  bool VisitCallExpr(CallExpr *Expr) {
+OnCall(Expr, Context);
+return true;
+  }
+
+  std::function OnCall;
+};
+
+std::string LocationToString(SourceLocation Loc, ASTContext *Context) {
+  return Loc.printToString(Context->getSourceManager());
+}
+
+TEST(FixitTest, getText) {
+  CallsVisitor Visitor;
+
+  Visitor.OnCall = [&](CallExpr *CE, ASTContext *Context) {
+EXPECT_EQ("foo(x, y)", tooling::fixit::getText(*CE, *Context));
+EXPECT_EQ("foo(x, y)",
+  tooling::fixit::getText(CE->getSourceRange(), *Context));
+
+Expr *P0 = CE->getArg(0);
+Expr *P1 = CE->getArg(1);
+EXPECT_EQ("x", tooling::fixit::getText(*P0, *Context));
+EXPECT_EQ("y", tooling::fixit::getText(*P1, *Context));
+  };
+  Visitor.runOver("void foo(int x, int y) { foo(x, y); }");
+
+  Visitor.OnCall = [&](CallExpr *CE, ASTContext *Context) {
+EXPECT_EQ("APPLY(foo, x, y)", tooling::fixit::getText(*CE, *Context));
+  };
+  Visitor.runOver("#define APPLY(f, x, y) f(x, y)\n"
+  "void foo(int x, int y) { APPLY(foo, x, y); }");
+}
+
+TEST(FixitTest, getTextWithMacro) {
+  CallsVisitor Visitor;
+
+  Visitor.OnCall = [](CallExpr *CE, ASTContext *Context) {
+EXPECT_EQ("F OO", tooling::fixit::getText(*CE, *Context));
+Expr *P0 = CE->getArg(0);
+Expr *P1 = CE->getArg(1);
+EXPECT_EQ("", tooling::fixit::getText(*P0, *Context));
+EXPECT_EQ("", tooling::fixit::getText(*P1, *Context));
+  };
+  Visitor.runOver("#define F foo(\n"
+  "#define OO x, y)\n"
+  "void foo(int x, int y) { F OO ; }");
+
+  Visitor.OnCall = [](CallExpr *CE, ASTContext *Context) {
+EXPECT_EQ("", tooling::fixit::getText(*CE, *Context));
+Expr *P0 = CE->getArg(0);
+Expr *P1 = CE->getArg(1);
+EXPECT_EQ("x", tooling::fixit::getText(*P0, *Context));
+EXPECT_EQ("y", tooling::fixit::getText(*P1, *Context));
+  };
+  Visitor.runOver("#define FOO(x, y) (void)x; (void)y; foo(x, y);\n"
+  "void foo(int x, int y) { FOO(x,y) }");
+}
+
+TEST(FixitTest, createRemoval) {
+  CallsVisitor Visitor;
+
+  Visitor.OnCall = [](CallExpr *CE, ASTContext *Context) {
+FixItHint Hint = tooling::fixit::createRemoval(*CE);
+EXPECT_EQ("foo(x, y)",
+  tooling::fixit::getText(Hint.RemoveRange.getAsRange(), *Context));
+EXPECT_TRUE(Hint.InsertFromRange.isInvalid());
+EXPECT_TRUE(Hint.CodeToInsert.empty());
+
+Expr *P0 = CE->getArg(0);
+FixItHint Hint0 = tooling::fixit::createRemoval(*P0);
+EXPECT_EQ(
+"x", tooling::fixit::getText(Hint0.RemoveRange.getAsRange(), *Context));
+EXPECT_TRUE(Hint0.InsertFromRange.isInvalid());
+EXPECT_TRUE(Hint0.CodeToInsert.empty());
+
+Expr *P1 = CE->getArg(1);
+FixItHint Hint1 = tooling::fixit::createRemoval(*P1);
+EXPECT_EQ(
+"y", tooling::fixit::getText(Hint1.RemoveRange.getAsRange(), *Context));
+EXPECT_TRUE(Hint1.InsertFromRange.isInvalid());
+EXPECT_TRUE(Hint1.CodeToInsert.empty());
+  };
+  Visitor.runOver("void foo(int x, int y) { foo(x, y); }");
+
+  Visitor.OnCall = [](CallExpr *CE, ASTContext *Context) {
+Expr *P0 = CE->getArg(0);
+FixItHint Hint0 = tooling::fixit::createRemoval(*P0);
+EXPECT_EQ("x + y", tooling::fixit::getText(Hint0.RemoveRange.getAsRange(),
+   *Context));
+
+Expr *P1 = CE->getArg(1);
+FixItHint Hint1 = tooling::fixit::createRemoval(*P1);
+EXPECT_EQ("y + x", tooling::fixit::getText(Hint1.RemoveRange.getAsRange(),
+   *Context));
+  };
+  Visitor.runOver("void foo(int x, int y) { foo(x + y, y + x); }");
+}
+
+TEST(FixitTest, createRemovalWithMacro) {
+  CallsVisitor Visitor;
+
+  Visitor.OnCall = [](CallExpr *CE, ASTContext *Context) {
+FixItHint Hint = tooling::fixit::createRemoval(*CE);
+EXPECT_EQ("FOO",
+  

Re: [PATCH] D20136: Get default -fms-compatibility-version from cl.exe's version

2016-05-10 Thread David Majnemer via cfe-commits
majnemer added inline comments.


Comment at: lib/Driver/MSVCToolChain.cpp:472
@@ +471,3 @@
+
+  const DWORD VersionSize = ::GetFileVersionInfoSizeA(ClExe.c_str(), nullptr);
+  if (VersionSize == 0) {

amccarth wrote:
> majnemer wrote:
> > Why not use the `GetFileVersionInfoSizeW` variant?
> I started down that road, but it seemed overkill to convert the path to a 
> wide string.  I'm happy to do it if you think it worthwhile.
I think it's worth it, we get bug reports whenever we break this sort of 
thing...


Comment at: lib/Driver/MSVCToolChain.cpp:477
@@ +476,3 @@
+  std::vector VersionBlock(VersionSize);
+  if (!::GetFileVersionInfoA(ClExe.c_str(), 0, VersionSize,
+ VersionBlock.data())) {

amccarth wrote:
> thakis wrote:
> > We already stat a bunch of directories to find the sdk include path. Can we 
> > use the result of that instead of looking at cl.exe? Then we wouldn't have 
> > to do additional stats.
> I'm just a simple CPM/VMS/Windows developer.  Your Linux terms (stat) 
> frighten and confuse me.
> 
> Seriously, though, this API isn't a file system check.  It's digging out the 
> version record from the file's resources.
> 
> We _could_ guess at the version from the names of the directories in the 
> path, but that would require mapping names to versions, and if it's installed 
> in a non-standard place it wouldn't help at all.
> 
> Also, `-fms-compatibility-version` is really about the version of the 
> compiler (cl.exe), not that of the standard library nor of the SDK, so trying 
> to check something else as a proxy for the version seems prone to obscure 
> failures.
> 
> I share your concern about speed, especially since getting the version 
> happens twice (once for the triple and once for the compatibility version), 
> but invoking clang and having it choose the wrong default costs a lot of 
> time, too.
> 
> The bug report correctly says we shouldn't spin up a process to run `cl 
> /version`--that would be far more expensive.  And if you put 
> `-fms-compatibility-version` on the command line, then this function won't be 
> called as it won't need to figure out the default.
> Seriously, though, this API isn't a file system check. It's digging out the 
> version record from the file's resources.

Isn't the content stored as a resource in the PE?  If so, that means that 
getting the version information requires reading bytes inside of cl.exe

With regard to `-fms-compatibility-version`, it shouldn't have anything to do 
with the platform SDK.  However, it is fundamentally the case that the CRT and 
the compiler have the same version.  Otherwise, really terrible things happen 
(MSVC19 assumes char16_t is a keyword which it provides but the MSVC18 STL 
assumes it is responsible for providing the keyword).


http://reviews.llvm.org/D20136



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


Re: [PATCH] D20136: Get default -fms-compatibility-version from cl.exe's version

2016-05-10 Thread Aaron Ballman via cfe-commits
On Tue, May 10, 2016 at 6:09 PM, Adrian McCarthy via cfe-commits
 wrote:
> amccarth added inline comments.
>
> 
> Comment at: lib/Driver/MSVCToolChain.cpp:472
> @@ +471,3 @@
> +
> +  const DWORD VersionSize = ::GetFileVersionInfoSizeA(ClExe.c_str(), 
> nullptr);
> +  if (VersionSize == 0) {
> 
> majnemer wrote:
>> Why not use the `GetFileVersionInfoSizeW` variant?
> I started down that road, but it seemed overkill to convert the path to a 
> wide string.  I'm happy to do it if you think it worthwhile.

Please use the W version instead of the A version. Not everyone
installs to the default path, and non-ASCII characters happen.

~Aaron

>
>
> http://reviews.llvm.org/D20136
>
>
>
> ___
> 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: [PATCH] D20136: Get default -fms-compatibility-version from cl.exe's version

2016-05-10 Thread Adrian McCarthy via cfe-commits
amccarth added inline comments.


Comment at: lib/Driver/MSVCToolChain.cpp:472
@@ +471,3 @@
+
+  const DWORD VersionSize = ::GetFileVersionInfoSizeA(ClExe.c_str(), nullptr);
+  if (VersionSize == 0) {

majnemer wrote:
> Why not use the `GetFileVersionInfoSizeW` variant?
I started down that road, but it seemed overkill to convert the path to a wide 
string.  I'm happy to do it if you think it worthwhile.


http://reviews.llvm.org/D20136



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


Re: [PATCH] D20136: Get default -fms-compatibility-version from cl.exe's version

2016-05-10 Thread Adrian McCarthy via cfe-commits
amccarth added inline comments.


Comment at: lib/Driver/MSVCToolChain.cpp:477
@@ +476,3 @@
+  std::vector VersionBlock(VersionSize);
+  if (!::GetFileVersionInfoA(ClExe.c_str(), 0, VersionSize,
+ VersionBlock.data())) {

thakis wrote:
> We already stat a bunch of directories to find the sdk include path. Can we 
> use the result of that instead of looking at cl.exe? Then we wouldn't have to 
> do additional stats.
I'm just a simple CPM/VMS/Windows developer.  Your Linux terms (stat) frighten 
and confuse me.

Seriously, though, this API isn't a file system check.  It's digging out the 
version record from the file's resources.

We _could_ guess at the version from the names of the directories in the path, 
but that would require mapping names to versions, and if it's installed in a 
non-standard place it wouldn't help at all.

Also, `-fms-compatibility-version` is really about the version of the compiler 
(cl.exe), not that of the standard library nor of the SDK, so trying to check 
something else as a proxy for the version seems prone to obscure failures.

I share your concern about speed, especially since getting the version happens 
twice (once for the triple and once for the compatibility version), but 
invoking clang and having it choose the wrong default costs a lot of time, too.

The bug report correctly says we shouldn't spin up a process to run `cl 
/version`--that would be far more expensive.  And if you put 
`-fms-compatibility-version` on the command line, then this function won't be 
called as it won't need to figure out the default.


http://reviews.llvm.org/D20136



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


Re: [PATCH] D20136: Get default -fms-compatibility-version from cl.exe's version

2016-05-10 Thread David Majnemer via cfe-commits
majnemer added a subscriber: majnemer.


Comment at: lib/Driver/MSVCToolChain.cpp:472
@@ +471,3 @@
+
+  const DWORD VersionSize = ::GetFileVersionInfoSizeA(ClExe.c_str(), nullptr);
+  if (VersionSize == 0) {

Why not use the `GetFileVersionInfoSizeW` variant?


http://reviews.llvm.org/D20136



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


Re: [PATCH] D20136: Get default -fms-compatibility-version from cl.exe's version

2016-05-10 Thread Nico Weber via cfe-commits
thakis added a subscriber: thakis.


Comment at: lib/Driver/MSVCToolChain.cpp:477
@@ +476,3 @@
+  std::vector VersionBlock(VersionSize);
+  if (!::GetFileVersionInfoA(ClExe.c_str(), 0, VersionSize,
+ VersionBlock.data())) {

We already stat a bunch of directories to find the sdk include path. Can we use 
the result of that instead of looking at cl.exe? Then we wouldn't have to do 
additional stats.


http://reviews.llvm.org/D20136



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


[PATCH] D20137: [PCH] Fixed bugs with preamble invalidation when files change (on Windows)

2016-05-10 Thread Cameron via cfe-commits
cameron314 created this revision.
cameron314 added a reviewer: rsmith.
cameron314 added a subscriber: cfe-commits.

There were two bugs relating to remapped files, that are not specific to 
Windows but happen more often there:
- When remapped files were changed, they would never cause the preamble's PCH 
to be invalidated, because the remapped path didn't necessarily match the 
include path (e.g. slash direction). I fixed this by moving to a 
`llvm::sys::fs::UniqueID`-based map instead of using strings for paths and 
hoping they match.
- Fixing the above bug revealed a new bug where remapped files would always 
cause the preamble's PCH to be invalidated (even if they hadn't changed) 
because the file manager was replacing the remapped virtual entry (no modified 
time) with a real file entry (which has a modified time) when the include 
directive was processed, again because it had a slightly different path due to 
slash direction.

http://reviews.llvm.org/D20137

Files:
  include/clang/Basic/FileManager.h
  include/clang/Frontend/ASTUnit.h
  lib/Basic/FileManager.cpp
  lib/Frontend/ASTUnit.cpp

Index: lib/Frontend/ASTUnit.cpp
===
--- lib/Frontend/ASTUnit.cpp
+++ lib/Frontend/ASTUnit.cpp
@@ -1378,7 +1378,7 @@
   
   // First, make a record of those files that have been overridden via
   // remapping or unsaved_files.
-  llvm::StringMap OverriddenFiles;
+  std::map OverriddenFiles;
   for (const auto  : PreprocessorOpts.RemappedFiles) {
 if (AnyFileChanged)
   break;
@@ -1391,40 +1391,47 @@
   break;
 }
 
-OverriddenFiles[R.first] = PreambleFileHash::createForFile(
+OverriddenFiles[Status.getUniqueID()] = PreambleFileHash::createForFile(
 Status.getSize(), Status.getLastModificationTime().toEpochTime());
   }
 
   for (const auto  : PreprocessorOpts.RemappedFileBuffers) {
 if (AnyFileChanged)
   break;
-OverriddenFiles[RB.first] =
+
+vfs::Status Status;
+if (FileMgr->getNoncachedStatValue(RB.first, Status)) {
+  AnyFileChanged = true;
+  break;
+}
+
+OverriddenFiles[Status.getUniqueID()] =
 PreambleFileHash::createForMemoryBuffer(RB.second);
   }

   // Check whether anything has changed.
-  for (llvm::StringMap::iterator 
+  for (FilesInPreambleMap::iterator
  F = FilesInPreamble.begin(), FEnd = FilesInPreamble.end();
!AnyFileChanged && F != FEnd; 
++F) {
-llvm::StringMap::iterator Overridden
-  = OverriddenFiles.find(F->first());
+std::map::iterator Overridden
+  = OverriddenFiles.find(F->first);
 if (Overridden != OverriddenFiles.end()) {
   // This file was remapped; check whether the newly-mapped file 
   // matches up with the previous mapping.
-  if (Overridden->second != F->second)
+  if (Overridden->second != F->second.second)
 AnyFileChanged = true;
   continue;
 }
 
 // The file was not remapped; check whether it has changed on disk.
 vfs::Status Status;
-if (FileMgr->getNoncachedStatValue(F->first(), Status)) {
+if (FileMgr->getNoncachedStatValue(F->second.first, Status)) {
   // If we can't stat the file, assume that something horrible happened.
   AnyFileChanged = true;
-} else if (Status.getSize() != uint64_t(F->second.Size) ||
+} else if (Status.getSize() != uint64_t(F->second.second.Size) ||
Status.getLastModificationTime().toEpochTime() !=
-   uint64_t(F->second.ModTime))
+   uint64_t(F->second.second.ModTime))
   AnyFileChanged = true;
   }
   
@@ -1612,12 +1619,14 @@
 if (!File || File == SourceMgr.getFileEntryForID(SourceMgr.getMainFileID()))
   continue;
 if (time_t ModTime = File->getModificationTime()) {
-  FilesInPreamble[File->getName()] = PreambleFileHash::createForFile(
-  File->getSize(), ModTime);
+  FilesInPreamble[File->getUniqueID()] = std::make_pair(
+StringRef(File->getName()),
+PreambleFileHash::createForFile(File->getSize(), ModTime));
 } else {
   llvm::MemoryBuffer *Buffer = SourceMgr.getMemoryBufferForFile(File);
-  FilesInPreamble[File->getName()] =
-  PreambleFileHash::createForMemoryBuffer(Buffer);
+  FilesInPreamble[File->getUniqueID()] = std::make_pair(
+StringRef(File->getName()),
+PreambleFileHash::createForMemoryBuffer(Buffer));
 }
   }
 
Index: lib/Basic/FileManager.cpp
===
--- lib/Basic/FileManager.cpp
+++ lib/Basic/FileManager.cpp
@@ -301,6 +301,11 @@
 return 
   }
 
+  if 

r269118 - Wildcard away paths in fixit CHECKs in my last testcase

2016-05-10 Thread Reid Kleckner via cfe-commits
Author: rnk
Date: Tue May 10 16:10:25 2016
New Revision: 269118

URL: http://llvm.org/viewvc/llvm-project?rev=269118=rev
Log:
Wildcard away paths in fixit CHECKs in my last testcase

Oops. :(

Modified:
cfe/trunk/test/Sema/callingconv-cast.c

Modified: cfe/trunk/test/Sema/callingconv-cast.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/callingconv-cast.c?rev=269118=269117=269118=diff
==
--- cfe/trunk/test/Sema/callingconv-cast.c (original)
+++ cfe/trunk/test/Sema/callingconv-cast.c Tue May 10 16:10:25 2016
@@ -43,12 +43,12 @@ int main() {
   take_callback((callback_t)(void*)mismatched);
 }
 
-// MSFIXIT: 
fix-it:"D:\\src\\llvm\\tools\\clang\\test\\Sema\\callingconv-cast.c":{16:6-16:6}:"WINAPI
 "
-// MSFIXIT: 
fix-it:"D:\\src\\llvm\\tools\\clang\\test\\Sema\\callingconv-cast.c":{16:6-16:6}:"WINAPI
 "
-// MSFIXIT: 
fix-it:"D:\\src\\llvm\\tools\\clang\\test\\Sema\\callingconv-cast.c":{16:6-16:6}:"WINAPI
 "
-// MSFIXIT: 
fix-it:"D:\\src\\llvm\\tools\\clang\\test\\Sema\\callingconv-cast.c":{7:6-7:6}:"__stdcall
 "
+// MSFIXIT: fix-it:"{{.*}}callingconv-cast.c":{16:6-16:6}:"WINAPI "
+// MSFIXIT: fix-it:"{{.*}}callingconv-cast.c":{16:6-16:6}:"WINAPI "
+// MSFIXIT: fix-it:"{{.*}}callingconv-cast.c":{16:6-16:6}:"WINAPI "
+// MSFIXIT: fix-it:"{{.*}}callingconv-cast.c":{7:6-7:6}:"__stdcall "
 
-// GNUFIXIT: 
fix-it:"D:\\src\\llvm\\tools\\clang\\test\\Sema\\callingconv-cast.c":{16:6-16:6}:"WINAPI
 "
-// GNUFIXIT: 
fix-it:"D:\\src\\llvm\\tools\\clang\\test\\Sema\\callingconv-cast.c":{16:6-16:6}:"WINAPI
 "
-// GNUFIXIT: 
fix-it:"D:\\src\\llvm\\tools\\clang\\test\\Sema\\callingconv-cast.c":{16:6-16:6}:"WINAPI
 "
-// GNUFIXIT: 
fix-it:"D:\\src\\llvm\\tools\\clang\\test\\Sema\\callingconv-cast.c":{7:6-7:6}:"__attribute__((stdcall))
 "
+// GNUFIXIT: fix-it:"{{.*}}callingconv-cast.c":{16:6-16:6}:"WINAPI "
+// GNUFIXIT: fix-it:"{{.*}}callingconv-cast.c":{16:6-16:6}:"WINAPI "
+// GNUFIXIT: fix-it:"{{.*}}callingconv-cast.c":{16:6-16:6}:"WINAPI "
+// GNUFIXIT: 
fix-it:"{{.*}}callingconv-cast.c":{7:6-7:6}:"__attribute__((stdcall)) "


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


Re: [PATCH] D19871: Add an AST matcher for CastExpr kind

2016-05-10 Thread Samuel Benzaquen via cfe-commits
sbenza added inline comments.


Comment at: lib/ASTMatchers/Dynamic/Registry.cpp:78
@@ -77,2 +77,3 @@
   // Need Variant/Parser fixes:
+  // hasCastKind  
   // ofKind

Is it registered or not?
You add this comment, but also add the matcher in the registry below.


http://reviews.llvm.org/D19871



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


[PATCH] D20134: [libclang] Fixed bug where ranges in spelling locations (in macro expansions) would get mangled

2016-05-10 Thread Cameron via cfe-commits
cameron314 created this revision.
cameron314 added a reviewer: rsmith.
cameron314 added a subscriber: cfe-commits.

The end location of the range would be changed into its expansion location, but 
the beginning of the range would stay as a spelling location. This would lead 
to very weird ranges spanning large parts of a file (starting in a macro until 
its expansion).

This bug was not previously caught because there was no way to actually obtain 
a spelling location via libclang (see my patch at D20125 which adds support for 
this), so later obtaining the start location of a range would result in an 
expansion location anyway, and wind up matching the expansion location that was 
stored in the end of the range.

http://reviews.llvm.org/D20134

Files:
  tools/libclang/CIndex.cpp

Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -142,8 +142,6 @@
   // We want the last character in this location, so we will adjust the
   // location accordingly.
   SourceLocation EndLoc = R.getEnd();
-  if (EndLoc.isValid() && EndLoc.isMacroID() && 
!SM.isMacroArgExpansion(EndLoc))
-EndLoc = SM.getExpansionRange(EndLoc).second;
   if (R.isTokenRange() && EndLoc.isValid()) {
 unsigned Length = Lexer::MeasureTokenLength(SM.getSpellingLoc(EndLoc),
 SM, LangOpts);


Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -142,8 +142,6 @@
   // We want the last character in this location, so we will adjust the
   // location accordingly.
   SourceLocation EndLoc = R.getEnd();
-  if (EndLoc.isValid() && EndLoc.isMacroID() && !SM.isMacroArgExpansion(EndLoc))
-EndLoc = SM.getExpansionRange(EndLoc).second;
   if (R.isTokenRange() && EndLoc.isValid()) {
 unsigned Length = Lexer::MeasureTokenLength(SM.getSpellingLoc(EndLoc),
 SM, LangOpts);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D17348: Add -Wcast-calling-convention to warn when casting away calling conventions

2016-05-10 Thread Reid Kleckner via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL269116: Add -Wcast-calling-convention to warn when casting 
away calling conventions (authored by rnk).

Changed prior to commit:
  http://reviews.llvm.org/D17348?vs=56802=56814#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D17348

Files:
  cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
  cfe/trunk/lib/Sema/SemaCast.cpp
  cfe/trunk/test/Sema/callingconv-cast.c

Index: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6621,7 +6621,13 @@
 def warn_function_def_in_objc_container : Warning<
   "function definition inside an Objective-C container is deprecated">,
   InGroup;
-  
+
+def warn_cast_calling_conv : Warning<
+  "cast between incompatible calling conventions '%0' and '%1'; "
+  "calls through this pointer may abort at runtime">,
+  InGroup>;
+def note_change_calling_conv_fixit : Note<
+  "consider defining %0 with the '%1' calling convention">;
 def warn_bad_function_cast : Warning<
   "cast from function call of type %0 to non-matching type %1">,
   InGroup, DefaultIgnore;
Index: cfe/trunk/test/Sema/callingconv-cast.c
===
--- cfe/trunk/test/Sema/callingconv-cast.c
+++ cfe/trunk/test/Sema/callingconv-cast.c
@@ -0,0 +1,54 @@
+// RUN: %clang_cc1 -fms-extensions -triple i686-pc-windows-msvc -Wcast-calling-convention -DMSVC -Wno-pointer-bool-conversion -verify -x c %s
+// RUN: %clang_cc1 -fms-extensions -triple i686-pc-windows-msvc -Wcast-calling-convention -DMSVC -Wno-pointer-bool-conversion -verify -x c++ %s
+// RUN: %clang_cc1 -fms-extensions -triple i686-pc-windows-msvc -Wcast-calling-convention -DMSVC -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s --check-prefix=MSFIXIT
+// RUN: %clang_cc1 -triple i686-pc-windows-gnu -Wcast-calling-convention -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s --check-prefix=GNUFIXIT
+
+// expected-note@+1 {{consider defining 'mismatched_before_winapi' with the 'stdcall' calling convention}}
+void mismatched_before_winapi(int x) {}
+
+#ifdef MSVC
+#define WINAPI __stdcall
+#else
+#define WINAPI __attribute__((stdcall))
+#endif
+
+// expected-note@+1 3 {{consider defining 'mismatched' with the 'stdcall' calling convention}}
+void mismatched(int x) {}
+
+typedef void (WINAPI *callback_t)(int);
+void take_callback(callback_t callback);
+
+int main() {
+  // expected-warning@+1 {{cast between incompatible calling conventions 'cdecl' and 'stdcall'}}
+  take_callback((callback_t)mismatched);
+
+  // expected-warning@+1 {{cast between incompatible calling conventions 'cdecl' and 'stdcall'}}
+  callback_t callback = (callback_t)mismatched; // warns
+  (void)callback;
+
+  // expected-warning@+1 {{cast between incompatible calling conventions 'cdecl' and 'stdcall'}}
+  callback = (callback_t) // warns
+
+  // No warning, just to show we don't drill through other kinds of unary operators.
+  callback = (callback_t)!mismatched;
+
+  // expected-warning@+1 {{cast between incompatible calling conventions 'cdecl' and 'stdcall'}}
+  callback = (callback_t)_before_winapi; // warns
+
+  // Probably a bug, but we don't warn.
+  void (*callback2)(int) = mismatched;
+  take_callback((callback_t)callback2);
+
+  // Another way to suppress the warning.
+  take_callback((callback_t)(void*)mismatched);
+}
+
+// MSFIXIT: fix-it:"D:\\src\\llvm\\tools\\clang\\test\\Sema\\callingconv-cast.c":{16:6-16:6}:"WINAPI "
+// MSFIXIT: fix-it:"D:\\src\\llvm\\tools\\clang\\test\\Sema\\callingconv-cast.c":{16:6-16:6}:"WINAPI "
+// MSFIXIT: fix-it:"D:\\src\\llvm\\tools\\clang\\test\\Sema\\callingconv-cast.c":{16:6-16:6}:"WINAPI "
+// MSFIXIT: fix-it:"D:\\src\\llvm\\tools\\clang\\test\\Sema\\callingconv-cast.c":{7:6-7:6}:"__stdcall "
+
+// GNUFIXIT: fix-it:"D:\\src\\llvm\\tools\\clang\\test\\Sema\\callingconv-cast.c":{16:6-16:6}:"WINAPI "
+// GNUFIXIT: fix-it:"D:\\src\\llvm\\tools\\clang\\test\\Sema\\callingconv-cast.c":{16:6-16:6}:"WINAPI "
+// GNUFIXIT: fix-it:"D:\\src\\llvm\\tools\\clang\\test\\Sema\\callingconv-cast.c":{16:6-16:6}:"WINAPI "
+// GNUFIXIT: fix-it:"D:\\src\\llvm\\tools\\clang\\test\\Sema\\callingconv-cast.c":{7:6-7:6}:"__attribute__((stdcall)) "
Index: cfe/trunk/lib/Sema/SemaCast.cpp
===
--- cfe/trunk/lib/Sema/SemaCast.cpp
+++ cfe/trunk/lib/Sema/SemaCast.cpp
@@ -22,6 +22,7 @@
 #include "clang/AST/RecordLayout.h"
 #include "clang/Basic/PartialDiagnostic.h"
 #include "clang/Basic/TargetInfo.h"
+#include "clang/Lex/Preprocessor.h"
 #include "clang/Sema/Initialization.h"
 #include "llvm/ADT/SmallVector.h"
 #include 
@@ -1726,6 +1727,89 @@
 }
 }
 
+/// Diagnose casts that change the calling convention of a pointer to a function

r269116 - Add -Wcast-calling-convention to warn when casting away calling conventions

2016-05-10 Thread Reid Kleckner via cfe-commits
Author: rnk
Date: Tue May 10 16:00:03 2016
New Revision: 269116

URL: http://llvm.org/viewvc/llvm-project?rev=269116=rev
Log:
Add -Wcast-calling-convention to warn when casting away calling conventions

Summary:
This only warns on casts of the address of a function defined in the
current TU. In this case, the fix is likely to be local and the warning
useful.

Here are some things we could experiment with in the future:
- Fire on declarations as well as definitions
- Limit the warning to non-void function prototypes
- Limit the warning to mismatches of caller and callee cleanup CCs

This warning is currently off by default while we study its usefulness.

Reviewers: thakis, rtrieu

Subscribers: cfe-commits

Differential Revision: http://reviews.llvm.org/D17348

Added:
cfe/trunk/test/Sema/callingconv-cast.c
Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaCast.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=269116=269115=269116=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue May 10 16:00:03 
2016
@@ -6621,7 +6621,13 @@ def warn_cast_pointer_from_sel : Warning
 def warn_function_def_in_objc_container : Warning<
   "function definition inside an Objective-C container is deprecated">,
   InGroup;
-  
+
+def warn_cast_calling_conv : Warning<
+  "cast between incompatible calling conventions '%0' and '%1'; "
+  "calls through this pointer may abort at runtime">,
+  InGroup>;
+def note_change_calling_conv_fixit : Note<
+  "consider defining %0 with the '%1' calling convention">;
 def warn_bad_function_cast : Warning<
   "cast from function call of type %0 to non-matching type %1">,
   InGroup, DefaultIgnore;

Modified: cfe/trunk/lib/Sema/SemaCast.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCast.cpp?rev=269116=269115=269116=diff
==
--- cfe/trunk/lib/Sema/SemaCast.cpp (original)
+++ cfe/trunk/lib/Sema/SemaCast.cpp Tue May 10 16:00:03 2016
@@ -22,6 +22,7 @@
 #include "clang/AST/RecordLayout.h"
 #include "clang/Basic/PartialDiagnostic.h"
 #include "clang/Basic/TargetInfo.h"
+#include "clang/Lex/Preprocessor.h"
 #include "clang/Sema/Initialization.h"
 #include "llvm/ADT/SmallVector.h"
 #include 
@@ -1726,6 +1727,89 @@ static void DiagnoseCastOfObjCSEL(Sema &
 }
 }
 
+/// Diagnose casts that change the calling convention of a pointer to a 
function
+/// defined in the current TU.
+static void DiagnoseCallingConvCast(Sema , const ExprResult ,
+QualType DstType, SourceRange OpRange) {
+  // Check if this cast would change the calling convention of a function
+  // pointer type.
+  QualType SrcType = SrcExpr.get()->getType();
+  if (Self.Context.hasSameType(SrcType, DstType) ||
+  !SrcType->isFunctionPointerType() || !DstType->isFunctionPointerType())
+return;
+  const auto *SrcFTy =
+  SrcType->castAs()->getPointeeType()->castAs();
+  const auto *DstFTy =
+  DstType->castAs()->getPointeeType()->castAs();
+  CallingConv SrcCC = SrcFTy->getCallConv();
+  CallingConv DstCC = DstFTy->getCallConv();
+  if (SrcCC == DstCC)
+return;
+
+  // We have a calling convention cast. Check if the source is a pointer to a
+  // known, specific function that has already been defined.
+  Expr *Src = SrcExpr.get()->IgnoreParenImpCasts();
+  if (auto *UO = dyn_cast(Src))
+if (UO->getOpcode() == UO_AddrOf)
+  Src = UO->getSubExpr()->IgnoreParenImpCasts();
+  auto *DRE = dyn_cast(Src);
+  if (!DRE)
+return;
+  auto *FD = dyn_cast(DRE->getDecl());
+  const FunctionDecl *Definition;
+  if (!FD || !FD->hasBody(Definition))
+return;
+
+  // The source expression is a pointer to a known function defined in this TU.
+  // Diagnose this cast, as it is probably bad.
+  StringRef SrcCCName = FunctionType::getNameForCallConv(SrcCC);
+  StringRef DstCCName = FunctionType::getNameForCallConv(DstCC);
+  Self.Diag(OpRange.getBegin(), diag::warn_cast_calling_conv)
+  << SrcCCName << DstCCName << OpRange;
+
+  // The checks above are cheaper than checking if the diagnostic is enabled.
+  // However, it's worth checking if the warning is enabled before we construct
+  // a fixit.
+  if (Self.Diags.isIgnored(diag::warn_cast_calling_conv, OpRange.getBegin()))
+return;
+
+  // Try to suggest a fixit to change the calling convention of the function
+  // whose address was taken. Try to use the latest macro for the convention.
+  // For example, users probably want to write "WINAPI" instead of "__stdcall"
+  // to match the Windows header declarations.
+  SourceLocation NameLoc = Definition->getNameInfo().getLoc();
+  

[PATCH] D20133: [OpenCL] Fix __builtin_astype for vec3 types.

2016-05-10 Thread Yaxun Liu via cfe-commits
yaxunl created this revision.
yaxunl added reviewers: Anastasia, pxli168, bader.
yaxunl added subscribers: cfe-commits, tstellarAMD.

__builtin_astype does not generate correct LLVM IR for vec3 types. This patch 
inserts bitcasts to/from vec4 when necessary in addition to generating vector 
shuffle. A codegen test is added.

http://reviews.llvm.org/D20133

Files:
  lib/CodeGen/CGExprScalar.cpp
  test/CodeGenOpenCL/as_type.cl

Index: test/CodeGenOpenCL/as_type.cl
===
--- /dev/null
+++ test/CodeGenOpenCL/as_type.cl
@@ -0,0 +1,54 @@
+// RUN: %clang_cc1 %s -emit-llvm -triple spir-unknown-unknown -o - | FileCheck %s
+
+typedef __attribute__(( ext_vector_type(3) )) char char3;
+typedef __attribute__(( ext_vector_type(4) )) char char4;
+
+//CHECK: define spir_func <3 x i8> @f1(<4 x i8> %[[x:.*]])
+//CHECK: %[[astype:.*]] = shufflevector <4 x i8> %[[x]], <4 x i8> undef, <3 x i32> 
+//CHECK: ret <3 x i8> %[[astype]]
+char3 f1(char4 x) {
+  return  __builtin_astype(x, char3);
+}
+
+//CHECK: define spir_func <4 x i8> @f2(<3 x i8> %[[x:.*]])
+//CHECK: %[[astype:.*]] = shufflevector <3 x i8> %[[x]], <3 x i8> undef, <4 x i32> 
+//CHECK: ret <4 x i8> %[[astype]]
+char4 f2(char3 x) {
+  return __builtin_astype(x, char4);
+}
+
+//CHECK: define spir_func <3 x i8> @f3(i32 %[[x:.*]])
+//CHECK: %[[cast:.*]] = bitcast i32 %[[x]] to <4 x i8>
+//CHECK: %[[astype:.*]] = shufflevector <4 x i8> %[[cast]], <4 x i8> undef, <3 x i32> 
+//CHECK: ret <3 x i8> %[[astype]]
+char3 f3(int x) {
+  return __builtin_astype(x, char3);
+}
+
+//CHECK: define spir_func <4 x i8> @f4(i32 %[[x:.*]])
+//CHECK: %[[astype:.*]] = bitcast i32 %[[x]] to <4 x i8>
+//CHECK: ret <4 x i8> %[[astype]]
+char4 f4(int x) {
+  return __builtin_astype(x, char4);
+}
+
+//CHECK: define spir_func i32 @f5(<3 x i8> %[[x:.*]])
+//CHECK: %[[shuffle:.*]] = shufflevector <3 x i8> %[[x]], <3 x i8> undef, <4 x i32> 
+//CHECK: %[[astype:.*]] = bitcast <4 x i8> %[[shuffle]] to i32
+//CHECK: ret i32 %[[astype]]
+int f5(char3 x) {
+  return __builtin_astype(x, int);
+}
+
+//CHECK: define spir_func i32 @f6(<4 x i8> %[[x:.*]])
+//CHECK: %[[astype]] = bitcast <4 x i8> %[[x]] to i32
+//CHECK: ret i32 %[[astype]]
+int f6(char4 x) {
+  return __builtin_astype(x, int);
+}
+
+//CHECK: define spir_func <3 x i8> @f7(<3 x i8> %[[x:.*]])
+//CHECK: ret <3 x i8> %[[x]]
+char3 f7(char3 x) {
+  return __builtin_astype(x, char3);
+}
Index: lib/CodeGen/CGExprScalar.cpp
===
--- lib/CodeGen/CGExprScalar.cpp
+++ lib/CodeGen/CGExprScalar.cpp
@@ -3394,50 +3394,48 @@
   return CGF.EmitBlockLiteral(block);
 }
 
+// Convert a vec3 to vec4, or vice versa.
+static Value *ConvertVec3AndVec4(CGBuilderTy , CodeGenFunction ,
+  Value *Src, unsigned numElementsDst) {
+  llvm::Value *UnV = llvm::UndefValue::get(Src->getType());
+  SmallVector Args;
+  Args.push_back(Builder.getInt32(0));
+  Args.push_back(Builder.getInt32(1));
+  Args.push_back(Builder.getInt32(2));
+  if (numElementsDst == 4)
+Args.push_back(llvm::UndefValue::get(CGF.Int32Ty));
+  llvm::Constant *Mask = llvm::ConstantVector::get(Args);
+  return Builder.CreateShuffleVector(Src, UnV, Mask);
+}
+
 Value *ScalarExprEmitter::VisitAsTypeExpr(AsTypeExpr *E) {
   Value *Src  = CGF.EmitScalarExpr(E->getSrcExpr());
   llvm::Type *DstTy = ConvertType(E->getType());
 
-  // Going from vec4->vec3 or vec3->vec4 is a special case and requires
-  // a shuffle vector instead of a bitcast.
   llvm::Type *SrcTy = Src->getType();
-  if (isa(DstTy) && isa(SrcTy)) {
-unsigned numElementsDst = cast(DstTy)->getNumElements();
-unsigned numElementsSrc = cast(SrcTy)->getNumElements();
-if ((numElementsDst == 3 && numElementsSrc == 4)
-|| (numElementsDst == 4 && numElementsSrc == 3)) {
-
-
-  // In the case of going from int4->float3, a bitcast is needed before
-  // doing a shuffle.
-  llvm::Type *srcElemTy =
-  cast(SrcTy)->getElementType();
-  llvm::Type *dstElemTy =
-  cast(DstTy)->getElementType();
-
-  if ((srcElemTy->isIntegerTy() && dstElemTy->isFloatTy())
-  || (srcElemTy->isFloatTy() && dstElemTy->isIntegerTy())) {
-// Create a float type of the same size as the source or destination.
-llvm::VectorType *newSrcTy = llvm::VectorType::get(dstElemTy,
- numElementsSrc);
-
-Src = Builder.CreateBitCast(Src, newSrcTy, "astypeCast");
-  }
-
-  llvm::Value *UnV = llvm::UndefValue::get(Src->getType());
-
-  SmallVector Args;
-  Args.push_back(Builder.getInt32(0));
-  Args.push_back(Builder.getInt32(1));
-  Args.push_back(Builder.getInt32(2));
-
-  if (numElementsDst == 4)
-Args.push_back(llvm::UndefValue::get(CGF.Int32Ty));
-
-  llvm::Constant *Mask = llvm::ConstantVector::get(Args);
+  unsigned numElementsSrc = isa(SrcTy) ?

[PATCH] D20132: [libclang] Add clang_getAllSkippedRanges function

2016-05-10 Thread Cameron via cfe-commits
cameron314 created this revision.
cameron314 added a reviewer: rsmith.
cameron314 added a subscriber: cfe-commits.

This complements the `clang_getSkippedRanges` function which returns skipped 
ranges filtered by a specific file.

This function is useful when all the ranges are desired (and a lot more 
efficient than the equivalent of asking for the ranges file by file, since the 
implementation of `clang_getSkippedRanges` iterates over all ranges anyway). We 
use this internally to populate a database that later gets queried by our IDE.

http://reviews.llvm.org/D20132

Files:
  include/clang-c/Index.h
  tools/libclang/CIndex.cpp

Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -7680,6 +7680,33 @@
   return skipped;
 }
 
+CXSourceRangeList *clang_getAllSkippedRanges(CXTranslationUnit TU) {
+  CXSourceRangeList *skipped = new CXSourceRangeList;
+  skipped->count = 0;
+  skipped->ranges = nullptr;
+
+  if (isNotUsableTU(TU)) {
+LOG_BAD_TU(TU);
+return skipped;
+  }
+
+  ASTUnit *astUnit = cxtu::getASTUnit(TU);
+  PreprocessingRecord *ppRec = 
astUnit->getPreprocessor().getPreprocessingRecord();
+  if (!ppRec)
+return skipped;
+
+  ASTContext  = astUnit->getASTContext();
+
+  const std::vector  = ppRec->getSkippedRanges();
+
+  skipped->count = SkippedRanges.size();
+  skipped->ranges = new CXSourceRange[skipped->count];
+  for (unsigned i = 0, ei = skipped->count; i != ei; ++i)
+skipped->ranges[i] = cxloc::translateSourceRange(Ctx, SkippedRanges[i]);
+
+  return skipped;
+}
+
 void clang_disposeSourceRangeList(CXSourceRangeList *ranges) {
   if (ranges) {
 delete[] ranges->ranges;
Index: include/clang-c/Index.h
===
--- include/clang-c/Index.h
+++ include/clang-c/Index.h
@@ -627,6 +627,15 @@
  CXFile file);
 
 /**
+ * \brief Retrieve all ranges from all files that were skipped by the
+ * preprocessor.
+ *
+ * The preprocessor will skip lines when they are surrounded by an
+ * if/ifdef/ifndef directive whose condition does not evaluate to true.
+ */
+CINDEX_LINKAGE CXSourceRangeList *clang_getAllSkippedRanges(CXTranslationUnit 
tu);
+
+/**
  * \brief Destroy the given \c CXSourceRangeList.
  */
 CINDEX_LINKAGE void clang_disposeSourceRangeList(CXSourceRangeList *ranges);


Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -7680,6 +7680,33 @@
   return skipped;
 }
 
+CXSourceRangeList *clang_getAllSkippedRanges(CXTranslationUnit TU) {
+  CXSourceRangeList *skipped = new CXSourceRangeList;
+  skipped->count = 0;
+  skipped->ranges = nullptr;
+
+  if (isNotUsableTU(TU)) {
+LOG_BAD_TU(TU);
+return skipped;
+  }
+
+  ASTUnit *astUnit = cxtu::getASTUnit(TU);
+  PreprocessingRecord *ppRec = astUnit->getPreprocessor().getPreprocessingRecord();
+  if (!ppRec)
+return skipped;
+
+  ASTContext  = astUnit->getASTContext();
+
+  const std::vector  = ppRec->getSkippedRanges();
+
+  skipped->count = SkippedRanges.size();
+  skipped->ranges = new CXSourceRange[skipped->count];
+  for (unsigned i = 0, ei = skipped->count; i != ei; ++i)
+skipped->ranges[i] = cxloc::translateSourceRange(Ctx, SkippedRanges[i]);
+
+  return skipped;
+}
+
 void clang_disposeSourceRangeList(CXSourceRangeList *ranges) {
   if (ranges) {
 delete[] ranges->ranges;
Index: include/clang-c/Index.h
===
--- include/clang-c/Index.h
+++ include/clang-c/Index.h
@@ -627,6 +627,15 @@
  CXFile file);
 
 /**
+ * \brief Retrieve all ranges from all files that were skipped by the
+ * preprocessor.
+ *
+ * The preprocessor will skip lines when they are surrounded by an
+ * if/ifdef/ifndef directive whose condition does not evaluate to true.
+ */
+CINDEX_LINKAGE CXSourceRangeList *clang_getAllSkippedRanges(CXTranslationUnit tu);
+
+/**
  * \brief Destroy the given \c CXSourceRangeList.
  */
 CINDEX_LINKAGE void clang_disposeSourceRangeList(CXSourceRangeList *ranges);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D17348: Add -Wcast-calling-convention to warn when casting away calling conventions

2016-05-10 Thread Reid Kleckner via cfe-commits
rnk added inline comments.


Comment at: include/clang/Basic/DiagnosticGroups.td:294
@@ -293,2 +293,3 @@
 def SelTypeCast : DiagGroup<"cast-of-sel-type">;
+def CastCallingConvention : DiagGroup<"cast-calling-convention">;
 def FunctionDefInObjCContainer : DiagGroup<"function-def-in-objc-container">;

thakis wrote:
> nit: since this isn't referenced in this file, consider using an unnamed 
> inline `InGroup` in the other file 
> instead
Sure. I dono, I never really liked that style since it makes it harder to add 
it to a group later.


Comment at: include/clang/Basic/DiagnosticSemaKinds.td:6516
@@ +6515,3 @@
+  "cast between incompatible calling conventions '%0' and '%1'">,
+  InGroup, DefaultIgnore;
+def note_change_calling_conv_fixit : Note<

thakis wrote:
> I'd build a large codebase of your choice with this enabled and see how it 
> does. If it does well I'd land it default-on. (I spent some time enabling 
> DefaultIgnore warnings that nobody turned on after they landed recently, and 
> I'd prefer to not having to do this for this warning too :-) )
My attempt to bait you into doing this work has failed. :) I'd still like to 
land it as ignored-by-default so I can run this warning across Chromium with a 
try job instead of doing it locally on my workstation. Sound good?


Comment at: lib/Sema/SemaCast.cpp:1752-1753
@@ +1751,4 @@
+  Expr *Src = SrcExpr.get()->IgnoreParenImpCasts();
+  if (auto *AddrOf = dyn_cast(Src))
+Src = AddrOf->getSubExpr()->IgnoreParenImpCasts();
+  auto *DRE = dyn_cast(Src);

rtrieu wrote:
> Which UnaryOperator are you attempting to strip off here?  This one will 
> strip off every UnaryOperator.
Oops, added a test for that.


Comment at: lib/Sema/SemaCast.cpp:1762
@@ +1761,3 @@
+
+  // The source expression is a pointer to a known function defined in this TU.
+  // Diagnose this cast, as it is probably bad.

thakis wrote:
> If it's an inline function in a header, would we want to warn? Should this 
> check if the body's SourceLocation is from the main file?
Yeah, we do want to warn even if it's from a header. If it were defined 
elsewhere with a different convention, that'd be an ODR violation.


Comment at: lib/Sema/SemaCast.cpp:1775
@@ +1774,3 @@
+  Self.Diag(NameLoc, diag::note_change_calling_conv_fixit)
+  << FD << DstCCName << FixItHint::CreateInsertion(NameLoc, CCAttrText);
+}

thakis wrote:
> Consider doing something like r164858 did to see if there's a define for the 
> calling convention (WINAPI or what have you) and suggest that instead
Done, that was fun. :)


http://reviews.llvm.org/D17348



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


Re: [PATCH] D17348: Add -Wcast-calling-convention to warn when casting away calling conventions

2016-05-10 Thread Reid Kleckner via cfe-commits
rnk updated this revision to Diff 56802.
rnk marked 3 inline comments as done.
rnk added a comment.

- address comments


http://reviews.llvm.org/D17348

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaCast.cpp
  test/Sema/callingconv-cast.c

Index: test/Sema/callingconv-cast.c
===
--- /dev/null
+++ test/Sema/callingconv-cast.c
@@ -0,0 +1,54 @@
+// RUN: %clang_cc1 -fms-extensions -triple i686-pc-windows-msvc -Wcast-calling-convention -DMSVC -Wno-pointer-bool-conversion -verify -x c %s
+// RUN: %clang_cc1 -fms-extensions -triple i686-pc-windows-msvc -Wcast-calling-convention -DMSVC -Wno-pointer-bool-conversion -verify -x c++ %s
+// RUN: %clang_cc1 -fms-extensions -triple i686-pc-windows-msvc -Wcast-calling-convention -DMSVC -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s --check-prefix=MSFIXIT
+// RUN: %clang_cc1 -triple i686-pc-windows-gnu -Wcast-calling-convention -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s --check-prefix=GNUFIXIT
+
+// expected-note@+1 {{consider defining 'mismatched_before_winapi' with the 'stdcall' calling convention}}
+void mismatched_before_winapi(int x) {}
+
+#ifdef MSVC
+#define WINAPI __stdcall
+#else
+#define WINAPI __attribute__((stdcall))
+#endif
+
+// expected-note@+1 3 {{consider defining 'mismatched' with the 'stdcall' calling convention}}
+void mismatched(int x) {}
+
+typedef void (WINAPI *callback_t)(int);
+void take_callback(callback_t callback);
+
+int main() {
+  // expected-warning@+1 {{cast between incompatible calling conventions 'cdecl' and 'stdcall'}}
+  take_callback((callback_t)mismatched);
+
+  // expected-warning@+1 {{cast between incompatible calling conventions 'cdecl' and 'stdcall'}}
+  callback_t callback = (callback_t)mismatched; // warns
+  (void)callback;
+
+  // expected-warning@+1 {{cast between incompatible calling conventions 'cdecl' and 'stdcall'}}
+  callback = (callback_t) // warns
+
+  // No warning, just to show we don't drill through other kinds of unary operators.
+  callback = (callback_t)!mismatched;
+
+  // expected-warning@+1 {{cast between incompatible calling conventions 'cdecl' and 'stdcall'}}
+  callback = (callback_t)_before_winapi; // warns
+
+  // Probably a bug, but we don't warn.
+  void (*callback2)(int) = mismatched;
+  take_callback((callback_t)callback2);
+
+  // Another way to suppress the warning.
+  take_callback((callback_t)(void*)mismatched);
+}
+
+// MSFIXIT: fix-it:"D:\\src\\llvm\\tools\\clang\\test\\Sema\\callingconv-cast.c":{16:6-16:6}:"WINAPI "
+// MSFIXIT: fix-it:"D:\\src\\llvm\\tools\\clang\\test\\Sema\\callingconv-cast.c":{16:6-16:6}:"WINAPI "
+// MSFIXIT: fix-it:"D:\\src\\llvm\\tools\\clang\\test\\Sema\\callingconv-cast.c":{16:6-16:6}:"WINAPI "
+// MSFIXIT: fix-it:"D:\\src\\llvm\\tools\\clang\\test\\Sema\\callingconv-cast.c":{7:6-7:6}:"__stdcall "
+
+// GNUFIXIT: fix-it:"D:\\src\\llvm\\tools\\clang\\test\\Sema\\callingconv-cast.c":{16:6-16:6}:"WINAPI "
+// GNUFIXIT: fix-it:"D:\\src\\llvm\\tools\\clang\\test\\Sema\\callingconv-cast.c":{16:6-16:6}:"WINAPI "
+// GNUFIXIT: fix-it:"D:\\src\\llvm\\tools\\clang\\test\\Sema\\callingconv-cast.c":{16:6-16:6}:"WINAPI "
+// GNUFIXIT: fix-it:"D:\\src\\llvm\\tools\\clang\\test\\Sema\\callingconv-cast.c":{7:6-7:6}:"__attribute__((stdcall)) "
Index: lib/Sema/SemaCast.cpp
===
--- lib/Sema/SemaCast.cpp
+++ lib/Sema/SemaCast.cpp
@@ -22,6 +22,7 @@
 #include "clang/AST/RecordLayout.h"
 #include "clang/Basic/PartialDiagnostic.h"
 #include "clang/Basic/TargetInfo.h"
+#include "clang/Lex/Preprocessor.h"
 #include "clang/Sema/Initialization.h"
 #include "llvm/ADT/SmallVector.h"
 #include 
@@ -1726,6 +1727,86 @@
 }
 }
 
+/// Diagnose casts that change the calling convention of a pointer to a function
+/// defined in the current TU.
+static void DiagnoseCallingConvCast(Sema , const ExprResult ,
+QualType DstType, SourceRange OpRange) {
+  if (Self.Diags.isIgnored(diag::warn_cast_calling_conv, OpRange.getBegin()))
+return;
+
+  // Check if this cast would change the calling convention of a function
+  // pointer type.
+  QualType SrcType = SrcExpr.get()->getType();
+  if (Self.Context.hasSameType(SrcType, DstType) ||
+  !SrcType->isFunctionPointerType() || !DstType->isFunctionPointerType())
+return;
+  const auto *SrcFTy =
+  SrcType->castAs()->getPointeeType()->castAs();
+  const auto *DstFTy =
+  DstType->castAs()->getPointeeType()->castAs();
+  CallingConv SrcCC = SrcFTy->getCallConv();
+  CallingConv DstCC = DstFTy->getCallConv();
+  if (SrcCC == DstCC)
+return;
+
+  // We have a calling convention cast. Check if the source is a pointer to a
+  // known, specific function that has already been defined.
+  Expr *Src = SrcExpr.get()->IgnoreParenImpCasts();
+  if (auto *UO = dyn_cast(Src))
+if (UO->getOpcode() == UO_AddrOf)
+  Src = 

Re: [PATCH] D18914: [clang-tidy] new readability-redundant-inline

2016-05-10 Thread Matthias Gehre via cfe-commits
mgehre updated this revision to Diff 56800.
mgehre added a comment.

Update for review comment:

- Return Optional when looking for 'inline'
- Add test that hides 'inline' in a macro


http://reviews.llvm.org/D18914

Files:
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tidy/readability/RedundantInlineCheck.cpp
  clang-tidy/readability/RedundantInlineCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-redundant-inline.rst
  test/clang-tidy/readability-redundant-inline.cpp

Index: test/clang-tidy/readability-redundant-inline.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-redundant-inline.cpp
@@ -0,0 +1,77 @@
+// RUN: %check_clang_tidy %s readability-redundant-inline %t
+
+struct S {
+  inline int f1() {
+// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: 'inline' is redundant because method body is defined inside class [readability-redundant-inline]
+// CHECK-FIXES: {{^}}  int f1()
+return 0;
+  }
+  inline int f2(); // OK
+
+  inline constexpr int f3() {
+// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: 'inline' is redundant because 'constexpr' implies 'inline' [readability-redundant-inline]
+// CHECK-FIXES: {{^}}  constexpr int f3()
+return 0;
+  }
+  static inline constexpr int f4() {
+// CHECK-MESSAGES: :[[@LINE-1]]:31: warning: 'inline' is redundant because 'constexpr' implies 'inline'
+// CHECK-FIXES: {{^}}  static constexpr int f4()
+return 0;
+  }
+
+  static inline int f5();
+
+  static inline int f6() {
+// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 'inline' is redundant because method body is defined inside class
+// CHECK-FIXES: {{^}}  static int f6()
+return 0;
+  }
+
+  inline friend int f7() {
+// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 'inline' is redundant because function body is defined inside class [readability-redundant-inline]
+// CHECK-FIXES: {{^}}  friend int f7()
+  return 0;
+  }
+
+  inline friend int f8(); // OK
+
+  template struct T{};
+  struct T inline f9() { return {}; }
+// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: 'inline' is redundant because method body is defined inside class
+// CHECK-FIXES: {{^}}  struct T f9() { return {}; }
+
+#define INLINE inline
+  INLINE void f10() {
+// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: 'inline' is redundant because method body is defined inside class
+  }
+};
+
+class S2 {
+  inline int f1() {
+// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: 'inline' is redundant because method body is defined inside class
+// CHECK-FIXES: {{^}}  int f1()
+return 0;
+  }
+};
+
+inline int S::f2() { // OK
+  return 0;
+}
+
+inline int S::f5() { // OK
+  return 0;
+}
+
+inline int f3() { // OK
+  return 0;
+}
+
+inline constexpr int f4() {
+// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: 'inline' is redundant because 'constexpr' implies 'inline'
+// CHECK-FIXES: {{^}}constexpr int f4()
+  return 1;
+}
+
+constexpr int f5() { // OK
+  return 1;
+}
Index: docs/clang-tidy/checks/readability-redundant-inline.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/readability-redundant-inline.rst
@@ -0,0 +1,16 @@
+.. title:: clang-tidy - readability-redundant-inline
+
+readability-redundant-inline
+
+
+This check flags redundant ``inline`` specifiers.
+It flags ``inline`` on member functions defined inside a class definition like
+.. code-block:: c++
+
+  struct S {
+inline int f() {
+  return 0;
+}
+  };
+
+and ``inline`` specifiers on functions that are also declared ``constexpr``.
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -105,6 +105,7 @@
readability-inconsistent-declaration-parameter-name
readability-named-parameter
readability-redundant-control-flow
+   readability-redundant-inline
readability-redundant-smartptr-get
readability-redundant-string-cstr
readability-redundant-string-init
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -160,6 +160,12 @@
   Looks for procedures (functions returning no value) with ``return`` statements
   at the end of the function.  Such `return` statements are redundant.
 
+- New `readability-redundant-inline
+  `_ check
+
+  Looks for redundant ``inline`` specifiers which are implied by defining a body within a class definition
+  or by ``constexpr``.
+
 - New `readability-redundant-string-init
   `_ check
 
Index: clang-tidy/readability/RedundantInlineCheck.h

[PATCH] D20131: Fixed crash during code completion in file included within declaration

2016-05-10 Thread Cameron via cfe-commits
cameron314 created this revision.
cameron314 added a reviewer: rsmith.
cameron314 added a subscriber: cfe-commits.

When triggering code completion within a file that is included in the middle of 
a declaration in another file, clang would crash while parsing the code.

This occurred with real-world code; there was an enum declaration that included 
a header in the middle of its declaration to specify the enum members.

http://reviews.llvm.org/D20131

Files:
  lib/Lex/PPLexerChange.cpp

Index: lib/Lex/PPLexerChange.cpp
===
--- lib/Lex/PPLexerChange.cpp
+++ lib/Lex/PPLexerChange.cpp
@@ -378,6 +378,8 @@
 Result.startToken();
 CurLexer->FormTokenWithChars(Result, CurLexer->BufferEnd, tok::eof);
 CurLexer.reset();
+if (CurLexerKind == CLK_Lexer)
+  CurLexerKind = CLK_CachingLexer;
   } else {
 assert(CurPTHLexer && "Got EOF but no current lexer set!");
 CurPTHLexer->getEOF(Result);


Index: lib/Lex/PPLexerChange.cpp
===
--- lib/Lex/PPLexerChange.cpp
+++ lib/Lex/PPLexerChange.cpp
@@ -378,6 +378,8 @@
 Result.startToken();
 CurLexer->FormTokenWithChars(Result, CurLexer->BufferEnd, tok::eof);
 CurLexer.reset();
+if (CurLexerKind == CLK_Lexer)
+  CurLexerKind = CLK_CachingLexer;
   } else {
 assert(CurPTHLexer && "Got EOF but no current lexer set!");
 CurPTHLexer->getEOF(Result);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D18425: [Sema] Make enable_if act correctly with value dependent conditions/arguments

2016-05-10 Thread George Burgess IV via cfe-commits
george.burgess.iv abandoned this revision.
george.burgess.iv added a comment.

Abandoning this; http://reviews.llvm.org/D20130 is our new approach.


http://reviews.llvm.org/D18425



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


[PATCH] D20130: Fix enable_if evaluation in value-dependent contexts.

2016-05-10 Thread George Burgess IV via cfe-commits
george.burgess.iv created this revision.
george.burgess.iv added a reviewer: rsmith.
george.burgess.iv added a subscriber: cfe-commits.

This patch is meant to be applied instead of D18425, but is a new review 
because we're taking an entirely different approach.

This patch makes us fail `enable_if` conditions in a value dependent context, 
if not all values are present when we're trying to evaluate the `enable_if`. 
e.g.

```
void foo(int i) __attribute__((enable_if(i, "")));
template  void callFoo() { foo(N); } // Error: No viable overload 
(emitted before any instantiations of callFoo are made), because the call to 
foo is resolved prior to instantiating callFoo, so we don't have a single value 
for N.
```

Our current behavior is that we'll happily resolve `foo` to the only existing 
`foo` overload, and be totally fine with `callFoo<0>();` as a result, which is 
incorrect. Were there two `foo` overloads with different `enable_if` 
attributes, we would always emit an error about an ambiguous overload set.

A complete, consistent fix for this would require that we're able to 
arbitrarily defer overload resolution. Additionally, we would need to handle 
type dependence appearing out of nowhere, as in:

```
double bar(int N) __attribute__((enable_if(N, "")));
void *bar(int N) __attribute__((enable_if(!N, "")));
template 
auto callBar() { return bar(N); }
```

Which seems like a very large, bug-prone change.

Given that it seems no one has tried to use `enable_if` in a value-dependent 
context yet (if they were, I'd assume we would have received a bug report about 
`bar(N)` always being ambiguous by now), I think this is the least bad of all 
of our options.

http://reviews.llvm.org/D20130

Files:
  include/clang/Basic/AttrDocs.td
  lib/Sema/SemaOverload.cpp
  test/SemaCXX/enable_if.cpp

Index: test/SemaCXX/enable_if.cpp
===
--- test/SemaCXX/enable_if.cpp
+++ test/SemaCXX/enable_if.cpp
@@ -116,9 +116,9 @@
   void g() { f(); }
 };
 
-int fn3(bool b) __attribute__((enable_if(b, "")));
+int fn3(bool b) __attribute__((enable_if(b, ""))); // FIXME: This test should net 0 error messages.
 template  void test3() {
-  fn3(sizeof(T) == 1);
+  fn3(sizeof(T) == 1); // expected-error{{no matching function for call to 'fn3'}} expected-note@-2{{candidate disabled}}
 }
 
 template 
@@ -138,7 +138,7 @@
 void h(int);
 template  void outer() {
   void local_function() __attribute__((enable_if(::h(T()), "")));
-  local_function();
+  local_function(); // expected-error{{no matching function for call to 'local_function'}} expected-note@-1{{candidate disabled}}
 };
 
 namespace PR20988 {
@@ -158,9 +158,9 @@
 fn2(expr);  // expected-error{{no matching function for call to 'fn2'}}
   }
 
-  int fn3(bool b) __attribute__((enable_if(b, "")));
+  int fn3(bool b) __attribute__((enable_if(b, ""))); // FIXME: This test should net 0 error messages.
   template  void test3() {
-fn3(sizeof(T) == 1);
+fn3(sizeof(T) == 1); // expected-error{{no matching function for call to 'fn3'}} expected-note@-2{{candidate disabled}}
   }
 }
 
@@ -386,3 +386,34 @@
   f.bar(1, 2); // expected-error{{too many arguments}}
 }
 }
+
+// Ideally, we should be able to handle value-dependent expressions sanely.
+// Sadly, that isn't the case at the moment.
+namespace dependent {
+int error(int N) __attribute__((enable_if(N, ""))); // expected-note{{candidate disabled}}
+int error(int N) __attribute__((enable_if(!N, ""))); // expected-note{{candidate disabled}}
+template  int callUnavailable() {
+  return error(N); // expected-error{{no matching function for call to 'error'}}
+}
+
+constexpr int noError(int N) __attribute__((enable_if(N, ""))) { return -1; }
+constexpr int noError(int N) __attribute__((enable_if(!N, ""))) { return -1; }
+constexpr int noError(int N) { return 0; }
+
+template 
+constexpr int callNoError() { return noError(N); }
+static_assert(callNoError<0>() == 0, "");
+static_assert(callNoError<1>() == 0, "");
+
+template  constexpr int templated() __attribute__((enable_if(N, ""))) {
+  return 1;
+}
+
+constexpr int A = templated<0>(); // expected-error{{no matching function for call to 'templated'}} expected-note@-4{{candidate disabled}}
+static_assert(templated<1>() == 1, "");
+
+template  constexpr int callTemplated() { return templated(); }
+
+constexpr int B = callTemplated<0>(); // expected-error{{initialized by a constant expression}} expected-error@-2{{no matching function for call to 'templated'}} expected-note{{in instantiation of function template}} expected-note@-9{{candidate disabled}}
+static_assert(callTemplated<1>() == 1, "");
+}
Index: lib/Sema/SemaOverload.cpp
===
--- lib/Sema/SemaOverload.cpp
+++ lib/Sema/SemaOverload.cpp
@@ -5984,7 +5984,6 @@
   SFINAETrap Trap(*this);
   SmallVector ConvertedArgs;
   bool InitializationFailed = false;
-  bool ContainsValueDependentExpr = false;
 
   // 

Re: [PATCH] D19941: [tooling] FixItHint Tooling refactoring

2016-05-10 Thread Alexander Kornienko via cfe-commits
alexfh added inline comments.


Comment at: include/clang/Tooling/Fixit.h:1
@@ +1,2 @@
+//===--- FixIt.h - FixIt Hint utilities -*- C++ 
-*-===//
+//

nit: s/FixIt Hint/FixItHint/, since this is a reference to the type. 


Comment at: lib/Tooling/CMakeLists.txt:10
@@ -9,2 +9,3 @@
   FileMatchTrie.cpp
+  Fixit.cpp
   JSONCompilationDatabase.cpp

Please rename the files as well (s/Fixit/FixIt/).


Comment at: unittests/Tooling/CMakeLists.txt:15
@@ -14,2 +14,3 @@
   CompilationDatabaseTest.cpp
+  FixitTest.cpp  
   LookupTest.cpp

s/FixitTest/FixItTest/


Comment at: unittests/Tooling/FixitTest.cpp:34
@@ +33,3 @@
+
+  Visitor.OnCall = [&](CallExpr *CE, ASTContext *Context) {
+EXPECT_EQ("foo(x, y)", tooling::fixit::getText(*CE, *Context));

Automatic captures always seem suspicious to me. What exactly does this lambda 
need to capture? If just `this`, I'd rather make the capture list explicit.


Comment at: unittests/Tooling/FixitTest.cpp:53
@@ +52,3 @@
+
+TEST(FixitTest, getTextWithMacro) {
+  CallsVisitor Visitor;

Could you add a test where getText returns an empty string?


Comment at: unittests/Tooling/FixitTest.cpp:119
@@ +118,3 @@
+  LocationToString(Hint0.RemoveRange.getBegin(), Context));
+EXPECT_EQ("input.cc:2:26 

r269111 - Update clang for LLVM API change.

2016-05-10 Thread Peter Collingbourne via cfe-commits
Author: pcc
Date: Tue May 10 15:23:29 2016
New Revision: 269111

URL: http://llvm.org/viewvc/llvm-project?rev=269111=rev
Log:
Update clang for LLVM API change.

Modified:
cfe/trunk/lib/CodeGen/CGVTables.cpp

Modified: cfe/trunk/lib/CodeGen/CGVTables.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGVTables.cpp?rev=269111=269110=269111=diff
==
--- cfe/trunk/lib/CodeGen/CGVTables.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGVTables.cpp Tue May 10 15:23:29 2016
@@ -156,9 +156,7 @@ CodeGenFunction::GenerateVarArgsThunk(ll
 
   // Clone to thunk.
   llvm::ValueToValueMapTy VMap;
-  llvm::Function *NewFn = llvm::CloneFunction(BaseFn, VMap,
-  /*ModuleLevelChanges=*/false);
-  CGM.getModule().getFunctionList().push_back(NewFn);
+  llvm::Function *NewFn = llvm::CloneFunction(BaseFn, VMap);
   Fn->replaceAllUsesWith(NewFn);
   NewFn->takeName(Fn);
   Fn->eraseFromParent();


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


r269108 - [VFS] Change unittest to try appeasing win10 buildbot

2016-05-10 Thread Bruno Cardoso Lopes via cfe-commits
Author: bruno
Date: Tue May 10 15:20:55 2016
New Revision: 269108

URL: http://llvm.org/viewvc/llvm-project?rev=269108=rev
Log:
[VFS] Change unittest to try appeasing win10 buildbot

http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/5103
Follow up from r269100.

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

Modified: cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp?rev=269108=269107=269108=diff
==
--- cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp (original)
+++ cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp Tue May 10 15:20:55 2016
@@ -1050,7 +1050,7 @@ TEST_F(VFSFromYAMLTest, DirectoryIterati
 "},\n"
 "{\n"
 "  'type': 'directory',\n"
-"  'name': '//root/baz',\n"
+"  'name': '//root/baz/',\n"
 "  'contents': [ {\n"
 "  'type': 'file',\n"
 "  'name': 'x',\n"
@@ -1060,7 +1060,7 @@ TEST_F(VFSFromYAMLTest, DirectoryIterati
 "},\n"
 "{\n"
 "  'type': 'directory',\n"
-"  'name': '//root/baz',\n"
+"  'name': '//root/baz/',\n"
 "  'contents': [ {\n"
 "  'type': 'file',\n"
 "  'name': 'y',\n"


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


[PATCH] D20129: [libclang] Properly expose linkage spec cursors

2016-05-10 Thread Cameron via cfe-commits
cameron314 created this revision.
cameron314 added a reviewer: rsmith.
cameron314 added a subscriber: cfe-commits.

All the groundwork for `CXCursor_LinkageSpec` already existed, but because of a 
missing case in a switch, cursors of that type were never actually created for 
linkage specifications in the AST. This patch fixes that.

I remember seeing another patch that fixed this a long time ago (that I found 
after fixing it myself), but it was never committed (it was just forgotten, if 
I recall correctly).

http://reviews.llvm.org/D20129

Files:
  lib/Sema/SemaCodeComplete.cpp

Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -3052,7 +3052,10 @@
 case Decl::UnresolvedUsingValue:
 case Decl::UnresolvedUsingTypename: 
   return CXCursor_UsingDeclaration;
-  
+
+case Decl::LinkageSpec:
+  return CXCursor_LinkageSpec;
+
 case Decl::ObjCPropertyImpl:
   switch (cast(D)->getPropertyImplementation()) {
   case ObjCPropertyImplDecl::Dynamic:


Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -3052,7 +3052,10 @@
 case Decl::UnresolvedUsingValue:
 case Decl::UnresolvedUsingTypename: 
   return CXCursor_UsingDeclaration;
-  
+
+case Decl::LinkageSpec:
+  return CXCursor_LinkageSpec;
+
 case Decl::ObjCPropertyImpl:
   switch (cast(D)->getPropertyImplementation()) {
   case ObjCPropertyImplDecl::Dynamic:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D20127: [libclang] Expose cursors for alias/weak attributes

2016-05-10 Thread Cameron via cfe-commits
cameron314 created this revision.
cameron314 added a reviewer: rsmith.
cameron314 added a subscriber: cfe-commits.

This patch introduces `CXCursor_AliasAttr`/`CXCursor_WeakAttr` and 
`clang_getAliasTargetSpelling()` for inspecting alias attributes via libclang.

http://reviews.llvm.org/D20127

Files:
  include/clang-c/Index.h
  tools/libclang/CIndex.cpp
  tools/libclang/CXCursor.cpp

Index: tools/libclang/CXCursor.cpp
===
--- tools/libclang/CXCursor.cpp
+++ tools/libclang/CXCursor.cpp
@@ -61,6 +61,8 @@
 case attr::Visibility: return CXCursor_VisibilityAttr;
 case attr::DLLExport: return CXCursor_DLLExport;
 case attr::DLLImport: return CXCursor_DLLImport;
+case attr::Weak: return CXCursor_WeakAttr;
+case attr::Alias: return CXCursor_AliasAttr;
   }
 
   return CXCursor_UnexposedAttr;
Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -4698,6 +4698,10 @@
 return cxstring::createRef("attribute(dllexport)");
   case CXCursor_DLLImport:
 return cxstring::createRef("attribute(dllimport)");
+  case CXCursor_WeakAttr:
+return cxstring::createRef("attribute(weak)");
+  case CXCursor_AliasAttr:
+return cxstring::createRef("attribute(alias)");
   case CXCursor_PreprocessingDirective:
 return cxstring::createRef("preprocessing directive");
   case CXCursor_MacroDefinition:
@@ -5861,6 +5865,16 @@
   return clang_getNullRange();
 }
 
+CXString clang_getAliasTargetSpelling(CXCursor C) {
+  if (C.kind != CXCursor_AliasAttr)
+return cxstring::createEmpty();
+
+  const AliasAttr *A =
+cast(cxcursor::getCursorAttr(C));
+
+  return cxstring::createDup(A->getAliasee());
+}
+
 void clang_enableStackTraces(void) {
   llvm::sys::PrintStackTraceOnErrorSignal();
 }
Index: include/clang-c/Index.h
===
--- include/clang-c/Index.h
+++ include/clang-c/Index.h
@@ -2338,7 +2338,9 @@
   CXCursor_VisibilityAttr= 417,
   CXCursor_DLLExport = 418,
   CXCursor_DLLImport = 419,
-  CXCursor_LastAttr  = CXCursor_DLLImport,
+  CXCursor_WeakAttr  = 420,
+  CXCursor_AliasAttr = 421,
+  CXCursor_LastAttr  = CXCursor_AliasAttr,
 
   /* Preprocessing */
   CXCursor_PreprocessingDirective= 500,
@@ -3589,6 +3591,12 @@
 CINDEX_LINKAGE CXType clang_getIBOutletCollectionType(CXCursor);
 
 /**
+ * \brief For cursors representing an alias attribute,
+ *  this function returns the aliased symbol name as written.
+ */
+CINDEX_LINKAGE CXString clang_getAliasTargetSpelling(CXCursor);
+
+/**
  * @}
  */
 


Index: tools/libclang/CXCursor.cpp
===
--- tools/libclang/CXCursor.cpp
+++ tools/libclang/CXCursor.cpp
@@ -61,6 +61,8 @@
 case attr::Visibility: return CXCursor_VisibilityAttr;
 case attr::DLLExport: return CXCursor_DLLExport;
 case attr::DLLImport: return CXCursor_DLLImport;
+case attr::Weak: return CXCursor_WeakAttr;
+case attr::Alias: return CXCursor_AliasAttr;
   }
 
   return CXCursor_UnexposedAttr;
Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -4698,6 +4698,10 @@
 return cxstring::createRef("attribute(dllexport)");
   case CXCursor_DLLImport:
 return cxstring::createRef("attribute(dllimport)");
+  case CXCursor_WeakAttr:
+return cxstring::createRef("attribute(weak)");
+  case CXCursor_AliasAttr:
+return cxstring::createRef("attribute(alias)");
   case CXCursor_PreprocessingDirective:
 return cxstring::createRef("preprocessing directive");
   case CXCursor_MacroDefinition:
@@ -5861,6 +5865,16 @@
   return clang_getNullRange();
 }
 
+CXString clang_getAliasTargetSpelling(CXCursor C) {
+  if (C.kind != CXCursor_AliasAttr)
+return cxstring::createEmpty();
+
+  const AliasAttr *A =
+cast(cxcursor::getCursorAttr(C));
+
+  return cxstring::createDup(A->getAliasee());
+}
+
 void clang_enableStackTraces(void) {
   llvm::sys::PrintStackTraceOnErrorSignal();
 }
Index: include/clang-c/Index.h
===
--- include/clang-c/Index.h
+++ include/clang-c/Index.h
@@ -2338,7 +2338,9 @@
   CXCursor_VisibilityAttr= 417,
   CXCursor_DLLExport = 418,
   CXCursor_DLLImport = 419,
-  CXCursor_LastAttr  = CXCursor_DLLImport,
+  CXCursor_WeakAttr  = 420,
+  CXCursor_AliasAttr = 421,
+  CXCursor_LastAttr  = CXCursor_AliasAttr,
 
   /* Preprocessing */
   CXCursor_PreprocessingDirective= 500,
@@ -3589,6 +3591,12 @@
 CINDEX_LINKAGE 

Re: [PATCH] D19941: [tooling] FixItHint Tooling refactoring

2016-05-10 Thread Etienne Bergeron via cfe-commits
etienneb updated this revision to Diff 56790.
etienneb added a comment.

more unittests


http://reviews.llvm.org/D19941

Files:
  include/clang/Tooling/Fixit.h
  lib/Tooling/CMakeLists.txt
  lib/Tooling/Fixit.cpp
  unittests/Tooling/CMakeLists.txt
  unittests/Tooling/FixitTest.cpp

Index: unittests/Tooling/FixitTest.cpp
===
--- /dev/null
+++ unittests/Tooling/FixitTest.cpp
@@ -0,0 +1,214 @@
+//===- unittest/Tooling/FixitTest.cpp ===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "TestVisitor.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Tooling/Fixit.h"
+
+using namespace clang;
+
+namespace {
+
+struct CallsVisitor : TestVisitor {
+  bool VisitCallExpr(CallExpr *Expr) {
+OnCall(Expr, Context);
+return true;
+  }
+
+  std::function OnCall;
+};
+
+std::string LocationToString(SourceLocation Loc, ASTContext *Context) {
+  return Loc.printToString(Context->getSourceManager());
+}
+
+TEST(FixitTest, getText) {
+  CallsVisitor Visitor;
+
+  Visitor.OnCall = [&](CallExpr *CE, ASTContext *Context) {
+EXPECT_EQ("foo(x, y)", tooling::fixit::getText(*CE, *Context));
+EXPECT_EQ("foo(x, y)",
+  tooling::fixit::getText(CE->getSourceRange(), *Context));
+
+Expr *P0 = CE->getArg(0);
+Expr *P1 = CE->getArg(1);
+EXPECT_EQ("x", tooling::fixit::getText(*P0, *Context));
+EXPECT_EQ("y", tooling::fixit::getText(*P1, *Context));
+  };
+  Visitor.runOver("void foo(int x, int y) { foo(x, y); }");
+
+  Visitor.OnCall = [&](CallExpr *CE, ASTContext *Context) {
+EXPECT_EQ("APPLY(foo, x, y)", tooling::fixit::getText(*CE, *Context));
+  };
+  Visitor.runOver("#define APPLY(f, x, y) f(x, y)\n"
+  "void foo(int x, int y) { APPLY(foo, x, y); }");
+}
+
+TEST(FixitTest, getTextWithMacro) {
+  CallsVisitor Visitor;
+
+  Visitor.OnCall = [&](CallExpr *CE, ASTContext *Context) {
+EXPECT_EQ("F OO", tooling::fixit::getText(*CE, *Context));
+Expr *P0 = CE->getArg(0);
+Expr *P1 = CE->getArg(1);
+EXPECT_EQ("", tooling::fixit::getText(*P0, *Context));
+EXPECT_EQ("", tooling::fixit::getText(*P1, *Context));
+  };
+  Visitor.runOver("#define F foo(\n"
+  "#define OO x, y)\n"
+  "void foo(int x, int y) { F OO ; }");
+
+  Visitor.OnCall = [&](CallExpr *CE, ASTContext *Context) {
+EXPECT_EQ("", tooling::fixit::getText(*CE, *Context));
+Expr *P0 = CE->getArg(0);
+Expr *P1 = CE->getArg(1);
+EXPECT_EQ("x", tooling::fixit::getText(*P0, *Context));
+EXPECT_EQ("y", tooling::fixit::getText(*P1, *Context));
+  };
+  Visitor.runOver("#define FOO(x, y) (void)x; (void)y; foo(x, y);\n"
+  "void foo(int x, int y) { FOO(x,y) }");
+}
+
+TEST(FixitTest, createRemoval) {
+  CallsVisitor Visitor;
+
+  Visitor.OnCall = [&](CallExpr *CE, ASTContext *Context) {
+FixItHint Hint = tooling::fixit::createRemoval(*CE);
+EXPECT_EQ("foo(x, y)",
+  tooling::fixit::getText(Hint.RemoveRange.getAsRange(), *Context));
+EXPECT_TRUE(Hint.InsertFromRange.isInvalid());
+EXPECT_TRUE(Hint.CodeToInsert.empty());
+
+Expr *P0 = CE->getArg(0);
+FixItHint Hint0 = tooling::fixit::createRemoval(*P0);
+EXPECT_EQ(
+"x", tooling::fixit::getText(Hint0.RemoveRange.getAsRange(), *Context));
+EXPECT_TRUE(Hint0.InsertFromRange.isInvalid());
+EXPECT_TRUE(Hint0.CodeToInsert.empty());
+
+Expr *P1 = CE->getArg(1);
+FixItHint Hint1 = tooling::fixit::createRemoval(*P1);
+EXPECT_EQ(
+"y", tooling::fixit::getText(Hint1.RemoveRange.getAsRange(), *Context));
+EXPECT_TRUE(Hint1.InsertFromRange.isInvalid());
+EXPECT_TRUE(Hint1.CodeToInsert.empty());
+  };
+  Visitor.runOver("void foo(int x, int y) { foo(x, y); }");
+}
+
+TEST(FixitTest, createRemovalWithMacro) {
+  CallsVisitor Visitor;
+
+  Visitor.OnCall = [&](CallExpr *CE, ASTContext *Context) {
+FixItHint Hint = tooling::fixit::createRemoval(*CE);
+EXPECT_EQ("FOO",
+  tooling::fixit::getText(Hint.RemoveRange.getAsRange(), *Context));
+EXPECT_TRUE(Hint.InsertFromRange.isInvalid());
+EXPECT_TRUE(Hint.CodeToInsert.empty());
+
+Expr *P0 = CE->getArg(0);
+FixItHint Hint0 = tooling::fixit::createRemoval(*P0);
+EXPECT_EQ("input.cc:2:26 

[PATCH] D20125: [libclang] Added clang_getRealSpellingLocation to compensate for clang_getSpellingLocation returning the expansion location

2016-05-10 Thread Cameron via cfe-commits
cameron314 created this revision.
cameron314 added a reviewer: rsmith.
cameron314 added a subscriber: cfe-commits.

All the libclang functions for expanding a location, namely 
`clang_getExpansionLocation`, `clang_getPresumedLocation`, 
`clang_getInstantiationLocation`, and most surprisingly 
`clang_getSpellingLocation` return expansion locations, not spelling locations. 
As it turns out, there is no way to get a spelling location via libclang. This 
patch adds such a function.

Note that there's a FIXME in `clang_getSpellingLocation` about this, but 
changing `clang_getSpellingLocation` to actually return a spelling location 
would almost certainly break a large body of existing (third-party) code, 
mostly in subtle ways. I think it's better to introduce a new function, though 
I know it's ugly. But, this is what we've been using for the past year, and 
we've gotten quite comfortable with it.

http://reviews.llvm.org/D20125

Files:
  include/clang-c/Index.h
  tools/libclang/CXSourceLocation.cpp

Index: tools/libclang/CXSourceLocation.cpp
===
--- tools/libclang/CXSourceLocation.cpp
+++ tools/libclang/CXSourceLocation.cpp
@@ -346,6 +346,42 @@
 *offset = FileOffset;
 }
 
+void clang_getRealSpellingLocation(CXSourceLocation location,
+   CXFile *file,
+   unsigned *line,
+   unsigned *column,
+   unsigned *offset) {
+
+  if (!isASTUnitSourceLocation(location)) {
+CXLoadedDiagnostic::decodeLocation(location, file, line, column, offset);
+return;
+  }
+
+  SourceLocation Loc = SourceLocation::getFromRawEncoding(location.int_data);
+
+  if (!location.ptr_data[0] || Loc.isInvalid())
+return createNullLocation(file, line, column, offset);
+
+  const SourceManager  =
+  *static_cast(location.ptr_data[0]);
+  SourceLocation SpellLoc = SM.getSpellingLoc(Loc);
+  std::pair LocInfo = SM.getDecomposedLoc(SpellLoc);
+  FileID FID = LocInfo.first;
+  unsigned FileOffset = LocInfo.second;
+
+  if (FID.isInvalid())
+return createNullLocation(file, line, column, offset);
+
+  if (file)
+*file = const_cast(SM.getFileEntryForID(FID));
+  if (line)
+*line = SM.getLineNumber(FID, FileOffset);
+  if (column)
+*column = SM.getColumnNumber(FID, FileOffset);
+  if (offset)
+*offset = FileOffset;
+}
+
 void clang_getFileLocation(CXSourceLocation location,
CXFile *file,
unsigned *line,
Index: include/clang-c/Index.h
===
--- include/clang-c/Index.h
+++ include/clang-c/Index.h
@@ -568,6 +568,36 @@
  * \brief Retrieve the file, line, column, and offset represented by
  * the given source location.
  *
+ * If the location refers into a macro instantiation, return where the
+ * location was originally spelled in the source file (i.e. where the token
+ * text is located). clang_getSpellingLocation() is supposed to do this
+ * but doesn't, hence this function.
+ *
+ * \param location the location within a source file that will be decomposed
+ * into its parts.
+ *
+ * \param file [out] if non-NULL, will be set to the file to which the given
+ * source location points.
+ *
+ * \param line [out] if non-NULL, will be set to the line to which the given
+ * source location points.
+ *
+ * \param column [out] if non-NULL, will be set to the column to which the 
given
+ * source location points.
+ *
+ * \param offset [out] if non-NULL, will be set to the offset into the
+ * buffer to which the given source location points.
+ */
+CINDEX_LINKAGE void clang_getRealSpellingLocation(CXSourceLocation location,
+  CXFile *file,
+  unsigned *line,
+  unsigned *column,
+  unsigned *offset);
+
+/**
+ * \brief Retrieve the file, line, column, and offset represented by
+ * the given source location.
+ *
  * If the location refers into a macro expansion, return where the macro was
  * expanded or where the macro argument was written, if the location points at
  * a macro argument.


Index: tools/libclang/CXSourceLocation.cpp
===
--- tools/libclang/CXSourceLocation.cpp
+++ tools/libclang/CXSourceLocation.cpp
@@ -346,6 +346,42 @@
 *offset = FileOffset;
 }
 
+void clang_getRealSpellingLocation(CXSourceLocation location,
+   CXFile *file,
+   unsigned *line,
+   unsigned *column,
+   unsigned *offset) {
+
+  if (!isASTUnitSourceLocation(location)) {
+CXLoadedDiagnostic::decodeLocation(location, file, line, column, 

[PATCH] D20124: [PCH] Serialize skipped preprocessor ranges

2016-05-10 Thread Cameron via cfe-commits
cameron314 created this revision.
cameron314 added a reviewer: rsmith.
cameron314 added a subscriber: cfe-commits.
cameron314 set the repository for this revision to rL LLVM.

This fixes, for example, libclang's `clang_getAllSkippedRanges` returning zero 
ranges after reparsing a translation unit.

Repository:
  rL LLVM

http://reviews.llvm.org/D20124

Files:
  include/clang/Lex/PreprocessingRecord.h
  include/clang/Serialization/ASTBitCodes.h
  include/clang/Serialization/ASTReader.h
  include/clang/Serialization/Module.h
  lib/Lex/PreprocessingRecord.cpp
  lib/Serialization/ASTReader.cpp
  lib/Serialization/ASTWriter.cpp
  lib/Serialization/Module.cpp

Index: lib/Serialization/Module.cpp
===
--- lib/Serialization/Module.cpp
+++ lib/Serialization/Module.cpp
@@ -31,6 +31,8 @@
 LocalNumMacros(0), MacroOffsets(nullptr),
 BasePreprocessedEntityID(0),
 PreprocessedEntityOffsets(nullptr), NumPreprocessedEntities(0),
+BasePreprocessedSkippedRangeID(0),
+PreprocessedSkippedRangeOffsets(nullptr), NumPreprocessedSkippedRanges(0),
 LocalNumHeaderFileInfos(0), 
 HeaderFileInfoTableData(nullptr), HeaderFileInfoTable(nullptr),
 LocalNumSubmodules(0), BaseSubmoduleID(0),
Index: lib/Serialization/ASTWriter.cpp
===
--- lib/Serialization/ASTWriter.cpp
+++ lib/Serialization/ASTWriter.cpp
@@ -953,6 +953,7 @@
   RECORD(UNUSED_FILESCOPED_DECLS);
   RECORD(PPD_ENTITIES_OFFSETS);
   RECORD(VTABLE_USES);
+  RECORD(PPD_SKIPPED_RANGES);
   RECORD(REFERENCED_SELECTOR_POOL);
   RECORD(TU_UPDATE_LEXICAL);
   RECORD(SEMA_DECL_REFS);
@@ -2408,6 +2409,26 @@
 Stream.EmitRecordWithBlob(PPEOffsetAbbrev, Record,
   bytes(PreprocessedEntityOffsets));
   }
+
+  // Write the skipped region table for the preprocessing record.
+  const std::vector  = PPRec.getSkippedRanges();
+  if (SkippedRanges.size() > 0) {
+std::vector SerializedSkippedRanges;
+SerializedSkippedRanges.reserve(SkippedRanges.size());
+for (auto const& Range : SkippedRanges)
+  SerializedSkippedRanges.emplace_back(Range);
+
+using namespace llvm;
+BitCodeAbbrev *Abbrev = new BitCodeAbbrev();
+Abbrev->Add(BitCodeAbbrevOp(PPD_SKIPPED_RANGES));
+Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob));
+unsigned PPESkippedRangeAbbrev = Stream.EmitAbbrev(Abbrev);
+
+Record.clear();
+Record.push_back(PPD_SKIPPED_RANGES);
+Stream.EmitRecordWithBlob(PPESkippedRangeAbbrev, Record,
+  bytes(SerializedSkippedRanges));
+  }
 }
 
 unsigned ASTWriter::getLocalOrImportedSubmoduleID(Module *Mod) {
Index: lib/Serialization/ASTReader.cpp
===
--- lib/Serialization/ASTReader.cpp
+++ lib/Serialization/ASTReader.cpp
@@ -3020,6 +3020,24 @@
 
   break;
 }
+
+case PPD_SKIPPED_RANGES: {
+  F.PreprocessedSkippedRangeOffsets = (const PPSkippedRange*)Blob.data();
+  assert(Blob.size() % sizeof(PPSkippedRange) == 0);
+  F.NumPreprocessedSkippedRanges = Blob.size() / sizeof(PPSkippedRange);
+
+  if (!PP.getPreprocessingRecord())
+PP.createPreprocessingRecord();
+  if (!PP.getPreprocessingRecord()->getExternalSource())
+PP.getPreprocessingRecord()->SetExternalSource(*this);
+  F.BasePreprocessedSkippedRangeID = PP.getPreprocessingRecord()
+->allocateSkippedRanges(F.NumPreprocessedSkippedRanges);
+  
+  if (F.NumPreprocessedSkippedRanges > 0)
+GlobalSkippedRangeMap.insert(
+std::make_pair(F.BasePreprocessedSkippedRangeID, ));
+  break;
+}
 
 case DECL_UPDATE_OFFSETS: {
   if (Record.size() % 2 != 0) {
@@ -4874,6 +4892,20 @@
  Mod.FileSortedDecls + Mod.NumFileSortedDecls));
 }
 
+SourceRange ASTReader::ReadSkippedRange(unsigned GlobalIndex) {
+  auto I = GlobalSkippedRangeMap.find(GlobalIndex);
+  assert(I != GlobalSkippedRangeMap.end() &&
+"Corrupted global skipped range map");
+  ModuleFile *M = I->second;
+  unsigned LocalIndex = GlobalIndex - M->BasePreprocessedSkippedRangeID;
+  assert(LocalIndex < M->NumPreprocessedSkippedRanges);
+  PPSkippedRange RawRange = M->PreprocessedSkippedRangeOffsets[LocalIndex];
+  SourceRange Range(TranslateSourceLocation(*M, RawRange.getBegin()),
+TranslateSourceLocation(*M, RawRange.getEnd()));
+  assert(Range.isValid());
+  return Range;
+}
+
 PreprocessedEntity *ASTReader::ReadPreprocessedEntity(unsigned Index) {
   PreprocessedEntityID PPID = Index+1;
   std::pair PPInfo = getModulePreprocessedEntity(Index);
Index: lib/Lex/PreprocessingRecord.cpp
===
--- lib/Lex/PreprocessingRecord.cpp
+++ lib/Lex/PreprocessingRecord.cpp
@@ -40,7 +40,8 @@
 
 PreprocessingRecord::PreprocessingRecord(SourceManager )
   : SourceMgr(SM),
-

Re: [PATCH] D19941: [clang-tidy] FixItHint Tooling refactoring

2016-05-10 Thread Etienne Bergeron via cfe-commits
etienneb updated this revision to Diff 56781.
etienneb marked 3 inline comments as done.
etienneb added a comment.

add unittests


http://reviews.llvm.org/D19941

Files:
  include/clang/Tooling/Fixit.h
  lib/Tooling/CMakeLists.txt
  lib/Tooling/Fixit.cpp
  unittests/Tooling/CMakeLists.txt
  unittests/Tooling/FixitTest.cpp

Index: unittests/Tooling/FixitTest.cpp
===
--- /dev/null
+++ unittests/Tooling/FixitTest.cpp
@@ -0,0 +1,163 @@
+//===- unittest/Tooling/FixitTest.cpp ===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "TestVisitor.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Tooling/Fixit.h"
+
+using namespace clang;
+
+namespace {
+
+struct CallsVisitor : TestVisitor {
+  bool VisitCallExpr(CallExpr *Expr) {
+OnCall(Expr, Context);
+return true;
+  }
+
+  std::function OnCall;
+};
+
+std::string LocationToString(SourceLocation Loc, ASTContext *Context) {
+  return Loc.printToString(Context->getSourceManager());
+}
+
+TEST(FixitTest, getText) {
+  CallsVisitor Visitor;
+
+  Visitor.OnCall = [&](CallExpr *CE, ASTContext *Context) {
+EXPECT_EQ("foo(x, y)", tooling::fixit::getText(*CE, *Context));
+EXPECT_EQ("foo(x, y)",
+  tooling::fixit::getText(CE->getSourceRange(), *Context));
+
+Expr *P0 = CE->getArg(0);
+Expr *P1 = CE->getArg(1);
+EXPECT_EQ("x", tooling::fixit::getText(*P0, *Context));
+EXPECT_EQ("y", tooling::fixit::getText(*P1, *Context));
+  };
+  Visitor.runOver("void foo(int x, int y) { foo(x, y); }");
+
+  Visitor.OnCall = [&](CallExpr *CE, ASTContext *Context) {
+EXPECT_EQ("APPLY(foo, x, y)", tooling::fixit::getText(*CE, *Context));
+  };
+  Visitor.runOver("#define APPLY(f, x, y) f(x, y)\n"
+  "void foo(int x, int y) { APPLY(foo, x, y); }");
+}
+
+TEST(FixitTest, createRemoval) {
+  CallsVisitor Visitor;
+
+  Visitor.OnCall = [&](CallExpr *CE, ASTContext *Context) {
+FixItHint Hint = tooling::fixit::createRemoval(*CE);
+EXPECT_EQ("foo(x, y)",
+  tooling::fixit::getText(Hint.RemoveRange.getAsRange(), *Context));
+EXPECT_TRUE(Hint.InsertFromRange.isInvalid());
+EXPECT_TRUE(Hint.CodeToInsert.empty());
+
+Expr *P0 = CE->getArg(0);
+FixItHint Hint0 = tooling::fixit::createRemoval(*P0);
+EXPECT_EQ(
+"x", tooling::fixit::getText(Hint0.RemoveRange.getAsRange(), *Context));
+EXPECT_TRUE(Hint0.InsertFromRange.isInvalid());
+EXPECT_TRUE(Hint0.CodeToInsert.empty());
+
+Expr *P1 = CE->getArg(1);
+FixItHint Hint1 = tooling::fixit::createRemoval(*P1);
+EXPECT_EQ(
+"y", tooling::fixit::getText(Hint1.RemoveRange.getAsRange(), *Context));
+EXPECT_TRUE(Hint1.InsertFromRange.isInvalid());
+EXPECT_TRUE(Hint1.CodeToInsert.empty());
+  };
+  Visitor.runOver("void foo(int x, int y) { foo(x, y); }");
+}
+
+TEST(FixitTest, createRemovalWithMacro) {
+  CallsVisitor Visitor;
+
+  Visitor.OnCall = [&](CallExpr *CE, ASTContext *Context) {
+FixItHint Hint = tooling::fixit::createRemoval(*CE);
+EXPECT_EQ("FOO",
+  tooling::fixit::getText(Hint.RemoveRange.getAsRange(), *Context));
+EXPECT_TRUE(Hint.InsertFromRange.isInvalid());
+EXPECT_TRUE(Hint.CodeToInsert.empty());
+
+Expr *P0 = CE->getArg(0);
+FixItHint Hint0 = tooling::fixit::createRemoval(*P0);
+EXPECT_EQ("input.cc:2:26 

Re: [PATCH] D20119: [libunwind] Improve unwinder stack usage

2016-05-10 Thread Asiri Rathnayake via cfe-commits
rmaprath marked an inline comment as done.
rmaprath added a comment.

http://reviews.llvm.org/D20119



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


Re: [PATCH] D20119: [libunwind] Improve unwinder stack usage

2016-05-10 Thread Asiri Rathnayake via cfe-commits
rmaprath updated this revision to Diff 56779.
rmaprath added a comment.

Address review comments by @rengolin:

- Define and use `_LIBUNWIND_CONTEXT_SIZE` and `_LIBUNWIND_CURSOR_SIZE` macros


http://reviews.llvm.org/D20119

Files:
  CMakeLists.txt
  include/__libunwind_config.h
  include/libunwind.h
  src/CompactUnwinder.hpp
  src/Registers.hpp
  src/UnwindCursor.hpp
  src/config.h
  src/libunwind.cpp

Index: src/libunwind.cpp
===
--- src/libunwind.cpp
+++ src/libunwind.cpp
@@ -45,30 +45,30 @@
   _LIBUNWIND_TRACE_API("unw_init_local(cursor=%p, context=%p)\n",
static_cast(cursor),
static_cast(context));
-  // Use "placement new" to allocate UnwindCursor in the cursor buffer.
 #if defined(__i386__)
-  new ((void *)cursor) UnwindCursor(
- context, LocalAddressSpace::sThisAddressSpace);
+# define REGISTER_KIND Registers_x86
 #elif defined(__x86_64__)
-  new ((void *)cursor) UnwindCursor(
- context, LocalAddressSpace::sThisAddressSpace);
+# define REGISTER_KIND Registers_x86_64
 #elif defined(__ppc__)
-  new ((void *)cursor) UnwindCursor(
- context, LocalAddressSpace::sThisAddressSpace);
-#elif defined(__arm64__) || defined(__aarch64__)
-  new ((void *)cursor) UnwindCursor(
- context, LocalAddressSpace::sThisAddressSpace);
+# define REGISTER_KIND Registers_ppc
+#elif defined(__aarch64__)
+# define REGISTER_KIND Registers_arm64
 #elif _LIBUNWIND_ARM_EHABI
-  new ((void *)cursor) UnwindCursor(
- context, LocalAddressSpace::sThisAddressSpace);
+# define REGISTER_KIND Registers_arm
 #elif defined(__or1k__)
-  new ((void *)cursor) UnwindCursor(
- context, LocalAddressSpace::sThisAddressSpace);
+# define REGISTER_KIND Registers_or1k
 #elif defined(__mips__)
-#warning The MIPS architecture is not supported.
+# warning The MIPS architecture is not supported.
 #else
-#error Architecture not supported
+# error Architecture not supported
 #endif
+  static_assert(check_fit,
+unw_cursor_t>::does_fit,
+"Unwind cursor does not fit in unw_cursor_t");
+  // Use "placement new" to allocate UnwindCursor in the cursor buffer.
+  new ((void *)cursor) UnwindCursor(
+ context, LocalAddressSpace::sThisAddressSpace);
+#undef REGISTER_KIND
   AbstractUnwindCursor *co = (AbstractUnwindCursor *)cursor;
   co->setInfoBasedOnIPRegister();
 
Index: src/config.h
===
--- src/config.h
+++ src/config.h
@@ -119,5 +119,25 @@
   #define _LIBUNWIND_TRACING_UNWINDING logUnwinding()
 #endif
 
+#ifdef __cplusplus
+// Used to fit UnwindCursor and Registers_xxx types against unw_context_t /
+// unw_cursor_t sized memory blocks.
+#if defined(_LIBUNWIND_IS_NATIVE_ONLY)
+# define COMP_OP ==
+#else
+# define COMP_OP <
+#endif
+template 
+struct check_fit {
+  template 
+  struct blk_count {
+static const uint32_t count =
+  (sizeof(T) + sizeof(uint64_t) - 1) / sizeof(uint64_t);
+  };
+  static const bool does_fit =
+(blk_count<_Type>::count COMP_OP blk_count<_Mem>::count);
+};
+#undef COMP_OP
+#endif // __cplusplus
 
 #endif // LIBUNWIND_CONFIG_H
Index: src/UnwindCursor.hpp
===
--- src/UnwindCursor.hpp
+++ src/UnwindCursor.hpp
@@ -481,30 +481,39 @@
 return stepWithCompactEncoding(dummy);
   }
 
+#if defined(_LIBUNWIND_TARGET_X86_64)
   int stepWithCompactEncoding(Registers_x86_64 &) {
 return CompactUnwinder_x86_64::stepWithCompactEncoding(
 _info.format, _info.start_ip, _addressSpace, _registers);
   }
+#endif
 
+#if defined(_LIBUNWIND_TARGET_I386)
   int stepWithCompactEncoding(Registers_x86 &) {
 return CompactUnwinder_x86::stepWithCompactEncoding(
 _info.format, (uint32_t)_info.start_ip, _addressSpace, _registers);
   }
+#endif
 
+#if defined(_LIBUNWIND_TARGET_PPC)
   int stepWithCompactEncoding(Registers_ppc &) {
 return UNW_EINVAL;
   }
+#endif
 
+#if defined(_LIBUNWIND_TARGET_AARCH64)
   int stepWithCompactEncoding(Registers_arm64 &) {
 return CompactUnwinder_arm64::stepWithCompactEncoding(
 _info.format, _info.start_ip, _addressSpace, _registers);
   }
+#endif
 
   bool compactSaysUseDwarf(uint32_t *offset=NULL) const {
 R dummy;
 return compactSaysUseDwarf(dummy, offset);
   }
 
+#if defined(_LIBUNWIND_TARGET_X86_64)
   bool compactSaysUseDwarf(Registers_x86_64 &, uint32_t *offset) const {
 if ((_info.format 

r269100 - [VFS] Reconstruct the VFS overlay tree for more accurate lookup

2016-05-10 Thread Bruno Cardoso Lopes via cfe-commits
Author: bruno
Date: Tue May 10 13:43:00 2016
New Revision: 269100

URL: http://llvm.org/viewvc/llvm-project?rev=269100=rev
Log:
[VFS] Reconstruct the VFS overlay tree for more accurate lookup

The way we currently build the internal VFS overlay representation leads
to inefficient path search and might yield wrong answers when asked for
recursive or regular directory iteration.

Currently, when reading an YAML file, each YAML root entry is placed
inside a new root in the filesystem overlay. In the crash reproducer, a
simple "@import Foundation" currently maps to 43 roots, and when looking
up paths, we traverse a directory tree for each of these different
roots, until we find a match (or don't). This has two consequences:

- It's slow.
- Directory iteration gives incomplete results since it only return
results within one root - since contents of the same directory can be
declared inside different roots, the result isn't accurate.

This is in part fault of the way we currently write out the YAML file
when emitting the crash reproducer - we could generate only one root and
that would make it fast and correct again. However, we should not rely
on how the client writes the YAML, but provide a good internal
representation regardless.

This patch builds a proper virtual directory tree out of the YAML
representation, allowing faster search and proper iteration. Besides the
crash reproducer, this potentially benefits other VFS clients.

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

Modified: cfe/trunk/lib/Basic/VirtualFileSystem.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/VirtualFileSystem.cpp?rev=269100=269099=269100=diff
==
--- cfe/trunk/lib/Basic/VirtualFileSystem.cpp (original)
+++ cfe/trunk/lib/Basic/VirtualFileSystem.cpp Tue May 10 13:43:00 2016
@@ -719,7 +719,13 @@ public:
 Status S)
   : Entry(EK_Directory, Name), Contents(std::move(Contents)),
 S(std::move(S)) {}
+  RedirectingDirectoryEntry(StringRef Name, Status S)
+  : Entry(EK_Directory, Name), S(std::move(S)) {}
   Status getStatus() { return S; }
+  void addContent(std::unique_ptr Content) {
+Contents.push_back(std::move(Content));
+  }
+  Entry *getLastContent() const { return Contents.back().get(); }
   typedef decltype(Contents)::iterator iterator;
   iterator contents_begin() { return Contents.begin(); }
   iterator contents_end() { return Contents.end(); }
@@ -747,6 +753,7 @@ public:
 return UseName == NK_NotSet ? GlobalUseExternalName
 : (UseName == NK_External);
   }
+  NameKind getUseName() const { return UseName; }
   static bool classof(const Entry *E) { return E->getKind() == EK_File; }
 };
 
@@ -1023,6 +1030,70 @@ class RedirectingFileSystemParser {
 return true;
   }
 
+  Entry *lookupOrCreateEntry(RedirectingFileSystem *FS, StringRef Name,
+ Entry *ParentEntry = nullptr) {
+if (!ParentEntry) { // Look for a existent root
+  for (const std::unique_ptr  : FS->Roots) {
+if (Name.equals(Root->getName())) {
+  ParentEntry = Root.get();
+  return ParentEntry;
+}
+  }
+} else { // Advance to the next component
+  auto *DE = dyn_cast(ParentEntry);
+  for (std::unique_ptr  :
+   llvm::make_range(DE->contents_begin(), DE->contents_end())) {
+auto *DirContent = dyn_cast(Content.get());
+if (DirContent && Name.equals(Content->getName()))
+  return DirContent;
+  }
+}
+
+// ... or create a new one
+std::unique_ptr E = llvm::make_unique(
+Name, Status("", getNextVirtualUniqueID(), sys::TimeValue::now(), 0, 0,
+ 0, file_type::directory_file, sys::fs::all_all));
+
+if (!ParentEntry) { // Add a new root to the overlay
+  FS->Roots.push_back(std::move(E));
+  ParentEntry = FS->Roots.back().get();
+  return ParentEntry;
+}
+
+auto *DE = dyn_cast(ParentEntry);
+DE->addContent(std::move(E));
+return DE->getLastContent();
+  }
+
+  void uniqueOverlayTree(RedirectingFileSystem *FS, Entry *SrcE,
+ Entry *NewParentE = nullptr) {
+StringRef Name = SrcE->getName();
+switch (SrcE->getKind()) {
+case EK_Directory: {
+  auto *DE = dyn_cast(SrcE);
+  assert(DE && "Must be a directory");
+  // Empty directories could be present in the YAML as a way to
+  // describe a file for a current directory after some of its subdir
+  // is parsed. This only leads to redundant walks, ignore it.
+  if (!Name.empty())
+NewParentE = lookupOrCreateEntry(FS, Name, NewParentE);
+  for (std::unique_ptr  :
+   llvm::make_range(DE->contents_begin(), DE->contents_end()))
+uniqueOverlayTree(FS, SubEntry.get(), NewParentE);
+  break;
+}
+case EK_File: {
+  

r269099 - [WebAssembly] Reduce strictness of static destructor test

2016-05-10 Thread Derek Schuff via cfe-commits
Author: dschuff
Date: Tue May 10 13:35:31 2016
New Revision: 269099

URL: http://llvm.org/viewvc/llvm-project?rev=269099=rev
Log:
[WebAssembly] Reduce strictness of static destructor test

It didn't work on non-asserts builds

Modified:
cfe/trunk/test/CodeGenCXX/static-destructor.cpp

Modified: cfe/trunk/test/CodeGenCXX/static-destructor.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/static-destructor.cpp?rev=269099=269098=269099=diff
==
--- cfe/trunk/test/CodeGenCXX/static-destructor.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/static-destructor.cpp Tue May 10 13:35:31 2016
@@ -17,20 +17,17 @@ Foo global;
 
 // X86 destructors have void return, and are registered directly with 
__cxa_atexit.
 // X86: define internal void @__cxx_global_var_init()
-// X86-NEXT: entry:
-// X86-NEXT:   %0 = call i32 @__cxa_atexit(void (i8*)* bitcast (void 
(%class.Foo*)* @_ZN3FooD1Ev to void (i8*)*), i8* getelementptr inbounds 
(%class.Foo, %class.Foo* @global, i32 0, i32 0), i8* @__dso_handle)
+// X86:   call i32 @__cxa_atexit(void (i8*)* bitcast (void (%class.Foo*)* 
@_ZN3FooD1Ev to void (i8*)*), i8* getelementptr inbounds (%class.Foo, 
%class.Foo* @global, i32 0, i32 0), i8* @__dso_handle)
 
 // ARM destructors return this, but can be registered directly with 
__cxa_atexit
 // because the calling conventions tolerate the mismatch.
 // ARM: define internal void @__cxx_global_var_init()
-// ARM-NEXT: entry:
-// ARM-NEXT:   %0 = call i32 @__cxa_atexit(void (i8*)* bitcast (%class.Foo* 
(%class.Foo*)* @_ZN3FooD1Ev to void (i8*)*), i8* getelementptr inbounds 
(%class.Foo, %class.Foo* @global, i32 0, i32 0), i8* @__dso_handle)
+// ARM:   call i32 @__cxa_atexit(void (i8*)* bitcast (%class.Foo* 
(%class.Foo*)* @_ZN3FooD1Ev to void (i8*)*), i8* getelementptr inbounds 
(%class.Foo, %class.Foo* @global, i32 0, i32 0), i8* @__dso_handle)
 
 // Wasm destructors return this, and use a wrapper function, which is 
registered
 // with __cxa_atexit.
 // WASM: define internal void @__cxx_global_var_init()
-// WASM-NEXT: entry:
-// WASM-NEXT: %0 = call i32 @__cxa_atexit(void (i8*)* 
@__cxx_global_array_dtor, i8* null, i8* @__dso_handle)
+// WASM: call i32 @__cxa_atexit(void (i8*)* @__cxx_global_array_dtor, i8* 
null, i8* @__dso_handle)
 
 // WASM: define internal void @__cxx_global_array_dtor(i8*)
 // WASM: %call = call %class.Foo* @_ZN3FooD1Ev(%class.Foo* @global)


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


RE: [Clang] Convergent Attribute

2016-05-10 Thread Anastasia Stulova via cfe-commits
> CUDA? In any case, I don't see how the restriction helps users, and the 
> attribute at the IR level has a well-defined meaning regardless. If a user 
> were to have a use case, they'd simply find the restriction arbitrary and 
> frustrating.

Yes, CUDA was already considered as well. I just think that compilers should 
help to reduce amount of erroneous or meaningless use cases. That's one of the 
reasons to have language options for the attributes. But I don't feel strongly 
about this particular case anyways. So let's make it language independent then. 
;)

Anastasia 
 
-Original Message-
From: Hal Finkel [mailto:hfin...@anl.gov] 
Sent: 10 May 2016 00:33
To: Anastasia Stulova
Cc: nd; Clang Commits; Matt Arsenault; Ettore Speziale; Aaron Ballman
Subject: Re: [Clang] Convergent Attribute

- Original Message -
> From: "Anastasia Stulova via cfe-commits" 
> To: "Matt Arsenault" , "Ettore Speziale" 
> , "Aaron Ballman"
> 
> Cc: "nd" , "Clang Commits" 
> Sent: Monday, May 9, 2016 12:39:19 PM
> Subject: RE: [Clang] Convergent Attribute
> 
> Since it's not a part of any official spec we could of course make it 
> accepted with anything.
> 
> Just out of curiosity what other programming models supported by Clang 
> do you think this attribute would be useful for?
> 
> Anastasia
> 
> -Original Message-
> From: Matt Arsenault [mailto:matthew.arsena...@amd.com]
> Sent: 07 May 2016 00:37
> To: Anastasia Stulova; Ettore Speziale; Aaron Ballman
> Cc: nd; Clang Commits
> Subject: Re: [Clang] Convergent Attribute
> 
> On 05/06/2016 12:11 PM, Anastasia Stulova via cfe-commits wrote:
> > I was just wondering whether it would make sense to restrict the 
> > usage of the attribute to OpenCL language i.e. to add  "let LangOpts 
> > = [OpenCL];" in the attribute definition.
> This seems to be a pointless arbitrary restriction to me
> 
> -Matt
> 
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
> 

--
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory

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


Re: [PATCH] D20090: [OPENCL] Fix wrongly vla error for OpenCL array.

2016-05-10 Thread Anastasia Stulova via cfe-commits
Anastasia added inline comments.


Comment at: lib/Sema/SemaType.cpp:2055
@@ -2054,3 +2054,3 @@
 
-  return S.VerifyIntegerConstantExpression(ArraySize, , Diagnoser,
-   S.LangOpts.GNUMode).isInvalid();
+  return S
+  .VerifyIntegerConstantExpression(ArraySize, , Diagnoser,

Formatting looks weird though... may be better to leave the first line 
unmodified?


Comment at: test/CodeGenOpenCL/vla.cl:1
@@ +1,2 @@
+// RUN: %clang_cc1 -emit-llvm -O0 -o - %s | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm -O0 -cl-std=CL2.0 -DCL20 -o - %s | FileCheck %s 
--check-prefix=CL20

Could we have a Sema test instead where we accept the VLA if constant AS object 
is used and give an error otherwise?

Also do we need to test CL2.0? We don't seem to be doing anything different for 
that version. 


http://reviews.llvm.org/D20090



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


Re: [PATCH] D19920: [libunwind][ARM] Improve unwinder stack usage on baremetal targets - part 1

2016-05-10 Thread Asiri Rathnayake via cfe-commits
rmaprath abandoned this revision.
rmaprath added a comment.

Superseded by http://reviews.llvm.org/D20119.


http://reviews.llvm.org/D19920



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


Re: [PATCH] D15674: [CodeGen] Fix assignments of inline layouts into the byref structure

2016-05-10 Thread Vedant Kumar via cfe-commits
vsk accepted this revision.
vsk added a reviewer: vsk.
vsk added a comment.
This revision is now accepted and ready to land.

Closing old review.


http://reviews.llvm.org/D15674



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


Re: [PATCH] D17299: [ASTReader] Report error when accessing corrupt record data

2016-05-10 Thread Vedant Kumar via cfe-commits
vsk abandoned this revision.
vsk added a comment.

I haven't measured the performance overhead of this patch. The failure case 
it's supposed to protect against is exceptionally rare: we haven't seen it 
happen again. In light of that, and considering that it clutters up the code, I 
think it'd be better to drop this.


http://reviews.llvm.org/D17299



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


Re: [PATCH] D20119: [libunwind] Improve unwinder stack usage

2016-05-10 Thread Renato Golin via cfe-commits
rengolin added a subscriber: rengolin.


Comment at: include/libunwind.h:49
@@ -48,1 +48,3 @@
 struct unw_context_t {
+#if defined(_LIBUNWIND_NATIVE_I386)
+  uint64_t data[8];

Wouldn't it be a lot simpler to just define a macro _LIBUNWIND_DATA_SIZE or 
something and have just one line here?


http://reviews.llvm.org/D20119



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


Re: [PATCH] D19932: [OpenCL] Add to_{global|local|private} builtin functions.

2016-05-10 Thread Anastasia Stulova via cfe-commits
Anastasia added inline comments.


Comment at: lib/Sema/SemaChecking.cpp:480
@@ +479,3 @@
+  .getQualifiers().getAddressSpace() == LangAS::opencl_constant) {
+S.Diag(Call->getLocStart(), diag::err_opencl_builtin_to_addr_invalid_arg)
+<< Call->getArg(0) << Call->getDirectCallee() << 
Call->getSourceRange();

I just think you should simply set the function return type in line 487 based 
on the passed argument type (RT variable declared in line 477).

The situation I would like us to avoid here is:

  int *gptr = ..;
  local char *lptr = to_local(gptr);

So basically by calling to_local here, we allow conversion of not only an 
address space but also an underlaying type silently. 

Could we add a test to make sure it's not happening.

Similarly, in C such situations trigger a warning:
  int *ptr1 = ...;
  char *ptr2 = ptr1; //expected-warning{{incompatible pointer types 
initializing 'char *' with an expression of type 'int *'}}


Comment at: lib/Sema/SemaExpr.cpp:5222-5232
@@ -5166,6 +5221,13 @@
 if (FDecl && FDecl->getBuiltinID()) {
-  // Rewrite the function decl for this builtin by replacing parameters
-  // with no explicit address space with the address space of the arguments
-  // in ArgExprs.
-  if ((FDecl = rewriteBuiltinFunctionDecl(this, Context, FDecl, 
ArgExprs))) {
+  // Rewrite the function decl for OpenCL to_addr builtin.
+  if (FunctionDecl *NFDecl = rewriteBuiltinFunctionDeclForOpenCLToAddr(
+this, Context, FDecl, ArgExprs))
+FDecl = NFDecl;
+  else {
+// Rewrite the function decl for this builtin by replacing parameters
+// with no explicit address space with the address space of the 
arguments
+// in ArgExprs.
+FDecl = rewriteBuiltinFunctionDecl(this, Context, FDecl, ArgExprs);
+  }
+  if (FDecl) {
 NDecl = FDecl;

I still don't understand the issue here. 

In my understanding what we need is:
(1) Check the passed argument is a pointer and set return type to match the 
pointer type exactly in SemaChecking.
(2) Generate to_addr with i8* in CGBuiltin casting to and back the original 
type used in the call.

See my comment to lib/Sema/SemaChecking.cpp. I think if you set the return type 
there correctly we won't need this change here.


Comment at: test/SemaOpenCL/to_addr_builtin.cl:21
@@ +20,3 @@
+#else
+  // expected-error@-4{{invalid argument x to function: 'to_global'}}
+#endif

May be we could add here something like: expecting a generic pointer type...


http://reviews.llvm.org/D19932



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


[PATCH] D20119: [libunwind] Improve unwinder stack usage

2016-05-10 Thread Asiri Rathnayake via cfe-commits
rmaprath created this revision.
rmaprath added reviewers: jroelofs, bcraig.
rmaprath added a subscriber: cfe-commits.
Herald added a subscriber: aemerson.

This patch is generalization of D19920.

A new native-only libunwind variant (selectable through the 
`-DLIBUNWIND_ENABLE_CROSS_UNWINDING=OFF` cmake option) is introduced.

The sizes of `Registers_xxx` and the associated `UnwindCursor` types are 
strictly matched against the sizes of `unw_context_t` and `unw_cursor_t` types 
defined in `libunwind.h`. This improves the stack usage of the unwinder as only 
just enough memory is reserved on the stack to hold the virtual register set on 
each architecture.

I've locally tested ARM, AArch64, x86 (compile-only) and x86_64 builds of the 
new variant. Could not perform any testing of PPC or OpenRISK as I couldn't get 
the cross compiling to work properly. Ideally, we'll have to switch the 
buildbots to use this new build option so that these strict limits will be 
validated (and will continue to be validated). This should be fine I think - if 
the library works for these restricted limits, the general case (which supports 
cross-unwinding) should follow.

If the patch is accepted, I will start a discussion on cfe-dev about switching 
the buildbot configurations (I'm assuming we don't have any cross-unwinding 
buildbot setups). 
 

http://reviews.llvm.org/D20119

Files:
  CMakeLists.txt
  include/__libunwind_config.h
  include/libunwind.h
  src/CompactUnwinder.hpp
  src/Registers.hpp
  src/UnwindCursor.hpp
  src/config.h
  src/libunwind.cpp

Index: src/libunwind.cpp
===
--- src/libunwind.cpp
+++ src/libunwind.cpp
@@ -45,30 +45,30 @@
   _LIBUNWIND_TRACE_API("unw_init_local(cursor=%p, context=%p)\n",
static_cast(cursor),
static_cast(context));
-  // Use "placement new" to allocate UnwindCursor in the cursor buffer.
 #if defined(__i386__)
-  new ((void *)cursor) UnwindCursor(
- context, LocalAddressSpace::sThisAddressSpace);
+# define REGISTER_KIND Registers_x86
 #elif defined(__x86_64__)
-  new ((void *)cursor) UnwindCursor(
- context, LocalAddressSpace::sThisAddressSpace);
+# define REGISTER_KIND Registers_x86_64
 #elif defined(__ppc__)
-  new ((void *)cursor) UnwindCursor(
- context, LocalAddressSpace::sThisAddressSpace);
-#elif defined(__arm64__) || defined(__aarch64__)
-  new ((void *)cursor) UnwindCursor(
- context, LocalAddressSpace::sThisAddressSpace);
+# define REGISTER_KIND Registers_ppc
+#elif defined(__aarch64__)
+# define REGISTER_KIND Registers_arm64
 #elif _LIBUNWIND_ARM_EHABI
-  new ((void *)cursor) UnwindCursor(
- context, LocalAddressSpace::sThisAddressSpace);
+# define REGISTER_KIND Registers_arm
 #elif defined(__or1k__)
-  new ((void *)cursor) UnwindCursor(
- context, LocalAddressSpace::sThisAddressSpace);
+# define REGISTER_KIND Registers_or1k
 #elif defined(__mips__)
-#warning The MIPS architecture is not supported.
+# warning The MIPS architecture is not supported.
 #else
-#error Architecture not supported
+# error Architecture not supported
 #endif
+  static_assert(check_fit,
+unw_cursor_t>::does_fit,
+"Unwind cursor does not fit in unw_cursor_t");
+  // Use "placement new" to allocate UnwindCursor in the cursor buffer.
+  new ((void *)cursor) UnwindCursor(
+ context, LocalAddressSpace::sThisAddressSpace);
+#undef REGISTER_KIND
   AbstractUnwindCursor *co = (AbstractUnwindCursor *)cursor;
   co->setInfoBasedOnIPRegister();
 
Index: src/config.h
===
--- src/config.h
+++ src/config.h
@@ -131,5 +131,25 @@
   #define _LIBUNWIND_TRACING_UNWINDING logUnwinding()
 #endif
 
+#ifdef __cplusplus
+// Used to fit UnwindCursor and Registers_xxx types against unw_context_t /
+// unw_cursor_t sized memory blocks.
+#if defined(_LIBUNWIND_IS_NATIVE_ONLY)
+# define COMP_OP == 
+#else
+# define COMP_OP <
+#endif
+template 
+struct check_fit {
+  template 
+  struct blk_count {
+static const uint32_t count =
+  (sizeof(T) + sizeof(uint64_t) - 1) / sizeof(uint64_t);
+  };
+  static const bool does_fit =
+(blk_count<_Type>::count COMP_OP blk_count<_Mem>::count);
+};
+#undef COMP_OP
+#endif // __cplusplus
 
 #endif // LIBUNWIND_CONFIG_H
Index: src/UnwindCursor.hpp
===
--- src/UnwindCursor.hpp
+++ src/UnwindCursor.hpp
@@ 

[PATCH] D20118: Add support for injected class names and constructor initializers in C++

2016-05-10 Thread Sean Callanan via cfe-commits
spyffe created this revision.
spyffe added reviewers: sepavloff, a.sidorin.
spyffe added a subscriber: cfe-commits.
spyffe set the repository for this revision to rL LLVM.
spyffe added a project: clang-c.

The AST importer currently does not handle injected class names properly (it 
does not bind their types to the type of the parent as the parser would) and it 
doesn't handle constructor initializers at all.

This patch adds support for both of those, and also forwards the `isImplicit()` 
and `isReferenced()` flags for *all* declarations as is done for `isUsed()`.

Repository:
  rL LLVM

http://reviews.llvm.org/D20118

Files:
  include/clang/AST/ASTImporter.h
  lib/AST/ASTImporter.cpp

Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -2820,6 +2820,18 @@
   return nullptr;
 D2CXX->setLambdaMangling(DCXX->getLambdaManglingNumber(),
  CDecl);
+  } else if (DCXX->isInjectedClassName()) {
+// We have to be careful to do a similar dance to the one in
+// Sema::ActOnStartCXXMemberDeclarations
+CXXRecordDecl * const PrevDecl = nullptr;
+const bool DelayTypeCreation = true;
+D2CXX = CXXRecordDecl::Create(Importer.getToContext(),
+  D->getTagKind(),
+  DC, StartLoc, Loc,
+  Name.getAsIdentifierInfo(),
+  PrevDecl, DelayTypeCreation);
+Importer.getToContext().getTypeDeclType(
+D2CXX, llvm::dyn_cast(DC));
   } else {
 D2CXX = CXXRecordDecl::Create(Importer.getToContext(),
   D->getTagKind(),
@@ -3018,6 +3030,22 @@
 D->isInlineSpecified(), 
 D->isImplicit(),
 D->isConstexpr());
+SmallVector CtorInitializers;
+for (const CXXCtorInitializer *I : FromConstructor->inits()) {
+  CXXCtorInitializer *ToI =cast_or_null(
+  Importer.Import(I));
+  if (!ToI && I)
+return nullptr;
+  CtorInitializers.push_back(ToI);
+}
+if (unsigned NumInitializers = CtorInitializers.size()) {
+  CXXCtorInitializer **Memory = new (Importer.getToContext())
+ CXXCtorInitializer*[NumInitializers];
+  std::copy(CtorInitializers.begin(), CtorInitializers.end(), Memory);
+  llvm::cast(ToFunction)->setCtorInitializers(Memory);
+  llvm::cast(ToFunction)->setNumCtorInitializers(
+  NumInitializers);
+}
   } else if (isa(D)) {
 ToFunction = CXXDestructorDecl::Create(Importer.getToContext(),
cast(DC),
@@ -6344,6 +6372,58 @@
   return ToID;
 }
 
+CXXCtorInitializer *ASTImporter::Import(const CXXCtorInitializer *From) {
+  Expr *ToExpr = Import(From->getInit());
+  if (!ToExpr && From->getInit())
+return nullptr;
+
+  if (From->isBaseInitializer()) {
+TypeSourceInfo *ToTInfo = Import(From->getTypeSourceInfo());
+if (!ToTInfo && From->getTypeSourceInfo())
+  return nullptr;
+
+return new (ToContext)
+  CXXCtorInitializer(ToContext, ToTInfo, From->isBaseVirtual(),
+ Import(From->getLParenLoc()), ToExpr, 
+ Import(From->getRParenLoc()),
+ From->isPackExpansion() ?
+ Import(From->getEllipsisLoc()) : SourceLocation());
+  } else if (From->isMemberInitializer()) {
+FieldDecl *ToField = llvm::cast_or_null(Import(
+From->getMember()));
+if (!ToField && From->getMember())
+  return nullptr;
+
+return new (ToContext)
+CXXCtorInitializer(ToContext, ToField,
+   Import(From->getMemberLocation()),
+   Import(From->getLParenLoc()), ToExpr, 
+   Import(From->getRParenLoc()));
+  } else if (From->isIndirectMemberInitializer()) {
+IndirectFieldDecl *ToIField = llvm::cast_or_null(Import(
+From->getIndirectMember()));
+if (!ToIField && From->getIndirectMember())
+  return nullptr;
+
+return new (ToContext)
+CXXCtorInitializer(ToContext, ToIField,
+   Import(From->getMemberLocation()),
+   Import(From->getLParenLoc()), ToExpr, 
+   Import(From->getRParenLoc()));
+  } else if (From->isDelegatingInitializer()) {
+TypeSourceInfo *ToTInfo = Import(From->getTypeSourceInfo());
+if (!ToTInfo && From->getTypeSourceInfo())
+  return nullptr;
+
+return new (ToContext)
+CXXCtorInitializer(ToContext, ToTInfo,
+   Import(From->getLParenLoc()), ToExpr,
+   Import(From->getRParenLoc()));
+  } else {

Re: [PATCH] D19275: Do not register incompatible C++ destructors with __cxa_atexit

2016-05-10 Thread Derek Schuff via cfe-commits
dschuff added a comment.

Well, I forgot to squash my local commits before landing, so this is r269085 
thru r269089 :(


Repository:
  rL LLVM

http://reviews.llvm.org/D19275



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


Re: [PATCH] D19275: Do not register incompatible C++ destructors with __cxa_atexit

2016-05-10 Thread Derek Schuff via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL269085: Do not register incompatible C++ destructors with 
__cxa_atexit (authored by dschuff).

Changed prior to commit:
  http://reviews.llvm.org/D19275?vs=56218=56751#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D19275

Files:
  cfe/trunk/lib/CodeGen/CGDeclCXX.cpp
  cfe/trunk/test/CodeGenCXX/runtimecc.cpp
  cfe/trunk/test/CodeGenCXX/static-destructor.cpp

Index: cfe/trunk/test/CodeGenCXX/static-destructor.cpp
===
--- cfe/trunk/test/CodeGenCXX/static-destructor.cpp
+++ cfe/trunk/test/CodeGenCXX/static-destructor.cpp
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 %s -triple=x86_64-pc-linux -emit-llvm -o - | FileCheck 
--check-prefix=X86 %s
+// RUN: %clang_cc1 %s -triple=wasm32 -emit-llvm -o - | FileCheck 
--check-prefix=WASM %s
+
+// Test that destructors are not passed directly to __cxa_atexit when their
+// signatures do not match the type of its first argument.
+// e.g. ARM and WebAssembly have destructors that return this instead of void.
+
+
+class Foo {
+ public:
+  ~Foo() {
+  }
+};
+
+Foo global;
+
+// X86 destructors have void return, and are registered directly with 
__cxa_atexit.
+// X86: define internal void @__cxx_global_var_init()
+// X86-NEXT: entry:
+// X86-NEXT:   %0 = call i32 @__cxa_atexit(void (i8*)* bitcast (void 
(%class.Foo*)* @_ZN3FooD1Ev to void (i8*)*), i8* getelementptr inbounds 
(%class.Foo, %class.Foo* @global, i32 0, i32 0), i8* @__dso_handle)
+
+// Wasm destructors return this, and use a wrapper function, which is 
registered
+// with __cxa_atexit.
+// WASM: define internal void @__cxx_global_var_init()
+// WASM-NEXT: entry:
+// WASM-NEXT: %0 = call i32 @__cxa_atexit(void (i8*)* 
@__cxx_global_array_dtor, i8* null, i8* @__dso_handle)
+
+// WASM: define internal void @__cxx_global_array_dtor(i8*)
+// WASM: %call = call %class.Foo* @_ZN3FooD1Ev(%class.Foo* @global)
Index: cfe/trunk/test/CodeGenCXX/runtimecc.cpp
===
--- cfe/trunk/test/CodeGenCXX/runtimecc.cpp
+++ cfe/trunk/test/CodeGenCXX/runtimecc.cpp
@@ -22,8 +22,13 @@
   A global;
 // CHECK-LABEL:define internal void @__cxx_global_var_init()
 // CHECK:  call [[A]]* @_ZN5test01AC1Ev([[A]]* @_ZN5test06globalE)
-// CHECK-NEXT: call i32 @__cxa_atexit(void (i8*)* bitcast ([[A]]* ([[A]]*)* 
@_ZN5test01AD1Ev to void (i8*)*), i8* bitcast ([[A]]* @_ZN5test06globalE to 
i8*), i8* @__dso_handle) [[NOUNWIND:#[0-9]+]]
+// CHECK-NEXT: call i32 @__cxa_atexit(void (i8*)* @__cxx_global_array_dtor, 
i8* null, i8* @__dso_handle) [[NOUNWIND:#[0-9]+]]
 // CHECK-NEXT: ret void
+
+// CHECK-LABEL: define internal void @__cxx_global_array_dtor(i8*)
+// CHECK: call [[A]]* @_ZN5test01AD1Ev([[A]]* @_ZN5test06globalE)
+// CHECK-NEXT: ret void
+
 }
 
 // CHECK: declare i32 @__cxa_atexit(void (i8*)*, i8*, i8*) [[NOUNWIND]]
Index: cfe/trunk/lib/CodeGen/CGDeclCXX.cpp
===
--- cfe/trunk/lib/CodeGen/CGDeclCXX.cpp
+++ cfe/trunk/lib/CodeGen/CGDeclCXX.cpp
@@ -86,11 +86,15 @@
   llvm::Constant *function;
   llvm::Constant *argument;
 
-  // Special-case non-array C++ destructors, where there's a function
-  // with the right signature that we can just call.
+  // Special-case non-array C++ destructors, if they have the right signature
+  // that can be directly registered with __cxa_atexit. If __cxa_atexit is
+  // disabled via a flag, a different helper function is generated anyway.
   const CXXRecordDecl *record = nullptr;
   if (dtorKind == QualType::DK_cxx_destructor &&
-  (record = type->getAsCXXRecordDecl())) {
+  (record = type->getAsCXXRecordDecl()) &&
+  (!CGM.getCXXABI().HasThisReturn(
+  GlobalDecl(record->getDestructor(), Dtor_Complete)) ||
+   !CGM.getCodeGenOpts().CXAAtExit)) {
 assert(!record->hasTrivialDestructor());
 CXXDestructorDecl *dtor = record->getDestructor();
 


Index: cfe/trunk/test/CodeGenCXX/static-destructor.cpp
===
--- cfe/trunk/test/CodeGenCXX/static-destructor.cpp
+++ cfe/trunk/test/CodeGenCXX/static-destructor.cpp
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 %s -triple=x86_64-pc-linux -emit-llvm -o - | FileCheck --check-prefix=X86 %s
+// RUN: %clang_cc1 %s -triple=wasm32 -emit-llvm -o - | FileCheck --check-prefix=WASM %s
+
+// Test that destructors are not passed directly to __cxa_atexit when their
+// signatures do not match the type of its first argument.
+// e.g. ARM and WebAssembly have destructors that return this instead of void.
+
+
+class Foo {
+ public:
+  ~Foo() {
+  }
+};
+
+Foo global;
+
+// X86 destructors have void return, and are registered directly with __cxa_atexit.
+// X86: define internal void @__cxx_global_var_init()
+// X86-NEXT: entry:
+// X86-NEXT:   %0 = call i32 @__cxa_atexit(void (i8*)* bitcast (void (%class.Foo*)* @_ZN3FooD1Ev to 

r269089 - Introduce CGCXXABI::canCallMismatchedFunctionType

2016-05-10 Thread Derek Schuff via cfe-commits
Author: dschuff
Date: Tue May 10 12:44:55 2016
New Revision: 269089

URL: http://llvm.org/viewvc/llvm-project?rev=269089=rev
Log:
Introduce CGCXXABI::canCallMismatchedFunctionType

Modified:
cfe/trunk/lib/CodeGen/CGCXXABI.h
cfe/trunk/lib/CodeGen/CGDeclCXX.cpp
cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
cfe/trunk/test/CodeGenCXX/runtimecc.cpp
cfe/trunk/test/CodeGenCXX/static-destructor.cpp

Modified: cfe/trunk/lib/CodeGen/CGCXXABI.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCXXABI.h?rev=269089=269088=269089=diff
==
--- cfe/trunk/lib/CodeGen/CGCXXABI.h (original)
+++ cfe/trunk/lib/CodeGen/CGCXXABI.h Tue May 10 12:44:55 2016
@@ -106,6 +106,16 @@ public:
 
   virtual bool hasMostDerivedReturn(GlobalDecl GD) const { return false; }
 
+  /// Returns true if the target allows calling a function through a pointer
+  /// with a different signature than the actual function (or equivalently,
+  /// bitcasting a function or function pointer to a different function type).
+  /// In principle in the most general case this could depend on the target, 
the
+  /// calling convention, and the actual types of the arguments and return
+  /// value. Here it just means whether the signature mismatch could *ever* be
+  /// allowed; in other words, does the target do strict checking of signatures
+  /// for all calls.
+  virtual bool canCallMismatchedFunctionType() const { return true; }
+
   /// If the C++ ABI requires the given type be returned in a particular way,
   /// this method sets RetAI and returns true.
   virtual bool classifyReturnType(CGFunctionInfo ) const = 0;

Modified: cfe/trunk/lib/CodeGen/CGDeclCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDeclCXX.cpp?rev=269089=269088=269089=diff
==
--- cfe/trunk/lib/CodeGen/CGDeclCXX.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDeclCXX.cpp Tue May 10 12:44:55 2016
@@ -88,11 +88,12 @@ static void EmitDeclDestroy(CodeGenFunct
 
   // Special-case non-array C++ destructors, if they have the right signature.
   // Under some ABIs, destructors return this instead of void, and cannot be
-  // passed directly to __cxa_atexit.
+  // passed directly to __cxa_atexit if the target does not allow this 
mismatch.
   const CXXRecordDecl *Record = type->getAsCXXRecordDecl();
-  bool CanRegisterDestructor = Record &&
-   !CGM.getCXXABI().HasThisReturn(GlobalDecl(
-   Record->getDestructor(), Dtor_Complete));
+  bool CanRegisterDestructor =
+  Record && (!CGM.getCXXABI().HasThisReturn(
+ GlobalDecl(Record->getDestructor(), Dtor_Complete)) ||
+ CGM.getCXXABI().canCallMismatchedFunctionType());
   // If __cxa_atexit is disabled via a flag, a different helper function is
   // generated elsewhere which uses atexit instead, and it takes the destructor
   // directly.

Modified: cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp?rev=269089=269088=269089=diff
==
--- cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp (original)
+++ cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp Tue May 10 12:44:55 2016
@@ -443,6 +443,7 @@ private:
(isa(GD.getDecl()) &&
 GD.getDtorType() != Dtor_Deleting);
   }
+  bool canCallMismatchedFunctionType() const override { return false; }
 };
 }
 

Modified: cfe/trunk/test/CodeGenCXX/runtimecc.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/runtimecc.cpp?rev=269089=269088=269089=diff
==
--- cfe/trunk/test/CodeGenCXX/runtimecc.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/runtimecc.cpp Tue May 10 12:44:55 2016
@@ -22,13 +22,8 @@ namespace test0 {
   A global;
 // CHECK-LABEL:define internal void @__cxx_global_var_init()
 // CHECK:  call [[A]]* @_ZN5test01AC1Ev([[A]]* @_ZN5test06globalE)
-// CHECK-NEXT: call i32 @__cxa_atexit(void (i8*)* @__cxx_global_array_dtor, 
i8* null, i8* @__dso_handle) [[NOUNWIND:#[0-9]+]]
+// CHECK-NEXT: call i32 @__cxa_atexit(void (i8*)* bitcast ([[A]]* ([[A]]*)* 
@_ZN5test01AD1Ev to void (i8*)*), i8* bitcast ([[A]]* @_ZN5test06globalE to 
i8*), i8* @__dso_handle) [[NOUNWIND:#[0-9]+]]
 // CHECK-NEXT: ret void
-
-// CHECK-LABEL: define internal void @__cxx_global_array_dtor(i8*)
-// CHECK: call [[A]]* @_ZN5test01AD1Ev([[A]]* @_ZN5test06globalE)
-// CHECK-NEXT: ret void
-
 }
 
 // CHECK: declare i32 @__cxa_atexit(void (i8*)*, i8*, i8*) [[NOUNWIND]]

Modified: cfe/trunk/test/CodeGenCXX/static-destructor.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/static-destructor.cpp?rev=269089=269088=269089=diff

r269088 - more cleanup

2016-05-10 Thread Derek Schuff via cfe-commits
Author: dschuff
Date: Tue May 10 12:44:52 2016
New Revision: 269088

URL: http://llvm.org/viewvc/llvm-project?rev=269088=rev
Log:
more cleanup

Modified:
cfe/trunk/lib/CodeGen/CGDeclCXX.cpp

Modified: cfe/trunk/lib/CodeGen/CGDeclCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDeclCXX.cpp?rev=269088=269087=269088=diff
==
--- cfe/trunk/lib/CodeGen/CGDeclCXX.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDeclCXX.cpp Tue May 10 12:44:52 2016
@@ -86,8 +86,9 @@ static void EmitDeclDestroy(CodeGenFunct
   llvm::Constant *function;
   llvm::Constant *argument;
 
-  // Special-case non-array C++ destructors, if they have the right signature
-  // that can be directly registered with __cxa_atexit.
+  // Special-case non-array C++ destructors, if they have the right signature.
+  // Under some ABIs, destructors return this instead of void, and cannot be
+  // passed directly to __cxa_atexit.
   const CXXRecordDecl *Record = type->getAsCXXRecordDecl();
   bool CanRegisterDestructor = Record &&
!CGM.getCXXABI().HasThisReturn(GlobalDecl(
@@ -96,8 +97,7 @@ static void EmitDeclDestroy(CodeGenFunct
   // generated elsewhere which uses atexit instead, and it takes the destructor
   // directly.
   bool UsingExternalHelper = !CGM.getCodeGenOpts().CXAAtExit;
-  if (Record &&
-  (CanRegisterDestructor || UsingExternalHelper)) {
+  if (Record && (CanRegisterDestructor || UsingExternalHelper)) {
 assert(!Record->hasTrivialDestructor());
 CXXDestructorDecl *dtor = Record->getDestructor();
 


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


r269085 - Do not register incompatible C++ destructors with __cxa_atexit

2016-05-10 Thread Derek Schuff via cfe-commits
Author: dschuff
Date: Tue May 10 12:44:46 2016
New Revision: 269085

URL: http://llvm.org/viewvc/llvm-project?rev=269085=rev
Log:
Do not register incompatible C++ destructors with __cxa_atexit

Summary:
For a static object with a nontrivial destructor, clang generates an
initializer function (__cxx_global_var_init) which registers that
object's destructor using __cxa_atexit. However some ABIs (ARM,
WebAssembly) use destructors that return 'this' instead of having void
return (which does not match the signature of function pointers passed
to __cxa_atexit). This results in undefined behavior when the destructors are
called. All the calling conventions I know of on ARM can tolerate this,
but WebAssembly requires the signatures of indirect calls to match the
called function.

This patch disables that direct registration of destructors for ABIs
that have this-returning destructors.

Subscribers: aemerson, jfb, cfe-commits, dschuff

Differential Revision: http://reviews.llvm.org/D19275

Added:
cfe/trunk/test/CodeGenCXX/static-destructor.cpp
Modified:
cfe/trunk/lib/CodeGen/CGDeclCXX.cpp
cfe/trunk/test/CodeGenCXX/runtimecc.cpp

Modified: cfe/trunk/lib/CodeGen/CGDeclCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDeclCXX.cpp?rev=269085=269084=269085=diff
==
--- cfe/trunk/lib/CodeGen/CGDeclCXX.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDeclCXX.cpp Tue May 10 12:44:46 2016
@@ -86,11 +86,15 @@ static void EmitDeclDestroy(CodeGenFunct
   llvm::Constant *function;
   llvm::Constant *argument;
 
-  // Special-case non-array C++ destructors, where there's a function
-  // with the right signature that we can just call.
+  // Special-case non-array C++ destructors, if they have the right signature
+  // that can be directly registered with __cxa_atexit. If __cxa_atexit is
+  // disabled via a flag, a different helper function is generated anyway.
   const CXXRecordDecl *record = nullptr;
   if (dtorKind == QualType::DK_cxx_destructor &&
-  (record = type->getAsCXXRecordDecl())) {
+  (record = type->getAsCXXRecordDecl()) &&
+  (!CGM.getCXXABI().HasThisReturn(
+  GlobalDecl(record->getDestructor(), Dtor_Complete)) ||
+   !CGM.getCodeGenOpts().CXAAtExit)) {
 assert(!record->hasTrivialDestructor());
 CXXDestructorDecl *dtor = record->getDestructor();
 

Modified: cfe/trunk/test/CodeGenCXX/runtimecc.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/runtimecc.cpp?rev=269085=269084=269085=diff
==
--- cfe/trunk/test/CodeGenCXX/runtimecc.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/runtimecc.cpp Tue May 10 12:44:46 2016
@@ -22,8 +22,13 @@ namespace test0 {
   A global;
 // CHECK-LABEL:define internal void @__cxx_global_var_init()
 // CHECK:  call [[A]]* @_ZN5test01AC1Ev([[A]]* @_ZN5test06globalE)
-// CHECK-NEXT: call i32 @__cxa_atexit(void (i8*)* bitcast ([[A]]* ([[A]]*)* 
@_ZN5test01AD1Ev to void (i8*)*), i8* bitcast ([[A]]* @_ZN5test06globalE to 
i8*), i8* @__dso_handle) [[NOUNWIND:#[0-9]+]]
+// CHECK-NEXT: call i32 @__cxa_atexit(void (i8*)* @__cxx_global_array_dtor, 
i8* null, i8* @__dso_handle) [[NOUNWIND:#[0-9]+]]
 // CHECK-NEXT: ret void
+
+// CHECK-LABEL: define internal void @__cxx_global_array_dtor(i8*)
+// CHECK: call [[A]]* @_ZN5test01AD1Ev([[A]]* @_ZN5test06globalE)
+// CHECK-NEXT: ret void
+
 }
 
 // CHECK: declare i32 @__cxa_atexit(void (i8*)*, i8*, i8*) [[NOUNWIND]]

Added: cfe/trunk/test/CodeGenCXX/static-destructor.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/static-destructor.cpp?rev=269085=auto
==
--- cfe/trunk/test/CodeGenCXX/static-destructor.cpp (added)
+++ cfe/trunk/test/CodeGenCXX/static-destructor.cpp Tue May 10 12:44:46 2016
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 %s -triple=x86_64-pc-linux -emit-llvm -o - | FileCheck 
--check-prefix=X86 %s
+// RUN: %clang_cc1 %s -triple=wasm32 -emit-llvm -o - | FileCheck 
--check-prefix=WASM %s
+
+// Test that destructors are not passed directly to __cxa_atexit when their
+// signatures do not match the type of its first argument.
+// e.g. ARM and WebAssembly have destructors that return this instead of void.
+
+
+class Foo {
+ public:
+  ~Foo() {
+  }
+};
+
+Foo global;
+
+// X86 destructors have void return, and are registered directly with 
__cxa_atexit.
+// X86: define internal void @__cxx_global_var_init()
+// X86-NEXT: entry:
+// X86-NEXT:   %0 = call i32 @__cxa_atexit(void (i8*)* bitcast (void 
(%class.Foo*)* @_ZN3FooD1Ev to void (i8*)*), i8* getelementptr inbounds 
(%class.Foo, %class.Foo* @global, i32 0, i32 0), i8* @__dso_handle)
+
+// Wasm destructors return this, and use a wrapper function, which is 
registered
+// with __cxa_atexit.
+// WASM: define internal void @__cxx_global_var_init()
+// WASM-NEXT: entry:
+// 

r269086 - Clean up condition, add ARM to test

2016-05-10 Thread Derek Schuff via cfe-commits
Author: dschuff
Date: Tue May 10 12:44:48 2016
New Revision: 269086

URL: http://llvm.org/viewvc/llvm-project?rev=269086=rev
Log:
Clean up condition, add ARM to test

Modified:
cfe/trunk/lib/CodeGen/CGDeclCXX.cpp
cfe/trunk/test/CodeGenCXX/static-destructor.cpp

Modified: cfe/trunk/lib/CodeGen/CGDeclCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDeclCXX.cpp?rev=269086=269085=269086=diff
==
--- cfe/trunk/lib/CodeGen/CGDeclCXX.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDeclCXX.cpp Tue May 10 12:44:48 2016
@@ -89,14 +89,15 @@ static void EmitDeclDestroy(CodeGenFunct
   // Special-case non-array C++ destructors, if they have the right signature
   // that can be directly registered with __cxa_atexit. If __cxa_atexit is
   // disabled via a flag, a different helper function is generated anyway.
-  const CXXRecordDecl *record = nullptr;
-  if (dtorKind == QualType::DK_cxx_destructor &&
-  (record = type->getAsCXXRecordDecl()) &&
-  (!CGM.getCXXABI().HasThisReturn(
-  GlobalDecl(record->getDestructor(), Dtor_Complete)) ||
-   !CGM.getCodeGenOpts().CXAAtExit)) {
-assert(!record->hasTrivialDestructor());
-CXXDestructorDecl *dtor = record->getDestructor();
+  const CXXRecordDecl *Record = type->getAsCXXRecordDecl();
+  bool CanRegisterDestructor = Record &&
+   !CGM.getCXXABI().HasThisReturn(GlobalDecl(
+   Record->getDestructor(), Dtor_Complete));
+
+  if (dtorKind == QualType::DK_cxx_destructor && Record &&
+  (CanRegisterDestructor || !CGM.getCodeGenOpts().CXAAtExit)) {
+assert(!Record->hasTrivialDestructor());
+CXXDestructorDecl *dtor = Record->getDestructor();
 
 function = CGM.getAddrOfCXXStructor(dtor, StructorType::Complete);
 argument = llvm::ConstantExpr::getBitCast(

Modified: cfe/trunk/test/CodeGenCXX/static-destructor.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/static-destructor.cpp?rev=269086=269085=269086=diff
==
--- cfe/trunk/test/CodeGenCXX/static-destructor.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/static-destructor.cpp Tue May 10 12:44:48 2016
@@ -1,5 +1,6 @@
 // RUN: %clang_cc1 %s -triple=x86_64-pc-linux -emit-llvm -o - | FileCheck 
--check-prefix=X86 %s
 // RUN: %clang_cc1 %s -triple=wasm32 -emit-llvm -o - | FileCheck 
--check-prefix=WASM %s
+// RUN: %clang_cc1 %s -triple=armv7-apple-darwin9 -emit-llvm -o - | FileCheck 
--check-prefix=WASM %s
 
 // Test that destructors are not passed directly to __cxa_atexit when their
 // signatures do not match the type of its first argument.


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


r269087 - Clarify condition, remove redundant check

2016-05-10 Thread Derek Schuff via cfe-commits
Author: dschuff
Date: Tue May 10 12:44:50 2016
New Revision: 269087

URL: http://llvm.org/viewvc/llvm-project?rev=269087=rev
Log:
Clarify condition, remove redundant check

Modified:
cfe/trunk/lib/CodeGen/CGDeclCXX.cpp

Modified: cfe/trunk/lib/CodeGen/CGDeclCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDeclCXX.cpp?rev=269087=269086=269087=diff
==
--- cfe/trunk/lib/CodeGen/CGDeclCXX.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDeclCXX.cpp Tue May 10 12:44:50 2016
@@ -87,15 +87,17 @@ static void EmitDeclDestroy(CodeGenFunct
   llvm::Constant *argument;
 
   // Special-case non-array C++ destructors, if they have the right signature
-  // that can be directly registered with __cxa_atexit. If __cxa_atexit is
-  // disabled via a flag, a different helper function is generated anyway.
+  // that can be directly registered with __cxa_atexit.
   const CXXRecordDecl *Record = type->getAsCXXRecordDecl();
   bool CanRegisterDestructor = Record &&
!CGM.getCXXABI().HasThisReturn(GlobalDecl(
Record->getDestructor(), Dtor_Complete));
-
-  if (dtorKind == QualType::DK_cxx_destructor && Record &&
-  (CanRegisterDestructor || !CGM.getCodeGenOpts().CXAAtExit)) {
+  // If __cxa_atexit is disabled via a flag, a different helper function is
+  // generated elsewhere which uses atexit instead, and it takes the destructor
+  // directly.
+  bool UsingExternalHelper = !CGM.getCodeGenOpts().CXAAtExit;
+  if (Record &&
+  (CanRegisterDestructor || UsingExternalHelper)) {
 assert(!Record->hasTrivialDestructor());
 CXXDestructorDecl *dtor = Record->getDestructor();
 


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


[PATCH] D20113: Fix mangled name of method with ns_consumed parameters.

2016-05-10 Thread Sylvain Defresne via cfe-commits
sdefresne created this revision.
sdefresne added a reviewer: rjmccall.
sdefresne added a subscriber: cfe-commits.

When a function/method use a parameter with "ns_consumed" attribute,
ensure that the mangled name is the same whether -fobjc-arc is used
or not.

Since "ns_consumed" attribute is generally used to inform ARC that
a function/method does sink the reference, it mean it is usually
implemented in a compilation unit compiled without -fobjc-arc but
used form a compilation unit compiled with it.

Originally found while trying to use "ns_consumed" attribute in an
Objective-C++ file in Chromium (http://crbug.com/599980) where it
caused a linker error.

Regression introduced by revision 262278 (previously the attribute
was incorrectly not part of the mangled name).

http://reviews.llvm.org/D20113

Files:
  lib/Sema/SemaType.cpp
  test/CodeGenObjCXX/arc-mangle.mm
  test/CodeGenObjCXX/mangle.mm

Index: test/CodeGenObjCXX/mangle.mm
===
--- test/CodeGenObjCXX/mangle.mm
+++ test/CodeGenObjCXX/mangle.mm
@@ -113,3 +113,10 @@
 
 // CHECK-LABEL: define void @_Z19parameterized_test3P13Parameterized
 void parameterized_test3(Parameterized *p) {}
+
+// CHECK-LABEL: define {{.*}}void @_Z1fU11ns_consumedP11objc_object
+void f(id __attribute((ns_consumed))) {}
+// CHECK-LABEL: define {{.*}}void @_Z1fPFP11objc_objectU11ns_consumedS0_S0_E
+void f(id (*fn)(__attribute__((ns_consumed)) id, id)) {}
+// CHECK-LABEL: define {{.*}}void @_Z1fPFP11objc_objectS0_U11ns_consumedS0_E
+void f(__strong id (*fn)(id, __attribute__((ns_consumed)) id)) {}
Index: test/CodeGenObjCXX/arc-mangle.mm
===
--- test/CodeGenObjCXX/arc-mangle.mm
+++ test/CodeGenObjCXX/arc-mangle.mm
@@ -18,6 +18,8 @@
 void f(const __unsafe_unretained id *) {}
 // CHECK-LABEL: define {{.*}}void @_Z1fPFU19ns_returns_retainedP11objc_objectvE
 void f(__attribute__((ns_returns_retained)) id (*fn)()) {}
+// CHECK-LABEL: define {{.*}}void @_Z1fU11ns_consumedP11objc_object
+void f(id __attribute((ns_consumed))) {}
 // CHECK-LABEL: define {{.*}}void @_Z1fPFP11objc_objectU11ns_consumedS0_S0_E
 void f(id (*fn)(__attribute__((ns_consumed)) id, id)) {}
 // CHECK-LABEL: define {{.*}}void @_Z1fPFP11objc_objectS0_U11ns_consumedS0_E
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -4229,7 +4229,7 @@
 }
   }
 
-  if (LangOpts.ObjCAutoRefCount && Param->hasAttr()) {
+  if (Param->hasAttr()) {
 ExtParameterInfos[i] = ExtParameterInfos[i].withIsConsumed(true);
 HasAnyInterestingExtParameterInfos = true;
   }


Index: test/CodeGenObjCXX/mangle.mm
===
--- test/CodeGenObjCXX/mangle.mm
+++ test/CodeGenObjCXX/mangle.mm
@@ -113,3 +113,10 @@
 
 // CHECK-LABEL: define void @_Z19parameterized_test3P13Parameterized
 void parameterized_test3(Parameterized *p) {}
+
+// CHECK-LABEL: define {{.*}}void @_Z1fU11ns_consumedP11objc_object
+void f(id __attribute((ns_consumed))) {}
+// CHECK-LABEL: define {{.*}}void @_Z1fPFP11objc_objectU11ns_consumedS0_S0_E
+void f(id (*fn)(__attribute__((ns_consumed)) id, id)) {}
+// CHECK-LABEL: define {{.*}}void @_Z1fPFP11objc_objectS0_U11ns_consumedS0_E
+void f(__strong id (*fn)(id, __attribute__((ns_consumed)) id)) {}
Index: test/CodeGenObjCXX/arc-mangle.mm
===
--- test/CodeGenObjCXX/arc-mangle.mm
+++ test/CodeGenObjCXX/arc-mangle.mm
@@ -18,6 +18,8 @@
 void f(const __unsafe_unretained id *) {}
 // CHECK-LABEL: define {{.*}}void @_Z1fPFU19ns_returns_retainedP11objc_objectvE
 void f(__attribute__((ns_returns_retained)) id (*fn)()) {}
+// CHECK-LABEL: define {{.*}}void @_Z1fU11ns_consumedP11objc_object
+void f(id __attribute((ns_consumed))) {}
 // CHECK-LABEL: define {{.*}}void @_Z1fPFP11objc_objectU11ns_consumedS0_S0_E
 void f(id (*fn)(__attribute__((ns_consumed)) id, id)) {}
 // CHECK-LABEL: define {{.*}}void @_Z1fPFP11objc_objectS0_U11ns_consumedS0_E
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -4229,7 +4229,7 @@
 }
   }
 
-  if (LangOpts.ObjCAutoRefCount && Param->hasAttr()) {
+  if (Param->hasAttr()) {
 ExtParameterInfos[i] = ExtParameterInfos[i].withIsConsumed(true);
 HasAnyInterestingExtParameterInfos = true;
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r269079 - clang/test/CodeGen/avx512f-builtins.c: Fix for -Asserts.

2016-05-10 Thread NAKAMURA Takumi via cfe-commits
Author: chapuni
Date: Tue May 10 12:16:12 2016
New Revision: 269079

URL: http://llvm.org/viewvc/llvm-project?rev=269079=rev
Log:
clang/test/CodeGen/avx512f-builtins.c: Fix for -Asserts.

Modified:
cfe/trunk/test/CodeGen/avx512f-builtins.c

Modified: cfe/trunk/test/CodeGen/avx512f-builtins.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/avx512f-builtins.c?rev=269079=269078=269079=diff
==
--- cfe/trunk/test/CodeGen/avx512f-builtins.c (original)
+++ cfe/trunk/test/CodeGen/avx512f-builtins.c Tue May 10 12:16:12 2016
@@ -93,8 +93,8 @@ void test_mm512_store_si512 (void *__P,
 {
   // CHECK-LABEL: @test_mm512_store_si512 
   // CHECK: load <8 x i64>, <8 x i64>* %__A.addr.i, align 64
-  // CHECK: load i8*, i8** %__P.addr.i, align 8
-  // CHECK: bitcast i8* %3 to <8 x i64>*
+  // CHECK: [[SI512_3:%.+]] = load i8*, i8** %__P.addr.i, align 8
+  // CHECK: bitcast i8* [[SI512_3]] to <8 x i64>*
   // CHECK: store <8 x i64>  
   _mm512_store_si512 ( __P,__A);
 }
@@ -103,8 +103,8 @@ void test_mm512_store_epi32 (void *__P,
 {
   // CHECK-LABEL: @test_mm512_store_epi32 
   // CHECK: load <8 x i64>, <8 x i64>* %__A.addr.i, align 64
-  // CHECK: load i8*, i8** %__P.addr.i, align 8
-  // CHECK: bitcast i8* %3 to <8 x i64>*
+  // CHECK: [[Si32_3:%.+]] = load i8*, i8** %__P.addr.i, align 8
+  // CHECK: bitcast i8* [[Si32_3]] to <8 x i64>*
   // CHECK: store <8 x i64>  
   _mm512_store_epi32 ( __P,__A);
 }
@@ -113,8 +113,8 @@ void test_mm512_store_epi64 (void *__P,
 {
   // CHECK-LABEL: @test_mm512_store_epi64 
   // CHECK: load <8 x i64>, <8 x i64>* %__A.addr.i, align 64
-  // CHECK: load i8*, i8** %__P.addr.i, align 8
-  // CHECK: bitcast i8* %3 to <8 x i64>*
+  // CHECK: [[SI64_3:%.+]] = load i8*, i8** %__P.addr.i, align 8
+  // CHECK: bitcast i8* [[SI64_3]] to <8 x i64>*
   // CHECK: store <8 x i64>  
   _mm512_store_epi64 ( __P,__A);
 }
@@ -192,27 +192,27 @@ __m512d test_mm512_mask_loadu_pd (__m512
 __m512i test_mm512_load_si512 (void *__P)
 {
   // CHECK-LABEL: @test_mm512_load_si512 
-  // CHECK: load i8*, i8** %__P.addr.i, align 8
-  // CHECK: bitcast i8* %1 to <8 x i64>*
-  // CHECK: load <8 x i64>, <8 x i64>* %2, align 64
+  // CHECK: [[LI512_1:%.+]] = load i8*, i8** %__P.addr.i, align 8
+  // CHECK: [[LI512_2:%.+]] = bitcast i8* [[LI512_1]] to <8 x i64>*
+  // CHECK: load <8 x i64>, <8 x i64>* [[LI512_2]], align 64
   return _mm512_load_si512 ( __P);
 }
 
 __m512i test_mm512_load_epi32 (void *__P)
 {
   // CHECK-LABEL: @test_mm512_load_epi32 
-  // CHECK: load i8*, i8** %__P.addr.i, align 8
-  // CHECK: bitcast i8* %1 to <8 x i64>*
-  // CHECK: load <8 x i64>, <8 x i64>* %2, align 64
+  // CHECK: [[LI32_1:%.+]] = load i8*, i8** %__P.addr.i, align 8
+  // CHECK: [[LI32_2:%.+]] = bitcast i8* [[LI32_1]] to <8 x i64>*
+  // CHECK: load <8 x i64>, <8 x i64>* [[LI32_2]], align 64
   return _mm512_load_epi32 ( __P);
 }
 
 __m512i test_mm512_load_epi64 (void *__P)
 {
   // CHECK-LABEL: @test_mm512_load_epi64 
-  // CHECK: load i8*, i8** %__P.addr.i, align 8
-  // CHECK: bitcast i8* %1 to <8 x i64>*
-  // CHECK: load <8 x i64>, <8 x i64>* %2, align 64
+  // CHECK: [[LI64_1:%.+]] = load i8*, i8** %__P.addr.i, align 8
+  // CHECK: [[LI64_2:%.+]] = bitcast i8* [[LI64_1]] to <8 x i64>*
+  // CHECK: load <8 x i64>, <8 x i64>* [[LI64_2]], align 64
   return _mm512_load_epi64 ( __P);
 }
 


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


Re: [PATCH] D20113: Fix mangled name of method with ns_consumed parameters.

2016-05-10 Thread John McCall via cfe-commits
rjmccall added a comment.

This is a good catch, thanks!

As a slight adjustment, It's probably better to just ignore this attribute when 
mangling the function type of an entity, the same way that we generally don't 
mangle return types because they don't affect overloading.  That will require 
an extra flag to be passed down in a few places, but that's pretty reasonable.  
This will generally allow NS_CONSUMED and NS_RETURNS_RETAINED to be placed on 
existing APIs without changing their mangling unless the attribute is used in a 
secondary position (such as the type of an argument).

Finally, you should give ns_returns_retained the same treatment, as well as the 
parameter-ABI attributes.


http://reviews.llvm.org/D20113



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


Re: [PATCH] D19275: Do not register incompatible C++ destructors with __cxa_atexit

2016-05-10 Thread Reid Kleckner via cfe-commits
rnk accepted this revision.
rnk added a reviewer: rnk.
rnk added a comment.

lgtm


http://reviews.llvm.org/D19275



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


[PATCH] D20112: [OpenMP] Add support for the 'private pointer' flag to signal variables captured in target regions and used in first-private clauses.

2016-05-10 Thread Samuel Antao via cfe-commits
sfantao created this revision.
sfantao added reviewers: ABataev, hfinkel, carlo.bertolli, arpith-jacob, kkwli0.
sfantao added subscribers: cfe-commits, caomhin.

If a variable is implicitly mapped (doesn't show in a map clause), the runtime 
library has to be informed if the corresponding capture shows up in 
first-private clause, so that the storage previously allocated in the device is 
used. This patch adds the support for that.

http://reviews.llvm.org/D20112

Files:
  lib/CodeGen/CGOpenMPRuntime.cpp
  test/OpenMP/target_firstprivate_codegen.cpp
  test/OpenMP/target_map_codegen.cpp

Index: test/OpenMP/target_map_codegen.cpp
===
--- test/OpenMP/target_map_codegen.cpp
+++ test/OpenMP/target_map_codegen.cpp
@@ -4281,8 +4281,17 @@
 // CK27: [[SIZE03:@.+]] = private {{.*}}constant [1 x i[[Z]]] zeroinitializer
 // CK27: [[MTYPE03:@.+]] = private {{.*}}constant [1 x i32] [i32 35]
 
-// CK27-LABEL: zero_size_section_maps
-void zero_size_section_maps (int ii){
+// CK27: [[SIZE05:@.+]] = private {{.*}}constant [1 x i[[Z]]] zeroinitializer
+// CK27: [[MTYPE05:@.+]] = private {{.*}}constant [1 x i32] [i32 32]
+
+// CK27: [[SIZE07:@.+]] = private {{.*}}constant [1 x i[[Z]]] [i[[Z]] 4]
+// CK27: [[MTYPE07:@.+]] = private {{.*}}constant [1 x i32] [i32 288]
+
+// CK27: [[SIZE09:@.+]] = private {{.*}}constant [1 x i[[Z]]] [i[[Z]] 40]
+// CK27: [[MTYPE09:@.+]] = private {{.*}}constant [1 x i32] [i32 161]
+
+// CK27-LABEL: zero_size_section_and_private_maps
+void zero_size_section_and_private_maps (int ii){
 
   // Map of a pointer.
   int *pa;
@@ -4367,12 +4376,99 @@
   {
 pa[50]++;
   }
+
+  int *pvtPtr;
+  int pvtScl;
+  int pvtArr[10];
+
+  // Region 04
+  // CK27: call i32 @__tgt_target(i32 {{[^,]+}}, i8* {{[^,]+}}, i32 0, i8** null, i8** null, i{{64|32}}* null, i32* null)
+  // CK27: call void [[CALL04:@.+]]()
+  #pragma omp target private(pvtPtr)
+  {
+pvtPtr[5]++;
+  }
+
+  // Region 05
+  // CK27-DAG: call i32 @__tgt_target(i32 {{[^,]+}}, i8* {{[^,]+}}, i32 1, i8** [[GEPBP:%.+]], i8** [[GEPP:%.+]], {{.+}}getelementptr {{.+}}[1 x i{{.+}}]* [[SIZE05]], {{.+}}getelementptr {{.+}}[1 x i{{.+}}]* [[MTYPE05]]{{.+}})
+  // CK27-DAG: [[GEPBP]] = getelementptr inbounds {{.+}}[[BP:%[^,]+]]
+  // CK27-DAG: [[GEPP]] = getelementptr inbounds {{.+}}[[P:%[^,]+]]
+
+  // CK27-DAG: [[BP0:%.+]] = getelementptr inbounds {{.+}}[[BP]], i{{.+}} 0, i{{.+}} 0
+  // CK27-DAG: [[P0:%.+]] = getelementptr inbounds {{.+}}[[P]], i{{.+}} 0, i{{.+}} 0
+  // CK27-DAG: store i8* [[CBPVAL0:%[^,]+]], i8** [[BP0]]
+  // CK27-DAG: store i8* [[CPVAL0:%[^,]+]], i8** [[P0]]
+  // CK27-DAG: [[CBPVAL0]] = bitcast i32* [[VAR0:%.+]] to i8*
+  // CK27-DAG: [[CPVAL0]] = bitcast i32* [[VAR0]] to i8*
+
+  // CK27: call void [[CALL05:@.+]](i32* {{[^,]+}})
+  #pragma omp target firstprivate(pvtPtr)
+  {
+pvtPtr[5]++;
+  }
+
+  // Region 06
+  // CK27: call i32 @__tgt_target(i32 {{[^,]+}}, i8* {{[^,]+}}, i32 0, i8** null, i8** null, i{{64|32}}* null, i32* null)
+  // CK27: call void [[CALL06:@.+]]()
+  #pragma omp target private(pvtScl)
+  {
+pvtScl++;
+  }
+
+  // Region 07
+  // CK27-DAG: call i32 @__tgt_target(i32 {{.+}}, i8* {{.+}}, i32 1, i8** [[BPGEP:%[0-9]+]], i8** [[PGEP:%[0-9]+]], {{.+}}[[SIZE07]]{{.+}}, {{.+}}[[MTYPE07]]{{.+}})
+  // CK27-DAG: [[BPGEP]] = getelementptr inbounds {{.+}}[[BPS:%[^,]+]], i32 0, i32 0
+  // CK27-DAG: [[PGEP]] = getelementptr inbounds {{.+}}[[PS:%[^,]+]], i32 0, i32 0
+  // CK27-DAG: [[BP1:%.+]] = getelementptr inbounds {{.+}}[[BPS]], i32 0, i32 0
+  // CK27-DAG: [[P1:%.+]] = getelementptr inbounds {{.+}}[[PS]], i32 0, i32 0
+  // CK27-DAG: store i8* [[VALBP:%.+]], i8** [[BP1]],
+  // CK27-DAG: store i8* [[VALP:%.+]], i8** [[P1]],
+  // CK27-DAG: [[VALBP]] = inttoptr i[[Z]] [[VAL:%.+]] to i8*
+  // CK27-DAG: [[VALP]] = inttoptr i[[Z]] [[VAL:%.+]] to i8*
+  // CK27-DAG: [[VAL]] = load i[[Z]], i[[Z]]* [[ADDR:%.+]],
+  // CK27-64-DAG: [[CADDR:%.+]] = bitcast i[[Z]]* [[ADDR]] to i32*
+  // CK27-64-DAG: store i32 {{.+}}, i32* [[CADDR]],
+
+  // CK27: call void [[CALL07:@.+]](i[[Z]] [[VAL]])
+  #pragma omp target firstprivate(pvtScl)
+  {
+pvtScl++;
+  }
+
+  // Region 08
+  // CK27: call i32 @__tgt_target(i32 {{[^,]+}}, i8* {{[^,]+}}, i32 0, i8** null, i8** null, i{{64|32}}* null, i32* null)
+  // CK27: call void [[CALL08:@.+]]()
+  #pragma omp target private(pvtArr)
+  {
+pvtArr[5]++;
+  }
+
+  // Region 09
+  // CK27-DAG: call i32 @__tgt_target(i32 {{[^,]+}}, i8* {{[^,]+}}, i32 1, i8** [[GEPBP:%.+]], i8** [[GEPP:%.+]], {{.+}}getelementptr {{.+}}[1 x i{{.+}}]* [[SIZE09]], {{.+}}getelementptr {{.+}}[1 x i{{.+}}]* [[MTYPE09]]{{.+}})
+  // CK27-DAG: [[GEPBP]] = getelementptr inbounds {{.+}}[[BP:%[^,]+]]
+  // CK27-DAG: [[GEPP]] = getelementptr inbounds {{.+}}[[P:%[^,]+]]
+
+  // CK27-DAG: [[BP0:%.+]] = getelementptr inbounds {{.+}}[[BP]], i{{.+}} 0, i{{.+}} 0
+  // CK27-DAG: [[P0:%.+]] = getelementptr inbounds {{.+}}[[P]], i{{.+}} 0, i{{.+}} 0
+  

Re: [PATCH] D19275: Do not register incompatible C++ destructors with __cxa_atexit

2016-05-10 Thread JF Bastien via cfe-commits
jfb accepted this revision.
jfb added a reviewer: jfb.
jfb added a comment.
This revision is now accepted and ready to land.

lgtm


http://reviews.llvm.org/D19275



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


Re: [PATCH] D19992: [libcxx] Prefer C++14 over C++11 when building libc++experimental.

2016-05-10 Thread Eric Fiselier via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL269070: [libcxx] Prefer C++14 over C++11 when building 
libc++experimental. (authored by EricWF).

Changed prior to commit:
  http://reviews.llvm.org/D19992?vs=56333=56732#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D19992

Files:
  libcxx/trunk/cmake/Modules/HandleLibcxxFlags.cmake
  libcxx/trunk/lib/CMakeLists.txt

Index: libcxx/trunk/lib/CMakeLists.txt
===
--- libcxx/trunk/lib/CMakeLists.txt
+++ libcxx/trunk/lib/CMakeLists.txt
@@ -142,9 +142,15 @@
   file(GLOB LIBCXX_EXPERIMENTAL_SOURCES ../src/experimental/*.cpp)
   add_library(cxx_experimental STATIC ${LIBCXX_EXPERIMENTAL_SOURCES})
   target_link_libraries(cxx_experimental cxx)
+
+  set(experimental_flags "${LIBCXX_COMPILE_FLAGS}")
+  check_flag_supported(-std=c++14)
+  if (NOT MSVC AND LIBCXX_SUPPORTS_STD_EQ_CXX14_FLAG)
+string(REPLACE "-std=c++11" "-std=c++14" experimental_flags 
"${LIBCXX_COMPILE_FLAGS}")
+  endif()
   set_target_properties(cxx_experimental
 PROPERTIES
-  COMPILE_FLAGS "${LIBCXX_COMPILE_FLAGS}"
+  COMPILE_FLAGS "${experimental_flags}"
   OUTPUT_NAME   "c++experimental"
   )
 endif()
Index: libcxx/trunk/cmake/Modules/HandleLibcxxFlags.cmake
===
--- libcxx/trunk/cmake/Modules/HandleLibcxxFlags.cmake
+++ libcxx/trunk/cmake/Modules/HandleLibcxxFlags.cmake
@@ -35,6 +35,11 @@
   endforeach()
 endmacro(remove_flags)
 
+macro(check_flag_supported flag)
+mangle_name("${flag}" flagname)
+check_cxx_compiler_flag("${flag}" "LIBCXX_SUPPORTS_${flagname}_FLAG")
+endmacro()
+
 # Add a macro definition if condition is true.
 macro(define_if condition def)
   if (${condition})


Index: libcxx/trunk/lib/CMakeLists.txt
===
--- libcxx/trunk/lib/CMakeLists.txt
+++ libcxx/trunk/lib/CMakeLists.txt
@@ -142,9 +142,15 @@
   file(GLOB LIBCXX_EXPERIMENTAL_SOURCES ../src/experimental/*.cpp)
   add_library(cxx_experimental STATIC ${LIBCXX_EXPERIMENTAL_SOURCES})
   target_link_libraries(cxx_experimental cxx)
+
+  set(experimental_flags "${LIBCXX_COMPILE_FLAGS}")
+  check_flag_supported(-std=c++14)
+  if (NOT MSVC AND LIBCXX_SUPPORTS_STD_EQ_CXX14_FLAG)
+string(REPLACE "-std=c++11" "-std=c++14" experimental_flags "${LIBCXX_COMPILE_FLAGS}")
+  endif()
   set_target_properties(cxx_experimental
 PROPERTIES
-  COMPILE_FLAGS "${LIBCXX_COMPILE_FLAGS}"
+  COMPILE_FLAGS "${experimental_flags}"
   OUTPUT_NAME   "c++experimental"
   )
 endif()
Index: libcxx/trunk/cmake/Modules/HandleLibcxxFlags.cmake
===
--- libcxx/trunk/cmake/Modules/HandleLibcxxFlags.cmake
+++ libcxx/trunk/cmake/Modules/HandleLibcxxFlags.cmake
@@ -35,6 +35,11 @@
   endforeach()
 endmacro(remove_flags)
 
+macro(check_flag_supported flag)
+mangle_name("${flag}" flagname)
+check_cxx_compiler_flag("${flag}" "LIBCXX_SUPPORTS_${flagname}_FLAG")
+endmacro()
+
 # Add a macro definition if condition is true.
 macro(define_if condition def)
   if (${condition})
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[libcxx] r269070 - [libcxx] Prefer C++14 over C++11 when building libc++experimental.

2016-05-10 Thread Eric Fiselier via cfe-commits
Author: ericwf
Date: Tue May 10 11:17:43 2016
New Revision: 269070

URL: http://llvm.org/viewvc/llvm-project?rev=269070=rev
Log:
[libcxx] Prefer C++14 over C++11 when building libc++experimental.

Summary:
Currently libc++experimental builds with C++11. This patch changes that to 
C++14 when supported by the compiler. Although nothing currently requires C++14 
the upcoming  implementation would benefit from 
it. [1]

Note that libc++.so continues to build with C++11 and is unaffected by this 
change.

[1]  provides global resources which must exist 
for the entire lifetime of the program. In order to ensure that a global 
resource can be used during program termination there destructors must never be 
invoked. The only way to do this, while also allowing "constant 
initialization", is to use a C++14 union.


Reviewers: mclow.lists

Subscribers: pete, cfe-commits

Differential Revision: http://reviews.llvm.org/D19992

Modified:
libcxx/trunk/cmake/Modules/HandleLibcxxFlags.cmake
libcxx/trunk/lib/CMakeLists.txt

Modified: libcxx/trunk/cmake/Modules/HandleLibcxxFlags.cmake
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/cmake/Modules/HandleLibcxxFlags.cmake?rev=269070=269069=269070=diff
==
--- libcxx/trunk/cmake/Modules/HandleLibcxxFlags.cmake (original)
+++ libcxx/trunk/cmake/Modules/HandleLibcxxFlags.cmake Tue May 10 11:17:43 2016
@@ -35,6 +35,11 @@ macro(remove_flags)
   endforeach()
 endmacro(remove_flags)
 
+macro(check_flag_supported flag)
+mangle_name("${flag}" flagname)
+check_cxx_compiler_flag("${flag}" "LIBCXX_SUPPORTS_${flagname}_FLAG")
+endmacro()
+
 # Add a macro definition if condition is true.
 macro(define_if condition def)
   if (${condition})

Modified: libcxx/trunk/lib/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/lib/CMakeLists.txt?rev=269070=269069=269070=diff
==
--- libcxx/trunk/lib/CMakeLists.txt (original)
+++ libcxx/trunk/lib/CMakeLists.txt Tue May 10 11:17:43 2016
@@ -142,9 +142,15 @@ if (LIBCXX_ENABLE_EXPERIMENTAL_LIBRARY)
   file(GLOB LIBCXX_EXPERIMENTAL_SOURCES ../src/experimental/*.cpp)
   add_library(cxx_experimental STATIC ${LIBCXX_EXPERIMENTAL_SOURCES})
   target_link_libraries(cxx_experimental cxx)
+
+  set(experimental_flags "${LIBCXX_COMPILE_FLAGS}")
+  check_flag_supported(-std=c++14)
+  if (NOT MSVC AND LIBCXX_SUPPORTS_STD_EQ_CXX14_FLAG)
+string(REPLACE "-std=c++11" "-std=c++14" experimental_flags 
"${LIBCXX_COMPILE_FLAGS}")
+  endif()
   set_target_properties(cxx_experimental
 PROPERTIES
-  COMPILE_FLAGS "${LIBCXX_COMPILE_FLAGS}"
+  COMPILE_FLAGS "${experimental_flags}"
   OUTPUT_NAME   "c++experimental"
   )
 endif()


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


[PATCH] D20111: [OpenMP] Adjust map type bits according to latest spec and use zero size array sections for pointers.

2016-05-10 Thread Samuel Antao via cfe-commits
sfantao created this revision.
sfantao added reviewers: ABataev, hfinkel, carlo.bertolli, arpith-jacob, kkwli0.
sfantao added subscribers: cfe-commits, caomhin.

This patch changes the bits used to specify the map types according to the 
latest version of the libomptarget document and add the support for zero size 
array section when pointers are being implicitly mapped. This completes the 
missing new 4.5 map semantics.

http://reviews.llvm.org/D20111

Files:
  lib/CodeGen/CGOpenMPRuntime.cpp
  test/OpenMP/target_codegen.cpp
  test/OpenMP/target_codegen_registration.cpp
  test/OpenMP/target_data_codegen.cpp
  test/OpenMP/target_enter_data_codegen.cpp
  test/OpenMP/target_exit_data_codegen.cpp
  test/OpenMP/target_firstprivate_codegen.cpp
  test/OpenMP/target_map_codegen.cpp

Index: test/OpenMP/target_map_codegen.cpp
===
--- test/OpenMP/target_map_codegen.cpp
+++ test/OpenMP/target_map_codegen.cpp
@@ -16,8 +16,8 @@
 #ifdef CK1
 
 // CK1-DAG: [[SIZES:@.+]] = {{.+}}constant [1 x i[[sz:64|32]]] [i{{64|32}} 4]
-// Map types: OMP_MAP_BYCOPY = 128
-// CK1-DAG: [[TYPES:@.+]] = {{.+}}constant [1 x i32] [i32 128]
+// Map types: OMP_MAP_PRIVATE_VAL | OMP_MAP_IS_FIRST = 288
+// CK1-DAG: [[TYPES:@.+]] = {{.+}}constant [1 x i32] [i32 288]
 
 // CK1-LABEL: implicit_maps_integer
 void implicit_maps_integer (int a){
@@ -61,8 +61,8 @@
 #ifdef CK2
 
 // CK2-DAG: [[SIZES:@.+]] = {{.+}}constant [1 x i[[sz:64|32]]] [i{{64|32}} 4]
-// Map types: OMP_MAP_BYCOPY = 128
-// CK2-DAG: [[TYPES:@.+]] = {{.+}}constant [1 x i32] [i32 128]
+// Map types: OMP_MAP_PRIVATE_VAL | OMP_MAP_IS_FIRST = 288
+// CK2-DAG: [[TYPES:@.+]] = {{.+}}constant [1 x i32] [i32 288]
 
 // CK2-LABEL: implicit_maps_integer_reference
 void implicit_maps_integer_reference (int a){
@@ -110,8 +110,8 @@
 #ifdef CK3
 
 // CK3-DAG: [[SIZES:@.+]] = {{.+}}constant [1 x i[[sz:64|32]]] [i{{64|32}} 4]
-// Map types: OMP_MAP_BYCOPY = 128
-// CK3-DAG: [[TYPES:@.+]] = {{.+}}constant [1 x i32] [i32 128]
+// Map types: OMP_MAP_PRIVATE_VAL | OMP_MAP_IS_FIRST = 288
+// CK3-DAG: [[TYPES:@.+]] = {{.+}}constant [1 x i32] [i32 288]
 
 // CK3-LABEL: implicit_maps_parameter
 void implicit_maps_parameter (int a){
@@ -154,8 +154,8 @@
 #ifdef CK4
 
 // CK4-DAG: [[SIZES:@.+]] = {{.+}}constant [1 x i[[sz:64|32]]] [i{{64|32}} 4]
-// Map types: OMP_MAP_BYCOPY = 128
-// CK4-DAG: [[TYPES:@.+]] = {{.+}}constant [1 x i32] [i32 128]
+// Map types: OMP_MAP_PRIVATE_VAL | OMP_MAP_IS_FIRST = 288
+// CK4-DAG: [[TYPES:@.+]] = {{.+}}constant [1 x i32] [i32 288]
 
 // CK4-LABEL: implicit_maps_nested_integer
 void implicit_maps_nested_integer (int a){
@@ -210,8 +210,8 @@
 #ifdef CK5
 
 // CK5-DAG: [[SIZES:@.+]] = {{.+}}constant [1 x i[[sz:64|32]]] [i{{64|32}} 4]
-// Map types: OMP_MAP_BYCOPY = 128
-// CK5-DAG: [[TYPES:@.+]] = {{.+}}constant [1 x i32] [i32 128]
+// Map types: OMP_MAP_PRIVATE_VAL | OMP_MAP_IS_FIRST = 288
+// CK5-DAG: [[TYPES:@.+]] = {{.+}}constant [1 x i32] [i32 288]
 
 // CK5-LABEL: implicit_maps_nested_integer_and_enum
 void implicit_maps_nested_integer_and_enum (int a){
@@ -261,8 +261,8 @@
 #ifdef CK6
 // CK6-DAG: [[GBL:@Gi]] = global i32 0
 // CK6-DAG: [[SIZES:@.+]] = {{.+}}constant [1 x i[[sz:64|32]]] [i{{64|32}} 4]
-// Map types: OMP_MAP_BYCOPY = 128
-// CK6-DAG: [[TYPES:@.+]] = {{.+}}constant [1 x i32] [i32 128]
+// Map types: OMP_MAP_PRIVATE_VAL | OMP_MAP_IS_FIRST = 288
+// CK6-DAG: [[TYPES:@.+]] = {{.+}}constant [1 x i32] [i32 288]
 
 // CK6-LABEL: implicit_maps_host_global
 int Gi;
@@ -310,10 +310,10 @@
 // therefore it is passed by reference with a map 'to' specification.
 
 // CK7-DAG: [[SIZES:@.+]] = {{.+}}constant [1 x i[[sz:64|32]]] [i{{64|32}} 8]
-// Map types: OMP_MAP_BYCOPY = 128
-// CK7-64-DAG: [[TYPES:@.+]] = {{.+}}constant [1 x i32] [i32 128]
-// Map types: OMP_MAP_TO = 1
-// CK7-32-DAG: [[TYPES:@.+]] = {{.+}}constant [1 x i32] [i32 1]
+// Map types: OMP_MAP_PRIVATE_VAL | OMP_MAP_IS_FIRST = 288
+// CK7-64-DAG: [[TYPES:@.+]] = {{.+}}constant [1 x i32] [i32 288]
+// Map types: OMP_MAP_TO  | OMP_MAP_IS_FIRST = 33
+// CK7-32-DAG: [[TYPES:@.+]] = {{.+}}constant [1 x i32] [i32 33]
 
 // CK7-LABEL: implicit_maps_double
 void implicit_maps_double (int a){
@@ -369,8 +369,8 @@
 #ifdef CK8
 
 // CK8-DAG: [[SIZES:@.+]] = {{.+}}constant [1 x i[[sz:64|32]]] [i{{64|32}} 4]
-// Map types: OMP_MAP_BYCOPY = 128
-// CK8-DAG: [[TYPES:@.+]] = {{.+}}constant [1 x i32] [i32 128]
+// Map types: OMP_MAP_PRIVATE_VAL | OMP_MAP_IS_FIRST = 288
+// CK8-DAG: [[TYPES:@.+]] = {{.+}}constant [1 x i32] [i32 288]
 
 // CK8-LABEL: implicit_maps_float
 void implicit_maps_float (int a){
@@ -413,8 +413,8 @@
 #ifdef CK9
 
 // CK9-DAG: [[SIZES:@.+]] = {{.+}}constant [1 x i[[sz:64|32]]] [i{{64|32}} 16]
-// Map types: OMP_MAP_TO + OMP_MAP_FROM = 2 + 1
-// CK9-DAG: [[TYPES:@.+]] = {{.+}}constant [1 x i32] [i32 3]
+// Map types: OMP_MAP_TO + OMP_MAP_FROM + OMP_MAP_IS_FIRST = 35
+// CK9-DAG: [[TYPES:@.+]] = {{.+}}constant [1 x i32] [i32 35]
 
 // 

r269069 - [CMake] Pass LLVM_LIBDIR_SUFFIX into Compiler-RT

2016-05-10 Thread Chris Bieneman via cfe-commits
Author: cbieneman
Date: Tue May 10 11:10:22 2016
New Revision: 269069

URL: http://llvm.org/viewvc/llvm-project?rev=269069=rev
Log:
[CMake] Pass LLVM_LIBDIR_SUFFIX into Compiler-RT

Not passing this causes Compiler-RT to fail to configure on multi-lib systems.

Modified:
cfe/trunk/runtime/CMakeLists.txt

Modified: cfe/trunk/runtime/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/runtime/CMakeLists.txt?rev=269069=269068=269069=diff
==
--- cfe/trunk/runtime/CMakeLists.txt (original)
+++ cfe/trunk/runtime/CMakeLists.txt Tue May 10 11:10:22 2016
@@ -82,6 +82,7 @@ if(LLVM_BUILD_EXTERNAL_COMPILER_RT AND E

-DCOMPILER_RT_INSTALL_PATH:STRING=lib${LLVM_LIBDIR_SUFFIX}/clang/${CLANG_VERSION}
-DCOMPILER_RT_INCLUDE_TESTS=${LLVM_INCLUDE_TESTS}
-DCMAKE_INSTALL_PREFIX=${CMAKE_INSTALL_PREFIX}
+   -LLVM_LIBDIR_SUFFIX=${LLVM_LIBDIR_SUFFIX}
-Dcmake_3_2_USES_TERMINAL=${cmake_3_2_USES_TERMINAL}
${COMPILER_RT_PASSTHROUGH_VARIABLES}
 INSTALL_COMMAND ""


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


Re: [PATCH] D20045: [ObjC][CodeGen] Remove an assert that is no longer correct.

2016-05-10 Thread Manman Ren via cfe-commits
manmanren added a comment.

After r231508 made changes to promote constant temporaries to globals, the 
assert fires when a std::initializer_list is constructed using Objective-C 
string literals.

--> Can you explain the code path after r231508 for your example? Will r231508 
change the code path if we start with a global?
r231508 seems to change the behavior constant temporaries only.

Thanks for working on this!
Manman



Comment at: lib/CodeGen/CGExpr.cpp:364
@@ +363,3 @@
+
+  if (Var->hasInitializer())
+return MakeAddrLValue(Object, M->getType(), AlignmentSource::Decl);

Can you add comments here?


http://reviews.llvm.org/D20045



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


Re: [PATCH] D20090: [OPENCL] Fix wrongly vla error for OpenCL array.

2016-05-10 Thread Alexey Bader via cfe-commits
bader accepted this revision.
bader added a reviewer: bader.
bader added a comment.
This revision is now accepted and ready to land.

LGTM


http://reviews.llvm.org/D20090



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


  1   2   >