[PATCH] D31745: [OpenCL] Added diagnostic for implicit declaration of function in OpenCL

2017-04-10 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia added inline comments.



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:8254
   "%0 cannot be used as the type of a kernel parameter">;
+def err_opencl_implicit_function_decl : Error<
+  "implicit declaration of function %0 is invalid in OpenCL">;

echuraev wrote:
> Anastasia wrote:
> > Could this be in OpenCL group please?
> But this is already in OpenCL group. Please, see line 8232. In this line is a 
> comment that OpenCL warnings and errors are below.
Ah yes! Sorry, I got confused by the absence of opencl in the neighboring 
diagnostics. :)



Comment at: test/SemaOpenCL/clang-builtin-version.cl:32
+  work_group_reserve_write_pipe(tmp, tmp); // expected-error{{implicit 
declaration of function 'work_group_reserve_write_pipe' is invalid in OpenCL}}
+  // expected-note@-1{{did you mean 'work_group_reserve_read_pipe'?}}
+  // expected-note@-2{{'work_group_reserve_write_pipe' declared here}}

echuraev wrote:
> Anastasia wrote:
> > Why do we get this note now? I believe work_group_reserve_read_pipe 
> > shouldn't be available either?
> May be I don't understand something but I think that it is the right 
> diagnostic message. We called work_group_reserve_read_pipe in line 29. So for 
> this case we will get the following message: 
> //clang-builtin-version.cl: 31 error: implicit declaration of function 
> 'work_group_reserve_write_pipe' is invalid in OpenCL
> clang-builtin-version.cl: 31 note: did you mean 
> 'work_group_reserve_read_pipe'?
> clang-builtin-version.cl: 29 note: 'work_group_reserve_read_pipe' declared 
> here//
But there is an error now given for the call to 'work_group_reserve_read_pipe'. 
Why is it still added to the declarations? I think we should prevent this.


https://reviews.llvm.org/D31745



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


[PATCH] D31885: Remove TBAA information from LValues representing union members

2017-04-10 Thread wenzel.ja...@epfl.ch via Phabricator via cfe-commits
wjakob added a comment.

Disclaimer: I am not an LLVM developer. Just looking at the patch, I wonder if 
it could be much less intrusive if `LValue CodeGenFunction::EmitLValue(const 
Expr *E)` is split into two methods -- a private one, and a public-facing one 
that applies your `ClearTBAA` method.


Repository:
  rL LLVM

https://reviews.llvm.org/D31885



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


[clang-tools-extra] r299854 - [clangd] Relax absolute path checking assertion

2017-04-10 Thread Reid Kleckner via cfe-commits
Author: rnk
Date: Mon Apr 10 12:03:44 2017
New Revision: 299854

URL: http://llvm.org/viewvc/llvm-project?rev=299854=rev
Log:
[clangd] Relax absolute path checking assertion

clangd can process absolute paths from systems that don't use the native
path style, so this assertion needs to check both Windows and Posix path
styles.

Fixes PR32596

Modified:
clang-tools-extra/trunk/clangd/ASTManager.cpp

Modified: clang-tools-extra/trunk/clangd/ASTManager.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ASTManager.cpp?rev=299854=299853=299854=diff
==
--- clang-tools-extra/trunk/clangd/ASTManager.cpp (original)
+++ clang-tools-extra/trunk/clangd/ASTManager.cpp Mon Apr 10 12:03:44 2017
@@ -232,7 +232,9 @@ tooling::CompilationDatabase *
 ASTManager::getOrCreateCompilationDatabaseForFile(StringRef File) {
   namespace path = llvm::sys::path;
 
-  assert(path::is_absolute(File) && "path must be absolute");
+  assert((path::is_absolute(File, path::Style::posix) ||
+  path::is_absolute(File, path::Style::windows)) &&
+ "path must be absolute");
 
   for (auto Path = path::parent_path(File); !Path.empty();
Path = path::parent_path(Path)) {


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


[clang-tools-extra] r299850 - Revert "XFAIL clangd tests on Windows"

2017-04-10 Thread Reid Kleckner via cfe-commits
Author: rnk
Date: Mon Apr 10 11:55:48 2017
New Revision: 299850

URL: http://llvm.org/viewvc/llvm-project?rev=299850=rev
Log:
Revert "XFAIL clangd tests on Windows"

This reverts r299849, apparently these tests only fail on my machine.

Modified:
clang-tools-extra/trunk/test/clangd/completion.test
clang-tools-extra/trunk/test/clangd/diagnostics.test
clang-tools-extra/trunk/test/clangd/fixits.test
clang-tools-extra/trunk/test/clangd/formatting.test

Modified: clang-tools-extra/trunk/test/clangd/completion.test
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/completion.test?rev=299850=299849=299850=diff
==
--- clang-tools-extra/trunk/test/clangd/completion.test (original)
+++ clang-tools-extra/trunk/test/clangd/completion.test Mon Apr 10 11:55:48 2017
@@ -1,8 +1,6 @@
 # RUN: clangd -run-synchronously < %s | FileCheck %s
 # It is absolutely vital that this file has CRLF line endings.
 #
-# FIXME: http://llvm.org/pr32596
-# XFAIL: win32
 Content-Length: 125
 
 
{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}

Modified: clang-tools-extra/trunk/test/clangd/diagnostics.test
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/diagnostics.test?rev=299850=299849=299850=diff
==
--- clang-tools-extra/trunk/test/clangd/diagnostics.test (original)
+++ clang-tools-extra/trunk/test/clangd/diagnostics.test Mon Apr 10 11:55:48 
2017
@@ -1,8 +1,6 @@
 # RUN: clangd -run-synchronously < %s | FileCheck %s
 # It is absolutely vital that this file has CRLF line endings.
 #
-# FIXME: http://llvm.org/pr32596
-# XFAIL: win32
 Content-Length: 125
 
 
{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}

Modified: clang-tools-extra/trunk/test/clangd/fixits.test
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/fixits.test?rev=299850=299849=299850=diff
==
--- clang-tools-extra/trunk/test/clangd/fixits.test (original)
+++ clang-tools-extra/trunk/test/clangd/fixits.test Mon Apr 10 11:55:48 2017
@@ -1,8 +1,6 @@
 # RUN: clangd -run-synchronously < %s | FileCheck %s
 # It is absolutely vital that this file has CRLF line endings.
 #
-# FIXME: http://llvm.org/pr32596
-# XFAIL: win32
 Content-Length: 125
 
 
{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}

Modified: clang-tools-extra/trunk/test/clangd/formatting.test
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/formatting.test?rev=299850=299849=299850=diff
==
--- clang-tools-extra/trunk/test/clangd/formatting.test (original)
+++ clang-tools-extra/trunk/test/clangd/formatting.test Mon Apr 10 11:55:48 2017
@@ -1,8 +1,6 @@
 # RUN: clangd < %s | FileCheck %s
 # It is absolutely vital that this file has CRLF line endings.
 #
-# FIXME: http://llvm.org/pr32596
-# XFAIL: win32
 Content-Length: 125
 
 
{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}


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


[clang-tools-extra] r299849 - XFAIL clangd tests on Windows

2017-04-10 Thread Reid Kleckner via cfe-commits
Author: rnk
Date: Mon Apr 10 11:50:55 2017
New Revision: 299849

URL: http://llvm.org/viewvc/llvm-project?rev=299849=rev
Log:
XFAIL clangd tests on Windows

They all assert. Filed as PR32596.

Modified:
clang-tools-extra/trunk/test/clangd/completion.test
clang-tools-extra/trunk/test/clangd/diagnostics.test
clang-tools-extra/trunk/test/clangd/fixits.test
clang-tools-extra/trunk/test/clangd/formatting.test

Modified: clang-tools-extra/trunk/test/clangd/completion.test
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/completion.test?rev=299849=299848=299849=diff
==
--- clang-tools-extra/trunk/test/clangd/completion.test (original)
+++ clang-tools-extra/trunk/test/clangd/completion.test Mon Apr 10 11:50:55 2017
@@ -1,6 +1,8 @@
 # RUN: clangd -run-synchronously < %s | FileCheck %s
 # It is absolutely vital that this file has CRLF line endings.
 #
+# FIXME: http://llvm.org/pr32596
+# XFAIL: win32
 Content-Length: 125
 
 
{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}

Modified: clang-tools-extra/trunk/test/clangd/diagnostics.test
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/diagnostics.test?rev=299849=299848=299849=diff
==
--- clang-tools-extra/trunk/test/clangd/diagnostics.test (original)
+++ clang-tools-extra/trunk/test/clangd/diagnostics.test Mon Apr 10 11:50:55 
2017
@@ -1,6 +1,8 @@
 # RUN: clangd -run-synchronously < %s | FileCheck %s
 # It is absolutely vital that this file has CRLF line endings.
 #
+# FIXME: http://llvm.org/pr32596
+# XFAIL: win32
 Content-Length: 125
 
 
{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}

Modified: clang-tools-extra/trunk/test/clangd/fixits.test
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/fixits.test?rev=299849=299848=299849=diff
==
--- clang-tools-extra/trunk/test/clangd/fixits.test (original)
+++ clang-tools-extra/trunk/test/clangd/fixits.test Mon Apr 10 11:50:55 2017
@@ -1,6 +1,8 @@
 # RUN: clangd -run-synchronously < %s | FileCheck %s
 # It is absolutely vital that this file has CRLF line endings.
 #
+# FIXME: http://llvm.org/pr32596
+# XFAIL: win32
 Content-Length: 125
 
 
{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}

Modified: clang-tools-extra/trunk/test/clangd/formatting.test
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/formatting.test?rev=299849=299848=299849=diff
==
--- clang-tools-extra/trunk/test/clangd/formatting.test (original)
+++ clang-tools-extra/trunk/test/clangd/formatting.test Mon Apr 10 11:50:55 2017
@@ -1,6 +1,8 @@
 # RUN: clangd < %s | FileCheck %s
 # It is absolutely vital that this file has CRLF line endings.
 #
+# FIXME: http://llvm.org/pr32596
+# XFAIL: win32
 Content-Length: 125
 
 
{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}


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


[clang-tools-extra] r299844 - [clangd] Fix nondeterminism in clangd test

2017-04-10 Thread Krasimir Georgiev via cfe-commits
Author: krasimir
Date: Mon Apr 10 09:06:54 2017
New Revision: 299844

URL: http://llvm.org/viewvc/llvm-project?rev=299844=rev
Log:
[clangd] Fix nondeterminism in clangd test

Modified:
clang-tools-extra/trunk/test/clangd/diagnostics.test

Modified: clang-tools-extra/trunk/test/clangd/diagnostics.test
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/diagnostics.test?rev=299844=299843=299844=diff
==
--- clang-tools-extra/trunk/test/clangd/diagnostics.test (original)
+++ clang-tools-extra/trunk/test/clangd/diagnostics.test Mon Apr 10 09:06:54 
2017
@@ -1,4 +1,4 @@
-# RUN: clangd < %s | FileCheck %s
+# RUN: clangd -run-synchronously < %s | FileCheck %s
 # It is absolutely vital that this file has CRLF line endings.
 #
 Content-Length: 125


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


[clang-tools-extra] r299843 - [clangd] Remove ASTUnits for closed documents and cache CompilationDatabase per directory.

2017-04-10 Thread Krasimir Georgiev via cfe-commits
Author: krasimir
Date: Mon Apr 10 08:31:39 2017
New Revision: 299843

URL: http://llvm.org/viewvc/llvm-project?rev=299843=rev
Log:
[clangd] Remove ASTUnits for closed documents and cache CompilationDatabase per 
directory.

Contributed by ilya-biryukov!

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

Modified:
clang-tools-extra/trunk/clangd/ASTManager.cpp
clang-tools-extra/trunk/clangd/ASTManager.h
clang-tools-extra/trunk/clangd/ClangDMain.cpp
clang-tools-extra/trunk/clangd/Protocol.cpp
clang-tools-extra/trunk/clangd/Protocol.h
clang-tools-extra/trunk/clangd/ProtocolHandlers.cpp
clang-tools-extra/trunk/clangd/ProtocolHandlers.h

Modified: clang-tools-extra/trunk/clangd/ASTManager.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ASTManager.cpp?rev=299843=299842=299843=diff
==
--- clang-tools-extra/trunk/clangd/ASTManager.cpp (original)
+++ clang-tools-extra/trunk/clangd/ASTManager.cpp Mon Apr 10 08:31:39 2017
@@ -20,6 +20,29 @@
 using namespace clang;
 using namespace clangd;
 
+void DocData::setAST(std::unique_ptr AST) {
+  this->AST = std::move(AST);
+}
+
+ASTUnit *DocData::getAST() const { return AST.get(); }
+
+void DocData::cacheFixIts(DiagnosticToReplacementMap FixIts) {
+  this->FixIts = std::move(FixIts);
+}
+
+std::vector
+DocData::getFixIts(const clangd::Diagnostic ) const {
+  auto it = FixIts.find(D);
+  if (it != FixIts.end())
+return it->second;
+  return {};
+}
+
+ASTManagerRequest::ASTManagerRequest(ASTManagerRequestType Type,
+ std::string File,
+ DocVersion Version)
+: Type(Type), File(File), Version(Version) {}
+
 /// Retrieve a copy of the contents of every file in the store, for feeding 
into
 /// ASTUnit.
 static std::vector
@@ -61,82 +84,125 @@ ASTManager::ASTManager(JSONOutput 
 
 void ASTManager::runWorker() {
   while (true) {
-std::string File;
+ASTManagerRequest Request;
 
+// Pick request from the queue
 {
   std::unique_lock Lock(RequestLock);
-  // Check if there's another request pending. We keep parsing until
-  // our one-element queue is empty.
+  // Wait for more requests.
   ClangRequestCV.wait(Lock,
   [this] { return !RequestQueue.empty() || Done; });
-
-  if (RequestQueue.empty() && Done)
+  if (Done)
 return;
+  assert(!RequestQueue.empty() && "RequestQueue was empty");
 
-  File = std::move(RequestQueue.back());
+  Request = std::move(RequestQueue.back());
   RequestQueue.pop_back();
-} // unlock.
 
+  // Skip outdated requests
+  if (Request.Version != DocVersions.find(Request.File)->second) {
+Output.log("Version for " + Twine(Request.File) +
+   " in request is outdated, skipping request\n");
+continue;
+  }
+} // unlock RequestLock
+
+handleRequest(Request.Type, Request.File);
+  }
+}
+
+void ASTManager::queueOrRun(ASTManagerRequestType RequestType, StringRef File) 
{
+  if (RunSynchronously) {
+handleRequest(RequestType, File);
+return;
+  }
+
+  std::lock_guard Guard(RequestLock);
+  // We increment the version of the added document immediately and schedule
+  // the requested operation to be run on a worker thread
+  DocVersion version = ++DocVersions[File];
+  RequestQueue.push_back(ASTManagerRequest(RequestType, File, version));
+  ClangRequestCV.notify_one();
+}
+
+void ASTManager::handleRequest(ASTManagerRequestType RequestType,
+   StringRef File) {
+  switch (RequestType) {
+  case ASTManagerRequestType::ParseAndPublishDiagnostics:
 parseFileAndPublishDiagnostics(File);
+break;
+  case ASTManagerRequestType::RemoveDocData: {
+std::lock_guard Lock(ClangObjectLock);
+auto DocDataIt = DocDatas.find(File);
+// We could get the remove request before parsing for the document is
+// started, just do nothing in that case, parsing request will be discarded
+// because it has a lower version value
+if (DocDataIt == DocDatas.end())
+  return;
+DocDatas.erase(DocDataIt);
+break;
+  } // unlock ClangObjectLock
   }
 }
 
 void ASTManager::parseFileAndPublishDiagnostics(StringRef File) {
-  DiagnosticToReplacementMap LocalFixIts; // Temporary storage
-  std::string Diagnostics;
-  {
-std::lock_guard ASTGuard(ASTLock);
-auto  = ASTs[File]; // Only one thread can access this at a time.
+  std::unique_lock ClangObjectLockGuard(ClangObjectLock);
 
-if (!Unit) {
-  Unit = createASTUnitForFile(File, this->Store);
-} else {
-  // Do a reparse if this wasn't the first parse.
-  // FIXME: This might have the wrong working directory if it changed in 
the
-  // meantime.
-  Unit->Reparse(PCHs, getRemappedFiles(this->Store));
-}
+  auto  = DocDatas[File];
+  ASTUnit *Unit = DocData.getAST();
+  if 

Re: r299774 - Sema: prevent __declspec(naked) use on x64

2017-04-10 Thread Aaron Ballman via cfe-commits
On Fri, Apr 7, 2017 at 4:48 PM, Saleem Abdulrasool
 wrote:
> Yeah, I shouldn't write commit messages late at night (I didn't commit until
> the morning to keep an eye on the bots).

Also, if you're going to make a change that might break users, having
a review for that change is probably a good idea. I don't know of any
users who WILL be broken by this, but having the discussion in advance
is preferred.

> I opted to follow cl in terms of erroring.  I can make it a warning if you
> feel strongly about that.
>
> I think for naked functions being stricter is better.  The user can just as
> easily accomplish the same thing with an out-of-line assembly input (or,
> something terrible like module level assembly blocks).
>
> With the GNU spelling, it restricts the function to just an asm blocks
> without operands.  Furthermore, GCC restricts it to ARM, AVR, MSP430 (and
> MCORE, NDS32, RL87, RX, SPU), which I think we should also do.  We currently
> don't check that.
>
> The declspec spelling is supposed to prevent inlining at all costs, and can
> only be applied to non-member functions.  What's even more interesting is
> that you don't have ASM blocks in ARM, so it being available is even more
> questionable [1].  Plus, I imagine that unwinding would be pretty fragile
> (since I don't think we could easily handle pdata/xdata emission in that
> case).
>
> Technically, it makes no sense for a target to not support the naked
> attribute: it just elides the frame setup/tear down - we could just skip
> functions with the naked attribute in PEI.  I don't think it's particularly
> difficult to support the concept at the IR level.  The problem arises in
> trying to analyze it.  If you want to completely skip that, why not just use
> out-of-line assembly?  Or how do you generate unwinding information?
>
> Am I overlooking a use case whuch cannot be accommodated with the same
> guarantees without lulling the developer into a false sense of safety?
>
> [1]: naked functions don't have a prologue/epilogue but you don't have an
> ASM block, so how do add that?  Windows ABI requires a FP, RA frame setup,
> how do you do that?  How do you generate the .pdata?

Given that this is an MS attribute, I think following the MS behavior
when first implementing the attribute is sensible (and similar for the
GNU behavior). We can always extend that behavior when we find a good
use case for it. However, this attribute has been in the wild for
quite some time, so I worry about the impact to users by restricting
it now -- do you know if this will break anyone?

~Aaron

>
> On Fri, Apr 7, 2017 at 12:06 PM Aaron Ballman via cfe-commits
>  wrote:
>>
>> On Fri, Apr 7, 2017 at 2:53 PM, Richard Smith 
>> wrote:
>> > On 7 Apr 2017 11:49 am, "David Majnemer via cfe-commits"
>> >  wrote:
>> >
>> >
>> >
>> > On Fri, Apr 7, 2017 at 8:30 AM, Aaron Ballman via cfe-commits
>> >  wrote:
>> >>
>> >> On Fri, Apr 7, 2017 at 11:13 AM, Saleem Abdulrasool via cfe-commits
>> >>  wrote:
>> >> > Author: compnerd
>> >> > Date: Fri Apr  7 10:13:47 2017
>> >> > New Revision: 299774
>> >> >
>> >> > URL: http://llvm.org/viewvc/llvm-project?rev=299774=rev
>> >> > Log:
>> >> > Sema: prevent __declspec(naked) use on x64
>> >> >
>> >> > MSDN (https://msdn.microsoft.com/en-us/library/h5w10wxs.aspx)
>> >> > indicates
>> >> > that `__declspec(naked)` is only permitted on x86 and ARM targets.
>> >> > Testing with cl does confirm this behaviour.  Provide a warning for
>> >> > use
>> >> > of `__declspec(naked)` on x64.
>> >>
>> >> Your patch is not providing a warning, it's providing an error, which
>> >> seems inappropriate (to me). Why is this attribute not silently
>> >> ignored there instead?
>> >
>> >
>> > FWIW, MSVC diagnoses this with an error: https://godbolt.org/g/T5sQr5
>> >
>> >
>> > Is there a reason we can't support this? Was our existing support broken
>> > in
>> > some way? We generally allow our features to be combined in arbitrary
>> > ways
>> > unless there's a reason not to.
>>
>> For the __declspec spelling of the attribute, we shouldn't really do
>> *more* than what Microsoft allows, should we (even if we can)?
>>
>> As for error vs warning, I was mostly pointing out that (1) the commit
>> comment doesn't match the behavior of the patch, which is always a
>> concern, and (2) most of our attributes warn rather than err, unless
>> there's a solid reason to err. If LLVM gets a naked attribute for a
>> function and the architecture doesn't support the concept, what
>> happens? I thought it ignored it (which suggests a warning rather than
>> an error, to a certain extent), but I could be wrong.
>>
>> ~Aaron
>>
>> >
>> >> (Btw, I did not see a review thread for this, did I miss one?) More
>> >> comments below.
>> >>
>> >> >
>> >> > Added:
>> >> > cfe/trunk/test/Sema/declspec-naked.c
>> >> > Modified:
>> 

<    1   2